GNU bug report logs - #78773
[PATCH] Speedup url-retrieve-synchronously for low-latency connections

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steven Allen <steven <at> stebalien.com>
Cc: 78773 <at> debbugs.gnu.org, rpluim <at> gmail.com, larsi <at> gnus.org
Subject: Re: bug#78773: [PATCH] Speedup url-retrieve-synchronously for
 low-latency connections
Date: Sat, 14 Jun 2025 14:38:35 +0300

> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Thu, 12 Jun 2025 13:54:01 -0700
> 
> Ok, I think I've figured it out. We loop twice and wait on a timeout
> (READ_OUTPUT_DELAY_INCREMENT) the second loop because we're already done
> reading.

We wait the second time because there could be more input, so I don't
see how this could be wrong in general.  I also don't quite see the
"gotcha!" in your analysis below:

> The first time through, we hit the following, recording that we've
> gotten some output:
> 
>   https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5974

OK.

> We hit the following, setting our timeout to READ_OUTPUT_DELAY_INCREMENT:
> 
>   https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5679

This is to _shorten_ the wait timeout:

	  /* If we've got some output and haven't limited our timeout
	     with adaptive read buffering, limit it. */
	  if (got_some_output > 0 && !process_skipped
	      && (timeout.tv_sec
		  || timeout.tv_nsec > READ_OUTPUT_DELAY_INCREMENT))
	    timeout = make_timespec (0, READ_OUTPUT_DELAY_INCREMENT);

So I don't quite see how this could be a problem here.  Am I missing
something?

> Then we hit the select call and wait the full timeout (nothing to read)
> and we finally exit here (because our timeout has passed):
> 
>   https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5832

The "full timeout" being the value of READ_OUTPUT_DELAY_INCREMENT, is
that right?  Then why is it not TRT, since it _shortens_ the wait?

> I think what we need to do is modify:
> 
>   https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5978
> 
> from
> 
>     		  if (wait_proc == XPROCESS (proc))
> 		    wait = MINIMUM;
> 
> to
> 
>     		  if (!wait_proc || wait_proc == XPROCESS (proc))
> 		    wait = MINIMUM;
> 
> So that we set the timeout to 0 here:
> 
>   https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
> 
> And exit early.

Why?  That code was there for a reason.  What if we are waiting for
other processes as well?

What did I miss in the above description that explains why the current
code is wrong?

Previous Next

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