GNU bug report logs - #23864
25.1.50; Poor network responsivenes using open-network-stream

Previous Next

Package: emacs;

Reported by: Christer Ekholm <che <at> chrekh.se>

Date: Tue, 28 Jun 2016 19:56:01 UTC

Severity: normal

Tags: patch

Found in version 25.1.50

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23864 in the body.
You can then email your comments to 23864 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Tue, 28 Jun 2016 19:56:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Christer Ekholm <che <at> chrekh.se>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 28 Jun 2016 19:56:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christer Ekholm <che <at> chrekh.se>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1.50; Poor network responsivenes using open-network-stream
Date: Tue, 28 Jun 2016 21:24:49 +0200
[Message part 1 (text/plain, inline)]
Hi,

I decided to test the next release, and observed that a elisp package
i'm using got very poor networ responsivenes.

I have bisected to commit
 ad236260 Avoid duplicate calls to current_timespec

And, The attached patch fixes it for me

[0001-Fix-condition-for-return-from-waiting-for-process-ou.patch (text/plain, inline)]
From ce0d739141770f885527a9f1a2db8ff79599ae11 Mon Sep 17 00:00:00 2001
From: Christer Ekholm <che <at> chrekh.se>
Date: Sun, 26 Jun 2016 01:53:42 +0200
Subject: [PATCH] Fix condition for return from waiting for process output.

* src/process.c (wait_reading_process_output): Move the test for
process output to outside setting of cmp_time, so that we don't miss
to exit if we have output, but got_output_end_time is not valid.
---
 src/process.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/process.c b/src/process.c
index ed0c529..23e2c52 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5271,12 +5271,13 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
              timers.  */
 	  if (wait < TIMEOUT)
 	    break;
+	  if (!process_skipped && got_some_output > 0
+	      && (!timespec_valid_p (got_output_end_time)
+		  || (timeout.tv_sec > 0 || timeout.tv_nsec > 0)))
+	    break;
 	  struct timespec cmp_time
 	    = (wait == TIMEOUT
 	       ? end_time
-	       : (!process_skipped && got_some_output > 0
-		  && (timeout.tv_sec > 0 || timeout.tv_nsec > 0))
-	       ? got_output_end_time
 	       : invalid_timespec ());
 	  if (timespec_valid_p (cmp_time))
 	    {
-- 
2.9.0

[Message part 3 (text/plain, inline)]

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.6)
 of 2016-06-28 built on jane
Repository revision: 2adc4ccd03d24660bcf7f8ff056c7f32b92b584d
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
System Description:	Gentoo Base System release 2.2

Configured using:
 'configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --disable-dependency-tracking
 --disable-silent-rules --docdir=/usr/share/doc/emacs-vcs-25.1.9999-r1
 --htmldir=/usr/share/doc/emacs-vcs-25.1.9999-r1/html
 --libdir=/usr/lib64 --program-suffix=-emacs-25-vcs
 --infodir=/usr/share/info/emacs-25-vcs --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --with-gameuser=:gamestat --without-compress-install
 --with-file-notification=inotify --disable-acl --with-dbus
 --without-gpm --without-hesiod --without-kerberos --without-kerberos5
 --with-xml2 --without-selinux --with-gnutls --without-wide-int
 --with-zlib --with-sound=alsa --with-x --without-ns --without-gconf
 --without-gsettings --with-toolkit-scroll-bars --with-gif --with-jpeg
 --with-png --with-rsvg --without-tiff --with-xpm --with-imagemagick
 --without-xft --without-cairo --without-libotf --without-m17n-flt
 --with-x-toolkit=gtk3 --without-xwidgets
 GENTOO_PACKAGE=app-editors/emacs-vcs-25.1.9999-r1'

Configured features:
XPM JPEG GIF PNG RSVG IMAGEMAGICK SOUND DBUS NOTIFY GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_MESSAGES: C
  value of $LANG: sv_SE.utf8
  locale-coding-system: utf-8-unix

Major mode: Info

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(misearch multi-isearch jka-compr info pp shadow sort mail-extr emacsbug
message puny seq byte-opt gv bytecomp byte-compile cl-extra help-mode
cconv cl-loaddefs pcase cl-lib dired dired-loaddefs format-spec rfc822
mml easymenu mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils site-gentoo w3m-load time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core term/tty-colors frame
cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese charscript
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote dbusbind inotify
dynamic-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 132582 36234)
 (symbols 48 21171 6)
 (miscs 40 121 488)
 (strings 32 24668 10672)
 (string-bytes 1 782873)
 (vectors 16 14829)
 (vector-slots 8 461770 9460)
 (floats 8 188 429)
 (intervals 56 5222 1)
 (buffers 976 15)
 (heap 1024 46495 2574))

-- 
 Christer

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Thu, 30 Jun 2016 01:51:01 GMT) Full text and rfc822 format available.

Message #8 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 23864 <at> debbugs.gnu.org, Christer Ekholm <che <at> chrekh.se>
Subject: Re: bug#23864: 25.1.50;
 Poor network responsivenes using open-network-stream
Date: Wed, 29 Jun 2016 21:50:47 -0400
Christer Ekholm wrote:

> I decided to test the next release, and observed that a elisp package
> i'm using got very poor networ responsivenes.
>
> I have bisected to commit
>  ad236260 Avoid duplicate calls to current_timespec


I see this is in emacs-25, so perhaps should be fixed there.


> And, The attached patch fixes it for me
>
>>From ce0d739141770f885527a9f1a2db8ff79599ae11 Mon Sep 17 00:00:00 2001
> From: Christer Ekholm <che <at> chrekh.se>
> Date: Sun, 26 Jun 2016 01:53:42 +0200
> Subject: [PATCH] Fix condition for return from waiting for process output.
>
> * src/process.c (wait_reading_process_output): Move the test for
> process output to outside setting of cmp_time, so that we don't miss
> to exit if we have output, but got_output_end_time is not valid.
> ---
>  src/process.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/process.c b/src/process.c
> index ed0c529..23e2c52 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -5271,12 +5271,13 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
>               timers.  */
>  	  if (wait < TIMEOUT)
>  	    break;
> +	  if (!process_skipped && got_some_output > 0
> +	      && (!timespec_valid_p (got_output_end_time)
> +		  || (timeout.tv_sec > 0 || timeout.tv_nsec > 0)))
> +	    break;
>  	  struct timespec cmp_time
>  	    = (wait == TIMEOUT
>  	       ? end_time
> -	       : (!process_skipped && got_some_output > 0
> -		  && (timeout.tv_sec > 0 || timeout.tv_nsec > 0))
> -	       ? got_output_end_time
>  	       : invalid_timespec ());
>  	  if (timespec_valid_p (cmp_time))
>  	    {
> -- 
> 2.9.0
>
> In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.6)
>  of 2016-06-28 built on jane
> Repository revision: 2adc4ccd03d24660bcf7f8ff056c7f32b92b584d
> Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
> System Description:	Gentoo Base System release 2.2




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Sun, 03 Jul 2016 09:03:02 GMT) Full text and rfc822 format available.

Message #11 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 23864 <at> debbugs.gnu.org, Christer Ekholm <che <at> chrekh.se>
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Sun, 3 Jul 2016 11:01:45 +0200
[Message part 1 (text/plain, inline)]
On 06/30/2016 03:50 AM, Glenn Morris wrote:
> I see this is in emacs-25, so perhaps should be fixed there. 

I agree; it's a regression and the fix is small and localized. Thanks, 
Christer, for debugging and reporting this.

I installed the attached patch into master; it should have the same 
effect as Christer's patch but avoids some code duplication. I can 
backport it into the emacs-25 branch if there is no objection.

Ouch. Now I see that this patch has the wrong bug number (transposed 
digits) and attribution. Sorry about that. I will fix that for the 
backport, if we decide to install it into emacs-25.
[0001-Fix-open-network-stream-responsiveness.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Sun, 03 Jul 2016 11:22:01 GMT) Full text and rfc822 format available.

Message #14 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Christer Ekholm <che <at> chrekh.se>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Glenn Morris <rgm <at> gnu.org>, 23864 <at> debbugs.gnu.org
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Sun, 3 Jul 2016 13:21:09 +0200
Paul Eggert writes:
 > On 06/30/2016 03:50 AM, Glenn Morris wrote:
 > > I see this is in emacs-25, so perhaps should be fixed there. 
 > 
 > I agree; it's a regression and the fix is small and localized. Thanks, 
 > Christer, for debugging and reporting this.
 > 
 > I installed the attached patch into master; it should have the same 
 > effect as Christer's patch but avoids some code duplication. I can 
 > backport it into the emacs-25 branch if there is no objection.

I have now tried 1e97ecb, but it didn't help. I observe the same
slowness as without it.

I'll examine further and report back here if I figure something out.

-- 
 Christer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Sun, 03 Jul 2016 15:23:02 GMT) Full text and rfc822 format available.

Message #17 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Christer Ekholm <che <at> chrekh.se>
Cc: 23864 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#23864: 25.1.50;
 Poor network responsivenes using open-network-stream
Date: Sun, 03 Jul 2016 18:21:45 +0300
> From: Christer Ekholm <che <at> chrekh.se>
> Date: Sun, 3 Jul 2016 13:21:09 +0200
> Cc: 23864 <at> debbugs.gnu.org
> 
> Paul Eggert writes:
>  > On 06/30/2016 03:50 AM, Glenn Morris wrote:
>  > > I see this is in emacs-25, so perhaps should be fixed there. 
>  > 
>  > I agree; it's a regression and the fix is small and localized. Thanks, 
>  > Christer, for debugging and reporting this.
>  > 
>  > I installed the attached patch into master; it should have the same 
>  > effect as Christer's patch but avoids some code duplication. I can 
>  > backport it into the emacs-25 branch if there is no objection.
> 
> I have now tried 1e97ecb, but it didn't help. I observe the same
> slowness as without it.

Isn't that strange?  The commit made exactly the change you proposed,
didn't it?

> I'll examine further and report back here if I figure something out.

Thanks.  When we understand what needs to be changed, we will be able
to decide whether it's safe for the release branch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Sun, 03 Jul 2016 18:55:02 GMT) Full text and rfc822 format available.

Message #20 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Christer Ekholm <che <at> chrekh.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23864 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#23864: 25.1.50;
 Poor network responsivenes using open-network-stream
Date: Sun, 3 Jul 2016 20:54:39 +0200
Eli Zaretskii writes:
 > 
 > Isn't that strange?  The commit made exactly the change you proposed,
 > didn't it?

Apparently not. But it was not easy to spot the difference. In my
try the test was added before the "if (WAIT == timeout)".

 > 
 > > I'll examine further and report back here if I figure something out.

I have tested some more now, and a simple change from "else if" to
just "if" does help.

diff --git a/src/process.c b/src/process.c
index 376e872..031967e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5275,7 +5275,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
            break;
          else if (wait == TIMEOUT)
            cmp_time = end_time, have_cmp_time = true;
-         else if (!process_skipped && got_some_output > 0
+         if (!process_skipped && got_some_output > 0
                   && (timeout.tv_sec > 0 || timeout.tv_nsec > 0))
            {
              if (!timespec_valid_p (got_output_end_time))

-- 
 Christer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Sun, 03 Jul 2016 22:52:02 GMT) Full text and rfc822 format available.

Message #23 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Christer Ekholm <che <at> chrekh.se>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 23864 <at> debbugs.gnu.org
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Mon, 4 Jul 2016 00:51:10 +0200
[Message part 1 (text/plain, inline)]
On 07/03/2016 08:54 PM, Christer Ekholm wrote:
> I have tested some more now, and a simple change from "else if" to
> just "if" does help.
>

Thanks, that fixes part of the remaining bug, but I see another case 
where there's a problem: we need to test the minimum of the two end 
times against "now". I installed the attached further patch into 
'master', which I hope puts this all to rest on 'master'. It is indeed a 
tricky area.
[0001-Re-fix-open-network-stream-responsiveness.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Mon, 04 Jul 2016 07:35:01 GMT) Full text and rfc822 format available.

Message #26 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Christer Ekholm <che <at> chrekh.se>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 23864 <at> debbugs.gnu.org
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Mon, 4 Jul 2016 09:34:21 +0200
Paul Eggert writes:
 > On 07/03/2016 08:54 PM, Christer Ekholm wrote:
 > > I have tested some more now, and a simple change from "else if" to
 > > just "if" does help.
 > >
 > 
 > Thanks, that fixes part of the remaining bug, but I see another case 
 > where there's a problem: we need to test the minimum of the two end 
 > times against "now". I installed the attached further patch into 
 > 'master', which I hope puts this all to rest on 'master'. It is indeed a 
 > tricky area.

I have tested commit 838f122, and it works for me. Thanks.


-- 
 Christer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Mon, 04 Jul 2016 14:36:02 GMT) Full text and rfc822 format available.

Message #29 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 23864 <at> debbugs.gnu.org, che <at> chrekh.se
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Mon, 04 Jul 2016 17:35:29 +0300
> Cc: 23864 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 4 Jul 2016 00:51:10 +0200
> 
> Thanks, that fixes part of the remaining bug, but I see another case 
> where there's a problem: we need to test the minimum of the two end 
> times against "now". I installed the attached further patch into 
> 'master', which I hope puts this all to rest on 'master'. It is indeed a 
> tricky area.

Thanks.

Do you consider these changes safe for back-porting to emacs-25?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Tue, 05 Jul 2016 18:45:01 GMT) Full text and rfc822 format available.

Message #32 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23864 <at> debbugs.gnu.org, che <at> chrekh.se
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Tue, 5 Jul 2016 20:43:48 +0200
[Message part 1 (text/plain, inline)]
On 07/04/2016 04:35 PM, Eli Zaretskii wrote:
> Do you consider these changes safe for back-porting to emacs-25?

Yes. Propose patch to emacs-25 attached.
[0001-Fix-open-network-stream-responsiveness.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#23864; Package emacs. (Tue, 05 Jul 2016 19:47:02 GMT) Full text and rfc822 format available.

Message #35 received at 23864 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 23864 <at> debbugs.gnu.org, che <at> chrekh.se
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Tue, 05 Jul 2016 22:46:08 +0300
> Cc: che <at> chrekh.se, 23864 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 5 Jul 2016 20:43:48 +0200
> 
> On 07/04/2016 04:35 PM, Eli Zaretskii wrote:
> > Do you consider these changes safe for back-porting to emacs-25?
> 
> Yes. Propose patch to emacs-25 attached.

Thanks, please push there.




Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Tue, 05 Jul 2016 23:11:02 GMT) Full text and rfc822 format available.

Notification sent to Christer Ekholm <che <at> chrekh.se>:
bug acknowledged by developer. (Tue, 05 Jul 2016 23:11:02 GMT) Full text and rfc822 format available.

Message #40 received at 23864-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23864-done <at> debbugs.gnu.org, che <at> chrekh.se
Subject: Re: bug#23864: 25.1.50; Poor network responsivenes using
 open-network-stream
Date: Wed, 6 Jul 2016 01:10:05 +0200
On 07/05/2016 09:46 PM, Eli Zaretskii wrote:
> please push there.

Done, and closing the bug report.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 03 Aug 2016 11:24:03 GMT) Full text and rfc822 format available.

This bug report was last modified 7 years and 239 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.