GNU bug report logs -
#78773
[PATCH] Speedup url-retrieve-synchronously for low-latency connections
Previous Next
Full log
Message #122 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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, ian <at> iankelling.org
>> Date: Sun, 15 Jun 2025 09:55:52 -0700
>>
>> However, the current code unconditionally waits 10ms when we're not
>> applying the adaptive read algorithm:
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=ebdad09c5a0a822acb52ec58b3319d77d156f0ce#n5679
>
> No, that's NOT what that code does. It _reduces_ the _existing_
> timeout to 10 ms if it is longer than 10 ms. So it can only _shorten_
> the wait, it can never add a wait that wasn't there. Here's the code
> again:
>
> /* 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);
>
> This clearly leaves any existing wait that is shorter than 10 ms
> unaltered.
Fair enough, we can be more precise here:
(a) after reading output from any process,
(b) when we didn't skip any of those processes due to adaptive read
buffering,
(c) when we're not waiting on any specific process†,
it waits an additional (after reading the output) 10ms or the remainder
of the timeout, whichever is shorter, before returning to the user.
My point is that it should reduce the timeout to 0 in this case because
we've just read process output (got_some_output > 0) and we KNOW that we
haven't read too little output from any of the processes we've read from
because process_skipped is false (i.e., we did NOT skip a process
because it had too little output available for us to read).
†If we are waiting on a specific process, we will have set wait to
MINIMAL here (on the previous pass through the loop):
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5979
And then the timeout will already have been set to 0 here:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
>> > Can we please agree that as long as we think the problem happens due
>> > to some specific feature of the code, such as
>> > process-adaptive-read-buffering, the fix should be limited _only_ to
>> > that feature, and should not affect any other uses? Because that's
>> > the only way to make changes in this code without risking regressions.
>>
>> I don't think the problem happens because of
>> `process-adaptive-read-buffering'. I think the problem happens because,
>> in an attempt to make adaptive read buffering work again, an
>> unconditional 10ms delay was added to `accept-process-output'.
>
> I'm not talking about the reason _why_ the problem happens, I'm
> talking about which parts of the code it is okay to touch to fix the
> problem to minimize the risk of regressions in unrelated scenarios.
> If the problem happens only when process-adaptive-read-buffering is
> nil, let's not change code that's used when it's non-nil, and vice
> versa.
The problem happens regardless of the value of
process-adaptive-read-buffering.
This bug report was last modified today.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.