From 1f5159febe263698042c8faf95009307df46217a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sat, 3 Dec 2022 16:49:26 +0100 Subject: [PATCH] use IdleTaskQueue & Date.now; some testcases --- src/common/TaskQueue.ts | 30 +++++++++++++++++--------- src/common/buffer/Buffer.test.ts | 30 ++++++++++++++++++++++++++ src/common/buffer/Buffer.ts | 37 +++++++++++++++++++------------- 3 files changed, 72 insertions(+), 25 deletions(-) diff --git a/src/common/TaskQueue.ts b/src/common/TaskQueue.ts index 94c5c53b..021bb7ce 100644 --- a/src/common/TaskQueue.ts +++ b/src/common/TaskQueue.ts @@ -8,8 +8,11 @@ import { isNode } from 'common/Platform'; interface ITaskQueue { /** * Adds a task to the queue which will run in a future idle callback. + * To avoid perceivable stalls on the mainthread, tasks with heavy workload + * should split their work into smaller pieces and return `true` to get + * called again until the work is done (on falsy return value). */ - enqueue(task: () => void): void; + enqueue(task: () => boolean | void): void; /** * Flushes the queue, running all remaining tasks synchronously. @@ -28,21 +31,23 @@ interface ITaskDeadline { type CallbackWithDeadline = (deadline: ITaskDeadline) => void; abstract class TaskQueue implements ITaskQueue { - private _tasks: (() => void)[] = []; + private _tasks: (() => boolean | void)[] = []; private _idleCallback?: number; private _i = 0; protected abstract _requestCallback(callback: CallbackWithDeadline): number; protected abstract _cancelCallback(identifier: number): void; - public enqueue(task: () => void): void { + public enqueue(task: () => boolean | void): void { this._tasks.push(task); this._start(); } public flush(): void { while (this._i < this._tasks.length) { - this._tasks[this._i++](); + if (!this._tasks[this._i]()) { + this._i++; + } } this.clear(); } @@ -67,9 +72,14 @@ abstract class TaskQueue implements ITaskQueue { let taskDuration = 0; let longestTask = 0; while (this._i < this._tasks.length) { - taskDuration = performance.now(); - this._tasks[this._i++](); - taskDuration = performance.now() - taskDuration; + taskDuration = Date.now(); + if (!this._tasks[this._i]()) { + this._i++; + } + // other than performance.now, Date.now might not be stable (changes on wall clock changes), + // this is not an issue here as a clock change during a short running task is very unlikely + // in case it still happened and leads to negative duration, simply assume 1 msec + taskDuration = Math.max(1, Date.now() - taskDuration); longestTask = Math.max(taskDuration, longestTask); // Guess the following task will take a similar time to the longest task in this batch, allow // additional room to try avoid exceeding the deadline @@ -97,9 +107,9 @@ export class PriorityTaskQueue extends TaskQueue { } private _createDeadline(duration: number): ITaskDeadline { - const end = performance.now() + duration; + const end = Date.now() + duration; return { - timeRemaining: () => Math.max(0, end - performance.now()) + timeRemaining: () => Math.max(0, end - Date.now()) }; } } @@ -136,7 +146,7 @@ export class DebouncedIdleTask { this._queue = new IdleTaskQueue(); } - public set(task: () => void): void { + public set(task: () => boolean | void): void { this._queue.clear(); this._queue.enqueue(task); } diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index e5ea7f5e..e854ce34 100644 --- a/src/common/buffer/Buffer.test.ts +++ b/src/common/buffer/Buffer.test.ts @@ -9,6 +9,7 @@ import { CircularList } from 'common/CircularList'; import { MockOptionsService, MockBufferService } from 'common/TestUtils.test'; import { BufferLine, DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; import { CellData } from 'common/buffer/CellData'; +import { ExtendedAttrs } from 'common/buffer/AttributeData'; const INIT_COLS = 80; const INIT_ROWS = 24; @@ -1177,4 +1178,33 @@ describe('Buffer', () => { assert.equal(str3, '😁a'); }); }); + + describe('memory cleanup after shrinking', () => { + it('should realign memory from idle task execution', async () => { + buffer.fillViewportRows(); + + // shrink more than 2 times to trigger lazy memory cleanup + buffer.resize(INIT_COLS / 2 - 1, INIT_ROWS); + + // sync + for (let i = 0; i < INIT_ROWS; i++) { + const line = buffer.lines.get(i)!; + // line memory is still at old size from initialization + assert.equal((line as any)._data.buffer.byteLength, INIT_COLS * 3 * 4); + // array.length and .length get immediately adjusted + assert.equal((line as any)._data.length, (INIT_COLS / 2 - 1) * 3); + assert.equal(line.length, INIT_COLS / 2 - 1); + } + + // wait for a bit to give IdleTaskQueue a chance to kick in + // and finish memory cleaning + await new Promise(r => setTimeout(r, 100)); + + // cleanup should have realigned memory with exact bytelength + for (let i = 0; i < INIT_ROWS; i++) { + const line = buffer.lines.get(i)!; + assert.equal((line as any)._data.buffer.byteLength, (INIT_COLS / 2 - 1) * 3 * 4); + } + }); + }); }); diff --git a/src/common/buffer/Buffer.ts b/src/common/buffer/Buffer.ts index 5c67be56..4890e7c2 100644 --- a/src/common/buffer/Buffer.ts +++ b/src/common/buffer/Buffer.ts @@ -14,7 +14,7 @@ import { Marker } from 'common/buffer/Marker'; import { IOptionsService, IBufferService } from 'common/services/Services'; import { DEFAULT_CHARSET } from 'common/data/Charsets'; import { ExtendedAttrs } from 'common/buffer/AttributeData'; -import { DebouncedIdleTask } from 'common/TaskQueue'; +import { DebouncedIdleTask, IdleTaskQueue } from 'common/TaskQueue'; export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1 @@ -151,8 +151,8 @@ export class Buffer implements IBuffer { // store reference to null cell with default attrs const nullCell = this.getNullCell(DEFAULT_ATTR_DATA); - // defer memory cleanup of bufferlines - let needsCleanup = 0; + // count bufferlines with overly big memory to be cleaned afterwards + let dirtyMemoryLines = 0; // Increase max length if needed before adjustments to allow space to fill // as required. @@ -168,7 +168,7 @@ export class Buffer implements IBuffer { if (this._cols < newCols) { for (let i = 0; i < this.lines.length; i++) { // +boolean for fast 0 or 1 conversion - needsCleanup |= +this.lines.get(i)!.resize(newCols, nullCell); + dirtyMemoryLines += +this.lines.get(i)!.resize(newCols, nullCell); } } @@ -248,7 +248,7 @@ export class Buffer implements IBuffer { if (this._cols > newCols) { for (let i = 0; i < this.lines.length; i++) { // +boolean for fast 0 or 1 conversion - needsCleanup |= +this.lines.get(i)!.resize(newCols, nullCell); + dirtyMemoryLines += +this.lines.get(i)!.resize(newCols, nullCell); } } } @@ -256,26 +256,33 @@ export class Buffer implements IBuffer { this._cols = newCols; this._rows = newRows; - if (needsCleanup) { - this._memoryCleanupTask.set(() => this._cleanupMemory()); - } else { - // FIXME: DebouncedIdleTask has no clear method? - this._memoryCleanupTask.set(() => {}); + this._memoryCleanupQueue.clear(); + // schedule memory cleanup only, if more than 10% of the lines are affected + if (dirtyMemoryLines > 0.1 * this.lines.length) { + this._memoryCleanupQueue.enqueue(() => this._batchedMemoryCleanup()); } } - private _memoryCleanupTask: DebouncedIdleTask = new DebouncedIdleTask(); + private _memoryCleanupQueue = new IdleTaskQueue(); - private _cleanupMemory(): void { + private _batchedMemoryCleanup(): boolean { let counted = 0; for (let i = 0; i < this.lines.length; i++) { counted += this.lines.get(i)!.cleanupMemory(); - // throttle to 5k lines + // throttle to 5k lines at once and + // return true to indicate, that the task is not finished yet if (counted > 5000) { - this._memoryCleanupTask.set(() => this._cleanupMemory()); - break; + return true; } } + return false; + } + + private _forceMemoryCleanup(): void { + this._memoryCleanupQueue.clear(); + for (let i = 0; i < this.lines.length; i++) { + this.lines.get(i)!.cleanupMemory(); + } } private get _isReflowEnabled(): boolean {