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] 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); } }