From c2ec16a16c834c4449547e3e1aab2da9eb4a4b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 13 Sep 2019 16:31:28 +0200 Subject: [PATCH] encapsulate deferred writing in WriteBuffer class --- src/InputHandler.ts | 1 + src/Terminal.ts | 100 +++--------------------- src/TestUtils.test.ts | 4 - src/common/input/WriteBuffer.ts | 110 +++++++++++++++++++++++++++ test/benchmark/Terminal.benchmark.ts | 8 +- 5 files changed, 127 insertions(+), 96 deletions(-) create mode 100644 src/common/input/WriteBuffer.ts diff --git a/src/InputHandler.ts b/src/InputHandler.ts index f3ed0454..74358583 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -344,6 +344,7 @@ export class InputHandler extends Disposable implements IInputHandler { if (buffer.x !== cursorStartX || buffer.y !== cursorStartY) { this._onCursorMove.fire(); } + this._terminal.refresh(this._dirtyRowService.start, this._dirtyRowService.end); } public print(data: Uint32Array, start: number, end: number): void { diff --git a/src/Terminal.ts b/src/Terminal.ts index 1b1d324f..aa55ebcb 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -62,35 +62,11 @@ import { ILinkifier, IMouseZoneManager, LinkMatcherHandler, ILinkMatcherOptions, import { DirtyRowService } from 'common/services/DirtyRowService'; import { InstantiationService } from 'common/services/InstantiationService'; import { CoreMouseService } from 'common/services/CoreMouseService'; +import { WriteBuffer } from 'common/input/WriteBuffer'; // Let it work inside Node.js for automated testing purposes. const document = (typeof window !== 'undefined') ? window.document : null; -/** - * Safety watermark to avoid memory exhaustion and browser engine crash on fast data input. - * Enable flow control to avoid this limit and make sure that your backend correctly - * propagates this to the underlying pty. (see docs for further instructions) - * Since this limit is meant as a safety parachute to prevent browser crashs, - * it is set to a very high number. Typically xterm.js gets unresponsive with - * a 100 times lower number (>500 kB). - */ -const DISCARD_WATERMARK = 50000000; // ~50 MB - -/** - * The max number of ms to spend on writes before allowing the renderer to - * catch up with a 0ms setTimeout. A value of < 33 to keep us close to - * 30fps, and a value of < 16 to try to run at 60fps. Of course, the real FPS - * depends on the time it takes for the renderer to draw the frame. - */ -const WRITE_TIMEOUT_MS = 12; - -/** - * Threshold of max held chunks in the write buffer, that were already processed. - * This is a tradeoff between extensive write buffer shifts (bad runtime) and high - * memory consumption by data thats not used anymore. - */ -const WRITE_BUFFER_LENGTH_THRESHOLD = 50; - export class Terminal extends Disposable implements ITerminal, IDisposable, IInputHandlingTerminal { public textarea: HTMLTextAreaElement; @@ -163,11 +139,8 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp public params: (string | number)[]; public currentParam: string | number; - // write data related containers - protected _writeBuffer: (Uint8Array | string)[] = []; - private _pendingWriteDataSize: number = 0; - private _writeChunkCallbacks: ((() => void) | undefined)[] = []; - private _writeInProgress = false; + // write buffer + private _deferredWriteBuffer: WriteBuffer; // Store if user went browsing history in scrollback private _userScrolling: boolean; @@ -258,6 +231,8 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp this._setupOptionsListeners(); this._setup(); + + this._deferredWriteBuffer = new WriteBuffer(data => this._inputHandler.parse(data)); } public dispose(): void { @@ -1137,60 +1112,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp } } - public write(data: string | Uint8Array, callback?: () => void): void { - // Ensure the terminal isn't disposed - // NOOP on empty data - if (this._isDisposed || !data.length) { - return; - } - - if (this._pendingWriteDataSize > DISCARD_WATERMARK) { - throw new Error('write data discarded, use flow control to avoid losing data'); - } - - this._pendingWriteDataSize += data.length; - this._writeBuffer.push(data); - this._writeChunkCallbacks.push(callback); - - if (!this._writeInProgress) { - this._writeInProgress = true; - setTimeout(() => this._innerWrite()); - } - } - - protected _innerWrite(bufferOffset: number = 0): void { - const startTime = Date.now(); - while (this._writeBuffer.length > bufferOffset) { - const data = this._writeBuffer[bufferOffset]; - const cb = this._writeChunkCallbacks[bufferOffset]; - bufferOffset++; - - this._inputHandler.parse(data); - this._pendingWriteDataSize -= data.length; - if (cb) cb(); - - this.refresh(this._dirtyRowService.start, this._dirtyRowService.end); - - if (Date.now() - startTime >= WRITE_TIMEOUT_MS) { - break; - } - } - if (this._writeBuffer.length > bufferOffset) { - // Allow renderer to catch up before processing the next batch - // trim already processed chunks if we are above threshold - if (bufferOffset > WRITE_BUFFER_LENGTH_THRESHOLD) { - this._writeBuffer = this._writeBuffer.slice(bufferOffset); - this._writeChunkCallbacks = this._writeChunkCallbacks.slice(bufferOffset); - bufferOffset = 0; - } - setTimeout(() => this._innerWrite(bufferOffset), 0); - } else { - this._writeInProgress = false; - this._writeBuffer = []; - this._writeChunkCallbacks = []; - } - } - public paste(data: string): void { paste(data, this.textarea, this.bracketedPasteMode, this._coreService); } @@ -1628,8 +1549,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp const customKeyEventHandler = this._customKeyEventHandler; const inputHandler = this._inputHandler; const cursorState = this.cursorState; - const writeBuffer = this._writeBuffer; - const writeInProgress = this._writeInProgress; const userScrolling = this._userScrolling; this._setup(); @@ -1644,8 +1563,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp this._customKeyEventHandler = customKeyEventHandler; this._inputHandler = inputHandler; this.cursorState = cursorState; - this._writeBuffer = writeBuffer; - this._writeInProgress = writeInProgress; this._userScrolling = userScrolling; // do a full screen refresh @@ -1676,6 +1593,13 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp // return this.options.bellStyle === 'sound' || // this.options.bellStyle === 'both'; } + + public write(data: string | Uint8Array, callback?: () => void): void { + this._deferredWriteBuffer.write(data, callback); + } + public writeSync(data: string | Uint8Array): void { + this._deferredWriteBuffer.writeSync(data); + } } /** diff --git a/src/TestUtils.test.ts b/src/TestUtils.test.ts index a30f60ce..a70f751c 100644 --- a/src/TestUtils.test.ts +++ b/src/TestUtils.test.ts @@ -19,10 +19,6 @@ import { IParams, IFunctionIdentifier } from 'common/parser/Types'; import { ISelectionService } from 'browser/services/Services'; export class TestTerminal extends Terminal { - writeSync(data: string): void { - this._writeBuffer.push(data); - this._innerWrite(); - } keyDown(ev: any): boolean { return this._keyDown(ev); } keyPress(ev: any): boolean { return this._keyPress(ev); } } diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts new file mode 100644 index 00000000..c7d82458 --- /dev/null +++ b/src/common/input/WriteBuffer.ts @@ -0,0 +1,110 @@ + +/** + * Copyright (c) 2019 The xterm.js authors. All rights reserved. + * @license MIT + */ + +declare const setTimeout: (handler: () => void, timeout?: number) => void; + +/** + * Safety watermark to avoid memory exhaustion and browser engine crash on fast data input. + * Enable flow control to avoid this limit and make sure that your backend correctly + * propagates this to the underlying pty. (see docs for further instructions) + * Since this limit is meant as a safety parachute to prevent browser crashs, + * it is set to a very high number. Typically xterm.js gets unresponsive with + * a 100 times lower number (>500 kB). + */ +const DISCARD_WATERMARK = 50000000; // ~50 MB + +/** + * The max number of ms to spend on writes before allowing the renderer to + * catch up with a 0ms setTimeout. A value of < 33 to keep us close to + * 30fps, and a value of < 16 to try to run at 60fps. Of course, the real FPS + * depends on the time it takes for the renderer to draw the frame. + */ +const WRITE_TIMEOUT_MS = 12; + +/** + * Threshold of max held chunks in the write buffer, that were already processed. + * This is a tradeoff between extensive write buffer shifts (bad runtime) and high + * memory consumption by data thats not used anymore. + */ +const WRITE_BUFFER_LENGTH_THRESHOLD = 50; + +export class WriteBuffer { + private _writeBuffer: (string | Uint8Array)[] = []; + private _callbacks: ((() => void) | undefined)[] = []; + private _pendingData = 0; + private _bufferOffset = 0; + + constructor(private _action: (data: string | Uint8Array) => void) { } + + public writeSync(data: string | Uint8Array): void { + // force sync processing on pending data chunks to avoid in-band data scrambling + // does the same as innerWrite but without event loop + if (this._writeBuffer.length) { + for (let i = this._bufferOffset; i < this._writeBuffer.length; ++i) { + const data = this._writeBuffer[i]; + const cb = this._callbacks[i]; + this._action(data); + if (cb) cb(); + } + // reset all to avoid reprocessing of chunks with scheduled innerWrite call + this._writeBuffer = []; + this._callbacks = []; + this._pendingData = 0; + // stop scheduled innerWrite by offset > length condition + this._bufferOffset = 0x7FFFFFFF; + } + // handle current data chunk + this._action(data); + } + + public write(data: string | Uint8Array, callback?: () => void): void { + if (this._pendingData > DISCARD_WATERMARK) { + throw new Error('write data discarded, use flow control to avoid losing data'); + } + + // schedule chunk processing for next event loop run + if (!this._writeBuffer.length) { + this._bufferOffset = 0; + setTimeout(() => this._innerWrite()); + } + + this._pendingData += data.length; + this._writeBuffer.push(data); + this._callbacks.push(callback); + } + + protected _innerWrite(): void { + const startTime = Date.now(); + while (this._writeBuffer.length > this._bufferOffset) { + const data = this._writeBuffer[this._bufferOffset]; + const cb = this._callbacks[this._bufferOffset]; + this._bufferOffset++; + + this._action(data); + this._pendingData -= data.length; + if (cb) cb(); + + if (Date.now() - startTime >= WRITE_TIMEOUT_MS) { + break; + } + } + if (this._writeBuffer.length > this._bufferOffset) { + // Allow renderer to catch up before processing the next batch + // trim already processed chunks if we are above threshold + if (this._bufferOffset > WRITE_BUFFER_LENGTH_THRESHOLD) { + this._writeBuffer = this._writeBuffer.slice(this._bufferOffset); + this._callbacks = this._callbacks.slice(this._bufferOffset); + this._bufferOffset = 0; + } + setTimeout(() => this._innerWrite(), 0); + } else { + this._writeBuffer = []; + this._callbacks = []; + this._pendingData = 0; + this._bufferOffset = 0; + } + } +} diff --git a/test/benchmark/Terminal.benchmark.ts b/test/benchmark/Terminal.benchmark.ts index 0e34ebb3..71a36cc7 100644 --- a/test/benchmark/Terminal.benchmark.ts +++ b/test/benchmark/Terminal.benchmark.ts @@ -49,8 +49,8 @@ perfContext('Terminal: ls -lR /usr/lib', () => { before(() => { terminal = new Terminal({cols: 80, rows: 25, scrollback: 1000}); }); - new ThroughputRuntimeCase('', async () => { - await new Promise(resolve => terminal.write(content, resolve)); + new ThroughputRuntimeCase('', () => { + terminal.writeSync(content); return {payloadSize: contentUtf8.length}; }, {fork: false}).showAverageThroughput(); }); @@ -60,8 +60,8 @@ perfContext('Terminal: ls -lR /usr/lib', () => { before(() => { terminal = new Terminal({cols: 80, rows: 25, scrollback: 1000}); }); - new ThroughputRuntimeCase('', async () => { - await new Promise(resolve => terminal.write(content, resolve)); + new ThroughputRuntimeCase('', () => { + terminal.writeSync(content); return {payloadSize: contentUtf8.length}; }, {fork: false}).showAverageThroughput(); });