GNU bug report logs -
#78773
[PATCH] Speedup url-retrieve-synchronously for low-latency connections
Previous Next
To reply to this bug, email your comments to 78773 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 04:10:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Steven Allen <steven <at> stebalien.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Thu, 12 Jun 2025 04:10:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Tags: patch
**Present context:**
I'm running into an issue in emacs-syncthing [1] where making a few
localhost network requests are taking a full second (blocking Emacs)
when they should be instantaneous. While digging into
`url-retrieve-synchronously', I found that waiting on the correct
network process instead of passing nil to `accept-process-output' made
these network requests instantaneous (although I have no idea why). See
the attached patch.
[1]: https://github.com/KeyWeeUsr/emacs-syncthing
To test this patch, you can:
1. Run a simple web server on localhost. I usually use
python -m http.server --bind 127.0.0.1 8000
2. Evaluate: (benchmark 100 '(url-retrieve-synchronously "http://127.0.0.1:8000"))
With this patch, this form is ~16x faster on my machine. I've also
tested this against a remote machine with a ~45ms latency and found a
1.25x speedup.
I've confirmed that this isn't busy-waiting by modifying this code to
print a message each time it loops: it loops the same number of times
with or without my patch.
I've confirmed that this speedup is strictly due to passing the target
process to `accept-process-output' by applying my patch then changing
JUST "proc" to "nil":
;; ms, so split the difference.
(accept-process-output proc 0.05))
to
;; ms, so split the difference.
(accept-process-output nil 0.05))
With this one change, this code goes back to being as slow as it was before.
**Historical context**
`url-retrieve-synchronously' was changed to wait on "nil" in:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49897,
Motivated by this bug report:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49861
However, the patch in question also changed the rest of
`url-retrieve-synchronously' so I'm hoping the issue lies elsewhere?
In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, cairo version
1.18.4) of 2025-06-11 built on Laptop
Repository revision: 87244ef1661f9e9d7c08f65047551e8a34cd92b2
Repository branch: makepkg
Windowing system distributor 'The X.Org Foundation', version 11.0.12101016
System Description: Arch Linux
Configured using:
'configure
'CPPFLAGS=-I/run/user/1000/build/emacs-git/src/mps-git/build/include '
'LDFLAGS=-L/run/user/1000/build/emacs-git/src/mps-git/build/lib -Wl,-O1
-Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now
-Wl,-z,pack-relative-relocs -flto=auto' --prefix=/usr --sysconfdir=/etc
--libexecdir=/usr/lib --localstatedir=/var --mandir=/usr/share/man
--with-gameuser=:games --with-modules --without-m17n-flt
--without-selinux --without-pop --without-gconf --disable-gc-mark-trace
--with-mps=yes --enable-link-time-optimization
--with-native-compilation=yes --with-xinput2 --with-x-toolkit=no
--without-toolkit-scroll-bars --without-xaw3d --without-gsettings
--with-cairo-xcb --without-xft --with-sound=no --with-tree-sitter
--without-gpm --without-compress-install
'--program-transform-name=s/\([ec]tags\)/\1.emacs/'
'CFLAGS=-march=native -mtune=native -O3 -pipe -fno-plt -fexceptions
-Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
-fstack-clash-protection -fcf-protection -fomit-frame-pointer
-fno-math-errno -fno-trapping-math -fno-math-errno -fno-trapping-math
-flto=auto''
[0001-Speedup-url-retrieve-synchronously-for-low-latency-c.patch (text/patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 06:46:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, dick <dick.r.chiang <at> gmail.com>
> Date: Wed, 11 Jun 2025 21:08:45 -0700
> From: Steven Allen via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>
> **Present context:**
>
> I'm running into an issue in emacs-syncthing [1] where making a few
> localhost network requests are taking a full second (blocking Emacs)
> when they should be instantaneous. While digging into
> `url-retrieve-synchronously', I found that waiting on the correct
> network process instead of passing nil to `accept-process-output' made
> these network requests instantaneous (although I have no idea why). See
> the attached patch.
>
> [1]: https://github.com/KeyWeeUsr/emacs-syncthing
>
> To test this patch, you can:
>
> 1. Run a simple web server on localhost. I usually use
>
> python -m http.server --bind 127.0.0.1 8000
>
> 2. Evaluate: (benchmark 100 '(url-retrieve-synchronously "http://127.0.0.1:8000"))
>
> With this patch, this form is ~16x faster on my machine. I've also
> tested this against a remote machine with a ~45ms latency and found a
> 1.25x speedup.
>
> I've confirmed that this isn't busy-waiting by modifying this code to
> print a message each time it loops: it loops the same number of times
> with or without my patch.
>
> I've confirmed that this speedup is strictly due to passing the target
> process to `accept-process-output' by applying my patch then changing
> JUST "proc" to "nil":
>
> ;; ms, so split the difference.
> (accept-process-output proc 0.05))
>
> to
>
> ;; ms, so split the difference.
> (accept-process-output nil 0.05))
>
> With this one change, this code goes back to being as slow as it was before.
What happens if you leave the 1st argument of accept-process-output at
its current nil value, but change the 2nd argument to be 0.005 instead
of 0.05 (i.e., 10 times smaller)?
Also, how many other sub-processes (of any kind, not just network
processes) do you have in that session when you are testing this
issue?
> **Historical context**
>
> `url-retrieve-synchronously' was changed to wait on "nil" in:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49897,
>
> Motivated by this bug report:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49861
>
> However, the patch in question also changed the rest of
> `url-retrieve-synchronously' so I'm hoping the issue lies elsewhere?
Unfortunately, I doubt that we will get any useful answers to this
question. We need to understand better why asking for output from a
single process has such a dramatic effect in your case with localhost
requests. If it happens only with localhost requests, perhaps we
could make some changes only for that case.
Robert, any ideas or suggestions?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 08:42:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 78773 <at> debbugs.gnu.org (full text, mbox):
(I dropped dick chiang from the CCʼs)
>>>>> On Thu, 12 Jun 2025 09:45:17 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
Eli> What happens if you leave the 1st argument of accept-process-output at
Eli> its current nil value, but change the 2nd argument to be 0.005 instead
Eli> of 0.05 (i.e., 10 times smaller)?
That speeds it up, although not as dramatically
Eli> Also, how many other sub-processes (of any kind, not just network
Eli> processes) do you have in that session when you are testing this
Eli> issue?
The answer to that is important, since there are a couple of loops in
`wait_reading_process_output' that depend on that, and can trigger DNS
or TLS operations as a result. But Iʼve just tested this in 'emacs -Q'
with zero processes, and I see the speedup.
>> **Historical context**
>>
>> `url-retrieve-synchronously' was changed to wait on "nil" in:
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49897,
>>
>> Motivated by this bug report:
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49861
>>
>> However, the patch in question also changed the rest of
>> `url-retrieve-synchronously' so I'm hoping the issue lies elsewhere?
Eli> Unfortunately, I doubt that we will get any useful answers to this
Eli> question. We need to understand better why asking for output from a
Eli> single process has such a dramatic effect in your case with localhost
Eli> requests. If it happens only with localhost requests, perhaps we
Eli> could make some changes only for that case.
Eli> Robert, any ideas or suggestions?
The fact that itʼs localhost shouldnʼt matter. The code in question is
convoluted, as you well know, so figuring out whatʼs going on needs us
to have a good understanding of which cases trigger this
behaviour. But the evidence so far points at the handling of the
timeout.
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 11:11:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: Steven Allen <steven <at> stebalien.com>, 78773 <at> debbugs.gnu.org,
> larsi <at> gnus.org
> Date: Thu, 12 Jun 2025 10:41:41 +0200
>
> >>>>> On Thu, 12 Jun 2025 09:45:17 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>
> Eli> What happens if you leave the 1st argument of accept-process-output at
> Eli> its current nil value, but change the 2nd argument to be 0.005 instead
> Eli> of 0.05 (i.e., 10 times smaller)?
>
> That speeds it up, although not as dramatically
>
> Eli> Also, how many other sub-processes (of any kind, not just network
> Eli> processes) do you have in that session when you are testing this
> Eli> issue?
>
> The answer to that is important, since there are a couple of loops in
> `wait_reading_process_output' that depend on that, and can trigger DNS
> or TLS operations as a result. But Iʼve just tested this in 'emacs -Q'
> with zero processes, and I see the speedup.
If this happens with a single sub-process, it's strange. It means
that we somehow wait for the full timeout even though some output from
the single sub-process arrives, which should have ended the wait. Or
what am I missing?
> Eli> Unfortunately, I doubt that we will get any useful answers to this
> Eli> question. We need to understand better why asking for output from a
> Eli> single process has such a dramatic effect in your case with localhost
> Eli> requests. If it happens only with localhost requests, perhaps we
> Eli> could make some changes only for that case.
>
> Eli> Robert, any ideas or suggestions?
>
> The fact that itʼs localhost shouldnʼt matter. The code in question is
> convoluted, as you well know, so figuring out whatʼs going on needs us
> to have a good understanding of which cases trigger this
> behaviour. But the evidence so far points at the handling of the
> timeout.
In the case of a remote host, the speedup was marginal, so I think
the localhost does matter, somehow.
Perhaps instrumenting wait_reading_process_output with printf's would
help to understand the control flow in the case of nil and non-nil
PROCESS argument?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 17:28:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 78773 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> With this one change, this code goes back to being as slow as it was before.
>
> What happens if you leave the 1st argument of accept-process-output at
> its current nil value, but change the 2nd argument to be 0.005 instead
> of 0.05 (i.e., 10 times smaller)?
Now testing with:
emacs -Q --load=url.el --batch --eval \
"(benchmark 100 '(url-retrieve-synchronously \"http://127.0.0.1:8000\"))"
- Changing the timeout from 0.05 to 0.005 drops the time from 1.25
seconds to 0.63 seconds (~2x faster).
- My patch drops it to 0.056 (22x faster in a clean environment).
However note: changing the timeout from 0.05 to 0.005 makes the wait
loop spin 6 times for higher latency (non-localhost, ~40ms) requests whereas it
loops exactly once with the default timeout.
> Also, how many other sub-processes (of any kind, not just network
> processes) do you have in that session when you are testing this
> issue?
From list-processes, 13-14 when testing this originally (including network
processes, listening sockets, etc.).
>> However, the patch in question also changed the rest of
>> `url-retrieve-synchronously' so I'm hoping the issue lies elsewhere?
>
> Unfortunately, I doubt that we will get any useful answers to this
> question. We need to understand better why asking for output from a
> single process has such a dramatic effect in your case with localhost
> requests. If it happens only with localhost requests, perhaps we
> could make some changes only for that case.
Testing with a server with a 30ms latency (tested with "emacs -Q"), I get:
1. A 1.4x speedup with my patch.
2. A 1.3x speedup by simply changing the timeout.
But there's a lot more noise in the higher-latency tests.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 18:03:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 78773 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> The fact that itʼs localhost shouldnʼt matter. The code in question is
>> convoluted, as you well know, so figuring out whatʼs going on needs us
>> to have a good understanding of which cases trigger this
>> behaviour. But the evidence so far points at the handling of the
>> timeout.
>
> In the case of a remote host, the speedup was marginal, so I think
> the localhost does matter, somehow.
It looks like a latency thing. It's not that it's localhost specific,
it's that the effect is more pronounced on low-latency connections
because something, somewhere is adding a tiny delay (likely the timeout).
When I test against my router (10ms latency, how awful!), I get a 1.25x
speedup (1.2x speedup with just your timeout change).
I've also tested with IP addresses and reproduced the same results, so
it's not related to DNS lookups.
> Perhaps instrumenting wait_reading_process_output with printf's would
> help to understand the control flow in the case of nil and non-nil
> PROCESS argument?
I'll do that today. I have a suspicion that fast requests waiting on a
single process exit early here:
https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
>> But the evidence so far points at the handling of the timeout.
I can say that we're definitely NOT waiting for the timeout: increasing
the timeout (to, e.g., 1-2s) doesn't slow things down. I suspect having
a really short timeout is causing us to abort
wait_reading_process_output early before it hits some expensive part.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 18:30:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Thu, 12 Jun 2025 11:02:41 -0700
>
> > Perhaps instrumenting wait_reading_process_output with printf's would
> > help to understand the control flow in the case of nil and non-nil
> > PROCESS argument?
>
> I'll do that today. I have a suspicion that fast requests waiting on a
> single process exit early here:
>
> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
The condition for that block is
if (wait_proc
&& ! EQ (wait_proc->status, Qrun)
&& ! connecting_status (wait_proc->status))
And the comment says "Don't wait for output from a non-running
process." Is the case here that the network connection was already
closed when we read from the sub-process? I thought that was not the
case.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 19:21:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 78773 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Steven Allen <steven <at> stebalien.com>
>> Cc: 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
>> Date: Thu, 12 Jun 2025 11:02:41 -0700
>>
>> > Perhaps instrumenting wait_reading_process_output with printf's would
>> > help to understand the control flow in the case of nil and non-nil
>> > PROCESS argument?
>>
>> I'll do that today. I have a suspicion that fast requests waiting on a
>> single process exit early here:
>>
>> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
>
> The condition for that block is
>
> if (wait_proc
> && ! EQ (wait_proc->status, Qrun)
> && ! connecting_status (wait_proc->status))
>
> And the comment says "Don't wait for output from a non-running
> process." Is the case here that the network connection was already
> closed when we read from the sub-process? I thought that was not the
> case.
With my patch, it does hit that case, but on the third loop through so
it's not exiting early.
I also tried setting the timeout in `accept-process-output' to nil and
that got me the same speedup as my fix but ended up looping 23 times so
there's something really strange going on with the timeout here.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Thu, 12 Jun 2025 20:55:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 78773 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Steven Allen <steven <at> stebalien.com> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Steven Allen <steven <at> stebalien.com>
>>> Cc: 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
>>> Date: Thu, 12 Jun 2025 11:02:41 -0700
>>>
>>> > Perhaps instrumenting wait_reading_process_output with printf's would
>>> > help to understand the control flow in the case of nil and non-nil
>>> > PROCESS argument?
>>>
>>> I'll do that today. I have a suspicion that fast requests waiting on a
>>> single process exit early here:
>>>
>>> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
>>
>> The condition for that block is
>>
>> if (wait_proc
>> && ! EQ (wait_proc->status, Qrun)
>> && ! connecting_status (wait_proc->status))
>>
>> And the comment says "Don't wait for output from a non-running
>> process." Is the case here that the network connection was already
>> closed when we read from the sub-process? I thought that was not the
>> case.
>
> With my patch, it does hit that case, but on the third loop through so
> it's not exiting early.
>
> I also tried setting the timeout in `accept-process-output' to nil and
> that got me the same speedup as my fix but ended up looping 23 times so
> there's something really strange going on with the timeout here.
Ok, I think I've figured it out. We loop twice and wait on a timeout
(READ_OUTPUT_DELAY_INCREMENT) the second loop because we're already done
reading.
The first time through, we hit the following, recording that we've
gotten some output:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5974
We hit the following, setting our timeout to READ_OUTPUT_DELAY_INCREMENT:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5679
Then we hit the select call and wait the full timeout (nothing to read)
and we finally exit here (because our timeout has passed):
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5832
***
I think what we need to do is modify:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5978
from
if (wait_proc == XPROCESS (proc))
wait = MINIMUM;
to
if (!wait_proc || wait_proc == XPROCESS (proc))
wait = MINIMUM;
So that we set the timeout to 0 here:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
And exit early.
[0001-Exit-accept-process-output-early-when-we-get-output-.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 06:35:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Thu, 12 Jun 2025 12:20:30 -0700
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> I'll do that today. I have a suspicion that fast requests waiting on a
> >> single process exit early here:
> >>
> >> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
> >
> > The condition for that block is
> >
> > if (wait_proc
> > && ! EQ (wait_proc->status, Qrun)
> > && ! connecting_status (wait_proc->status))
> >
> > And the comment says "Don't wait for output from a non-running
> > process." Is the case here that the network connection was already
> > closed when we read from the sub-process? I thought that was not the
> > case.
>
> With my patch, it does hit that case, but on the third loop through so
> it's not exiting early.
How come? What is wait_proc->status in that case?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 08:59:04 GMT)
Full text and
rfc822 format available.
Message #35 received at 78773 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Thu, 12 Jun 2025 13:54:01 -0700, Steven Allen <steven <at> stebalien.com> said:
Steven> Ok, I think I've figured it out. We loop twice and wait on a timeout
Steven> (READ_OUTPUT_DELAY_INCREMENT) the second loop because we're already done
Steven> reading.
Steven> The first time through, we hit the following, recording that we've
Steven> gotten some output:
Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5974
Steven> We hit the following, setting our timeout to READ_OUTPUT_DELAY_INCREMENT:
Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5679
Steven> Then we hit the select call and wait the full timeout (nothing to read)
Steven> and we finally exit here (because our timeout has passed):
Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5832
Steven> ***
Steven> I think what we need to do is modify:
Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5978
Steven> from
Steven> if (wait_proc == XPROCESS (proc))
Steven> wait = MINIMUM;
Steven> to
Steven> if (!wait_proc || wait_proc == XPROCESS (proc))
Steven> wait = MINIMUM;
Steven> So that we set the timeout to 0 here:
Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
Steven> And exit early.
That analysis looks correct to me. It works for the original test
case, at least.
Iʼm wondering why weʼre setting the timeout to zero and calling
`pselect' again, which means recomputing the wait set again, if I
havenʼt gotten lost in the twisty maze of ifʼs and global variables
😀. Weʼve got input, why canʼt we just `break'? There might be some
work we donʼt do, but that will get done the next time
`wait_reading_process_output' is called.
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 10:49:06 GMT)
Full text and
rfc822 format available.
Message #38 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Fri, 13 Jun 2025 10:58:07 +0200
>
> >>>>> On Thu, 12 Jun 2025 13:54:01 -0700, Steven Allen <steven <at> stebalien.com> said:
>
> Steven> Ok, I think I've figured it out. We loop twice and wait on a timeout
> Steven> (READ_OUTPUT_DELAY_INCREMENT) the second loop because we're already done
> Steven> reading.
>
> Steven> The first time through, we hit the following, recording that we've
> Steven> gotten some output:
>
> Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5974
>
>
> Steven> We hit the following, setting our timeout to READ_OUTPUT_DELAY_INCREMENT:
>
> Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5679
>
> Steven> Then we hit the select call and wait the full timeout (nothing to read)
> Steven> and we finally exit here (because our timeout has passed):
>
> Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5832
>
> Steven> ***
>
> Steven> I think what we need to do is modify:
>
> Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5978
>
> Steven> from
>
> Steven> if (wait_proc == XPROCESS (proc))
> Steven> wait = MINIMUM;
>
> Steven> to
>
> Steven> if (!wait_proc || wait_proc == XPROCESS (proc))
> Steven> wait = MINIMUM;
>
> Steven> So that we set the timeout to 0 here:
>
> Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
>
> Steven> And exit early.
>
> That analysis looks correct to me. It works for the original test
> case, at least.
I didn't yet have time to look into this closely enough to have an
opinion.
> Iʼm wondering why weʼre setting the timeout to zero and calling
> `pselect' again, which means recomputing the wait set again, if I
> havenʼt gotten lost in the twisty maze of ifʼs and global variables
> 😀. Weʼve got input, why canʼt we just `break'? There might be some
> work we donʼt do, but that will get done the next time
> `wait_reading_process_output' is called.
Try looking at the Git history of this code, maybe it will tell you
how this code evolved, and bring up past discussions and bug reports
which caused us to make the code as it is today.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 12:00:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 78773 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Fri, 13 Jun 2025 13:48:08 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>> Iʼm wondering why weʼre setting the timeout to zero and calling
>> `pselect' again, which means recomputing the wait set again, if I
>> havenʼt gotten lost in the twisty maze of ifʼs and global variables
>> 😀. Weʼve got input, why canʼt we just `break'? There might be some
>> work we donʼt do, but that will get done the next time
>> `wait_reading_process_output' is called.
Eli> Try looking at the Git history of this code, maybe it will tell you
Eli> how this code evolved, and bring up past discussions and bug reports
Eli> which caused us to make the code as it is today.
Yes, itʼs gnarly. I think the following would be ok, and it passes the
test case for Bug#20978.
But I canʼt detect any difference in Stevenʼs test case, so maybe we
let sleeping pselectʼs lie.
Robert
--
diff --git a/src/process.c b/src/process.c
index e61ec425f7e..0119330f7fe 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5934,6 +5934,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
channel_start
= process_prioritize_lower_fds ? 0 : last_read_channel + 1;
+ bool break_now = false;
for (channel = channel_start, wrapped = false;
!wrapped || (channel < channel_start && channel <= max_desc);
channel++)
@@ -5975,8 +5976,11 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
if (nread > 0)
{
/* Vacuum up any leftovers without waiting. */
- if (wait_proc == XPROCESS (proc))
- wait = MINIMUM;
+ if (!wait_proc || wait_proc == XPROCESS (proc))
+ {
+ wait = MINIMUM;
+ break_now = true;
+ }
/* Since read_process_output can run a filter,
which can call accept-process-output,
don't try to read from any other processes
@@ -6119,6 +6123,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
}
}
} /* End for each file descriptor. */
+ if (break_now) /* Don't rerun select. */
+ break;
} /* End while exit conditions not met. */
unbind_to (count, Qnil);
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 14:46:03 GMT)
Full text and
rfc822 format available.
Message #44 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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: Thu, 12 Jun 2025 12:20:30 -0700
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> I'll do that today. I have a suspicion that fast requests waiting on a
>> >> single process exit early here:
>> >>
>> >> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
>> >
>> > The condition for that block is
>> >
>> > if (wait_proc
>> > && ! EQ (wait_proc->status, Qrun)
>> > && ! connecting_status (wait_proc->status))
>> >
>> > And the comment says "Don't wait for output from a non-running
>> > process." Is the case here that the network connection was already
>> > closed when we read from the sub-process? I thought that was not the
>> > case.
>>
>> With my patch, it does hit that case, but on the third loop through so
>> it's not exiting early.
>
> How come? What is wait_proc->status in that case?
Note: this is WITH my original patch (to url.el), the one that makes it
only wait on one process.
In that case, the status is "run" the 1-2 loops through, then "exit" on
the last loop (triggering this branch).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 14:52:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 78773 <at> debbugs.gnu.org (full text, mbox):
Robert Pluim <rpluim <at> gmail.com> writes:
>>>>>> On Fri, 13 Jun 2025 13:48:08 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>
> >> Iʼm wondering why weʼre setting the timeout to zero and calling
> >> `pselect' again, which means recomputing the wait set again, if I
> >> havenʼt gotten lost in the twisty maze of ifʼs and global variables
> >> 😀. Weʼve got input, why canʼt we just `break'? There might be some
> >> work we donʼt do, but that will get done the next time
> >> `wait_reading_process_output' is called.
>
> Eli> Try looking at the Git history of this code, maybe it will tell you
> Eli> how this code evolved, and bring up past discussions and bug reports
> Eli> which caused us to make the code as it is today.
>
> Yes, itʼs gnarly. I think the following would be ok, and it passes the
> test case for Bug#20978.
>
> But I canʼt detect any difference in Stevenʼs test case, so maybe we
> let sleeping pselectʼs lie.
I think this patch will cause us to the final drain loop when waiting on
a single connection. It's probably fine, but I assume this drain loop
existed for a reason?
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=bec823b107ef7d3b51b8e430ccab82c81bd63d24#n5525
> Robert
> --
> diff --git a/src/process.c b/src/process.c
> index e61ec425f7e..0119330f7fe 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -5934,6 +5934,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> channel_start
> = process_prioritize_lower_fds ? 0 : last_read_channel + 1;
>
> + bool break_now = false;
> for (channel = channel_start, wrapped = false;
> !wrapped || (channel < channel_start && channel <= max_desc);
> channel++)
> @@ -5975,8 +5976,11 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> if (nread > 0)
> {
> /* Vacuum up any leftovers without waiting. */
> - if (wait_proc == XPROCESS (proc))
> - wait = MINIMUM;
> + if (!wait_proc || wait_proc == XPROCESS (proc))
> + {
> + wait = MINIMUM;
> + break_now = true;
> + }
> /* Since read_process_output can run a filter,
> which can call accept-process-output,
> don't try to read from any other processes
> @@ -6119,6 +6123,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> }
> }
> } /* End for each file descriptor. */
> + if (break_now) /* Don't rerun select. */
> + break;
> } /* End while exit conditions not met. */
>
> unbind_to (count, Qnil);
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 15:00:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 78773 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Fri, 13 Jun 2025 07:50:59 -0700, Steven Allen <steven <at> stebalien.com> said:
Steven> I think this patch will cause us to the final drain loop when waiting on
Steven> a single connection. It's probably fine, but I assume this drain loop
Steven> existed for a reason?
Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=bec823b107ef7d3b51b8e430ccab82c81bd63d24#n5525
It will, but weʼll already have read from the process at line 5972 or
thereabouts.
Since I canʼt definitively say it wonʼt break anything, we should go
with the minimal patch, ie yours.
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 15:15:06 GMT)
Full text and
rfc822 format available.
Message #53 received at 78773 <at> debbugs.gnu.org (full text, mbox):
Robert Pluim <rpluim <at> gmail.com> writes:
>>>>>> On Fri, 13 Jun 2025 07:50:59 -0700, Steven Allen <steven <at> stebalien.com> said:
>
> Steven> I think this patch will cause us to the final drain loop when waiting on
> Steven> a single connection. It's probably fine, but I assume this drain loop
> Steven> existed for a reason?
>
> Steven> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=bec823b107ef7d3b51b8e430ccab82c81bd63d24#n5525
>
> It will, but weʼll already have read from the process at line 5972 or
> thereabouts.
>
> Since I canʼt definitively say it wonʼt break anything, we should go
> with the minimal patch, ie yours.
Your approach is correct, I think. It just may be less performant as the
user will now need to accept output twice when we could have just
"finished the job" the first time around.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 15:32:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 78773 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Fri, 13 Jun 2025 08:14:41 -0700, Steven Allen <steven <at> stebalien.com> said:
>>
>> Since I canʼt definitively say it wonʼt break anything, we should go
>> with the minimal patch, ie yours.
Steven> Your approach is correct, I think. It just may be less performant as the
Steven> user will now need to accept output twice when we could have just
Steven> "finished the job" the first time around.
Weʼll let Eli decide 😺
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 15:49:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Fri, 13 Jun 2025 07:45:25 -0700
>
>
> 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: Thu, 12 Jun 2025 12:20:30 -0700
> >>
> >> Eli Zaretskii <eliz <at> gnu.org> writes:
> >>
> >> >> I'll do that today. I have a suspicion that fast requests waiting on a
> >> >> single process exit early here:
> >> >>
> >> >> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
> >> >
> >> > The condition for that block is
> >> >
> >> > if (wait_proc
> >> > && ! EQ (wait_proc->status, Qrun)
> >> > && ! connecting_status (wait_proc->status))
> >> >
> >> > And the comment says "Don't wait for output from a non-running
> >> > process." Is the case here that the network connection was already
> >> > closed when we read from the sub-process? I thought that was not the
> >> > case.
> >>
> >> With my patch, it does hit that case, but on the third loop through so
> >> it's not exiting early.
> >
> > How come? What is wait_proc->status in that case?
>
> Note: this is WITH my original patch (to url.el), the one that makes it
> only wait on one process.
>
> In that case, the status is "run" the 1-2 loops through, then "exit" on
> the last loop (triggering this branch).
So your patch is only useful when the sub-process exits soon after
producing its single chunk of output, is that right?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Fri, 13 Jun 2025 17:46:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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: Fri, 13 Jun 2025 07:45:25 -0700
>>
>>
>> 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: Thu, 12 Jun 2025 12:20:30 -0700
>> >>
>> >> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >>
>> >> >> I'll do that today. I have a suspicion that fast requests waiting on a
>> >> >> single process exit early here:
>> >> >>
>> >> >> https://https.git.savannah.gnu.org/cgit/git/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5562
>> >> >
>> >> > The condition for that block is
>> >> >
>> >> > if (wait_proc
>> >> > && ! EQ (wait_proc->status, Qrun)
>> >> > && ! connecting_status (wait_proc->status))
>> >> >
>> >> > And the comment says "Don't wait for output from a non-running
>> >> > process." Is the case here that the network connection was already
>> >> > closed when we read from the sub-process? I thought that was not the
>> >> > case.
>> >>
>> >> With my patch, it does hit that case, but on the third loop through so
>> >> it's not exiting early.
>> >
>> > How come? What is wait_proc->status in that case?
>>
>> Note: this is WITH my original patch (to url.el), the one that makes it
>> only wait on one process.
>>
>> In that case, the status is "run" the 1-2 loops through, then "exit" on
>> the last loop (triggering this branch).
>
> So your patch is only useful when the sub-process exits soon after
> producing its single chunk of output, is that right?
Not quite. That condition will trigger on the last call to
`accept-process-output` when a PROCESS is specified, regardless of how
short the total response was.
But there's a more to the story; let's start at the top.
First, the actual bug: When called with no PROCESS,
`accept-process-output' would wait an additional 10ms
(READ_OUTPUT_DELAY_INCREMENT) after successfully reading data from some
(any) process. `url-retrieve-synchronously' was slow for localhost calls
because 10ms is a long time for fast requests. This bug would add 10ms
of latency to every request.
My SECOND patch (0772d96d, the one to process.c) fixes this
underlying bug by setting the timeout to 0 in this case: when we manage
to read data from some process and aren't waiting on any specific
process.
My FIRST patch (4529f63f, the one to `url-retrieve-synchronously')
worked around this bug by specifying the process to wait on. With that
patch I happened to hit the branch in question because the requests were
fast enough to complete in one call to `accept-process-output'. However,
the FIRST patch would still have worked around the underlying bug (the
issue fixed in the SECOND patch) because the FIRST patch specifies the
process to wait on and the underlying bug is only triggered when the
process is NOT specified.
Next steps:
1. The SECOND patch fixes the actual bug and should be installed to fix
the underlying bug (assuming I correctly diagnosed the problem/solution,
but I'm pretty sure I did).
2. Personally, I would install the FIRST patch as well as it avoids
returning to `url-retrieve-synchronously' until we've actually read
relevant data. This is how `url-retrieve-synchronously' behaved before
bug#49897 changed it to work around then undiagnosed bugs in
`accept-process-output'. However, that's very much a maintainer decision
because it might cause the bug in `accept-process-output' that motivated
this change (bug#49861) to rear its head again.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 11:39:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Thu, 12 Jun 2025 13:54:01 -0700
>
> Ok, I think I've figured it out. We loop twice and wait on a timeout
> (READ_OUTPUT_DELAY_INCREMENT) the second loop because we're already done
> reading.
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:
> The first time through, we hit the following, recording that we've
> gotten some output:
>
> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5974
OK.
> We hit the following, setting our timeout to READ_OUTPUT_DELAY_INCREMENT:
>
> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5679
This is to _shorten_ the wait timeout:
/* If we've got some output and haven't limited our timeout
with adaptive read buffering, limit it. */
if (got_some_output > 0 && !process_skipped
&& (timeout.tv_sec
|| timeout.tv_nsec > READ_OUTPUT_DELAY_INCREMENT))
timeout = make_timespec (0, READ_OUTPUT_DELAY_INCREMENT);
So I don't quite see how this could be a problem here. Am I missing
something?
> Then we hit the select call and wait the full timeout (nothing to read)
> and we finally exit here (because our timeout has passed):
>
> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5832
The "full timeout" being the value of READ_OUTPUT_DELAY_INCREMENT, is
that right? Then why is it not TRT, since it _shortens_ the wait?
> I think what we need to do is modify:
>
> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5978
>
> from
>
> if (wait_proc == XPROCESS (proc))
> wait = MINIMUM;
>
> to
>
> if (!wait_proc || wait_proc == XPROCESS (proc))
> wait = MINIMUM;
>
> So that we set the timeout to 0 here:
>
> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
>
> And exit early.
Why? That code was there for a reason. What if we are waiting for
other processes as well?
What did I miss in the above description that explains why the current
code is wrong?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 11:46:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: steven <at> stebalien.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Fri, 13 Jun 2025 13:59:40 +0200
>
> >>>>> On Fri, 13 Jun 2025 13:48:08 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>
> >> Iʼm wondering why weʼre setting the timeout to zero and calling
> >> `pselect' again, which means recomputing the wait set again, if I
> >> havenʼt gotten lost in the twisty maze of ifʼs and global variables
> >> 😀. Weʼve got input, why canʼt we just `break'? There might be some
> >> work we donʼt do, but that will get done the next time
> >> `wait_reading_process_output' is called.
>
> Eli> Try looking at the Git history of this code, maybe it will tell you
> Eli> how this code evolved, and bring up past discussions and bug reports
> Eli> which caused us to make the code as it is today.
>
> Yes, itʼs gnarly. I think the following would be ok, and it passes the
> test case for Bug#20978.
Please explain why you think this change is okay, by talking us
through what will happen with it in the case in pint.
I generally don't like to touch this code with a 3-mile pole, and if
this means the current use case will run slowly, so be it. So I need
a clear evidence that (a) the current code is wrong (not just
sub-optimal, but wrong), and (b) that it can never affect other cases
handled by wait_reading_process_output, of which there are a legion.
> But I canʼt detect any difference in Stevenʼs test case, so maybe we
> let sleeping pselectʼs lie.
You are saying that Stevenʼs patch also doesn't break bug#20978? If
so, why did you post a different one?
And I don't yet understand why short-circuiting like Steven suggests
is TRT.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 11:53:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org
> Date: Fri, 13 Jun 2025 10:45:06 -0700
>
> First, the actual bug: When called with no PROCESS,
> `accept-process-output' would wait an additional 10ms
> (READ_OUTPUT_DELAY_INCREMENT) after successfully reading data from some
> (any) process. `url-retrieve-synchronously' was slow for localhost calls
> because 10ms is a long time for fast requests. This bug would add 10ms
> of latency to every request.
But it's an unneeded delay only if there's no more output from the
subprocess during those 10 ms, isn't that so? And we don't really
know if all the process output was read, do we?
> My SECOND patch (0772d96d, the one to process.c) fixes this
> underlying bug by setting the timeout to 0 in this case: when we manage
> to read data from some process and aren't waiting on any specific
> process.
But what if we are also waiting for output from other processes? Your
patch will slow those down, won't it? In you case, you only care
about a single process, but that's not so in general.
> Next steps:
>
> 1. The SECOND patch fixes the actual bug and should be installed to fix
> the underlying bug (assuming I correctly diagnosed the problem/solution,
> but I'm pretty sure I did).
I disagree for now. I need to understand better why you think it's
TRT in the general case.
> 2. Personally, I would install the FIRST patch as well as it avoids
> returning to `url-retrieve-synchronously' until we've actually read
> relevant data.
What if url-retrieve was called for more than a single connection?
Won't your FIRST patch cause a regression in that case?
> This is how `url-retrieve-synchronously' behaved before
> bug#49897 changed it to work around then undiagnosed bugs in
> `accept-process-output'.
Exactly. I'm unwilling to risk reintroducing bugs we already fixed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 16:16:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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: Thu, 12 Jun 2025 13:54:01 -0700
>>
>> Ok, I think I've figured it out. We loop twice and wait on a timeout
>> (READ_OUTPUT_DELAY_INCREMENT) the second loop because we're already done
>> reading.
>
> 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, but we
do know that there's no more available right now because we poll pselect
with a zero timeout when `wait' is `MINIMAL'. The following check
prevents us from exiting the loop (in this case) when there are file
descriptors with waiting data, regardless of what the timeout is:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=bec823b107ef7d3b51b8e430ccab82c81bd63d24#n5810
There is still a design question of how this SHOULD behave (discussed
at the end).
>> The first time through, we hit the following, recording that we've
>> gotten some output:
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5974
>
> OK.
>
>> We hit the following, setting our timeout to READ_OUTPUT_DELAY_INCREMENT:
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5679
>
> This is to _shorten_ the wait timeout:
>
> /* If we've got some output and haven't limited our timeout
> with adaptive read buffering, limit it. */
> if (got_some_output > 0 && !process_skipped
> && (timeout.tv_sec
> || timeout.tv_nsec > READ_OUTPUT_DELAY_INCREMENT))
> timeout = make_timespec (0, READ_OUTPUT_DELAY_INCREMENT);
>
> So I don't quite see how this could be a problem here. Am I missing
> something?
Nothing is wrong here, I'm explaining why we're adding exactly 10ms to
every call to `accept-process-output' with a nil PROCESS. The fact that
it only decreases the timeout also explains why your patch to reduce the
timeout in `url-retrieve-synchronously' makes this faster (as long as
the new timeout is less than 10ms).
>> Then we hit the select call and wait the full timeout (nothing to read)
>> and we finally exit here (because our timeout has passed):
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5832
>
> The "full timeout" being the value of READ_OUTPUT_DELAY_INCREMENT, is
> that right? Then why is it not TRT, since it _shortens_ the wait?
Yes.
>
>> I think what we need to do is modify:
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5978
>>
>> from
>>
>> if (wait_proc == XPROCESS (proc))
>> wait = MINIMUM;
>>
>> to
>>
>> if (!wait_proc || wait_proc == XPROCESS (proc))
>> wait = MINIMUM;
>>
>> So that we set the timeout to 0 here:
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
>>
>> And exit early.
>
> Why? That code was there for a reason. What if we are waiting for
> other processes as well?
>
> What did I miss in the above description that explains why the current
> code is wrong?
My understanding is that `accept-process-output' is supposed to return
immediately when output has been read BOTH when PROCESS is nil and when
it is non-nil. If this is not the case, then:
1. We now understand why passing a nil process to
`accept-process-output' is slower than passing a specific process.
2. The fix to the original bug report is to apply the first patch I sent
to have `url-retrieve-synchronously' wait on a specific process.
Looking at the history, it looks like the code in question was changed
in bug#20978. However, from what I can tell, we should only apply
adaptive read buffering when `process-adaptive-read-buffering' is
non-nil. I think we may need to change the check in question to:
if (!process_output_skip && (!wait_proc || wait_proc == XPROCESS (proc)))
wait = MINIMUM;
But I'm very much not sure about that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 16:26:06 GMT)
Full text and
rfc822 format available.
Message #77 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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: Fri, 13 Jun 2025 10:45:06 -0700
>>
>> First, the actual bug: When called with no PROCESS,
>> `accept-process-output' would wait an additional 10ms
>> (READ_OUTPUT_DELAY_INCREMENT) after successfully reading data from some
>> (any) process. `url-retrieve-synchronously' was slow for localhost calls
>> because 10ms is a long time for fast requests. This bug would add 10ms
>> of latency to every request.
>
> But it's an unneeded delay only if there's no more output from the
> subprocess during those 10 ms, isn't that so? And we don't really
> know if all the process output was read, do we?
(discussed in the other email)
>> My SECOND patch (0772d96d, the one to process.c) fixes this
>> underlying bug by setting the timeout to 0 in this case: when we manage
>> to read data from some process and aren't waiting on any specific
>> process.
>
> But what if we are also waiting for output from other processes? Your
> patch will slow those down, won't it? In you case, you only care
> about a single process, but that's not so in general.
(discussed in the other email)
>> Next steps:
>>
>> 1. The SECOND patch fixes the actual bug and should be installed to fix
>> the underlying bug (assuming I correctly diagnosed the problem/solution,
>> but I'm pretty sure I did).
>
> I disagree for now. I need to understand better why you think it's
> TRT in the general case.
(discussed in the other email)
>> 2. Personally, I would install the FIRST patch as well as it avoids
>> returning to `url-retrieve-synchronously' until we've actually read
>> relevant data.
>
> What if url-retrieve was called for more than a single connection?
> Won't your FIRST patch cause a regression in that case?
I assume you're talking about redirects here? That's the only
multiple-process case I know about. In this case, it'll just return back
to the top of the `url-retrieve-synchronously' loop (same as it would
before) and wait on the next process.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 16:35:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> 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 09:15:00 -0700
>
> Eli Zaretskii <eliz <at> gnu.org> 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".
> My understanding is that `accept-process-output' is supposed to return
> immediately when output has been read BOTH when PROCESS is nil and when
> it is non-nil.
That's true, but wait_reading_process_output is called not just by
accept-process-output, it is called by several other callers, and in
that case it is not necessarily true that we need to exit immediately
upon the first available channel. Keep in mind that reading from a
subprocess also invokes a filter function, if there is one, so it
might make sense in some cases to read from more than one source,
perhaps even from all of the ones that have output ready to be read.
> Looking at the history, it looks like the code in question was changed
> in bug#20978. However, from what I can tell, we should only apply
> adaptive read buffering when `process-adaptive-read-buffering' is
> non-nil. I think we may need to change the check in question to:
>
> if (!process_output_skip && (!wait_proc || wait_proc == XPROCESS (proc)))
> wait = MINIMUM;
That could indeed fly, but process-adaptive-read-buffering is non-nil
by default. Does url set it to nil?
In any case, I think the addition of process-adaptive-read-buffering
test should only affect the case when wait_proc is zero.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 17:05:03 GMT)
Full text and
rfc822 format available.
Message #83 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> 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 09:25:40 -0700
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > What if url-retrieve was called for more than a single connection?
> > Won't your FIRST patch cause a regression in that case?
>
> I assume you're talking about redirects here? That's the only
> multiple-process case I know about. In this case, it'll just return back
> to the top of the `url-retrieve-synchronously' loop (same as it would
> before) and wait on the next process.
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 18:25:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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 09:25:40 -0700
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > What if url-retrieve was called for more than a single connection?
>> > Won't your FIRST patch cause a regression in that case?
>>
>> I assume you're talking about redirects here? That's the only
>> multiple-process case I know about. In this case, it'll just return back
>> to the top of the `url-retrieve-synchronously' loop (same as it would
>> before) and wait on the next process.
>
> 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).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 18:30:03 GMT)
Full text and
rfc822 format available.
Message #89 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> 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 11:23:58 -0700
>
> Eli Zaretskii <eliz <at> gnu.org> 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. So I suggest to explore other ways of
solving this first.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 19:38:03 GMT)
Full text and
rfc822 format available.
Message #92 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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 11:23:58 -0700
>>
>> Eli Zaretskii <eliz <at> gnu.org> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 20:02:04 GMT)
Full text and
rfc822 format available.
Message #95 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> 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?
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 20:13:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 78773 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
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 09:15:00 -0700
>>
>> Eli Zaretskii <eliz <at> gnu.org> 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.
The exception to this rule is adaptive read buffering where explicitly
_don't_ want to yield too little data at a time if we expect we'll
receive more in the near future. I agree we should wait a bit longer in
that case (the attached patch should handle that).
>> My understanding is that `accept-process-output' is supposed to return
>> immediately when output has been read BOTH when PROCESS is nil and when
>> it is non-nil.
>
> That's true, but wait_reading_process_output is called not just by
> accept-process-output, it is called by several other callers, and in
> that case it is not necessarily true that we need to exit immediately
> upon the first available channel. Keep in mind that reading from a
> subprocess also invokes a filter function, if there is one, so it
> might make sense in some cases to read from more than one source,
> perhaps even from all of the ones that have output ready to be read.
>
>> Looking at the history, it looks like the code in question was changed
>> in bug#20978. However, from what I can tell, we should only apply
>> adaptive read buffering when `process-adaptive-read-buffering' is
>> non-nil. I think we may need to change the check in question to:
>>
>> if (!process_output_skip && (!wait_proc || wait_proc == XPROCESS (proc)))
>> wait = MINIMUM;
>
> 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).
> In any case, I think the addition of process-adaptive-read-buffering
> test should only affect the case when wait_proc is zero.
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.
[0001-Exit-accept-process-output-early-when-we-get-output-.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sat, 14 Jun 2025 20:52:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 78773 <at> debbugs.gnu.org (full text, mbox):
Steven Allen <steven <at> stebalien.com> writes:
>
> 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.
>
> From 8ab170b8ad4718e6d5d12bbc85e9bcda02fffc0d Mon Sep 17 00:00:00 2001
> From: Steven Allen <steven <at> stebalien.com>
> Date: Sat, 14 Jun 2025 10:05:05 -0700
> Subject: [PATCH] Exit accept-process-output early when we get output from any
> process
>
This last patch is definitely buggy (causes random hangs) and didn't
make the performance any faster. I'm going to poke at this code a bit
more.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sun, 15 Jun 2025 00:39:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 78773 <at> debbugs.gnu.org (full text, mbox):
[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)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sun, 15 Jun 2025 05:18:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org, Ian Kelling
> <ian <at> iankelling.org>
> 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 <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 09:15:00 -0700
> >>
> >> Eli Zaretskii <eliz <at> gnu.org> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sun, 15 Jun 2025 16:57:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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, Ian Kelling
>> <ian <at> iankelling.org>
>> 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 <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 09:15:00 -0700
>> >>
>> >> Eli Zaretskii <eliz <at> gnu.org> 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.
Hence my second paragraph:
>> The exception to this rule is adaptive read buffering where explicitly
>> _don't_ want to yield too little data at a time if we expect we'll
>> receive more in the near future. I agree we should wait a bit longer in
>> that case (the attached patch should handle that).
When adaptive read buffering is enabled, if we read less then 256 bytes
from one or more processes, we wait a bit for more output to arrive from
these processes (the process_output_skip flag is set to 1 when the
adaptive read buffering algorithm kicks in):
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=ebdad09c5a0a822acb52ec58b3319d77d156f0ce#n5646
However, the current code unconditionally waits 10ms when we're not
applying the adaptive read algorithm:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=ebdad09c5a0a822acb52ec58b3319d77d156f0ce#n5679
>> > 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'.
(see my first response at the top of this message)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Sun, 15 Jun 2025 17:09:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org, ian <at> iankelling.org
> Date: Sun, 15 Jun 2025 09:55:52 -0700
>
> Eli Zaretskii <eliz <at> gnu.org> 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.
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.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Mon, 16 Jun 2025 07:41:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org, ian <at> iankelling.org
> Date: Sun, 15 Jun 2025 09:55:52 -0700
>
> However, the current code unconditionally waits 10ms when we're not
> applying the adaptive read algorithm:
>
> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=ebdad09c5a0a822acb52ec58b3319d77d156f0ce#n5679
No, that's NOT what that code does. It _reduces_ the _existing_
timeout to 10 ms if it is longer than 10 ms. So it can only _shorten_
the wait, it can never add a wait that wasn't there. Here's the code
again:
/* If we've got some output and haven't limited our timeout
with adaptive read buffering, limit it. */
if (got_some_output > 0 && !process_skipped
&& (timeout.tv_sec
|| timeout.tv_nsec > READ_OUTPUT_DELAY_INCREMENT))
timeout = make_timespec (0, READ_OUTPUT_DELAY_INCREMENT);
This clearly leaves any existing wait that is shorter than 10 ms
unaltered.
> > 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'm not talking about the reason _why_ the problem happens, I'm
talking about which parts of the code it is okay to touch to fix the
problem to minimize the risk of regressions in unrelated scenarios.
If the problem happens only when process-adaptive-read-buffering is
nil, let's not change code that's used when it's non-nil, and vice
versa.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Mon, 16 Jun 2025 15:52:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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, ian <at> iankelling.org
>> Date: Sun, 15 Jun 2025 09:55:52 -0700
>>
>> Eli Zaretskii <eliz <at> gnu.org> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Mon, 16 Jun 2025 16:11:03 GMT)
Full text and
rfc822 format available.
Message #122 received at 78773 <at> debbugs.gnu.org (full text, mbox):
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, ian <at> iankelling.org
>> Date: Sun, 15 Jun 2025 09:55:52 -0700
>>
>> However, the current code unconditionally waits 10ms when we're not
>> applying the adaptive read algorithm:
>>
>> https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=ebdad09c5a0a822acb52ec58b3319d77d156f0ce#n5679
>
> No, that's NOT what that code does. It _reduces_ the _existing_
> timeout to 10 ms if it is longer than 10 ms. So it can only _shorten_
> the wait, it can never add a wait that wasn't there. Here's the code
> again:
>
> /* If we've got some output and haven't limited our timeout
> with adaptive read buffering, limit it. */
> if (got_some_output > 0 && !process_skipped
> && (timeout.tv_sec
> || timeout.tv_nsec > READ_OUTPUT_DELAY_INCREMENT))
> timeout = make_timespec (0, READ_OUTPUT_DELAY_INCREMENT);
>
> This clearly leaves any existing wait that is shorter than 10 ms
> unaltered.
Fair enough, we can be more precise here:
(a) after reading output from any process,
(b) when we didn't skip any of those processes due to adaptive read
buffering,
(c) when we're not waiting on any specific process†,
it waits an additional (after reading the output) 10ms or the remainder
of the timeout, whichever is shorter, before returning to the user.
My point is that it should reduce the timeout to 0 in this case because
we've just read process output (got_some_output > 0) and we KNOW that we
haven't read too little output from any of the processes we've read from
because process_skipped is false (i.e., we did NOT skip a process
because it had too little output available for us to read).
† If we are waiting on a specific process, we will have set wait to
MINIMAL here (on the previous pass through the loop):
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5979
And then the timeout will already have been set to 0 here:
https://cgit.git.savannah.gnu.org/cgit/emacs.git/tree/src/process.c?h=81a3e4e51167be51c63eae682331210bc62f7280#n5439
>> > 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'm not talking about the reason _why_ the problem happens, I'm
> talking about which parts of the code it is okay to touch to fix the
> problem to minimize the risk of regressions in unrelated scenarios.
> If the problem happens only when process-adaptive-read-buffering is
> nil, let's not change code that's used when it's non-nil, and vice
> versa.
The problem happens regardless of the value of
process-adaptive-read-buffering.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#78773; Package
emacs.
(Mon, 16 Jun 2025 16:27:03 GMT)
Full text and
rfc822 format available.
Message #125 received at 78773 <at> debbugs.gnu.org (full text, mbox):
> From: Steven Allen <steven <at> stebalien.com>
> Cc: rpluim <at> gmail.com, 78773 <at> debbugs.gnu.org, larsi <at> gnus.org, ian <at> iankelling.org
> Date: Mon, 16 Jun 2025 09:10:28 -0700
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > /* If we've got some output and haven't limited our timeout
> > with adaptive read buffering, limit it. */
> > if (got_some_output > 0 && !process_skipped
> > && (timeout.tv_sec
> > || timeout.tv_nsec > READ_OUTPUT_DELAY_INCREMENT))
> > timeout = make_timespec (0, READ_OUTPUT_DELAY_INCREMENT);
> >
> > This clearly leaves any existing wait that is shorter than 10 ms
> > unaltered.
>
> Fair enough, we can be more precise here:
>
> (a) after reading output from any process,
>
> (b) when we didn't skip any of those processes due to adaptive read
> buffering,
>
> (c) when we're not waiting on any specific process†,
>
> it waits an additional (after reading the output) 10ms or the remainder
> of the timeout, whichever is shorter, before returning to the user.
Agreed.
> My point is that it should reduce the timeout to 0 in this case because
> we've just read process output (got_some_output > 0) and we KNOW that we
> haven't read too little output from any of the processes we've read from
> because process_skipped is false (i.e., we did NOT skip a process
> because it had too little output available for us to read).
I think you are looking at this only from the POV of how fast should
the caller of wait_reading_process_output return. But that's not the
only use of this function: it is also used in applications where
reading large amounts of sub-process output should be as fast as
possible. In those other applications, waiting for a short while in
case there's another chunk soon might be preferable, for minimizing
the time it takes to read the entire output from the sub-process.
> >> > 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'm not talking about the reason _why_ the problem happens, I'm
> > talking about which parts of the code it is okay to touch to fix the
> > problem to minimize the risk of regressions in unrelated scenarios.
> > If the problem happens only when process-adaptive-read-buffering is
> > nil, let's not change code that's used when it's non-nil, and vice
> > versa.
>
> The problem happens regardless of the value of
> process-adaptive-read-buffering.
Once again, I'm talking about which case the modifications should
affect. If url-retrieve-synchronously always uses the nil value, then
whatever changes we make should not affect the case when this variable
is non-nil, because then we are running the risk of introducing
unintended regressions.
This bug report was last modified today.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.