Subject: Re: bug#78773: [PATCH] Speedup url-retrieve-synchronously for low-latency connections

From: Steven Allen <[email-masked]>
Date: Mon, 16 Jun 2025 08:50:56 -0700

Eli Zaretskii <[email-masked]> writes:

>> From: Steven Allen <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked], [email-masked]
>> Date: Sun, 15 Jun 2025 09:55:52 -0700
>> 
>> Eli Zaretskii <[email-masked]> writes:
>> 
>> >> > That could indeed fly, but process-adaptive-read-buffering is non-nil
>> >> > by default.  Does url set it to nil?
>> >>
>> >> It is nil by default as of bug#75574 (8a669b6be523e043423b81571a8c94cb49ccc8e5).
>> >
>> > So the problem happens only with the master branch, and doesn't exist
>> > in Emacs 30 and older?
>> 
>> The problem being discussed here happens in the latest release as well.
>> I'm simply pointing out that your assertion that
>> process-adaptive-read-buffering is non-nil by default is incorrect on
>> the latest master so we're all on the same page.
>> 
>> Additionally, I should note that adaptive read buffering only applies to
>> system processes and pipes, not network connections (it's not enabled in
>> `make-network-process') so it's not relevant to
>> `url-retrieve-synchronously'. I only bring it up is that I agree that
>> it's important to preserve its behavior in
>> `wait_reading_process_output'.
>> 
>> >> Looking at this more, I'm becoming more and more convinced that this was
>> >> a bug inadvertently introduced by
>> >> 12a2691fb254db20607b2067a12b795a6d7c6649. That patch was trying to make
>> >> adaptive read buffering work again but it went too far and applied the
>> >> delay everywhere. I've attached what I think is the correct patch, but
>> >> I've also CCed Ian (author of #20978 and the associated patch) in hopes
>> >> of getting a bit more context here.
>> >
>> > This cannot be TRT, as you have already discovered.
>> >
>> > 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 don't disagree, I'm just saying that if the problem happens only
> when process-adaptive-read-buffering is (or should be) nil, then
> whatever changes we discuss should not touch the cases where
> process-adaptive-read-buffering is non-nil.

Fair enough. I'm saying that the problem happens regardless of the value
of `process-adaptive-read-buffering'.

> Did you have a chance to investigate how come these small waits add up
> to a full second?  I think understanding that is very important for
> the decision what fixes should be considered.

Not yet, but that's next on my agenda.