Subject: bug#71355: 30.0.50; [PATCH] Improve performance of buffered output in Eshell

From: Jim Porter <[email-masked]>
Date: Tue, 4 Jun 2024 18:55:00 -0700

On 6/4/2024 2:52 PM, Stefan Kangas wrote:
> Nice, thanks for working on this.  Your patch makes sense to me at first
> glance, but I didn't test it.  The performance improvement definitely
> seems highly worthwhile based on your measurements.

If anyone has time to test, I'd be interested to see if the results are 
similarly good on other platforms (I'm testing on x86_64 GNU/Linux), but 
I imagine they're comparable. The main areas of improvement are:

1. Batching output into larger chunks reduces the constant-time 
overheads associated with each write to the screen.

2. Scanning for control characters in output is much faster thanks to 
're-search-forward'.

3. The above improvements made it possible to add periodic redisplays, 
which eat into the perf improvements. I mitigated this by throttling the 
redisplay to 40Hz (the slowest value I found where the throttling isn't 
obvious).

> Bonus points for adding tests, as always.

I wrote some tests for the control character fix, since my changes to 
that function were nontrivial. I'll see if I can think of a few others, 
though I'm partly relying on the now-fairly-extensive Eshell test suite 
to catch any mistakes.

>> +(defcustom eshell-buffered-print-size 2048
>> +  "The size of the print queue in characters, for doing buffered printing.
>>   This is basically a speed enhancement, to avoid blocking the Lisp code
>>   from executing while Emacs is redisplaying."
> 
> How did you decide on this value?

Basically, I tried a few different command invocations that normally 
take a while (the main ones being "cat config.log" and "ls -Al 
/usr/bin") and scaled up the value by factors of two until I stopped 
seeing any major perf improvements. (Larger values are faster still, but 
not by a whole lot.)

> Could the docstring be expanded to explain what a user can expect to
> happen if they increase or decrease this value?

Sure, that makes sense. Essentially, smaller values will be slower, but 
may update faster (subject to the redisplay throttling), whereas larger 
values are the opposite.