Eli Zaretskii writes: >> From: Steven Allen >> Cc: rpluim@gmail.com, 78773@debbugs.gnu.org, larsi@gnus.org >> Date: Sat, 14 Jun 2025 12:37:38 -0700 >> >> Eli Zaretskii 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.