xterm.js
xterm.js copied to clipboard
Avoid creating new ArrayBuffers when reducing the size of the buffer
See https://github.com/xtermjs/xterm.js/pull/4109#issuecomment-1243690016
I think we could actually change this:
https://github.com/xtermjs/xterm.js/blob/f9db65c52e4702cf906c14cda1c640342a7b6dca/src/common/buffer/BufferLine.ts#L335-L336
To:
this._data = this._data.subarray(0, cols * CELL_SIZE);
That will keep the same ArrayBuffer, but use a different view of it. We wouldn't free the memory but maybe we should only do that either when the view is < 1/2 the size of the buffer, or defer the clean up to an idle timeout? It doesn't make sense to do so many allocations if they're being thrown away immediately after several resizes.
We could do something similar here:
https://github.com/xtermjs/xterm.js/blob/f9db65c52e4702cf906c14cda1c640342a7b6dca/src/common/buffer/BufferLine.ts#L347-L348
@Tyriar This a really good idea, it would lower CPU stress for multiple shrinking events in a row. The halving also sounds good to me, it would also lift some burden from expanding resizes within the old upper limit.
Are the resize events de-bounced? Like the event would not get triggered for every single number, if one changes from 80 to 50 cols fast enough?
Did a quick test with a 100k scrollback terminal filled (only testing col shrinking): old ~320ms vs. new ~160ms for resize runtime - speed doubled. :smiley_cat:
Seems nodejs does not support requestIdleCallback
due to the way the libuv event loop works. So this careful shimming for the headless variant to not let it hog tons of memory.
Here a few numbers with the new approach. Again done on 100k scrollback, from shriniking/enlarging between 55 -65 cols. They should be JIT'ed (did some warmup before profiling), and show 2 +-1 col events:
-
shrink by one col:
-
enlarge by one col w'o new buffer alloc/copy (within
buffer.bytelength
): -
enlarge by one col with full buffer alloc/copy (outside of
buffer.bytelength
):
Without the patches all actions look like the last image. With the patches we gain:
- shrinking ~4x faster
- enlarging within
bytelength
~1.9x faster - enlarging outside
bytelength
- same speed as before
Further take aways:
- main cost during resize comes from alloc + data moving - alloc accounts for ~half of the runtime, avoiding all data moves gives another 2x speedup (as seen for shrinking where no data has to be moved in
BufferLine.resize
) -
getTrimmedLength
has constant runtime of ~25ms (~10%), prolly not worth be optimized further -
_reflowSmaller
has constant runtime of ~80 ms, accounting for ~45% during shrinking and 12 - 30% during enlarging
@Tyriar Idk how your reflow logic works, but wonder why _reflowSmaller
would appear for enlarging at all?
Something weird is going on during enlarging in the demo. If I increase cols by one I get the following onResize
dispatching:
Note that I changed cols to 87, which is correctly announce by the first onResize
event, but gets reverted by a second event shrinking to 86. Therefore cols in the demo is off by one (stty size
also reports 86 afterwards). For some reason the resize dispatching is messed up. It also explains why I see _reflowSmaller
above in the profiler stats.
Edit:
Oh well, this is a result of updateTerminalSize()
in client.ts
and FitAddon.fit
not yielding the same numbers, if you have a zoom level on the browser (which I had by accident). With zoomlevel 100% I dont get the re-adjustment to cols-1 anymore. So the numbers above are flawed for enlarging (subtract the _reflowSmaller
runtime). Not sure if this is worthing being fixed in client.ts
, it might create frictions on integrator side, if they c&p the code? (created #4113).
Runtimes w'o zoomlevel madness:
- shrinking ~85ms (~3x faster)
- enlarging within bounds ~93ms (~2.7x faster)
- enlarging out of bounds ~250ms (same speed)
Are the resize events de-bounced?
This is up to the embedder. It's debounced by 50ms in VS Code as we still want it to be relatively responsive, so multiple resizes can easily happen in a second.