WIP: replace _combined map and string cache with _chars variable - #6160
Open
PerBothner wants to merge 24 commits into
Open
PerBothner wants to merge 24 commits into
PerBothner wants to merge 24 commits into
Conversation
Instead to change isWrapped state use a Buffer.setWrapped method. This allows for potential flexibility in how BufferLine and line-wrapping are implemented.
A BufferLine is now the sub-range of a LogicalLine for a specific visible line, while LogicalLine is independent of window width.
This is used by InputHandler.print to "batch" multiple characters.
This typically reduces memory usage, allocation, and copying.
Also restored CircularList.recycle, pending possible future use - though the initial attempt actually slowed things down.
Seems slightly faster to recycle BufferLine but not LogicalLine.
The image addon now builds but has some problems still.
I think this is slightly cleaner and possibly faster, since no cloning needed.
(Still a good chunk to go.)
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
1 participant
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.
[This is a work-in-progress - there are a number of testsuite failures that I haven't debugged yet.]
This is an unfinished implementation of the
_charsproposal from this comment, which is copied/edited below.This builds on the
LogicalLinePR #5797.Quick summary: add a new field
_charstoLogicalLinewhich replaces both the_combinedmap and the string cache.The idea is that each cell is in one of two modes (see the comment before enum Content in common/buffer/Constants.ts): Either the existing way (a codepoint as a 21-bit value in the _data array), or as substring of the
_charsstring. SeegetStringinLogicalLine. All combined characters would use the latter variant. But so do all other characters after translateToString is called, which also sets the _charsIsTextValue of the LogicalLine. If that property is true, all cells use the substring-of-_chars representations and all the substrings are in order. Thus translateToString can trivially and safely return the _chars string. If translateToString is called when _charsIsTextValue is false, we do basically the same computation as the current implementation (using a StringBuilder). So _chars functions as a cache for translateToString, and _charsIsTextValue indicates if the cache is valid. There is no need to trim the string cache because the amount of space for the _chars string is quite modest and I believe more than made up by removing the _combined map and StringCacheEntry, simpler data structures, simpler gc (no need for a bunch of expensive WeakRefs), and better memory locality.@jerch mentioned earlier a concern that using a string for cell character data is very expensive in the presense of updates. But this hybrid approach avoids that issue: We generally don't update the _chars string during normal writes. (A combined characher is handled by just appending at the end of _chars.) However, the _chars array gets normalized when the line is rendered or translateToString is called for other reasons.