Repository navigation
Conversation
runewav1
added a commit
to runewav1/xterm.js
that referenced
this pull request
Oct 6, 2026
apply a change introduced by a pull request on the main xterm.js repository, proposed by @slbls (thanks!) to make scrolling operations faster/more efficient within margin by recycling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scrolling within DECSTBM margins allocates a new
BufferLineon every line feed, even though the line leaving the region is discarded. This change reuses that line instead, as the full-screen path already does withrecycle():shiftElementsoverwrites the region's top line. That line is now captured first, reset withcopyFrom(newLine, true), and placed at the bottom.splicetrims the oldest line, which is now reused as the inserted line.Benchmarks
Using
xterm-benchmarkin the same way asTerminal.benchmark.ts, I wrote a 4.3 MBls -lR /System/Library/Frameworkscapture into an 80×25CoreBrowserTerminal, once at full screen, once inside a top margin of2;25, and once inside a bottom margin of1;24. Figures are medians across 6 sessions of 30 runs per build, alternating builds in ABBA order, with 95% bootstrap intervals. This was on Node 26 and an Apple M5.masterAn
--evalrun against amasterbaseline reported +41% and +29% throughput for the margin cases and −0.4% for full screen.The existing
Terminal.benchmark.tscases were unchanged across the same sessions, at 1.00 [0.99, 1.01] forwrite/stringand 1.00 [0.81, 1.23] forwrite/utf8. On macOS,ls -lR /usr/libproduces only about 50 KB, so single--evalruns of those cases varied by up to 9% even between twomasterbuilds.Replaying vtebench's region scrolling payloads, which scroll one short line at a time, through headless bundles built with
esbuild-package-headless-onlyshowed larger gains. These are medians across 6 processes of 8 samples of 8 MiB each on a 158×41 grid:masterscrolling_bottom_region1;40scrolling_bottom_small_region1;20scrolling_top_region2;41scrolling_top_small_region20;41On
master, CPU time exceeds wall time for these payloads, since the allocations keep V8's GC threads busy. The other 8 vtebench payloads don't reach these paths and stayed within 0.96–1.02.If it would help, I can add margin variants of the
ls -lR /usr/libcase toTerminal.benchmark.tsin a follow-up.Tests
Two new tests in
BufferService.test.tscover repeated scrolls within top and bottom margins, writing into the recycled row between scrolls, and a bottom-only margin with a full buffer. Both also pass onmaster. Inserting the cached blank line directly instead of recycling fails the first new test, along with the existing DECSTBM and alt-screen tests.In the demo, scrolling 2,000 lines inside a top margin and 30,000 inside a bottom margin with a full scrollback left the same buffer on
masterand on this branch.Overlap with #5797
#5797 rewrites
scroll()aroundLogicalLine, so this will conflict. Its region path still allocates a line per scroll, so the same reuse could apply there. I'm happy to port this onto it or hold off, whichever you prefer.