ptyread eating CPU on Cygwin

ptyread eating CPU on Cygwin

[Andrej Borsenkow]

I could finally run (almost) all Zsh tests under lasy Cygwin snapshot.
Unfortunately, doing it I found, that reading from pty currently takes about
98% CPU time :-( (I wonder, why we do not notice it on Unix). That is well
seen running completion tests.

Peter, you that select does not work under Cygwin (read_poll). I asked on
Cygwin list, but there are no known problems and many applications do use
select.

What specific problems did you have?

-andrej

Have a nice DOS!
B >>

Re: ptyread eating CPU on Cygwin

[Peter Stephenson]

> Peter, you that select does not work under Cygwin (read_poll). I asked on
> Cygwin list, but there are no known problems and many applications do use
> select.

The problem I was having was with putting the terminal temporarily into raw
mode without the full majesty of zle and setting a timeout there (in
bin_read()).  select is working fine in the main terminal code.  I didn't
have the week necessary to resolve all the possible ways of setting
timeouts, so I gave up.  I expect it can be done.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
Cambridge Silicon Radio, Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070

PATCH: ptyread eating CPU on Cygwin

[Andrej Borsenkow]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

>
> The problem I was having was with putting the terminal temporarily into raw
> mode without the full majesty of zle and setting a timeout there (in
> bin_read()).  select is working fine in the main terminal code.  I didn't
> have the week necessary to resolve all the possible ways of setting
> timeouts, so I gave up.  I expect it can be done.
>

O.K., this is very simple patch that makes ptyread use select. Sven, could you
look at conditions there? It should be O.K. (blocking select can return either
positive value or -1 if interrupted, in which case we simply exit read loop).

With this patch 'make check' has acceptable CPU load.

Attached due to line length.

-andrej

[-- Attachment #2: zsh-zpty.diff --]
[-- Type: application/octet-stream, Size: 986 bytes --]

Index: Src/Modules/zpty.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/zpty.c,v
retrieving revision 1.11
diff -u -r1.11 zpty.c
--- Src/Modules/zpty.c	2000/06/27 14:25:05	1.11
+++ Src/Modules/zpty.c	2000/10/18 11:26:20
@@ -446,6 +446,9 @@
     int blen = 256, used = 0, ret = 1;
     char *buf = (char *) zhalloc(blen + 1);
     Patprog prog = NULL;
+#ifdef HAVE_SELECT
+    fd_set foofd;
+#endif
 
     if (*args && args[1]) {
 	char *p;
@@ -468,7 +471,16 @@
 	    if (cmd->fin)
 		break;
 	}
-	if ((ret = read(cmd->fd, buf + used, 1)) == 1) {
+#ifdef HAVE_SELECT
+        FD_ZERO(&foofd);
+        FD_SET(cmd->fd, &foofd);
+#endif
+        if (
+#ifdef HAVE_SELECT
+	    (ret = select(cmd->fd+1, (SELECT_ARG_2_T) &foofd, NULL, NULL, NULL) > 0) &&
+#endif
+
+	(ret = read(cmd->fd, buf + used, 1)) == 1) {
 	    if (++used == blen) {
 		buf = hrealloc(buf, blen, blen << 1);
 		blen <<= 1;

Re: PATCH: ptyread eating CPU on Cygwin

[Bart Schaefer]

On Oct 18,  3:30pm, Andrej Borsenkow wrote:
}
} O.K., this is very simple patch that makes ptyread use select. Sven,
} could you look at conditions there?

I'm not Sven, but HAVE_SELECT does not imply having FD_ZERO or an fd_set
typedef.

On the other hand, I've just noticed that other code assumes in several
places that it does imply those things, so it's probably OK.

However, using a NULL timeout structure in the select() call has turned a
non-blocking read into a fully blocking one.  If you're going to do that,
you might as well throw out the select() call and just set the descriptor
back to blocking state before calling read().  I suspect there's a good
reason it was a non-blocking read in the first place, though.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

RE: PATCH: ptyread eating CPU on Cygwin

[Andrej Borsenkow]

>
> I'm not Sven, but HAVE_SELECT does not imply having FD_ZERO or an fd_set
> typedef.
>
> On the other hand, I've just noticed that other code assumes in several
> places that it does imply those things, so it's probably OK.
>

Yes, it was just copy'n'paste.

> However, using a NULL timeout structure in the select() call has turned a
> non-blocking read into a fully blocking one.  If you're going to do that,
> you might as well throw out the select() call and just set the descriptor
> back to blocking state before calling read().  I suspect there's a good
> reason it was a non-blocking read in the first place, though.
>

Erm. You are right, of course.

Non-blocking read made sense in initial implemetation, because it was the
condition to exit read loop (without pattern matching) - 9390:

+    do {
+	while ((ret = read(cmd->fd, buf + used, 1)) == 1) {
+	    if (++used == blen) {
+		buf = hrealloc(buf, blen, blen << 1);
+		blen <<= 1;
+	    }
+	}
+	buf[used] = '\0';
+    } while (prog && !pattry(prog, buf));

Currently zsh tries to always read the whole input (until EOF) Actually, in
pattern-matching mode it even completely ignores EOF. That makes absolutely no
functional difference between blocking and non-blocking mode but makes
performance in non-blocking mode terrible.

As it stands now, we really can just use blocking read.

-andrej