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] 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,