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

Previous Next

Full log

View this message in rfc822 format

From: Steven Allen <steven <at> stebalien.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78773 <at> debbugs.gnu.org, rpluim <at> gmail.com, larsi <at> gnus.org
Subject: bug#78773: [PATCH] Speedup url-retrieve-synchronously for low-latency connections
Date: Fri, 13 Jun 2025 10:45:06 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Steven Allen <steven <at> stebalien.com>
>> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
>> Date: Fri, 13 Jun 2025 07:45:25 -0700
>>
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> 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 12:20:30 -0700
>> >>
>> >> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >>
>> >> >> I'll do that today. I have a suspicion that fast requests waiting on a
>> >> >> single process exit early here:
>> >> >>
>> >> >> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
>> >> >
>> >> > The condition for that block is
>> >> >
>> >> >       if (wait_proc
>> >> > 	  && ! EQ (wait_proc->status, Qrun)
>> >> > 	  && ! connecting_status (wait_proc->status))
>> >> >
>> >> > And the comment says "Don't wait for output from a non-running
>> >> > process."  Is the case here that the network connection was already
>> >> > closed when we read from the sub-process?  I thought that was not the
>> >> > case.
>> >>
>> >> With my patch, it does hit that case, but on the third loop through so
>> >> it's not exiting early.
>> >
>> > How come?  What is wait_proc->status in that case?
>>
>> Note: this is WITH my original patch (to url.el), the one that makes it
>> only wait on one process.
>>
>> In that case, the status is "run" the 1-2 loops through, then "exit" on
>> the last loop (triggering this branch).
>
> So your patch is only useful when the sub-process exits soon after
> producing its single chunk of output, is that right?

Not quite. That condition will trigger on the last call to
`accept-process-output` when a PROCESS is specified, regardless of how
short the total response was.

But there's a more to the story; let's start at the top.

First, the actual bug: When called with no PROCESS,
`accept-process-output' would wait an additional 10ms
(READ_OUTPUT_DELAY_INCREMENT) after successfully reading data from some
(any) process. `url-retrieve-synchronously' was slow for localhost calls
because 10ms is a long time for fast requests. This bug would add 10ms
of latency to every request.

My SECOND patch (0772d96d, the one to process.c) fixes this
underlying bug by setting the timeout to 0 in this case: when we manage
to read data from some process and aren't waiting on any specific
process.

My FIRST patch (4529f63f, the one to `url-retrieve-synchronously')
worked around this bug by specifying the process to wait on. With that
patch I happened to hit the branch in question because the requests were
fast enough to complete in one call to `accept-process-output'. However,
the FIRST patch would still have worked around the underlying bug (the
issue fixed in the SECOND patch) because the FIRST patch specifies the
process to wait on and the underlying bug is only triggered when the
process is NOT specified.

Next steps:

1. The SECOND patch fixes the actual bug and should be installed to fix
the underlying bug (assuming I correctly diagnosed the problem/solution,
but I'm pretty sure I did).

2. Personally, I would install the FIRST patch as well as it avoids
returning to `url-retrieve-synchronously' until we've actually read
relevant data. This is how `url-retrieve-synchronously' behaved before
bug#49897 changed it to work around then undiagnosed bugs in
`accept-process-output'. However, that's very much a maintainer decision
because it might cause the bug in `accept-process-output' that motivated
this change (bug#49861) to rear its head again.

Previous Next

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