From 2a18600d18bc010104859bcfdf1955dde0a69f68 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 May 2026 14:06:39 -0700 Subject: [PATCH 01/11] Cache translateToString calls inside BufferLine Fixes #5863 --- src/common/buffer/BufferLine.test.ts | 57 ++++++++++++++++++++++++++++ src/common/buffer/BufferLine.ts | 26 ++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/common/buffer/BufferLine.test.ts b/src/common/buffer/BufferLine.test.ts index dce68ebbf..10781745d 100644 --- a/src/common/buffer/BufferLine.test.ts +++ b/src/common/buffer/BufferLine.test.ts @@ -807,5 +807,62 @@ describe('BufferLine', function(): void { assert.equal(extendedAttributes(line, 3), extendedAttributes(initial, 3)); assert.equal(extendedAttributes(line, 4), extendedAttributes(initial, 4)); }); + + it('should cache canonical trimmed string translations', () => { + const line = new TestBufferLine(3); + line.setCell(0, createCellData(1, 'a', 1)); + line.setCell(1, createCellData(1, 'b', 1)); + line.setCell(2, createCellData(1, 'c', 1)); + + const originalGetTrimmedLength = line.getTrimmedLength.bind(line); + let trimmedLengthCalls = 0; + line.getTrimmedLength = (): number => { + trimmedLengthCalls++; + return originalGetTrimmedLength(); + }; + + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); + assert.equal(trimmedLengthCalls, 1); + }); + + it('should invalidate cached trimmed strings on line mutations', () => { + const assertCacheInvalidated = (mutate: (line: TestBufferLine) => void): void => { + const line = new TestBufferLine(5); + line.fill(createCellData(1, 'a', 1)); + const originalGetTrimmedLength = line.getTrimmedLength.bind(line); + let trimmedLengthCalls = 0; + line.getTrimmedLength = (): number => { + trimmedLengthCalls++; + return originalGetTrimmedLength(); + }; + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'aaaaa'); + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'aaaaa'); + assert.equal(trimmedLengthCalls, 1); + mutate(line); + line.translateToString(true, undefined, undefined, undefined); + assert.equal(trimmedLengthCalls, 2); + }; + + assertCacheInvalidated(line => line.set(0, [0, 'b', 1, 'b'.charCodeAt(0)])); + assertCacheInvalidated(line => line.setCell(0, createCellData(1, 'b', 1))); + assertCacheInvalidated(line => line.setCellFromCodepoint(0, 'b'.charCodeAt(0), 1, createCellData(1, 'b', 1))); + assertCacheInvalidated(line => line.addCodepointToCell(0, 0x301, 0)); + assertCacheInvalidated(line => line.insertCells(1, 1, createCellData(1, 'b', 1))); + assertCacheInvalidated(line => line.deleteCells(1, 1, createCellData(1, 'b', 1))); + assertCacheInvalidated(line => line.replaceCells(1, 3, createCellData(1, 'b', 1))); + assertCacheInvalidated(line => line.resize(6, createCellData(1, 'b', 1))); + assertCacheInvalidated(line => line.fill(createCellData(1, 'b', 1))); + assertCacheInvalidated(line => { + const src = new TestBufferLine(5); + src.fill(createCellData(1, 'x', 1)); + line.copyFrom(src); + }); + assertCacheInvalidated(line => { + const src = new TestBufferLine(5); + src.fill(createCellData(1, 'x', 1)); + line.copyCellsFrom(src, 0, 0, 2, false); + }); + }); }); }); diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index e415851a0..d8e911906 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -63,6 +63,7 @@ export class BufferLine implements IBufferLine { protected _data: Uint32Array; protected _combined: {[index: number]: string} = {}; protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} = {}; + private _cachedTrimmedString: string | undefined; public length: number; constructor(cols: number, fillCellData?: ICellData, public isWrapped: boolean = false) { @@ -98,6 +99,7 @@ export class BufferLine implements IBufferLine { * @deprecated */ public set(index: number, value: CharData): void { + this._invalidateStringCache(); this._data[index * CELL_SIZE + Cell.FG] = value[CHAR_DATA_ATTR_INDEX]; if (value[CHAR_DATA_CHAR_INDEX].length > 1) { this._combined[index] = value[1]; @@ -197,6 +199,7 @@ export class BufferLine implements IBufferLine { * Set data at `index` to `cell`. */ public setCell(index: number, cell: ICellData): void { + this._invalidateStringCache(); if (cell.content & Content.IS_COMBINED_MASK) { this._combined[index] = cell.combinedData; } @@ -214,6 +217,7 @@ export class BufferLine implements IBufferLine { * it gets an optimized access method. */ public setCellFromCodepoint(index: number, codePoint: number, width: number, attrs: IAttributeData): void { + this._invalidateStringCache(); if (attrs.bg & BgFlags.HAS_EXTENDED) { this._extendedAttrs[index] = attrs.extended; } @@ -229,6 +233,7 @@ export class BufferLine implements IBufferLine { * by the previous `setDataFromCodePoint` call, we can omit it here. */ public addCodepointToCell(index: number, codePoint: number, width: number): void { + this._invalidateStringCache(); let content = this._data[index * CELL_SIZE + Cell.CONTENT]; if (content & Content.IS_COMBINED_MASK) { // we already have a combined string, simply add @@ -255,6 +260,7 @@ export class BufferLine implements IBufferLine { } public insertCells(pos: number, n: number, fillCellData: ICellData): void { + this._invalidateStringCache(); pos %= this.length; // handle fullwidth at pos: reset cell one to the left if pos is second cell of a wide char @@ -282,6 +288,7 @@ export class BufferLine implements IBufferLine { } public deleteCells(pos: number, n: number, fillCellData: ICellData): void { + this._invalidateStringCache(); pos %= this.length; if (n < this.length - pos) { for (let i = 0; i < this.length - pos - n; ++i) { @@ -308,6 +315,7 @@ export class BufferLine implements IBufferLine { } public replaceCells(start: number, end: number, fillCellData: ICellData, respectProtect: boolean = false): void { + this._invalidateStringCache(); // full branching on respectProtect==true, hopefully getting fast JIT for standard case if (respectProtect) { if (start && this.getWidth(start - 1) === 2 && !this.isProtected(start - 1)) { @@ -347,6 +355,7 @@ export class BufferLine implements IBufferLine { * excess memory (true after shrinking > CLEANUP_THRESHOLD). */ public resize(cols: number, fillCellData: ICellData): boolean { + this._invalidateStringCache(); if (cols === this.length) { return this._data.length * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength; } @@ -406,6 +415,7 @@ export class BufferLine implements IBufferLine { /** fill a line with fillCharData */ public fill(fillCellData: ICellData, respectProtect: boolean = false): void { + this._invalidateStringCache(); // full branching on respectProtect==true, hopefully getting fast JIT for standard case if (respectProtect) { for (let i = 0; i < this.length; ++i) { @@ -424,6 +434,7 @@ export class BufferLine implements IBufferLine { /** alter to a full copy of line */ public copyFrom(line: BufferLine): void { + this._invalidateStringCache(); if (this.length !== line.length) { this._data = new Uint32Array(line._data); } else { @@ -476,6 +487,7 @@ export class BufferLine implements IBufferLine { } public copyCellsFrom(src: BufferLine, srcCol: number, destCol: number, length: number, applyInReverse: boolean): void { + this._invalidateStringCache(); const srcData = src._data; if (applyInReverse) { for (let cell = length - 1; cell >= 0; cell--) { @@ -508,7 +520,8 @@ export class BufferLine implements IBufferLine { } /** - * Translates the buffer line to a string. + * Translates the buffer line to a string. Using any of the optional arguments will disable the + * cache. * * @param trimRight Whether to trim any empty cells on the right. * @param startCol The column to start the string (0-based inclusive). @@ -521,6 +534,10 @@ export class BufferLine implements IBufferLine { * returned string, the corresponding entries in `outColumns` will have the same column number. */ public translateToString(trimRight?: boolean, startCol?: number, endCol?: number, outColumns?: number[]): string { + const isCanonicalTrimmedRequest = !trimRight && !startCol && endCol === undefined && outColumns === undefined; + if (isCanonicalTrimmedRequest && this._cachedTrimmedString !== undefined) { + return this._cachedTrimmedString; + } startCol = startCol ?? 0; endCol = endCol ?? this.length; if (trimRight) { @@ -545,6 +562,13 @@ export class BufferLine implements IBufferLine { if (outColumns) { outColumns.push(startCol); } + if (isCanonicalTrimmedRequest) { + this._cachedTrimmedString = result; + } return result; } + + private _invalidateStringCache(): void { + this._cachedTrimmedString = undefined; + } } From 66fcdb2583edf3e23ee7f5a9d3069b72c3917c5f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 May 2026 14:11:14 -0700 Subject: [PATCH 02/11] Fix test/startCol handling --- src/common/buffer/BufferLine.test.ts | 32 ++++++++++------------------ src/common/buffer/BufferLine.ts | 6 +++--- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/common/buffer/BufferLine.test.ts b/src/common/buffer/BufferLine.test.ts index 10781745d..f7d434532 100644 --- a/src/common/buffer/BufferLine.test.ts +++ b/src/common/buffer/BufferLine.test.ts @@ -808,40 +808,30 @@ describe('BufferLine', function(): void { assert.equal(extendedAttributes(line, 4), extendedAttributes(initial, 4)); }); - it('should cache canonical trimmed string translations', () => { + it('should cache canonical non-trimmed string translations', () => { const line = new TestBufferLine(3); line.setCell(0, createCellData(1, 'a', 1)); line.setCell(1, createCellData(1, 'b', 1)); line.setCell(2, createCellData(1, 'c', 1)); - const originalGetTrimmedLength = line.getTrimmedLength.bind(line); - let trimmedLengthCalls = 0; - line.getTrimmedLength = (): number => { - trimmedLengthCalls++; - return originalGetTrimmedLength(); - }; - + const translated = line.translateToString(false, undefined, undefined, undefined); + assert.equal(translated, 'abc'); + assert.equal((line as any)._cachedTrimmedString, 'abc'); + (line as any)._cachedTrimmedString = 'cached'; + assert.equal(line.translateToString(false, undefined, undefined, undefined), 'cached'); + // Any optional translation argument should bypass cache. + assert.equal(line.translateToString(false, 0, 2, undefined), 'ab'); assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); - assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); - assert.equal(trimmedLengthCalls, 1); }); it('should invalidate cached trimmed strings on line mutations', () => { const assertCacheInvalidated = (mutate: (line: TestBufferLine) => void): void => { const line = new TestBufferLine(5); line.fill(createCellData(1, 'a', 1)); - const originalGetTrimmedLength = line.getTrimmedLength.bind(line); - let trimmedLengthCalls = 0; - line.getTrimmedLength = (): number => { - trimmedLengthCalls++; - return originalGetTrimmedLength(); - }; - assert.equal(line.translateToString(true, undefined, undefined, undefined), 'aaaaa'); - assert.equal(line.translateToString(true, undefined, undefined, undefined), 'aaaaa'); - assert.equal(trimmedLengthCalls, 1); + line.translateToString(false, undefined, undefined, undefined); + assert.equal((line as any)._cachedTrimmedString, 'aaaaa'); mutate(line); - line.translateToString(true, undefined, undefined, undefined); - assert.equal(trimmedLengthCalls, 2); + assert.equal((line as any)._cachedTrimmedString, undefined); }; assertCacheInvalidated(line => line.set(0, [0, 'b', 1, 'b'.charCodeAt(0)])); diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index d8e911906..40c963190 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -520,8 +520,8 @@ export class BufferLine implements IBufferLine { } /** - * Translates the buffer line to a string. Using any of the optional arguments will disable the - * cache. + * Translates the buffer line to a string. Caching only applies to canonical non-trimmed + * translation requests. Using `trimRight` or any optional range/mapping arguments disables cache. * * @param trimRight Whether to trim any empty cells on the right. * @param startCol The column to start the string (0-based inclusive). @@ -534,7 +534,7 @@ export class BufferLine implements IBufferLine { * returned string, the corresponding entries in `outColumns` will have the same column number. */ public translateToString(trimRight?: boolean, startCol?: number, endCol?: number, outColumns?: number[]): string { - const isCanonicalTrimmedRequest = !trimRight && !startCol && endCol === undefined && outColumns === undefined; + const isCanonicalTrimmedRequest = !trimRight && (startCol === undefined || startCol === 0) && endCol === undefined && outColumns === undefined; if (isCanonicalTrimmedRequest && this._cachedTrimmedString !== undefined) { return this._cachedTrimmedString; } From 4d4b994c3593041379524997742727684dababc4 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 04:31:47 -0700 Subject: [PATCH 03/11] Support caching of trimRight --- src/common/buffer/BufferLine.test.ts | 58 +++++++++++++++++++++++----- src/common/buffer/BufferLine.ts | 28 +++++++++----- src/common/tsconfig.json | 3 +- 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/common/buffer/BufferLine.test.ts b/src/common/buffer/BufferLine.test.ts index f7d434532..1e7451589 100644 --- a/src/common/buffer/BufferLine.test.ts +++ b/src/common/buffer/BufferLine.test.ts @@ -16,6 +16,22 @@ class TestBufferLine extends BufferLine { return this._combined; } + public get cachedCanonicalString(): string | undefined { + return this._cachedCanonicalString; + } + + public set cachedCanonicalString(value: string | undefined) { + this._cachedCanonicalString = value; + } + + public get cachedCanonicalTrimmedString(): string | undefined { + return this._cachedCanonicalTrimmedString; + } + + public set cachedCanonicalTrimmedString(value: string | undefined) { + this._cachedCanonicalTrimmedString = value; + } + public toArray(): CharData[] { const result = []; for (let i = 0; i < this.length; ++i) { @@ -808,30 +824,52 @@ describe('BufferLine', function(): void { assert.equal(extendedAttributes(line, 4), extendedAttributes(initial, 4)); }); - it('should cache canonical non-trimmed string translations', () => { - const line = new TestBufferLine(3); + it('should cache canonical string translations', () => { + const line = new TestBufferLine(5); line.setCell(0, createCellData(1, 'a', 1)); line.setCell(1, createCellData(1, 'b', 1)); line.setCell(2, createCellData(1, 'c', 1)); + // Trimmed-only canonical request should cache the trimmed value. + const trimmed = line.translateToString(true, undefined, undefined, undefined); + assert.equal(trimmed, 'abc'); + assert.equal(line.cachedCanonicalTrimmedString, 'abc'); + + // Non-trimmed canonical request should cache the full value and free trimmed cache. const translated = line.translateToString(false, undefined, undefined, undefined); - assert.equal(translated, 'abc'); - assert.equal((line as any)._cachedTrimmedString, 'abc'); - (line as any)._cachedTrimmedString = 'cached'; - assert.equal(line.translateToString(false, undefined, undefined, undefined), 'cached'); + assert.equal(translated, 'abc '); + assert.equal(line.cachedCanonicalString, 'abc '); + assert.equal(line.cachedCanonicalTrimmedString, undefined); + + // Once non-trimmed is cached, trimmed should be derived from trimEnd() (not re-cached). + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); + assert.equal(line.cachedCanonicalTrimmedString, undefined); + + line.cachedCanonicalString = 'cached-non-trimmed '; + line.cachedCanonicalTrimmedString = 'cached-trimmed'; + assert.equal(line.translateToString(false, undefined, undefined, undefined), 'cached-non-trimmed '); + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'cached-trimmed'); + + line.cachedCanonicalTrimmedString = undefined; + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'cached-non-trimmed'); + // Any optional translation argument should bypass cache. assert.equal(line.translateToString(false, 0, 2, undefined), 'ab'); - assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); + assert.equal(line.translateToString(true, 0, 2, undefined), 'ab'); }); - it('should invalidate cached trimmed strings on line mutations', () => { + it('should invalidate cached canonical strings on line mutations', () => { const assertCacheInvalidated = (mutate: (line: TestBufferLine) => void): void => { const line = new TestBufferLine(5); line.fill(createCellData(1, 'a', 1)); + line.translateToString(true, undefined, undefined, undefined); + assert.equal(line.cachedCanonicalTrimmedString, 'aaaaa'); line.translateToString(false, undefined, undefined, undefined); - assert.equal((line as any)._cachedTrimmedString, 'aaaaa'); + assert.equal(line.cachedCanonicalString, 'aaaaa'); + assert.equal(line.cachedCanonicalTrimmedString, undefined); mutate(line); - assert.equal((line as any)._cachedTrimmedString, undefined); + assert.equal(line.cachedCanonicalString, undefined); + assert.equal(line.cachedCanonicalTrimmedString, undefined); }; assertCacheInvalidated(line => line.set(0, [0, 'b', 1, 'b'.charCodeAt(0)])); diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 40c963190..66bf124e3 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -63,7 +63,9 @@ export class BufferLine implements IBufferLine { protected _data: Uint32Array; protected _combined: {[index: number]: string} = {}; protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} = {}; - private _cachedTrimmedString: string | undefined; + // Cache for canonical full-line translations. + protected _cachedCanonicalString: string | undefined; + protected _cachedCanonicalTrimmedString: string | undefined; public length: number; constructor(cols: number, fillCellData?: ICellData, public isWrapped: boolean = false) { @@ -520,8 +522,8 @@ export class BufferLine implements IBufferLine { } /** - * Translates the buffer line to a string. Caching only applies to canonical non-trimmed - * translation requests. Using `trimRight` or any optional range/mapping arguments disables cache. + * Translates the buffer line to a string. Caching only applies to canonical full-line translation + * requests (regardless of `trimRight` value). * * @param trimRight Whether to trim any empty cells on the right. * @param startCol The column to start the string (0-based inclusive). @@ -534,9 +536,10 @@ export class BufferLine implements IBufferLine { * returned string, the corresponding entries in `outColumns` will have the same column number. */ public translateToString(trimRight?: boolean, startCol?: number, endCol?: number, outColumns?: number[]): string { - const isCanonicalTrimmedRequest = !trimRight && (startCol === undefined || startCol === 0) && endCol === undefined && outColumns === undefined; - if (isCanonicalTrimmedRequest && this._cachedTrimmedString !== undefined) { - return this._cachedTrimmedString; + const isCanonicalRequest = (startCol === undefined || startCol === 0) && endCol === undefined && outColumns === undefined; + const cachedResult = trimRight ? this._cachedCanonicalTrimmedString ?? this._cachedCanonicalString?.trimEnd() : this._cachedCanonicalString; + if (isCanonicalRequest && cachedResult !== undefined) { + return cachedResult; } startCol = startCol ?? 0; endCol = endCol ?? this.length; @@ -562,13 +565,20 @@ export class BufferLine implements IBufferLine { if (outColumns) { outColumns.push(startCol); } - if (isCanonicalTrimmedRequest) { - this._cachedTrimmedString = result; + if (isCanonicalRequest) { + if (trimRight) { + this._cachedCanonicalTrimmedString = result; + } else { + this._cachedCanonicalString = result; + // Free the trimmed string as trimEnd() will be used to save memory + this._cachedCanonicalTrimmedString = undefined; + } } return result; } private _invalidateStringCache(): void { - this._cachedTrimmedString = undefined; + this._cachedCanonicalString = undefined; + this._cachedCanonicalTrimmedString = undefined; } } diff --git a/src/common/tsconfig.json b/src/common/tsconfig.json index 22501b9cf..279e9391e 100644 --- a/src/common/tsconfig.json +++ b/src/common/tsconfig.json @@ -3,7 +3,8 @@ "compilerOptions": { "lib": [ "es2015", - "es2016.Array.Include" + "es2016.Array.Include", + "es2019.string" ], "outDir": "../../out", "types": [ From 7615cd210a3c4dc2c4976b4f082f7029c062aa0a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 04:34:05 -0700 Subject: [PATCH 04/11] Store a boolean on whether the cache is trimmed --- src/common/buffer/BufferLine.test.ts | 55 ++++++++++++++++------------ src/common/buffer/BufferLine.ts | 28 +++++++------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/common/buffer/BufferLine.test.ts b/src/common/buffer/BufferLine.test.ts index 1e7451589..7ac943d25 100644 --- a/src/common/buffer/BufferLine.test.ts +++ b/src/common/buffer/BufferLine.test.ts @@ -16,20 +16,20 @@ class TestBufferLine extends BufferLine { return this._combined; } - public get cachedCanonicalString(): string | undefined { - return this._cachedCanonicalString; + public get cachedString(): string | undefined { + return this._cachedString; } - public set cachedCanonicalString(value: string | undefined) { - this._cachedCanonicalString = value; + public set cachedString(value: string | undefined) { + this._cachedString = value; } - public get cachedCanonicalTrimmedString(): string | undefined { - return this._cachedCanonicalTrimmedString; + public get isCachedStringTrimmed(): boolean { + return this._isCachedStringTrimmed; } - public set cachedCanonicalTrimmedString(value: string | undefined) { - this._cachedCanonicalTrimmedString = value; + public set isCachedStringTrimmed(value: boolean) { + this._isCachedStringTrimmed = value; } public toArray(): CharData[] { @@ -833,26 +833,32 @@ describe('BufferLine', function(): void { // Trimmed-only canonical request should cache the trimmed value. const trimmed = line.translateToString(true, undefined, undefined, undefined); assert.equal(trimmed, 'abc'); - assert.equal(line.cachedCanonicalTrimmedString, 'abc'); + assert.equal(line.cachedString, 'abc'); + assert.equal(line.isCachedStringTrimmed, true); - // Non-trimmed canonical request should cache the full value and free trimmed cache. + // Non-trimmed canonical request should refresh cache with the full value. const translated = line.translateToString(false, undefined, undefined, undefined); assert.equal(translated, 'abc '); - assert.equal(line.cachedCanonicalString, 'abc '); - assert.equal(line.cachedCanonicalTrimmedString, undefined); + assert.equal(line.cachedString, 'abc '); + assert.equal(line.isCachedStringTrimmed, false); - // Once non-trimmed is cached, trimmed should be derived from trimEnd() (not re-cached). + // Once non-trimmed is cached, trimmed should be derived via trimEnd(). assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc'); - assert.equal(line.cachedCanonicalTrimmedString, undefined); + assert.equal(line.cachedString, 'abc '); + assert.equal(line.isCachedStringTrimmed, false); - line.cachedCanonicalString = 'cached-non-trimmed '; - line.cachedCanonicalTrimmedString = 'cached-trimmed'; + line.cachedString = 'cached-non-trimmed '; + line.isCachedStringTrimmed = false; assert.equal(line.translateToString(false, undefined, undefined, undefined), 'cached-non-trimmed '); - assert.equal(line.translateToString(true, undefined, undefined, undefined), 'cached-trimmed'); - - line.cachedCanonicalTrimmedString = undefined; assert.equal(line.translateToString(true, undefined, undefined, undefined), 'cached-non-trimmed'); + line.cachedString = 'cached-trimmed'; + line.isCachedStringTrimmed = true; + assert.equal(line.translateToString(true, undefined, undefined, undefined), 'cached-trimmed'); + assert.equal(line.translateToString(false, undefined, undefined, undefined), 'abc '); + assert.equal(line.cachedString, 'abc '); + assert.equal(line.isCachedStringTrimmed, false); + // Any optional translation argument should bypass cache. assert.equal(line.translateToString(false, 0, 2, undefined), 'ab'); assert.equal(line.translateToString(true, 0, 2, undefined), 'ab'); @@ -863,13 +869,14 @@ describe('BufferLine', function(): void { const line = new TestBufferLine(5); line.fill(createCellData(1, 'a', 1)); line.translateToString(true, undefined, undefined, undefined); - assert.equal(line.cachedCanonicalTrimmedString, 'aaaaa'); + assert.equal(line.cachedString, 'aaaaa'); + assert.equal(line.isCachedStringTrimmed, true); line.translateToString(false, undefined, undefined, undefined); - assert.equal(line.cachedCanonicalString, 'aaaaa'); - assert.equal(line.cachedCanonicalTrimmedString, undefined); + assert.equal(line.cachedString, 'aaaaa'); + assert.equal(line.isCachedStringTrimmed, false); mutate(line); - assert.equal(line.cachedCanonicalString, undefined); - assert.equal(line.cachedCanonicalTrimmedString, undefined); + assert.equal(line.cachedString, undefined); + assert.equal(line.isCachedStringTrimmed, false); }; assertCacheInvalidated(line => line.set(0, [0, 'b', 1, 'b'.charCodeAt(0)])); diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 66bf124e3..64639c114 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -63,9 +63,8 @@ export class BufferLine implements IBufferLine { protected _data: Uint32Array; protected _combined: {[index: number]: string} = {}; protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} = {}; - // Cache for canonical full-line translations. - protected _cachedCanonicalString: string | undefined; - protected _cachedCanonicalTrimmedString: string | undefined; + protected _cachedString: string | undefined; + protected _isCachedStringTrimmed: boolean = false; public length: number; constructor(cols: number, fillCellData?: ICellData, public isWrapped: boolean = false) { @@ -537,9 +536,13 @@ export class BufferLine implements IBufferLine { */ public translateToString(trimRight?: boolean, startCol?: number, endCol?: number, outColumns?: number[]): string { const isCanonicalRequest = (startCol === undefined || startCol === 0) && endCol === undefined && outColumns === undefined; - const cachedResult = trimRight ? this._cachedCanonicalTrimmedString ?? this._cachedCanonicalString?.trimEnd() : this._cachedCanonicalString; - if (isCanonicalRequest && cachedResult !== undefined) { - return cachedResult; + if (isCanonicalRequest && this._cachedString !== undefined) { + if (trimRight) { + return this._isCachedStringTrimmed ? this._cachedString : this._cachedString.trimEnd(); + } + if (!this._isCachedStringTrimmed) { + return this._cachedString; + } } startCol = startCol ?? 0; endCol = endCol ?? this.length; @@ -566,19 +569,14 @@ export class BufferLine implements IBufferLine { outColumns.push(startCol); } if (isCanonicalRequest) { - if (trimRight) { - this._cachedCanonicalTrimmedString = result; - } else { - this._cachedCanonicalString = result; - // Free the trimmed string as trimEnd() will be used to save memory - this._cachedCanonicalTrimmedString = undefined; - } + this._cachedString = result; + this._isCachedStringTrimmed = !!trimRight; } return result; } private _invalidateStringCache(): void { - this._cachedCanonicalString = undefined; - this._cachedCanonicalTrimmedString = undefined; + this._cachedString = undefined; + this._isCachedStringTrimmed = false; } } From 52d4b2719cfc0d9cbce391c5ad03cfe322a9e384 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 05:32:43 -0700 Subject: [PATCH 05/11] Move cache into Buffer owned object, use WeakRefs This allows O(1) clearing of the cache, a single TTL timer and we can avoid making BufferLine's Disposable --- .../dom/DomRendererRowFactory.test.ts | 4 +- .../services/CharacterJoinerService.test.ts | 7 +- src/browser/services/SelectionService.test.ts | 9 ++- src/common/buffer/Buffer.test.ts | 67 +++++++++++++++++-- src/common/buffer/Buffer.ts | 37 ++++++++-- src/common/buffer/BufferLine.test.ts | 17 +++-- src/common/buffer/BufferLine.ts | 63 +++++++++++++---- src/common/buffer/BufferLineStringCache.ts | 37 ++++++++++ src/common/buffer/BufferReflow.test.ts | 15 +++-- src/common/tsconfig.json | 3 +- 10 files changed, 219 insertions(+), 40 deletions(-) create mode 100644 src/common/buffer/BufferLineStringCache.ts diff --git a/src/browser/renderer/dom/DomRendererRowFactory.test.ts b/src/browser/renderer/dom/DomRendererRowFactory.test.ts index 05fc8000c..b7c60e8f3 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.test.ts @@ -8,6 +8,7 @@ import { assert } from 'chai'; import { DomRendererRowFactory } from 'browser/renderer/dom/DomRendererRowFactory'; import { NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR, DEFAULT_ATTR, FgFlags, BgFlags, Attributes, UnderlineStyle } from 'common/buffer/Constants'; import { BufferLine, DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { IBufferLine } from 'common/Types'; import { CellData } from 'common/buffer/CellData'; import { MockCoreService, MockDecorationService, MockOptionsService, createCellData, NULL_CELL_DATA } from 'common/TestUtils.test'; @@ -15,6 +16,7 @@ import { MockCharacterJoinerService, MockCoreBrowserService, MockThemeService } import { TestWidthCache } from 'browser/renderer/dom/WidthCache.test'; const dom = new jsdom.JSDOM(''); +const TEST_STRING_CACHE = new BufferLineStringCache(); describe('DomRendererRowFactory', () => { @@ -517,7 +519,7 @@ describe('DomRendererRowFactory', () => { } function createEmptyLineData(cols: number): IBufferLine { - const lineData = new BufferLine(cols); + const lineData = new BufferLine(TEST_STRING_CACHE, cols); for (let i = 0; i < cols; i++) { lineData.setCell(i, NULL_CELL_DATA); } diff --git a/src/browser/services/CharacterJoinerService.test.ts b/src/browser/services/CharacterJoinerService.test.ts index a0ccc67d6..06730c7ac 100644 --- a/src/browser/services/CharacterJoinerService.test.ts +++ b/src/browser/services/CharacterJoinerService.test.ts @@ -7,10 +7,13 @@ import { assert } from 'chai'; import { ICharacterJoinerService } from 'browser/services/Services'; import { CharacterJoinerService } from 'browser/services/CharacterJoinerService'; import { BufferLine } from 'common/buffer/BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { IBufferLine } from 'common/Types'; import { CellData } from 'common/buffer/CellData'; import { MockBufferService, createCellData } from 'common/TestUtils.test'; +const TEST_STRING_CACHE = new BufferLineStringCache(); + describe('CharacterJoinerService', () => { let service: ICharacterJoinerService; @@ -22,7 +25,7 @@ describe('CharacterJoinerService', () => { lines.set(2, lineData([['a -> b -', 0xFFFFFFFF], ['> c -> d', 0]])); lines.set(3, lineData([['no joined ranges']])); - lines.set(4, new BufferLine(0)); + lines.set(4, new BufferLine(TEST_STRING_CACHE, 0)); lines.set(5, lineData([['a', 0x11111111], [' -> b -> c -> '], ['d', 0x22222222]])); const line6 = lineData([['wi']]); line6.resize(line6.length + 1, createCellData(0, '￥', 2)); @@ -267,7 +270,7 @@ describe('CharacterJoinerService', () => { type IPartialLineData = ([string] | [string, number]); function lineData(data: IPartialLineData[]): IBufferLine { - const tline = new BufferLine(0); + const tline = new BufferLine(TEST_STRING_CACHE, 0); for (let i = 0; i < data.length; ++i) { const line = data[i][0]; const attr = (data[i][1] || 0) as number; diff --git a/src/browser/services/SelectionService.test.ts b/src/browser/services/SelectionService.test.ts index 8deedb751..be62ccc6b 100644 --- a/src/browser/services/SelectionService.test.ts +++ b/src/browser/services/SelectionService.test.ts @@ -9,12 +9,15 @@ import { SelectionModel } from 'browser/selection/SelectionModel'; import { IBufferLine } from 'common/Types'; import { MockBufferService, MockOptionsService, MockCoreService, createCellData } from 'common/TestUtils.test'; import { BufferLine } from 'common/buffer/BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { IBufferService, IOptionsService } from 'common/services/Services'; import { MockCoreBrowserService, MockMouseService, MockRenderService } from 'browser/TestUtils.test'; import { CellData } from 'common/buffer/CellData'; import { IBuffer } from 'common/buffer/Types'; import { IRenderService } from 'browser/services/Services'; +const TEST_STRING_CACHE = new BufferLineStringCache(); + class TestSelectionService extends SelectionService { constructor( bufferService: IBufferService, @@ -55,7 +58,7 @@ describe('SelectionService', () => { }); function stringToRow(text: string): IBufferLine { - const result = new BufferLine(text.length); + const result = new BufferLine(TEST_STRING_CACHE, text.length); for (let i = 0; i < text.length; i++) { result.setCell(i, createCellData(0, text.charAt(i), 1)); } @@ -63,7 +66,7 @@ describe('SelectionService', () => { } function stringArrayToRow(chars: string[]): IBufferLine { - const line = new BufferLine(chars.length); + const line = new BufferLine(TEST_STRING_CACHE, chars.length); chars.map((c, idx) => line.setCell(idx, createCellData(0, c, 1))); return line; } @@ -118,7 +121,7 @@ describe('SelectionService', () => { [0, 'o', 1, 'o'.charCodeAt(0)], [0, 'o', 1, 'o'.charCodeAt(0)] ]; - const line = new BufferLine(data.length); + const line = new BufferLine(TEST_STRING_CACHE, data.length); for (let i = 0; i < data.length; ++i) line.setCell(i, CellData.fromCharData(data[i])); buffer.lines.set(0, line); // Ensure wide characters take up 2 columns diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index 9c013c5de..93cc6882b 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -8,12 +8,14 @@ import { Buffer } from 'common/buffer/Buffer'; import { CircularList } from 'common/CircularList'; import { MockOptionsService, MockBufferService, MockLogService, createCellData } from 'common/TestUtils.test'; import { BufferLine, DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { CellData } from 'common/buffer/CellData'; import { ExtendedAttrs } from 'common/buffer/AttributeData'; const INIT_COLS = 80; const INIT_ROWS = 24; const INIT_SCROLLBACK = 1000; +const TEST_STRING_CACHE = new BufferLineStringCache(); describe('Buffer', () => { let optionsService: MockOptionsService; @@ -1104,7 +1106,7 @@ describe('Buffer', () => { describe ('translateBufferLineToString', () => { it('should handle selecting a section of ascii text', () => { - const line = new BufferLine(4); + const line = new BufferLine(TEST_STRING_CACHE, 4); line.setCell(0, createCellData(0, 'a', 1)); line.setCell(1, createCellData(0, 'b', 1)); line.setCell(2, createCellData(0, 'c', 1)); @@ -1116,7 +1118,7 @@ describe('Buffer', () => { }); it('should handle a cut-off double width character by including it', () => { - const line = new BufferLine(3); + const line = new BufferLine(TEST_STRING_CACHE, 3); line.setCell(0, createCellData(0, '語', 2)); line.setCell(1, createCellData(0, '', 0)); line.setCell(2, createCellData(0, 'a', 1)); @@ -1127,7 +1129,7 @@ describe('Buffer', () => { }); it('should handle a zero width character in the middle of the string by not including it', () => { - const line = new BufferLine(3); + const line = new BufferLine(TEST_STRING_CACHE, 3); line.setCell(0, createCellData(0, '語', 2)); line.setCell(1, createCellData(0, '', 0)); line.setCell(2, createCellData(0, 'a', 1)); @@ -1144,7 +1146,7 @@ describe('Buffer', () => { }); it('should handle single width emojis', () => { - const line = new BufferLine(2); + const line = new BufferLine(TEST_STRING_CACHE, 2); line.setCell(0, createCellData(0, '😁', 1)); line.setCell(1, createCellData(0, 'a', 1)); buffer.lines.set(0, line); @@ -1157,7 +1159,7 @@ describe('Buffer', () => { }); it('should handle double width emojis', () => { - const line = new BufferLine(2); + const line = new BufferLine(TEST_STRING_CACHE, 2); line.setCell(0, createCellData(0, '😁', 2)); line.setCell(1, createCellData(0, '', 0)); buffer.lines.set(0, line); @@ -1168,7 +1170,7 @@ describe('Buffer', () => { const str2 = buffer.translateBufferLineToString(0, true, 0, 2); assert.equal(str2, '😁'); - const line2 = new BufferLine(3); + const line2 = new BufferLine(TEST_STRING_CACHE, 3); line2.setCell(0, createCellData(0, '😁', 2)); line2.setCell(1, createCellData(0, '', 0)); line2.setCell(2, createCellData(0, 'a', 1)); @@ -1179,6 +1181,59 @@ describe('Buffer', () => { }); }); + describe('line string cache cleanup', () => { + it('should clear shared cache entries with a single timer', async () => { + buffer.fillViewportRows(); + buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); + buffer.lines.get(1)!.setCell(0, createCellData(0, 'b', 1)); + + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.equal(buffer.translateBufferLineToString(1, false), `b${' '.repeat(INIT_COLS - 1)}`); + + const cache = (buffer as any)._stringCache; + assert.equal(cache.entries.size, 2); + const previousTimer = (buffer as any)._stringCacheClearTimeout; + assert.notEqual(previousTimer, undefined); + + // Another translation should refresh the same shared timer, not create a second queue. + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.notEqual((buffer as any)._stringCacheClearTimeout, undefined); + assert.notEqual((buffer as any)._stringCacheClearTimeout, previousTimer); + + await new Promise(r => setTimeout(r, 70)); + + assert.equal(cache.entries.size, 0); + assert.equal((buffer as any)._stringCacheClearTimeout, undefined); + + // Cache entries should be recreated lazily after a timer-based reset. + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.equal(cache.entries.size, 1); + }); + + it('should reset line string cache state on clear and resize', () => { + buffer.fillViewportRows(); + buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); + buffer.translateBufferLineToString(0, false); + + const cache = (buffer as any)._stringCache; + assert.equal(cache.entries.size, 1); + assert.notEqual((buffer as any)._stringCacheClearTimeout, undefined); + + buffer.clear(); + assert.equal(cache.entries.size, 0); + assert.equal((buffer as any)._stringCacheClearTimeout, undefined); + + buffer.fillViewportRows(); + buffer.lines.get(0)!.setCell(0, createCellData(0, 'b', 1)); + buffer.translateBufferLineToString(0, false); + assert.equal(cache.entries.size, 1); + + buffer.resize(INIT_COLS - 1, INIT_ROWS); + assert.equal(cache.entries.size, 0); + assert.equal((buffer as any)._stringCacheClearTimeout, undefined); + }); + }); + describe('memory cleanup after shrinking', () => { it('should realign memory from idle task execution', async () => { buffer.fillViewportRows(); diff --git a/src/common/buffer/Buffer.ts b/src/common/buffer/Buffer.ts index 8efefd60a..bf78e473c 100644 --- a/src/common/buffer/Buffer.ts +++ b/src/common/buffer/Buffer.ts @@ -8,6 +8,7 @@ import { IdleTaskQueue } from 'common/TaskQueue'; import { IAttributeData, IBufferLine, ICellData, ICharset } from 'common/Types'; import { ExtendedAttrs } from 'common/buffer/AttributeData'; import { BufferLine, DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { getWrappedLineTrimmedLength, reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths } from 'common/buffer/BufferReflow'; import { CellData } from 'common/buffer/CellData'; import { NULL_CELL_CHAR, NULL_CELL_CODE, NULL_CELL_WIDTH, WHITESPACE_CELL_CHAR, WHITESPACE_CELL_CODE, WHITESPACE_CELL_WIDTH } from 'common/buffer/Constants'; @@ -17,6 +18,7 @@ import { DEFAULT_CHARSET } from 'common/data/Charsets'; import { IBufferService, ILogService, IOptionsService } from 'common/services/Services'; export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1 +const STRING_CACHE_CLEAR_DELAY_MS = 50; /** * This class represents a terminal buffer (an internal state of the terminal), where the @@ -50,6 +52,8 @@ export class Buffer implements IBuffer { private _isClearing: boolean = false; private _memoryCleanupQueue: InstanceType; private _memoryCleanupPosition = 0; + private readonly _stringCache: BufferLineStringCache; + private _stringCacheClearTimeout: ReturnType | undefined; constructor( private _hasScrollback: boolean, @@ -64,6 +68,7 @@ export class Buffer implements IBuffer { this.scrollBottom = this._rows - 1; this.setupTabStops(); this._memoryCleanupQueue = new IdleTaskQueue(this._logService); + this._stringCache = new BufferLineStringCache(() => this._scheduleStringCacheCleanup()); } public getNullCell(attr?: IAttributeData): ICellData { @@ -93,7 +98,7 @@ export class Buffer implements IBuffer { } public getBlankLine(attr: IAttributeData, isWrapped?: boolean): IBufferLine { - return new BufferLine(this._bufferService.cols, this.getNullCell(attr), isWrapped); + return new BufferLine(this._stringCache, this._bufferService.cols, this.getNullCell(attr), isWrapped); } public get hasScrollback(): boolean { @@ -138,6 +143,7 @@ export class Buffer implements IBuffer { * Clears the buffer to it's initial state, discarding all previous data. */ public clear(): void { + this._resetStringCache(); this.ydisp = 0; this.ybase = 0; this.y = 0; @@ -156,6 +162,7 @@ export class Buffer implements IBuffer { public resize(newCols: number, newRows: number): void { // store reference to null cell with default attrs const nullCell = this.getNullCell(DEFAULT_ATTR_DATA); + this._resetStringCache(); // count bufferlines with overly big memory to be cleaned afterwards let dirtyMemoryLines = 0; @@ -190,7 +197,7 @@ export class Buffer implements IBuffer { if (this._optionsService.rawOptions.windowsPty.backend !== undefined || this._optionsService.rawOptions.windowsPty.buildNumber !== undefined) { // Just add the new missing rows on Windows as conpty reprints the screen with it's // view of the world. Once a line enters scrollback for conpty it remains there - this.lines.push(new BufferLine(newCols, nullCell)); + this.lines.push(new BufferLine(this._stringCache, newCols, nullCell, false)); } else { if (this.ybase > 0 && this.lines.length <= this.ybase + this.y + addToY + 1) { // There is room above the buffer and there are no empty elements below the line, @@ -204,7 +211,7 @@ export class Buffer implements IBuffer { } else { // Add a blank line if there is no buffer left at the top to scroll to, or if there // are blank lines after the cursor - this.lines.push(new BufferLine(newCols, nullCell)); + this.lines.push(new BufferLine(this._stringCache, newCols, nullCell, false)); } } } @@ -303,6 +310,28 @@ export class Buffer implements IBuffer { return normalRun; } + private _scheduleStringCacheCleanup(): void { + if (this._stringCacheClearTimeout !== undefined) { + clearTimeout(this._stringCacheClearTimeout); + } + this._stringCacheClearTimeout = setTimeout(() => { + this._stringCacheClearTimeout = undefined; + this._clearStringCache(); + }, STRING_CACHE_CLEAR_DELAY_MS); + } + + private _resetStringCache(): void { + if (this._stringCacheClearTimeout !== undefined) { + clearTimeout(this._stringCacheClearTimeout); + this._stringCacheClearTimeout = undefined; + } + this._clearStringCache(); + } + + private _clearStringCache(): void { + this._stringCache.clear(); + } + private get _isReflowEnabled(): boolean { const windowsPty = this._optionsService.rawOptions.windowsPty; if (windowsPty && windowsPty.buildNumber) { @@ -345,7 +374,7 @@ export class Buffer implements IBuffer { } if (this.lines.length < newRows) { // Add an extra row at the bottom of the viewport - this.lines.push(new BufferLine(newCols, nullCell)); + this.lines.push(new BufferLine(this._stringCache, newCols, nullCell, false)); } } else { if (this.ydisp === this.ybase) { diff --git a/src/common/buffer/BufferLine.test.ts b/src/common/buffer/BufferLine.test.ts index 7ac943d25..3aec28872 100644 --- a/src/common/buffer/BufferLine.test.ts +++ b/src/common/buffer/BufferLine.test.ts @@ -4,32 +4,39 @@ */ import { NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE, DEFAULT_ATTR, Content, UnderlineStyle, BgFlags, Attributes, FgFlags } from 'common/buffer/Constants'; import { BufferLine } from 'common/buffer//BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { CellData } from 'common/buffer/CellData'; -import { CharData, IBufferLine } from '../Types'; +import { CharData, IBufferLine, ICellData } from '../Types'; import { assert } from 'chai'; import { AttributeData } from 'common/buffer/AttributeData'; import { createCellData, NULL_CELL_DATA, extendedAttributes } from 'common/TestUtils.test'; +const TEST_STRING_CACHE = new BufferLineStringCache(); + class TestBufferLine extends BufferLine { + constructor(cols: number, fillCellData?: ICellData, isWrapped: boolean = false) { + super(TEST_STRING_CACHE, cols, fillCellData, isWrapped); + } + public get combined(): {[index: number]: string} { return this._combined; } public get cachedString(): string | undefined { - return this._cachedString; + return this._getStringCacheEntry(false)?.value; } public set cachedString(value: string | undefined) { - this._cachedString = value; + this._getStringCacheEntry(true)!.value = value; } public get isCachedStringTrimmed(): boolean { - return this._isCachedStringTrimmed; + return this._getStringCacheEntry(false)?.isTrimmed ?? false; } public set isCachedStringTrimmed(value: boolean) { - this._isCachedStringTrimmed = value; + this._getStringCacheEntry(true)!.isTrimmed = value; } public toArray(): CharData[] { diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 64639c114..682747b16 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -44,6 +44,18 @@ const $workCell = new CellData(); /** Factor when to cleanup underlying array buffer after shrinking. */ const CLEANUP_THRESHOLD = 2; +export interface IBufferLineStringCacheEntry { + value: string | undefined; + isTrimmed: boolean; + generation: number; +} + +export interface IBufferLineStringCache { + generation: number; + allocateEntry(): IBufferLineStringCacheEntry; + touch?(): void; +} + /** * Typed array based bufferline implementation. * @@ -63,11 +75,15 @@ export class BufferLine implements IBufferLine { protected _data: Uint32Array; protected _combined: {[index: number]: string} = {}; protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} = {}; - protected _cachedString: string | undefined; - protected _isCachedStringTrimmed: boolean = false; + protected _stringCacheEntryRef: WeakRef | undefined; public length: number; - constructor(cols: number, fillCellData?: ICellData, public isWrapped: boolean = false) { + constructor( + protected readonly _stringCache: IBufferLineStringCache, + cols: number, + fillCellData?: ICellData, + public isWrapped: boolean = false + ) { this._data = new Uint32Array(cols * CELL_SIZE); const cell = fillCellData ?? CellData.fromCharData([0, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]); for (let i = 0; i < cols; ++i) { @@ -456,7 +472,7 @@ export class BufferLine implements IBufferLine { /** create a new clone */ public clone(): IBufferLine { - const newLine = new BufferLine(0); + const newLine = new BufferLine(this._stringCache, 0, undefined, false); newLine._data = new Uint32Array(this._data); newLine.length = this.length; for (const el in this._combined) { @@ -536,12 +552,16 @@ export class BufferLine implements IBufferLine { */ public translateToString(trimRight?: boolean, startCol?: number, endCol?: number, outColumns?: number[]): string { const isCanonicalRequest = (startCol === undefined || startCol === 0) && endCol === undefined && outColumns === undefined; - if (isCanonicalRequest && this._cachedString !== undefined) { + if (isCanonicalRequest) { + this._stringCache.touch?.(); + } + const stringCacheEntry = isCanonicalRequest ? this._getStringCacheEntry(false) : undefined; + if (isCanonicalRequest && stringCacheEntry?.value !== undefined) { if (trimRight) { - return this._isCachedStringTrimmed ? this._cachedString : this._cachedString.trimEnd(); + return stringCacheEntry.isTrimmed ? stringCacheEntry.value : stringCacheEntry.value.trimEnd(); } - if (!this._isCachedStringTrimmed) { - return this._cachedString; + if (!stringCacheEntry.isTrimmed) { + return stringCacheEntry.value; } } startCol = startCol ?? 0; @@ -569,14 +589,33 @@ export class BufferLine implements IBufferLine { outColumns.push(startCol); } if (isCanonicalRequest) { - this._cachedString = result; - this._isCachedStringTrimmed = !!trimRight; + const cacheEntry = this._getStringCacheEntry(true)!; + cacheEntry.value = result; + cacheEntry.isTrimmed = !!trimRight; } return result; } + protected _getStringCacheEntry(createIfNeeded: boolean): IBufferLineStringCacheEntry | undefined { + const cachedEntry = this._stringCacheEntryRef?.deref(); + if (cachedEntry) { + if (cachedEntry.generation === this._stringCache.generation) { + return cachedEntry; + } + } + if (!createIfNeeded) { + return undefined; + } + const cacheEntry = this._stringCache.allocateEntry(); + this._stringCacheEntryRef = new WeakRef(cacheEntry); + return cacheEntry; + } + private _invalidateStringCache(): void { - this._cachedString = undefined; - this._isCachedStringTrimmed = false; + const cacheEntry = this._getStringCacheEntry(false); + if (cacheEntry) { + cacheEntry.value = undefined; + cacheEntry.isTrimmed = false; + } } } diff --git a/src/common/buffer/BufferLineStringCache.ts b/src/common/buffer/BufferLineStringCache.ts new file mode 100644 index 000000000..9365c847d --- /dev/null +++ b/src/common/buffer/BufferLineStringCache.ts @@ -0,0 +1,37 @@ +/** + * Copyright (c) 2026 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import type { IBufferLineStringCache, IBufferLineStringCacheEntry } from 'common/buffer/BufferLine'; + +export class BufferLineStringCache implements IBufferLineStringCache { + public generation: number = 0; + public readonly entries: Set = new Set(); + + constructor(private readonly _onCacheUsed: () => void = () => { }) { } + + public touch(): void { + this._onCacheUsed(); + } + + public allocateEntry(): IBufferLineStringCacheEntry { + const entry: IBufferLineStringCacheEntry = { + value: undefined, + isTrimmed: false, + generation: this.generation + }; + this.entries.add(entry); + this._onCacheUsed(); + return entry; + } + + public clear(): void { + this.generation++; + for (const entry of this.entries) { + entry.value = undefined; + entry.isTrimmed = false; + } + this.entries.clear(); + } +} diff --git a/src/common/buffer/BufferReflow.test.ts b/src/common/buffer/BufferReflow.test.ts index b351b89c4..d71186ddd 100644 --- a/src/common/buffer/BufferReflow.test.ts +++ b/src/common/buffer/BufferReflow.test.ts @@ -4,13 +4,16 @@ */ import { assert } from 'chai'; import { BufferLine } from 'common/buffer/BufferLine'; +import { BufferLineStringCache } from 'common/buffer/BufferLineStringCache'; import { NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE } from 'common/buffer/Constants'; import { reflowSmallerGetNewLineLengths } from 'common/buffer/BufferReflow'; +const TEST_STRING_CACHE = new BufferLineStringCache(); + describe('BufferReflow', () => { describe('reflowSmallerGetNewLineLengths', () => { it('should return correct line lengths for a small line with wide characters', () => { - const line = new BufferLine(4); + const line = new BufferLine(TEST_STRING_CACHE, 4); line.set(0, [0, '汉', 2, '汉'.charCodeAt(0)]); line.set(1, [0, '', 0, 0]); line.set(2, [0, '语', 2, '语'.charCodeAt(0)]); @@ -20,7 +23,7 @@ describe('BufferReflow', () => { assert.deepEqual(reflowSmallerGetNewLineLengths([line], 4, 2), [2, 2], 'line: 汉, 语'); }); it('should return correct line lengths for a large line with wide characters', () => { - const line = new BufferLine(12); + const line = new BufferLine(TEST_STRING_CACHE, 12); for (let i = 0; i < 12; i += 4) { line.set(i, [0, '汉', 2, '汉'.charCodeAt(0)]); line.set(i + 2, [0, '语', 2, '语'.charCodeAt(0)]); @@ -42,7 +45,7 @@ describe('BufferReflow', () => { assert.deepEqual(reflowSmallerGetNewLineLengths([line], 12, 2), [2, 2, 2, 2, 2, 2], 'line: 汉, 语, 汉, 语, 汉, 语'); }); it('should return correct line lengths for a string with wide and single characters', () => { - const line = new BufferLine(6); + const line = new BufferLine(TEST_STRING_CACHE, 6); line.set(0, [0, 'a', 1, 'a'.charCodeAt(0)]); line.set(1, [0, '汉', 2, '汉'.charCodeAt(0)]); line.set(2, [0, '', 0, 0]); @@ -56,14 +59,14 @@ describe('BufferReflow', () => { assert.deepEqual(reflowSmallerGetNewLineLengths([line], 6, 2), [1, 2, 2, 1], 'line: a, 汉, 语, b'); }); it('should return correct line lengths for a wrapped line with wide and single characters', () => { - const line1 = new BufferLine(6); + const line1 = new BufferLine(TEST_STRING_CACHE, 6); line1.set(0, [0, 'a', 1, 'a'.charCodeAt(0)]); line1.set(1, [0, '汉', 2, '汉'.charCodeAt(0)]); line1.set(2, [0, '', 0, 0]); line1.set(3, [0, '语', 2, '语'.charCodeAt(0)]); line1.set(4, [0, '', 0, 0]); line1.set(5, [0, 'b', 1, 'b'.charCodeAt(0)]); - const line2 = new BufferLine(6, undefined, true); + const line2 = new BufferLine(TEST_STRING_CACHE, 6, undefined, true); line2.set(0, [0, 'a', 1, 'a'.charCodeAt(0)]); line2.set(1, [0, '汉', 2, '汉'.charCodeAt(0)]); line2.set(2, [0, '', 0, 0]); @@ -78,7 +81,7 @@ describe('BufferReflow', () => { assert.deepEqual(reflowSmallerGetNewLineLengths([line1, line2], 6, 2), [1, 2, 2, 2, 2, 2, 1], 'lines: a, 汉, 语, ba, 汉, 语, b'); }); it('should work on lines ending in null space', () => { - const line = new BufferLine(5); + const line = new BufferLine(TEST_STRING_CACHE, 5); line.set(0, [0, '汉', 2, '汉'.charCodeAt(0)]); line.set(1, [0, '', 0, 0]); line.set(2, [0, '语', 2, '语'.charCodeAt(0)]); diff --git a/src/common/tsconfig.json b/src/common/tsconfig.json index 279e9391e..31a893756 100644 --- a/src/common/tsconfig.json +++ b/src/common/tsconfig.json @@ -4,7 +4,8 @@ "lib": [ "es2015", "es2016.Array.Include", - "es2019.string" + "es2019.string", + "es2021.weakref" ], "outDir": "../../out", "types": [ From 6287c6ddfef9aa13cee36288f3409bcc8ed1058f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 05:44:13 -0700 Subject: [PATCH 06/11] Cache/buffer lifecycle --- src/common/buffer/Buffer.test.ts | 14 +++---- src/common/buffer/Buffer.ts | 36 ++++-------------- src/common/buffer/BufferLineStringCache.ts | 27 +++++++++++--- src/common/buffer/BufferSet.test.ts | 43 +++++++++++++++++++++- src/common/buffer/BufferSet.ts | 6 ++- 5 files changed, 84 insertions(+), 42 deletions(-) diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index 93cc6882b..76e13cc72 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -1192,18 +1192,18 @@ describe('Buffer', () => { const cache = (buffer as any)._stringCache; assert.equal(cache.entries.size, 2); - const previousTimer = (buffer as any)._stringCacheClearTimeout; + const previousTimer = (cache as any)._clearTimeout; assert.notEqual(previousTimer, undefined); // Another translation should refresh the same shared timer, not create a second queue. assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); - assert.notEqual((buffer as any)._stringCacheClearTimeout, undefined); - assert.notEqual((buffer as any)._stringCacheClearTimeout, previousTimer); + assert.notEqual((cache as any)._clearTimeout, undefined); + assert.notEqual((cache as any)._clearTimeout, previousTimer); await new Promise(r => setTimeout(r, 70)); assert.equal(cache.entries.size, 0); - assert.equal((buffer as any)._stringCacheClearTimeout, undefined); + assert.equal((cache as any)._clearTimeout, undefined); // Cache entries should be recreated lazily after a timer-based reset. assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); @@ -1217,11 +1217,11 @@ describe('Buffer', () => { const cache = (buffer as any)._stringCache; assert.equal(cache.entries.size, 1); - assert.notEqual((buffer as any)._stringCacheClearTimeout, undefined); + assert.notEqual((cache as any)._clearTimeout, undefined); buffer.clear(); assert.equal(cache.entries.size, 0); - assert.equal((buffer as any)._stringCacheClearTimeout, undefined); + assert.equal((cache as any)._clearTimeout, undefined); buffer.fillViewportRows(); buffer.lines.get(0)!.setCell(0, createCellData(0, 'b', 1)); @@ -1230,7 +1230,7 @@ describe('Buffer', () => { buffer.resize(INIT_COLS - 1, INIT_ROWS); assert.equal(cache.entries.size, 0); - assert.equal((buffer as any)._stringCacheClearTimeout, undefined); + assert.equal((cache as any)._clearTimeout, undefined); }); }); diff --git a/src/common/buffer/Buffer.ts b/src/common/buffer/Buffer.ts index bf78e473c..99ae5d00b 100644 --- a/src/common/buffer/Buffer.ts +++ b/src/common/buffer/Buffer.ts @@ -4,6 +4,7 @@ */ import { CircularList, IInsertEvent } from 'common/CircularList'; +import { Disposable, toDisposable } from 'common/Lifecycle'; import { IdleTaskQueue } from 'common/TaskQueue'; import { IAttributeData, IBufferLine, ICellData, ICharset } from 'common/Types'; import { ExtendedAttrs } from 'common/buffer/AttributeData'; @@ -18,7 +19,6 @@ import { DEFAULT_CHARSET } from 'common/data/Charsets'; import { IBufferService, ILogService, IOptionsService } from 'common/services/Services'; export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1 -const STRING_CACHE_CLEAR_DELAY_MS = 50; /** * This class represents a terminal buffer (an internal state of the terminal), where the @@ -27,7 +27,7 @@ const STRING_CACHE_CLEAR_DELAY_MS = 50; * - cursor position * - scroll position */ -export class Buffer implements IBuffer { +export class Buffer extends Disposable implements IBuffer { public lines: CircularList; public ydisp: number = 0; public ybase: number = 0; @@ -53,7 +53,6 @@ export class Buffer implements IBuffer { private _memoryCleanupQueue: InstanceType; private _memoryCleanupPosition = 0; private readonly _stringCache: BufferLineStringCache; - private _stringCacheClearTimeout: ReturnType | undefined; constructor( private _hasScrollback: boolean, @@ -61,6 +60,8 @@ export class Buffer implements IBuffer { private _bufferService: IBufferService, private readonly _logService: ILogService ) { + super(); + this._register(toDisposable(() => this.clearAllMarkers())); this._cols = this._bufferService.cols; this._rows = this._bufferService.rows; this.lines = new CircularList(this._getCorrectBufferLength(this._rows)); @@ -68,7 +69,8 @@ export class Buffer implements IBuffer { this.scrollBottom = this._rows - 1; this.setupTabStops(); this._memoryCleanupQueue = new IdleTaskQueue(this._logService); - this._stringCache = new BufferLineStringCache(() => this._scheduleStringCacheCleanup()); + this._register(toDisposable(() => this._memoryCleanupQueue.clear())); + this._stringCache = this._register(new BufferLineStringCache()); } public getNullCell(attr?: IAttributeData): ICellData { @@ -143,7 +145,7 @@ export class Buffer implements IBuffer { * Clears the buffer to it's initial state, discarding all previous data. */ public clear(): void { - this._resetStringCache(); + this._stringCache.clear(); this.ydisp = 0; this.ybase = 0; this.y = 0; @@ -162,7 +164,7 @@ export class Buffer implements IBuffer { public resize(newCols: number, newRows: number): void { // store reference to null cell with default attrs const nullCell = this.getNullCell(DEFAULT_ATTR_DATA); - this._resetStringCache(); + this._stringCache.clear(); // count bufferlines with overly big memory to be cleaned afterwards let dirtyMemoryLines = 0; @@ -310,28 +312,6 @@ export class Buffer implements IBuffer { return normalRun; } - private _scheduleStringCacheCleanup(): void { - if (this._stringCacheClearTimeout !== undefined) { - clearTimeout(this._stringCacheClearTimeout); - } - this._stringCacheClearTimeout = setTimeout(() => { - this._stringCacheClearTimeout = undefined; - this._clearStringCache(); - }, STRING_CACHE_CLEAR_DELAY_MS); - } - - private _resetStringCache(): void { - if (this._stringCacheClearTimeout !== undefined) { - clearTimeout(this._stringCacheClearTimeout); - this._stringCacheClearTimeout = undefined; - } - this._clearStringCache(); - } - - private _clearStringCache(): void { - this._stringCache.clear(); - } - private get _isReflowEnabled(): boolean { const windowsPty = this._optionsService.rawOptions.windowsPty; if (windowsPty && windowsPty.buildNumber) { diff --git a/src/common/buffer/BufferLineStringCache.ts b/src/common/buffer/BufferLineStringCache.ts index 9365c847d..9382eaf83 100644 --- a/src/common/buffer/BufferLineStringCache.ts +++ b/src/common/buffer/BufferLineStringCache.ts @@ -4,15 +4,18 @@ */ import type { IBufferLineStringCache, IBufferLineStringCacheEntry } from 'common/buffer/BufferLine'; +import { disposableTimeout } from 'common/Async'; +import type { IDisposable } from 'common/Lifecycle'; -export class BufferLineStringCache implements IBufferLineStringCache { +const STRING_CACHE_CLEAR_DELAY_MS = 50; + +export class BufferLineStringCache implements IBufferLineStringCache, IDisposable { public generation: number = 0; public readonly entries: Set = new Set(); - - constructor(private readonly _onCacheUsed: () => void = () => { }) { } + private _clearTimeout: IDisposable | undefined; public touch(): void { - this._onCacheUsed(); + this._scheduleClear(); } public allocateEntry(): IBufferLineStringCacheEntry { @@ -22,11 +25,13 @@ export class BufferLineStringCache implements IBufferLineStringCache { generation: this.generation }; this.entries.add(entry); - this._onCacheUsed(); + this._scheduleClear(); return entry; } public clear(): void { + this._clearTimeout?.dispose(); + this._clearTimeout = undefined; this.generation++; for (const entry of this.entries) { entry.value = undefined; @@ -34,4 +39,16 @@ export class BufferLineStringCache implements IBufferLineStringCache { } this.entries.clear(); } + + public dispose(): void { + this.clear(); + } + + private _scheduleClear(): void { + this._clearTimeout?.dispose(); + this._clearTimeout = disposableTimeout(() => { + this._clearTimeout = undefined; + this.clear(); + }, STRING_CACHE_CLEAR_DELAY_MS); + } } diff --git a/src/common/buffer/BufferSet.test.ts b/src/common/buffer/BufferSet.test.ts index 96737fee9..072095346 100644 --- a/src/common/buffer/BufferSet.test.ts +++ b/src/common/buffer/BufferSet.test.ts @@ -6,7 +6,7 @@ import { assert } from 'chai'; import { BufferSet } from 'common/buffer/BufferSet'; import { Buffer } from 'common/buffer/Buffer'; -import { MockOptionsService, MockBufferService, MockLogService } from 'common/TestUtils.test'; +import { MockOptionsService, MockBufferService, MockLogService, createCellData } from 'common/TestUtils.test'; describe('BufferSet', () => { let bufferSet: BufferSet; @@ -82,4 +82,45 @@ describe('BufferSet', () => { assert.equal(bufferSet.alt.markers.length, 0); }); }); + + describe('lifecycle', () => { + it('should dispose previous buffers on reset', () => { + const oldNormal = bufferSet.normal as any; + oldNormal.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); + oldNormal.translateBufferLineToString(0, false); + + const oldCache = oldNormal._stringCache; + assert.equal(oldCache.entries.size, 1); + assert.notEqual(oldCache._clearTimeout, undefined); + + bufferSet.reset(); + + assert.notEqual(bufferSet.normal, oldNormal); + assert.equal(oldCache.entries.size, 0); + assert.equal(oldCache._clearTimeout, undefined); + }); + + it('should dispose both buffers when disposed', () => { + const normal = bufferSet.normal as any; + normal.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); + normal.translateBufferLineToString(0, false); + + bufferSet.activateAltBuffer(); + const alt = bufferSet.alt as any; + alt.lines.get(0)!.setCell(0, createCellData(0, 'b', 1)); + alt.translateBufferLineToString(0, false); + + const normalCache = normal._stringCache; + const altCache = alt._stringCache; + assert.notEqual(normalCache._clearTimeout, undefined); + assert.notEqual(altCache._clearTimeout, undefined); + + bufferSet.dispose(); + + assert.equal(normalCache.entries.size, 0); + assert.equal(altCache.entries.size, 0); + assert.equal(normalCache._clearTimeout, undefined); + assert.equal(altCache._clearTimeout, undefined); + }); + }); }); diff --git a/src/common/buffer/BufferSet.ts b/src/common/buffer/BufferSet.ts index 772a76442..32a4bf3b4 100644 --- a/src/common/buffer/BufferSet.ts +++ b/src/common/buffer/BufferSet.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Disposable } from 'common/Lifecycle'; +import { Disposable, MutableDisposable } from 'common/Lifecycle'; import { IAttributeData } from 'common/Types'; import { Buffer } from 'common/buffer/Buffer'; import { IBuffer, IBufferSet } from 'common/buffer/Types'; @@ -18,6 +18,8 @@ export class BufferSet extends Disposable implements IBufferSet { private _normal!: Buffer; private _alt!: Buffer; private _activeBuffer!: Buffer; + private readonly _normalBuffer = this._register(new MutableDisposable()); + private readonly _altBuffer = this._register(new MutableDisposable()); private readonly _onBufferActivate = this._register(new Emitter<{ activeBuffer: IBuffer, inactiveBuffer: IBuffer }>()); public readonly onBufferActivate = this._onBufferActivate.event; @@ -38,11 +40,13 @@ export class BufferSet extends Disposable implements IBufferSet { public reset(): void { this._normal = new Buffer(true, this._optionsService, this._bufferService, this._logService); + this._normalBuffer.value = this._normal; this._normal.fillViewportRows(); // The alt buffer should never have scrollback. // See http://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-The-Alternate-Screen-Buffer this._alt = new Buffer(false, this._optionsService, this._bufferService, this._logService); + this._altBuffer.value = this._alt; this._activeBuffer = this._normal; this._onBufferActivate.fire({ activeBuffer: this._normal, From 3dbfa7ce3e84426e77e612d85ff455fb8551e6b9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 05:48:08 -0700 Subject: [PATCH 07/11] Convert cache to Disposable --- src/common/buffer/Buffer.test.ts | 66 ++++++++++++++-------- src/common/buffer/BufferLineStringCache.ts | 28 ++++----- src/common/buffer/BufferSet.test.ts | 12 ++-- 3 files changed, 64 insertions(+), 42 deletions(-) diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index 76e13cc72..5a5cb3522 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -1182,32 +1182,52 @@ describe('Buffer', () => { }); describe('line string cache cleanup', () => { - it('should clear shared cache entries with a single timer', async () => { - buffer.fillViewportRows(); - buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); - buffer.lines.get(1)!.setCell(0, createCellData(0, 'b', 1)); + it('should clear shared cache entries with a single timer', () => { + const originalSetTimeout = globalThis.setTimeout; + const originalClearTimeout = globalThis.clearTimeout; + let timeoutId = 0; + const scheduledTimeouts = new Map void>(); + (globalThis as any).setTimeout = ((handler: (...args: any[]) => void) => { + const id = ++timeoutId; + scheduledTimeouts.set(id, () => handler()); + return id as ReturnType; + }) as typeof setTimeout; + (globalThis as any).clearTimeout = ((id: ReturnType) => { + scheduledTimeouts.delete(id as unknown as number); + }) as typeof clearTimeout; + try { + buffer.fillViewportRows(); + buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); + buffer.lines.get(1)!.setCell(0, createCellData(0, 'b', 1)); - assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); - assert.equal(buffer.translateBufferLineToString(1, false), `b${' '.repeat(INIT_COLS - 1)}`); + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.equal(buffer.translateBufferLineToString(1, false), `b${' '.repeat(INIT_COLS - 1)}`); - const cache = (buffer as any)._stringCache; - assert.equal(cache.entries.size, 2); - const previousTimer = (cache as any)._clearTimeout; - assert.notEqual(previousTimer, undefined); + const cache = (buffer as any)._stringCache; + assert.equal(cache.entries.size, 2); + const previousTimer = (cache as any)._clearTimeout.value; + assert.notEqual(previousTimer, undefined); - // Another translation should refresh the same shared timer, not create a second queue. - assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); - assert.notEqual((cache as any)._clearTimeout, undefined); - assert.notEqual((cache as any)._clearTimeout, previousTimer); + // Another translation should refresh the same shared timer, not create a second queue. + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.notEqual((cache as any)._clearTimeout.value, undefined); + assert.notEqual((cache as any)._clearTimeout.value, previousTimer); - await new Promise(r => setTimeout(r, 70)); + for (const timeout of scheduledTimeouts.values()) { + timeout(); + } + scheduledTimeouts.clear(); - assert.equal(cache.entries.size, 0); - assert.equal((cache as any)._clearTimeout, undefined); + assert.equal(cache.entries.size, 0); + assert.equal((cache as any)._clearTimeout.value, undefined); - // Cache entries should be recreated lazily after a timer-based reset. - assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); - assert.equal(cache.entries.size, 1); + // Cache entries should be recreated lazily after a timer-based reset. + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.equal(cache.entries.size, 1); + } finally { + globalThis.setTimeout = originalSetTimeout; + globalThis.clearTimeout = originalClearTimeout; + } }); it('should reset line string cache state on clear and resize', () => { @@ -1217,11 +1237,11 @@ describe('Buffer', () => { const cache = (buffer as any)._stringCache; assert.equal(cache.entries.size, 1); - assert.notEqual((cache as any)._clearTimeout, undefined); + assert.notEqual((cache as any)._clearTimeout.value, undefined); buffer.clear(); assert.equal(cache.entries.size, 0); - assert.equal((cache as any)._clearTimeout, undefined); + assert.equal((cache as any)._clearTimeout.value, undefined); buffer.fillViewportRows(); buffer.lines.get(0)!.setCell(0, createCellData(0, 'b', 1)); @@ -1230,7 +1250,7 @@ describe('Buffer', () => { buffer.resize(INIT_COLS - 1, INIT_ROWS); assert.equal(cache.entries.size, 0); - assert.equal((cache as any)._clearTimeout, undefined); + assert.equal((cache as any)._clearTimeout.value, undefined); }); }); diff --git a/src/common/buffer/BufferLineStringCache.ts b/src/common/buffer/BufferLineStringCache.ts index 9382eaf83..547afc85f 100644 --- a/src/common/buffer/BufferLineStringCache.ts +++ b/src/common/buffer/BufferLineStringCache.ts @@ -5,14 +5,22 @@ import type { IBufferLineStringCache, IBufferLineStringCacheEntry } from 'common/buffer/BufferLine'; import { disposableTimeout } from 'common/Async'; +import { Disposable, MutableDisposable, toDisposable } from 'common/Lifecycle'; import type { IDisposable } from 'common/Lifecycle'; -const STRING_CACHE_CLEAR_DELAY_MS = 50; +const enum Constants { + CacheTtlMs = 15000 +} -export class BufferLineStringCache implements IBufferLineStringCache, IDisposable { +export class BufferLineStringCache extends Disposable implements IBufferLineStringCache { public generation: number = 0; public readonly entries: Set = new Set(); - private _clearTimeout: IDisposable | undefined; + private readonly _clearTimeout = this._register(new MutableDisposable()); + + constructor() { + super(); + this._register(toDisposable(() => this.entries.clear())); + } public touch(): void { this._scheduleClear(); @@ -30,8 +38,7 @@ export class BufferLineStringCache implements IBufferLineStringCache, IDisposabl } public clear(): void { - this._clearTimeout?.dispose(); - this._clearTimeout = undefined; + this._clearTimeout.clear(); this.generation++; for (const entry of this.entries) { entry.value = undefined; @@ -40,15 +47,10 @@ export class BufferLineStringCache implements IBufferLineStringCache, IDisposabl this.entries.clear(); } - public dispose(): void { - this.clear(); - } - private _scheduleClear(): void { - this._clearTimeout?.dispose(); - this._clearTimeout = disposableTimeout(() => { - this._clearTimeout = undefined; + this._clearTimeout.value = disposableTimeout(() => { + this._clearTimeout.clear(); this.clear(); - }, STRING_CACHE_CLEAR_DELAY_MS); + }, Constants.CacheTtlMs); } } diff --git a/src/common/buffer/BufferSet.test.ts b/src/common/buffer/BufferSet.test.ts index 072095346..8809a3cb6 100644 --- a/src/common/buffer/BufferSet.test.ts +++ b/src/common/buffer/BufferSet.test.ts @@ -91,13 +91,13 @@ describe('BufferSet', () => { const oldCache = oldNormal._stringCache; assert.equal(oldCache.entries.size, 1); - assert.notEqual(oldCache._clearTimeout, undefined); + assert.notEqual(oldCache._clearTimeout.value, undefined); bufferSet.reset(); assert.notEqual(bufferSet.normal, oldNormal); assert.equal(oldCache.entries.size, 0); - assert.equal(oldCache._clearTimeout, undefined); + assert.equal(oldCache._clearTimeout.value, undefined); }); it('should dispose both buffers when disposed', () => { @@ -112,15 +112,15 @@ describe('BufferSet', () => { const normalCache = normal._stringCache; const altCache = alt._stringCache; - assert.notEqual(normalCache._clearTimeout, undefined); - assert.notEqual(altCache._clearTimeout, undefined); + assert.notEqual(normalCache._clearTimeout.value, undefined); + assert.notEqual(altCache._clearTimeout.value, undefined); bufferSet.dispose(); assert.equal(normalCache.entries.size, 0); assert.equal(altCache.entries.size, 0); - assert.equal(normalCache._clearTimeout, undefined); - assert.equal(altCache._clearTimeout, undefined); + assert.equal(normalCache._clearTimeout.value, undefined); + assert.equal(altCache._clearTimeout.value, undefined); }); }); }); From d911b40a47e5bad40c65d1a21c7b2658f8bd229c Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 05:52:33 -0700 Subject: [PATCH 08/11] Avoid any in test --- .../dom/DomRendererRowFactory.test.ts | 2 - src/common/buffer/Buffer.test.ts | 44 ++++++++++++------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.test.ts b/src/browser/renderer/dom/DomRendererRowFactory.test.ts index b7c60e8f3..6ca84569a 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.test.ts @@ -15,10 +15,8 @@ import { MockCoreService, MockDecorationService, MockOptionsService, createCellD import { MockCharacterJoinerService, MockCoreBrowserService, MockThemeService } from 'browser/TestUtils.test'; import { TestWidthCache } from 'browser/renderer/dom/WidthCache.test'; -const dom = new jsdom.JSDOM(''); const TEST_STRING_CACHE = new BufferLineStringCache(); - describe('DomRendererRowFactory', () => { let dom: jsdom.JSDOM; let rowFactory: DomRendererRowFactory; diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index 5a5cb3522..ae086554e 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -17,15 +17,25 @@ const INIT_ROWS = 24; const INIT_SCROLLBACK = 1000; const TEST_STRING_CACHE = new BufferLineStringCache(); +class TestBuffer extends Buffer { + public getStringCache(): BufferLineStringCache { + return (this as unknown as { _stringCache: BufferLineStringCache })._stringCache; + } + + public getStringCacheClearTimeout(): unknown { + return (this.getStringCache() as unknown as { _clearTimeout: { value: unknown } })._clearTimeout.value; + } +} + describe('Buffer', () => { let optionsService: MockOptionsService; let bufferService: MockBufferService; - let buffer: Buffer; + let buffer: TestBuffer; beforeEach(() => { optionsService = new MockOptionsService({ scrollback: INIT_SCROLLBACK }); bufferService = new MockBufferService(INIT_COLS, INIT_ROWS); - buffer = new Buffer(true, optionsService, bufferService, new MockLogService()); + buffer = new TestBuffer(true, optionsService, bufferService, new MockLogService()); }); describe('constructor', () => { @@ -153,7 +163,7 @@ describe('Buffer', () => { describe('no scrollback', () => { it('should trim from the top of the buffer when the cursor reaches the bottom', () => { - buffer = new Buffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); + buffer = new TestBuffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); assert.equal(buffer.lines.maxLength, INIT_ROWS); buffer.y = INIT_ROWS - 1; buffer.fillViewportRows(); @@ -1056,7 +1066,7 @@ describe('Buffer', () => { describe('buffer marked to have no scrollback', () => { it('should always have a scrollback of 0', () => { // Test size on initialization - buffer = new Buffer(false, new MockOptionsService({ scrollback: 1000 }), bufferService, new MockLogService()); + buffer = new TestBuffer(false, new MockOptionsService({ scrollback: 1000 }), bufferService, new MockLogService()); buffer.fillViewportRows(); assert.equal(buffer.lines.maxLength, INIT_ROWS); // Test size on buffer increase @@ -1070,7 +1080,7 @@ describe('Buffer', () => { describe('addMarker', () => { it('should adjust a marker line when the buffer is trimmed', () => { - buffer = new Buffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); + buffer = new TestBuffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); buffer.fillViewportRows(); const marker = buffer.addMarker(buffer.lines.length - 1); assert.equal(marker.line, buffer.lines.length - 1); @@ -1078,7 +1088,7 @@ describe('Buffer', () => { assert.equal(marker.line, buffer.lines.length - 2); }); it('should dispose of a marker if it is trimmed off the buffer', () => { - buffer = new Buffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); + buffer = new TestBuffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); buffer.fillViewportRows(); assert.equal(buffer.markers.length, 0); const marker = buffer.addMarker(0); @@ -1090,7 +1100,7 @@ describe('Buffer', () => { }); it('should call onDispose', () => { const eventStack: string[] = []; - buffer = new Buffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); + buffer = new TestBuffer(true, new MockOptionsService({ scrollback: 0 }), bufferService, new MockLogService()); buffer.fillViewportRows(); assert.equal(buffer.markers.length, 0); const marker = buffer.addMarker(0); @@ -1203,15 +1213,15 @@ describe('Buffer', () => { assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); assert.equal(buffer.translateBufferLineToString(1, false), `b${' '.repeat(INIT_COLS - 1)}`); - const cache = (buffer as any)._stringCache; + const cache = buffer.getStringCache(); assert.equal(cache.entries.size, 2); - const previousTimer = (cache as any)._clearTimeout.value; - assert.notEqual(previousTimer, undefined); + const previousTimer = buffer.getStringCacheClearTimeout(); + assert.ok(previousTimer !== undefined); // Another translation should refresh the same shared timer, not create a second queue. assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); - assert.notEqual((cache as any)._clearTimeout.value, undefined); - assert.notEqual((cache as any)._clearTimeout.value, previousTimer); + assert.ok(buffer.getStringCacheClearTimeout() !== undefined); + assert.notEqual(buffer.getStringCacheClearTimeout(), previousTimer); for (const timeout of scheduledTimeouts.values()) { timeout(); @@ -1219,7 +1229,7 @@ describe('Buffer', () => { scheduledTimeouts.clear(); assert.equal(cache.entries.size, 0); - assert.equal((cache as any)._clearTimeout.value, undefined); + assert.equal(buffer.getStringCacheClearTimeout(), undefined); // Cache entries should be recreated lazily after a timer-based reset. assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); @@ -1235,13 +1245,13 @@ describe('Buffer', () => { buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); buffer.translateBufferLineToString(0, false); - const cache = (buffer as any)._stringCache; + const cache = buffer.getStringCache(); assert.equal(cache.entries.size, 1); - assert.notEqual((cache as any)._clearTimeout.value, undefined); + assert.ok(buffer.getStringCacheClearTimeout() !== undefined); buffer.clear(); assert.equal(cache.entries.size, 0); - assert.equal((cache as any)._clearTimeout.value, undefined); + assert.equal(buffer.getStringCacheClearTimeout(), undefined); buffer.fillViewportRows(); buffer.lines.get(0)!.setCell(0, createCellData(0, 'b', 1)); @@ -1250,7 +1260,7 @@ describe('Buffer', () => { buffer.resize(INIT_COLS - 1, INIT_ROWS); assert.equal(cache.entries.size, 0); - assert.equal((cache as any)._clearTimeout.value, undefined); + assert.equal(buffer.getStringCacheClearTimeout(), undefined); }); }); From e04dd2501f9779e739782e1b8152a03ab3dae164 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 05:58:56 -0700 Subject: [PATCH 09/11] Avoid thrashing timeouts --- src/common/buffer/Buffer.test.ts | 50 +++++++++++++++------- src/common/buffer/Buffer.ts | 2 +- src/common/buffer/BufferLineStringCache.ts | 20 +++++++-- 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index ae086554e..38fd513d3 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -1195,16 +1195,28 @@ describe('Buffer', () => { it('should clear shared cache entries with a single timer', () => { const originalSetTimeout = globalThis.setTimeout; const originalClearTimeout = globalThis.clearTimeout; + const originalDateNow = Date.now; let timeoutId = 0; - const scheduledTimeouts = new Map void>(); - (globalThis as any).setTimeout = ((handler: (...args: any[]) => void) => { + let now = 0; + const clearedTimeouts: number[] = []; + const scheduledTimeouts = new Map void }>(); + (globalThis as any).setTimeout = ((handler: (...args: any[]) => void, timeout?: number) => { const id = ++timeoutId; - scheduledTimeouts.set(id, () => handler()); + scheduledTimeouts.set(id, { + delay: timeout ?? 0, + fire: () => { + scheduledTimeouts.delete(id); + handler(); + } + }); return id as ReturnType; }) as typeof setTimeout; (globalThis as any).clearTimeout = ((id: ReturnType) => { - scheduledTimeouts.delete(id as unknown as number); + const numericId = id as unknown as number; + clearedTimeouts.push(numericId); + scheduledTimeouts.delete(numericId); }) as typeof clearTimeout; + Date.now = () => now; try { buffer.fillViewportRows(); buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1)); @@ -1215,26 +1227,34 @@ describe('Buffer', () => { const cache = buffer.getStringCache(); assert.equal(cache.entries.size, 2); - const previousTimer = buffer.getStringCacheClearTimeout(); - assert.ok(previousTimer !== undefined); - - // Another translation should refresh the same shared timer, not create a second queue. - assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); assert.ok(buffer.getStringCacheClearTimeout() !== undefined); - assert.notEqual(buffer.getStringCacheClearTimeout(), previousTimer); + assert.equal(scheduledTimeouts.size, 1); + assert.equal([...scheduledTimeouts.values()][0].delay, 15000); + const initialTimerCreationCount = timeoutId; - for (const timeout of scheduledTimeouts.values()) { - timeout(); - } - scheduledTimeouts.clear(); + now = 5000; + assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); + assert.equal(timeoutId, initialTimerCreationCount); + assert.equal(scheduledTimeouts.size, 1); + assert.deepEqual(clearedTimeouts, []); + + now = 15000; + [...scheduledTimeouts.values()][0].fire(); + assert.equal(timeoutId, initialTimerCreationCount + 1); + assert.ok(buffer.getStringCacheClearTimeout() !== undefined); + assert.equal(scheduledTimeouts.size, 1); + assert.equal([...scheduledTimeouts.values()][0].delay, 5000); + + now = 20000; + [...scheduledTimeouts.values()][0].fire(); assert.equal(cache.entries.size, 0); assert.equal(buffer.getStringCacheClearTimeout(), undefined); - // Cache entries should be recreated lazily after a timer-based reset. assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`); assert.equal(cache.entries.size, 1); } finally { + Date.now = originalDateNow; globalThis.setTimeout = originalSetTimeout; globalThis.clearTimeout = originalClearTimeout; } diff --git a/src/common/buffer/Buffer.ts b/src/common/buffer/Buffer.ts index 99ae5d00b..34ef02f6a 100644 --- a/src/common/buffer/Buffer.ts +++ b/src/common/buffer/Buffer.ts @@ -61,7 +61,6 @@ export class Buffer extends Disposable implements IBuffer { private readonly _logService: ILogService ) { super(); - this._register(toDisposable(() => this.clearAllMarkers())); this._cols = this._bufferService.cols; this._rows = this._bufferService.rows; this.lines = new CircularList(this._getCorrectBufferLength(this._rows)); @@ -70,6 +69,7 @@ export class Buffer extends Disposable implements IBuffer { this.setupTabStops(); this._memoryCleanupQueue = new IdleTaskQueue(this._logService); this._register(toDisposable(() => this._memoryCleanupQueue.clear())); + this._register(toDisposable(() => this.clearAllMarkers())); this._stringCache = this._register(new BufferLineStringCache()); } diff --git a/src/common/buffer/BufferLineStringCache.ts b/src/common/buffer/BufferLineStringCache.ts index 547afc85f..0918a66ba 100644 --- a/src/common/buffer/BufferLineStringCache.ts +++ b/src/common/buffer/BufferLineStringCache.ts @@ -16,6 +16,7 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri public generation: number = 0; public readonly entries: Set = new Set(); private readonly _clearTimeout = this._register(new MutableDisposable()); + private _lastAccessTimestamp: number = 0; constructor() { super(); @@ -39,6 +40,7 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri public clear(): void { this._clearTimeout.clear(); + this._lastAccessTimestamp = 0; this.generation++; for (const entry of this.entries) { entry.value = undefined; @@ -48,9 +50,21 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri } private _scheduleClear(): void { + this._lastAccessTimestamp = Date.now(); + if (this._clearTimeout.value) { + return; + } + this._scheduleClearTimeout(Constants.CacheTtlMs); + } + + private _scheduleClearTimeout(timeoutMs: number): void { this._clearTimeout.value = disposableTimeout(() => { - this._clearTimeout.clear(); - this.clear(); - }, Constants.CacheTtlMs); + const elapsed = Date.now() - this._lastAccessTimestamp; + if (elapsed >= Constants.CacheTtlMs) { + this.clear(); + return; + } + this._scheduleClearTimeout(Constants.CacheTtlMs - elapsed); + }, timeoutMs); } } From 093e81b9a7243ceb23292920345a15a691f7a51d Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 06:07:41 -0700 Subject: [PATCH 10/11] Fix build, lint --- src/common/buffer/Buffer.test.ts | 2 +- src/common/buffer/BufferLineStringCache.ts | 11 +++++------ src/headless/tsconfig.json | 3 ++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index 38fd513d3..abfea7b06 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -1199,7 +1199,7 @@ describe('Buffer', () => { let timeoutId = 0; let now = 0; const clearedTimeouts: number[] = []; - const scheduledTimeouts = new Map void }>(); + const scheduledTimeouts = new Map void }>(); (globalThis as any).setTimeout = ((handler: (...args: any[]) => void, timeout?: number) => { const id = ++timeoutId; scheduledTimeouts.set(id, { diff --git a/src/common/buffer/BufferLineStringCache.ts b/src/common/buffer/BufferLineStringCache.ts index 0918a66ba..b365eef70 100644 --- a/src/common/buffer/BufferLineStringCache.ts +++ b/src/common/buffer/BufferLineStringCache.ts @@ -5,11 +5,10 @@ import type { IBufferLineStringCache, IBufferLineStringCacheEntry } from 'common/buffer/BufferLine'; import { disposableTimeout } from 'common/Async'; -import { Disposable, MutableDisposable, toDisposable } from 'common/Lifecycle'; -import type { IDisposable } from 'common/Lifecycle'; +import { Disposable, MutableDisposable, toDisposable, type IDisposable } from 'common/Lifecycle'; const enum Constants { - CacheTtlMs = 15000 + CACHE_TTL_MS = 15000 } export class BufferLineStringCache extends Disposable implements IBufferLineStringCache { @@ -54,17 +53,17 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri if (this._clearTimeout.value) { return; } - this._scheduleClearTimeout(Constants.CacheTtlMs); + this._scheduleClearTimeout(Constants.CACHE_TTL_MS); } private _scheduleClearTimeout(timeoutMs: number): void { this._clearTimeout.value = disposableTimeout(() => { const elapsed = Date.now() - this._lastAccessTimestamp; - if (elapsed >= Constants.CacheTtlMs) { + if (elapsed >= Constants.CACHE_TTL_MS) { this.clear(); return; } - this._scheduleClearTimeout(Constants.CacheTtlMs - elapsed); + this._scheduleClearTimeout(Constants.CACHE_TTL_MS - elapsed); }, timeoutMs); } } diff --git a/src/headless/tsconfig.json b/src/headless/tsconfig.json index 6ae7b1dc9..0c445bcfc 100644 --- a/src/headless/tsconfig.json +++ b/src/headless/tsconfig.json @@ -3,7 +3,8 @@ "compilerOptions": { "lib": [ "es2015", - "es2016.Array.Include" + "es2016.Array.Include", + "es2021.weakref" ], "outDir": "../../out", "types": [ From 4ebb3fffe9856d672a6254378f83da37e28ab13a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 2 May 2026 06:21:42 -0700 Subject: [PATCH 11/11] Ignore cache keys in serialize addon Bit hacky, but this is due to the way we're serializing the line in the test --- addons/addon-serialize/test/SerializeAddon.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/addons/addon-serialize/test/SerializeAddon.test.ts b/addons/addon-serialize/test/SerializeAddon.test.ts index ee5355fa6..fb42d807d 100644 --- a/addons/addon-serialize/test/SerializeAddon.test.ts +++ b/addons/addon-serialize/test/SerializeAddon.test.ts @@ -49,7 +49,14 @@ test.describe('SerializeAddon', () => { for (let i = 0; i < buffer.length; i++) { // Do this intentionally to get content of underlining source const bufferLine = buffer.getLine(i)._line; - lines.push(JSON.stringify(bufferLine)); + lines.push(JSON.stringify(bufferLine, (key, value) => { + // BufferLine caches are internal/transient and can legitimately differ + // across equivalent terminal states. + if (key === '_stringCache' || key === '_stringCacheEntryRef') { + return undefined; + } + return value; + })); } return { x: buffer.cursorX,