diff --git a/src/common/TaskQueue.ts b/src/common/TaskQueue.ts index 94c5c53b..29c29f64 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(); } @@ -66,17 +71,31 @@ abstract class TaskQueue implements ITaskQueue { this._idleCallback = undefined; let taskDuration = 0; let longestTask = 0; + let lastDeadlineRemaining = deadline.timeRemaining(); + let deadlineRemaining = 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 - if (longestTask * 1.5 > deadline.timeRemaining()) { + deadlineRemaining = deadline.timeRemaining(); + if (longestTask * 1.5 > deadlineRemaining) { + // Warn when the time exceeding the deadline is over 20ms, if this happens in practice the + // task should be split into sub-tasks to ensure the UI remains responsive. + if (lastDeadlineRemaining - taskDuration < -20) { + console.warn(`task queue exceeded allotted deadline by ${Math.abs(Math.round(lastDeadlineRemaining - taskDuration))}ms`); + } this._start(); return; } + lastDeadlineRemaining = deadlineRemaining; } this.clear(); } @@ -97,9 +116,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 +155,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/Types.d.ts b/src/common/Types.d.ts index d44bb197..c4c470ad 100644 --- a/src/common/Types.d.ts +++ b/src/common/Types.d.ts @@ -205,7 +205,8 @@ export interface IBufferLine { insertCells(pos: number, n: number, ch: ICellData, eraseAttr?: IAttributeData): void; deleteCells(pos: number, n: number, fill: ICellData, eraseAttr?: IAttributeData): void; replaceCells(start: number, end: number, fill: ICellData, eraseAttr?: IAttributeData, respectProtect?: boolean): void; - resize(cols: number, fill: ICellData): void; + resize(cols: number, fill: ICellData): boolean; + cleanupMemory(): number; fill(fillCellData: ICellData, respectProtect?: boolean): void; copyFrom(line: IBufferLine): void; clone(): IBufferLine; diff --git a/src/common/buffer/Buffer.test.ts b/src/common/buffer/Buffer.test.ts index e5ea7f5e..39cffb49 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, 30)); + + // 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 c929ba2d..b935b2a3 100644 --- a/src/common/buffer/Buffer.ts +++ b/src/common/buffer/Buffer.ts @@ -14,6 +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, IdleTaskQueue } from 'common/TaskQueue'; export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1 @@ -150,6 +151,9 @@ export class Buffer implements IBuffer { // store reference to null cell with default attrs const nullCell = this.getNullCell(DEFAULT_ATTR_DATA); + // 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. const newMaxLength = this._getCorrectBufferLength(newRows); @@ -163,7 +167,8 @@ export class Buffer implements IBuffer { // Deal with columns increasing (reducing needs to happen after reflow) if (this._cols < newCols) { for (let i = 0; i < this.lines.length; i++) { - this.lines.get(i)!.resize(newCols, nullCell); + // +boolean for fast 0 or 1 conversion + dirtyMemoryLines += +this.lines.get(i)!.resize(newCols, nullCell); } } @@ -242,13 +247,46 @@ export class Buffer implements IBuffer { // Trim the end of the line off if cols shrunk if (this._cols > newCols) { for (let i = 0; i < this.lines.length; i++) { - this.lines.get(i)!.resize(newCols, nullCell); + // +boolean for fast 0 or 1 conversion + dirtyMemoryLines += +this.lines.get(i)!.resize(newCols, nullCell); } } } this._cols = newCols; this._rows = newRows; + + this._memoryCleanupQueue.clear(); + // schedule memory cleanup only, if more than 10% of the lines are affected + if (dirtyMemoryLines > 0.1 * this.lines.length) { + this._memoryCleanupPosition = 0; + this._memoryCleanupQueue.enqueue(() => this._batchedMemoryCleanup()); + } + } + + private _memoryCleanupQueue = new IdleTaskQueue(); + private _memoryCleanupPosition = 0; + + private _batchedMemoryCleanup(): boolean { + let normalRun = true; + if (this._memoryCleanupPosition >= this.lines.length) { + // cleanup made it once through all lines, thus rescan in loop below to also catch shifted lines, + // which should finish rather quick if there are no more cleanups pending + this._memoryCleanupPosition = 0; + normalRun = false; + } + let counted = 0; + while (this._memoryCleanupPosition < this.lines.length) { + counted += this.lines.get(this._memoryCleanupPosition++)!.cleanupMemory(); + // cleanup max 100 lines per batch + if (counted > 100) { + return true; + } + } + // normal runs always need another rescan afterwards + // if we made it here with normalRun=false, we are in a final run + // and can end the cleanup task for sure + return normalRun; } private get _isReflowEnabled(): boolean { diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 5a192203..d5f43844 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -40,6 +40,9 @@ export const DEFAULT_ATTR_DATA = Object.freeze(new AttributeData()); // Work variables to avoid garbage collection let $startIndex = 0; +/** Factor when to cleanup underlying array buffer after shrinking. */ +const CLEANUP_THRESHOLD = 2; + /** * Typed array based bufferline implementation. * @@ -333,42 +336,69 @@ export class BufferLine implements IBufferLine { } } - public resize(cols: number, fillCellData: ICellData): void { + /** + * Resize BufferLine to `cols` filling excess cells with `fillCellData`. + * The underlying array buffer will not change if there is still enough space + * to hold the new buffer line data. + * Returns a boolean indicating, whether a `cleanupMemory` call would free + * excess memory (true after shrinking > CLEANUP_THRESHOLD). + */ + public resize(cols: number, fillCellData: ICellData): boolean { if (cols === this.length) { - return; + return this._data.length * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength; } + const uint32Cells = cols * CELL_SIZE; if (cols > this.length) { - const data = new Uint32Array(cols * CELL_SIZE); - if (this.length) { - if (cols * CELL_SIZE < this._data.length) { - data.set(this._data.subarray(0, cols * CELL_SIZE)); - } else { - data.set(this._data); - } + if (this._data.buffer.byteLength >= uint32Cells * 4) { + // optimization: avoid alloc and data copy if buffer has enough room + this._data = new Uint32Array(this._data.buffer, 0, uint32Cells); + } else { + // slow path: new alloc and full data copy + const data = new Uint32Array(uint32Cells); + data.set(this._data); + this._data = data; } - this._data = data; for (let i = this.length; i < cols; ++i) { this.setCell(i, fillCellData); } } else { - if (cols) { - const data = new Uint32Array(cols * CELL_SIZE); - data.set(this._data.subarray(0, cols * CELL_SIZE)); - this._data = data; - // Remove any cut off combined data, FIXME: repeat this for extended attrs - const keys = Object.keys(this._combined); - for (let i = 0; i < keys.length; i++) { - const key = parseInt(keys[i], 10); - if (key >= cols) { - delete this._combined[key]; - } + // optimization: just shrink the view on existing buffer + this._data = this._data.subarray(0, uint32Cells); + // Remove any cut off combined data + const keys = Object.keys(this._combined); + for (let i = 0; i < keys.length; i++) { + const key = parseInt(keys[i], 10); + if (key >= cols) { + delete this._combined[key]; + } + } + // remove any cut off extended attributes + const extKeys = Object.keys(this._extendedAttrs); + for (let i = 0; i < extKeys.length; i++) { + const key = parseInt(extKeys[i], 10); + if (key >= cols) { + delete this._extendedAttrs[key]; } - } else { - this._data = new Uint32Array(0); - this._combined = {}; } } this.length = cols; + return uint32Cells * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength; + } + + /** + * Cleanup underlying array buffer. + * A cleanup will be triggered if the array buffer exceeds the actual used + * memory by a factor of CLEANUP_THRESHOLD. + * Returns 0 or 1 indicating whether a cleanup happened. + */ + public cleanupMemory(): number { + if (this._data.length * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength) { + const data = new Uint32Array(this._data.length); + data.set(this._data); + this._data = data; + return 1; + } + return 0; } /** fill a line with fillCharData */ diff --git a/src/common/input/Keyboard.test.ts b/src/common/input/Keyboard.test.ts index 68fbaac1..f2d0936b 100644 --- a/src/common/input/Keyboard.test.ts +++ b/src/common/input/Keyboard.test.ts @@ -125,6 +125,12 @@ describe('Keyboard', () => { it('should return \\x1ba for alt+a', () => { assert.equal(testEvaluateKeyboardEvent({ altKey: true, keyCode: 65 }, { isMac: false }).key, '\x1ba'); }); + it('should return \\x1b\\x20 for alt+space', () => { + assert.equal(testEvaluateKeyboardEvent({ altKey: true, keyCode: 32 }, { isMac: false }).key, '\x1b\x20'); + }); + it('should return \\x1b\\x00 for ctrl+alt+space', () => { + assert.equal(testEvaluateKeyboardEvent({ altKey: true, ctrlKey: true, keyCode: 32 }, { isMac: false }).key, '\x1b\x00'); + }); }); describe('On macOS platforms', () => { diff --git a/src/common/input/Keyboard.ts b/src/common/input/Keyboard.ts index 013a7711..225c914e 100644 --- a/src/common/input/Keyboard.ts +++ b/src/common/input/Keyboard.ts @@ -360,6 +360,8 @@ export function evaluateKeyboardEvent( keyString = keyString.toUpperCase(); } result.key = C0.ESC + keyString; + } else if (ev.keyCode === 32) { + result.key = C0.ESC + (ev.ctrlKey ? C0.NUL : ' '); } else if (ev.key === 'Dead' && ev.code.startsWith('Key')) { // Reference: https://github.com/xtermjs/xterm.js/issues/3725 // Alt will produce a "dead key" (initate composition) with some