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

From: Steven Allen <[email-masked]>
Date: Sat, 14 Jun 2025 12:37:38 -0700

Eli Zaretskii <[email-masked]> writes:

>> From: Steven Allen <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked]
>> Date: Sat, 14 Jun 2025 11:23:58 -0700
>> 
>> Eli Zaretskii <[email-masked]> writes:
>> 
>> > No, I mean other callers of the same low-level functionality, for
>> > example, url-retrieve.
>> >
>> > Once again, the C function where you propose changes is used in a wide
>> > variety of scenarios, and several clients of it could be active at the
>> > same time.
>> 
>> In that case, `accept-process-output' will process output for those
>> other processes as usual (calling filters/sentinels, etc.) but won't
>> return call until we have data that's relevant to this call to
>> `url-retrieve-synchronously'. We haven't set JUST-THIS-ONE in this call
>> to `accept-process-output' so we won't simply *ignore* all other process
>> output.
>> 
>> To be clear, calling `accept-process-output' with a nil process is the
>> exception, not the norm (8 out of 105 call sites).
>
> That might be, but in this particular case we changed the call to give
> it the nil argument for a reason, and it will be hard to convince me
> to go back on that decision.

It was changed in 93e1248c2085dfb675d7ed916ec5621e3fe6e2c6 as a blind
attempt to fix a hard to reproduce bug. The commit message was:

    * lisp/url/url.el (url-retrieve-synchronously): Use
    `accept-process-output' on a null process.  That is the safer, more
    conventional way of achieving non-blocking sleep-for (bug#49897).

I understand if you want to leave well enough alone, but I also want to
find SOME way to fix this bug. Which leads us to...

> So I suggest to explore other ways of solving this first.

This is what I've been trying to do. There are only two ways to fix
this:

1. Change `accept-process-output' to not unconditionally wait an
additional 10ms after receiving output when PROCESS is nil "just in
case" there's some additional data to read.

2. Change `url-retrieve-synchronously' pass a non-nil PROCESS to
`accept-process-output' thereby avoiding the additional 10ms wait.

I've been trying to go down the first path (I think that *is* the
correct path) but fixing it will necessarily require
`accept-process-output` to return immediately after receiving output
when PROCESS is nil, instead of waiting around in case some additional
output arrives.