Skip to content

Commit e04dd25

Browse files
committed
Avoid thrashing timeouts
1 parent d911b40 commit e04dd25

3 files changed

Lines changed: 51 additions & 17 deletions

File tree

src/common/buffer/Buffer.test.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,16 +1195,28 @@ describe('Buffer', () => {
11951195
it('should clear shared cache entries with a single timer', () => {
11961196
const originalSetTimeout = globalThis.setTimeout;
11971197
const originalClearTimeout = globalThis.clearTimeout;
1198+
const originalDateNow = Date.now;
11981199
let timeoutId = 0;
1199-
const scheduledTimeouts = new Map<number, () => void>();
1200-
(globalThis as any).setTimeout = ((handler: (...args: any[]) => void) => {
1200+
let now = 0;
1201+
const clearedTimeouts: number[] = [];
1202+
const scheduledTimeouts = new Map<number, { delay: number; fire: () => void }>();
1203+
(globalThis as any).setTimeout = ((handler: (...args: any[]) => void, timeout?: number) => {
12011204
const id = ++timeoutId;
1202-
scheduledTimeouts.set(id, () => handler());
1205+
scheduledTimeouts.set(id, {
1206+
delay: timeout ?? 0,
1207+
fire: () => {
1208+
scheduledTimeouts.delete(id);
1209+
handler();
1210+
}
1211+
});
12031212
return id as ReturnType<typeof setTimeout>;
12041213
}) as typeof setTimeout;
12051214
(globalThis as any).clearTimeout = ((id: ReturnType<typeof setTimeout>) => {
1206-
scheduledTimeouts.delete(id as unknown as number);
1215+
const numericId = id as unknown as number;
1216+
clearedTimeouts.push(numericId);
1217+
scheduledTimeouts.delete(numericId);
12071218
}) as typeof clearTimeout;
1219+
Date.now = () => now;
12081220
try {
12091221
buffer.fillViewportRows();
12101222
buffer.lines.get(0)!.setCell(0, createCellData(0, 'a', 1));
@@ -1215,26 +1227,34 @@ describe('Buffer', () => {
12151227

12161228
const cache = buffer.getStringCache();
12171229
assert.equal(cache.entries.size, 2);
1218-
const previousTimer = buffer.getStringCacheClearTimeout();
1219-
assert.ok(previousTimer !== undefined);
1230+
assert.ok(buffer.getStringCacheClearTimeout() !== undefined);
1231+
assert.equal(scheduledTimeouts.size, 1);
1232+
assert.equal([...scheduledTimeouts.values()][0].delay, 15000);
1233+
const initialTimerCreationCount = timeoutId;
12201234

1221-
// Another translation should refresh the same shared timer, not create a second queue.
1235+
now = 5000;
12221236
assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`);
1237+
assert.equal(timeoutId, initialTimerCreationCount);
1238+
assert.equal(scheduledTimeouts.size, 1);
1239+
assert.deepEqual(clearedTimeouts, []);
1240+
1241+
now = 15000;
1242+
[...scheduledTimeouts.values()][0].fire();
1243+
assert.equal(timeoutId, initialTimerCreationCount + 1);
12231244
assert.ok(buffer.getStringCacheClearTimeout() !== undefined);
1224-
assert.notEqual(buffer.getStringCacheClearTimeout(), previousTimer);
1245+
assert.equal(scheduledTimeouts.size, 1);
1246+
assert.equal([...scheduledTimeouts.values()][0].delay, 5000);
12251247

1226-
for (const timeout of scheduledTimeouts.values()) {
1227-
timeout();
1228-
}
1229-
scheduledTimeouts.clear();
1248+
now = 20000;
1249+
[...scheduledTimeouts.values()][0].fire();
12301250

12311251
assert.equal(cache.entries.size, 0);
12321252
assert.equal(buffer.getStringCacheClearTimeout(), undefined);
12331253

1234-
// Cache entries should be recreated lazily after a timer-based reset.
12351254
assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`);
12361255
assert.equal(cache.entries.size, 1);
12371256
} finally {
1257+
Date.now = originalDateNow;
12381258
globalThis.setTimeout = originalSetTimeout;
12391259
globalThis.clearTimeout = originalClearTimeout;
12401260
}

src/common/buffer/Buffer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ export class Buffer extends Disposable implements IBuffer {
6161
private readonly _logService: ILogService
6262
) {
6363
super();
64-
this._register(toDisposable(() => this.clearAllMarkers()));
6564
this._cols = this._bufferService.cols;
6665
this._rows = this._bufferService.rows;
6766
this.lines = new CircularList<IBufferLine>(this._getCorrectBufferLength(this._rows));
@@ -70,6 +69,7 @@ export class Buffer extends Disposable implements IBuffer {
7069
this.setupTabStops();
7170
this._memoryCleanupQueue = new IdleTaskQueue(this._logService);
7271
this._register(toDisposable(() => this._memoryCleanupQueue.clear()));
72+
this._register(toDisposable(() => this.clearAllMarkers()));
7373
this._stringCache = this._register(new BufferLineStringCache());
7474
}
7575

src/common/buffer/BufferLineStringCache.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri
1616
public generation: number = 0;
1717
public readonly entries: Set<IBufferLineStringCacheEntry> = new Set();
1818
private readonly _clearTimeout = this._register(new MutableDisposable<IDisposable>());
19+
private _lastAccessTimestamp: number = 0;
1920

2021
constructor() {
2122
super();
@@ -39,6 +40,7 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri
3940

4041
public clear(): void {
4142
this._clearTimeout.clear();
43+
this._lastAccessTimestamp = 0;
4244
this.generation++;
4345
for (const entry of this.entries) {
4446
entry.value = undefined;
@@ -48,9 +50,21 @@ export class BufferLineStringCache extends Disposable implements IBufferLineStri
4850
}
4951

5052
private _scheduleClear(): void {
53+
this._lastAccessTimestamp = Date.now();
54+
if (this._clearTimeout.value) {
55+
return;
56+
}
57+
this._scheduleClearTimeout(Constants.CacheTtlMs);
58+
}
59+
60+
private _scheduleClearTimeout(timeoutMs: number): void {
5161
this._clearTimeout.value = disposableTimeout(() => {
52-
this._clearTimeout.clear();
53-
this.clear();
54-
}, Constants.CacheTtlMs);
62+
const elapsed = Date.now() - this._lastAccessTimestamp;
63+
if (elapsed >= Constants.CacheTtlMs) {
64+
this.clear();
65+
return;
66+
}
67+
this._scheduleClearTimeout(Constants.CacheTtlMs - elapsed);
68+
}, timeoutMs);
5569
}
5670
}

0 commit comments

Comments
 (0)