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: Sat, 14 Jun 2025 17:38:34 -0700

[Message part 1 (text/plain, inline)]

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: Sat, 14 Jun 2025 12:37:38 -0700
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> 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.
>
> Taking a step back, you never explained how come waiting for 10 msec
> causes some operation to take a full second.  Could you please
> describe what happens in more details, so that such a significant
> effect of such a small delay could be understood without knowing what
> emacs-syncthing is or does?

emacs-syncthing makes 36+ RPC calls to the local syncthing process. Each
request takes ~1-2ms on my machine so the 10ms extra delay dominates the
request time.

However, you do bring up a good point as this change (either the first
or second patch) improves performance more than I'd expect:

1. Before this patch, opening the syncthing buffer took 900ms (much
greater than the expected 36*10ms).

2. After this patch, it takes 50ms (expected given 1-2ms for 36 RPC
calls plus the time to draw the buffer).

So it would appear that there's something going on here beyond the 10ms
delay from READ_OUTPUT_DELAY_INCREMENT (more like a 25ms delay). I'll do
some more digging.

> One aspect of this which I'd like to understand is how general is this
> problem and what kind of use patterns will bump into it.  Because if
> it turns out that just emacs-syncthing is affected, a simple solution
> would be for emacs-syncthing to have its own variant of
> url-retrieve-synchronously, in which case it can do anything it likes
> with its variant of that function.  By contrast, if we want to modify
> the code in Emacs, we need at least to understand what use cases need
> that and consider their importance vs the other kinds, which might
> suffer degradation in performance due to such a change.

This applies to any case where one makes multiple sequential RPC
requests to a local server, waiting on that request via
`accept-process-output' with a nil PROCESS.

However, there appears to be an escape hatch that lets, e.g., jsonrpc
avoid this issue: you can throw an error out of
`accept-process-output'. I've attached a patch that does this and it
appears to work, but throwing from a sentinel seems sketchy.

[0001-Rewrite-url-retrieve-synchronously-to-throw-when-don.patch (text/x-patch, attachment)]

Previous Next

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