From fb64c527a58e9c1ba84c2ed46516010c77b320e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 30 Oct 2018 17:40:11 +0100 Subject: [PATCH] revert to precheck pushWouldTrim --- src/BufferLine.ts | 6 ++---- src/Terminal.ts | 18 ++++++++++-------- src/Types.ts | 2 +- src/common/CircularList.test.ts | 16 ---------------- src/common/CircularList.ts | 20 +++++--------------- src/common/Types.ts | 2 +- typings/xterm.d.ts | 7 ++++++- 7 files changed, 25 insertions(+), 46 deletions(-) diff --git a/src/BufferLine.ts b/src/BufferLine.ts index c1ec2609..7c697334 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -94,11 +94,10 @@ export class BufferLine implements IBufferLine { } } - public copyFrom(line: BufferLine): IBufferLine { + public copyFrom(line: BufferLine): void { this._data = line._data.slice(0); this.length = line.length; this.isWrapped = line.isWrapped; - return this; } public clone(): IBufferLine { @@ -249,7 +248,7 @@ export class BufferLineTypedArray implements IBufferLine { } /** alter to a full copy of line */ - public copyFrom(line: BufferLineTypedArray): IBufferLine { + public copyFrom(line: BufferLineTypedArray): void { if (this.length !== line.length) { this._data = new Uint32Array(line._data); } else { @@ -262,7 +261,6 @@ export class BufferLineTypedArray implements IBufferLine { this._combined[el] = line._combined[el]; } this.isWrapped = line.isWrapped; - return this; } /** create a new clone */ diff --git a/src/Terminal.ts b/src/Terminal.ts index df73ca58..420db146 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -107,7 +107,7 @@ const DEFAULT_OPTIONS: ITerminalOptions = { rightClickSelectsWord: Browser.isMac, rendererType: 'canvas', experimentalBufferLineImpl: 'JsArray', - experimentalPushRecycling: false + experimentalBufferLineRecycling: false }; export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal { @@ -1179,16 +1179,16 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * Scroll the terminal down 1 row, creating a blank line. * @param isWrapped Whether the new line is wrapped from the previous line. */ - public scroll(isWrapped?: boolean): void { + public scroll(isWrapped: boolean = false): void { let newLine: IBufferLine; - const useRecycling = this.options.experimentalPushRecycling; + const useRecycling = this.options.experimentalBufferLineRecycling; if (useRecycling) { newLine = this._blankLine; if (!newLine || newLine.length !== this.cols || newLine.get(0)[CHAR_DATA_ATTR_INDEX] !== this.eraseAttr()) { newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); this._blankLine = newLine; } - newLine.isWrapped = !!(isWrapped); + newLine.isWrapped = isWrapped; } else { newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); } @@ -1198,15 +1198,17 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (this.buffer.scrollTop === 0) { // Determine whether the buffer is going to be trimmed after insertion. - const willBufferBeTrimmed = this.buffer.lines.length === this.buffer.lines.maxLength; + const willBufferBeTrimmed = this.buffer.lines.pushWouldTrim(); // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { if (willBufferBeTrimmed) { - // Warning: Never call .trimAndRecycle() without the - // willBufferBeTrimmed guard! - this.buffer.lines.trimAndRecycle().copyFrom(newLine); + // push would trim the oldest line in the ringbuffer + // therefore we can recycle it here as the new line + const recycled = this.buffer.lines.get(0); + recycled.copyFrom(newLine); + this.buffer.lines.push(recycled); } else { this.buffer.lines.push(newLine.clone()); } diff --git a/src/Types.ts b/src/Types.ts index cafe60db..e8578426 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -522,7 +522,7 @@ export interface IBufferLine { replaceCells(start: number, end: number, fill: CharData): void; resize(cols: number, fill: CharData, shrink?: boolean): void; fill(fillCharData: CharData): void; - copyFrom(line: IBufferLine): IBufferLine; + copyFrom(line: IBufferLine): void; clone(): IBufferLine; } diff --git a/src/common/CircularList.test.ts b/src/common/CircularList.test.ts index 5f0419ab..4c07b16c 100644 --- a/src/common/CircularList.test.ts +++ b/src/common/CircularList.test.ts @@ -257,20 +257,4 @@ describe('CircularList', () => { assert.equal(list.get(3), 4); }); }); - describe('trimAndRecycle', function(): void { - it('should return correct element', function(): void { - const list = new CircularList(5); - list.push([0]); - list.push([1]); - list.push([2]); - list.push([3]); - list.push([4]); - assert.equal(list.trimAndRecycle()[0], 0); - assert.equal(list.trimAndRecycle()[0], 1); - assert.equal(list.trimAndRecycle()[0], 2); - assert.equal(list.trimAndRecycle()[0], 3); - assert.equal(list.trimAndRecycle()[0], 4); - assert.equal(list.trimAndRecycle()[0], 0); - }); - }); }); diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 345eb106..00d5a520 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -90,10 +90,7 @@ export class CircularList extends EventEmitter implements ICircularList { public push(value: T): void { this._array[this._getCyclicIndex(this._length)] = value; if (this._length === this._maxLength) { - this._startIndex++; - if (this._startIndex === this._maxLength) { - this._startIndex = 0; - } + this._startIndex = ++this._startIndex % this._maxLength; this.emit('trim', 1); } else { this._length++; @@ -101,18 +98,11 @@ export class CircularList extends EventEmitter implements ICircularList { } /** - * Recycling trim. - * This is used to recycle buffer lines in Terminal.scroll when - * the list is at maxLength as a push replacement. - * Returns the old line as new one to be recycled. - * Note: There are no bound checks for performance reasons, - * the method is a special optimization for Terminal.scroll, - * do not use it anywhere else. + * Whether a push would trim. + * True when the ringbuffer is full. */ - public trimAndRecycle(): T | undefined { - this._startIndex = ++this._startIndex % this._maxLength; - this.emit('trim', 1); - return this._array[this._getCyclicIndex(this._length - 1)]; + public pushWouldTrim(): boolean { + return this._length === this._maxLength; } /** diff --git a/src/common/Types.ts b/src/common/Types.ts index a1c12adf..c866581d 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,7 +28,7 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; - trimAndRecycle(): T | undefined; + pushWouldTrim(): boolean; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 68e1e188..8b36dd75 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -112,7 +112,12 @@ declare module 'xterm' { */ experimentalBufferLineImpl?: 'JsArray' | 'TypedArray'; - experimentalPushRecycling?: boolean; + /** + * (EXPERIMENTAL) Enable recycling of buffer lines. + * + * This option will be removed in the future. + */ + experimentalBufferLineRecycling?: boolean; /** * The font size used to render text.