Add caching strategy for BufferLine.translateToString

[Tyriar]

Idea: At a minimum, store scrollback lines (since they're immutable) for use later. This will ease perf in search and other advanced use cases in embedders.

[PerBothner]

If we store the result of translateToString, we can consider getting rid of the _combined array. A string is likely more compact than an array.

The cell code field can be replaced by an offset into the translated string. That can free up a few extra bits, depending of the maximum offset we allow.

Furthermore, CellData.getChars and BufferLine.getString becomes substring operations, which are simpler and probably cheaper.

The Dom renderer calls CellData.getChars for each cell (or pair of cells for wide characters), and concatenates a "run" of cell strings. This can be easily optimized into a single substring operation for the whole run.

This idea works even better combined with the LogicalLine idea - but does not depend on it.

[PerBothner]

Suppose we store the translated string in a _stringData property, and get rid of the _combined array. We replace the the code part of a cell with the end offset in the _stringData. The start offset is 0 for cell 0; other other cells it is the end of the the previous cell.

public getString(index: number): string {
  return this._stringData.substring(index >= 0 ? this._stringEndOffset(index - 1) : 0,
    this._stringEndOffset(index);
}
_stringEndOffset(index: number): number {
  return this._data[index * CELL_SIZE + Cell.CONTENT] & Content.OFFSET_MASK;
}

[Tyriar]

If we store the result of translateToString, we can consider getting rid of the _combined array. A string is likely more compact than an array.

_combined is an edge case, when present there's a little memory loss from the object and it's a little slower, but that only impacts these special unoptimized lines. This is one of those things that I'm not too keen on changing the foundation too much due to the impact v risk trade off, as opposed to just adding a low risk layer of caching on top.

The Dom renderer calls CellData.getChars for each cell (or pair of cells for wide characters), and concatenates a "run" of cell strings. This can be easily optimized into a single substring operation for the whole run.

Wouldn't this fall over on zero width characters/multiple spaces/etc. The grid alignment piece is an important part of the DOM renderer

[PerBothner]

Wouldn't this fall over on zero width characters/multiple spaces/etc. The grid alignment piece is an important part of the DOM renderer

I don't understand how zero width characters/multiple spaces are a problem: The idea is the text string would be implicitly represented by a (start-offset, end-offset) pair. If the text += chars concatenation is valid it should be equally valid to just adjust the end-ofset value, though there may be a slight complication for edge cases with '\xa0' and invisible characters. Checking the widthCache adds a further complication: You still have to call getChars for each cell (unless you also change the WidthCache API non-trivially). The question then is whether saving a concatenation is worth an extra substring operation.

[Tyriar]

Didn't meant to start a discussion around this, but to do a targeted fix solve a performance problem in vscode/cursor. #5864 once polished should do the job.

You might be right that it may be more optimal of a solution for _combined

[jerch]

I think we should avoid restoring the old buffer layout by accident. It will be much slower for almost every task except for those that need to search on JS strings.

So while I like the idea to introduce a JS string cache for cells that will not change anymore, we also should watch the impact of those cached lines on overall memory usage and GC pressure. Because that will go up again. (and was a major reason the old buffer layout with JS strings was so hard to get performant)

[PerBothner]

I don't know the "old buffer layout".
It might make sense to have two representation of a BufferLine/LogicalLine:

• A "dynamic" representation optimized for random access and updates. The current representation can work, though it can be tweaked since space usage would not be a concern.
• A compact "static" representation. The actual text is the translated string. The attributes only store ones that change, similar to issue Proposal: more compact and flexible BufferLine data structure #4800 or PR BufferLine rewrite [WIP] #4928. (However, we don't need to cache complications of PR BufferLine rewrite [WIP] #4928.) The _data array (or its replacement) is only as long as needed. The _data array needs to encode offsets into the _text string, but can be optimized: For example "30 single-wide non-composed BMP characters with the current attributes" would use one entry in the _data array, rather than 3*30. This representation is optimized for compactness, and sequential-access whole-line processing, including rendering, search, and serialization.

The "normal" representation is the "static" one. If a line needs to be updated, it is converted to dynamic. Any dynamic lines are switched to static lazily, before rendering, searching, or reflow. The CellData class can be used to read either representation.