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

From: Eli Zaretskii <[email-masked]>
Date: Sun, 15 Jun 2025 08:17:25 +0300

> From: Steven Allen <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked], Ian Kelling
>  <[email-masked]>
> Date: Sat, 14 Jun 2025 13:12:00 -0700
> 
> Ian (CCed): I've run into an issue where `accept-process-output' is
> always waiting an extra 10ms (READ_OUTPUT_DELAY_INCREMENT) after
> receiving output as long as the PROCESS passed to it is nil. This
> appears to be related to the change you made in bug#20978 and I'm
> wondering if you can shed more light on when `accept-process-output'
> should wait for more input.

Yes, Ian's input will be appreciated, especially if he can explain the
rationale for this part of the patch in enough detail for us to
understand what parts of it are essential and which aren't.

> Eli Zaretskii <[email-masked]> writes:
> 
> >> From: Steven Allen <[email-masked]>
> >> Cc: [email-masked], [email-masked], [email-masked]
> >> Date: Sat, 14 Jun 2025 09:15:00 -0700
> >>
> >> Eli Zaretskii <[email-masked]> writes:
> >>
> >> > 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:
> >>
> >> There is no available input. There might be more input in 10ms
> >
> > That's what I meant by "there could be more input".
> 
> I'd rather make use of what we know: we don't know if we'll get more
> data in the near future but we *do* know that we've just received some
> data that the caller (of `accept-process-output`) may want to handle. So
> we might as well return to the caller and give them a chance to do
> something about it.

This would be wrong.  In most cases, especially those where
performance matters, sub-process output arrives in many chunks, and
there almost always is another chunk coming very soon.  That's how I
understand the rationale for READ_OUTPUT_DELAY_INCREMENT stuff and its
use.

> > 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?

> 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.

Thanks.