Skip to content

Speed up BufferApiView.getLine and remove BufferLineApiView - #6193

Merged
jerch merged 2 commits into
xtermjs:masterfrom
PerBothner:BufferApi
Oct 12, 2026
Merged

jerch merged 2 commits into
xtermjs:masterfrom
PerBothner:BufferApi

Conversation

@PerBothner

Copy link
Copy Markdown
Contributor

The IBufferApi in the "public API" has a getLine method which is implemented by (on each call) creating a new BufferLineApiView object as a proxy for a BufferLine. This is quite expensive. Other parts of the API just coerce the internal interface to the public API interface. We can't coerce BufferLine to a IBufferLineApi because BufferLine doesn't implement getCell. However, it is trivial to add getCell, so I did.

runewav1 added a commit to runewav1/xterm.js that referenced this pull request Oct 7, 2026
Speed up BufferApiView.getLine and remove BufferLineApiView (thanks @PerBothner)
@PerBothner

Copy link
Copy Markdown
Contributor Author

This is part of the large PR #6160.

@jerch jerch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting rid of costly abstractions is a good thing. ❤️

One minor nag: Please add a comment in the bufferline source, that the internal API is directly exposed to public as a warning. I am not sure, if the public API paths are fully tested and the least we want is surprising sideeffects on public API from internal changes.

@PerBothner

Copy link
Copy Markdown
Contributor Author

Not sure I'd say "the internal API is directly exposed to public" - you still have to do a cast - and I suspect you have to go via unknown: pline as unknown as BufferLine.

As opposed to the old code: pline._line using the _line pointer to the internal BufferLine object.

It seems weird adding a comment in BufferLine about the public API, since in general the internal APIs don't mention the public APIs.

@jerch
jerch merged commit 5f141a3 into xtermjs:master Oct 12, 2026
12 checks passed
@PerBothner
PerBothner deleted the BufferApi branch October 13, 2026 02:32
@neomparam neomparam mentioned this pull request Oct 19, 2026
2 of 9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants