From 33a358033ca26fd9d7d54f01127446357fac8b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 9 Oct 2018 23:48:58 +0200 Subject: [PATCH 1/9] new approach --- src/BufferLine.ts | 11 +++++------ src/Terminal.ts | 17 ++++++++++++----- src/common/CircularList.ts | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/BufferLine.ts b/src/BufferLine.ts index 53f5d4d4..4134673e 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -94,13 +94,11 @@ export class BufferLine implements IBufferLine { } } - public copyFrom(line: IBufferLine): void { - this._data = []; - for (let i = 0; i < line.length; ++i) { - this._push(line.get(i)); - } + public copyFrom(line: BufferLine): IBufferLine { + this._data = line._data.slice(0); this.length = line.length; this.isWrapped = line.isWrapped; + return this; } public clone(): IBufferLine { @@ -249,7 +247,7 @@ export class BufferLineTypedArray implements IBufferLine { } /** alter to a full copy of line */ - public copyFrom(line: BufferLineTypedArray): void { + public copyFrom(line: BufferLineTypedArray): IBufferLine { if (this.length !== line.length) { this._data = new Uint32Array(line._data); } else { @@ -262,6 +260,7 @@ 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 c995af62..919b039e 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -21,7 +21,7 @@ * http://linux.die.net/man/7/urxvt */ -import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, CharacterJoinerHandler } from './Types'; +import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, CharacterJoinerHandler, IBufferLine } from './Types'; import { IMouseZoneManager } from './ui/Types'; import { IRenderer } from './renderer/Types'; import { BufferSet } from './BufferSet'; @@ -1174,7 +1174,14 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { - const newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + // TODO: make blank a member + let blank: IBufferLine = (this as any)._blank; + if (!blank || blank.length !== this.cols) { + blank = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + (this as any)._blank = blank; + } + blank.isWrapped = !!(isWrapped); + const topRow = this.buffer.ybase + this.buffer.scrollTop; const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; @@ -1184,9 +1191,9 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { - this.buffer.lines.push(newLine); + (this.buffer.lines as any).pushRecycling((line: IBufferLine | undefined) => (line) ? line.copyFrom(blank) : blank.clone()); } else { - this.buffer.lines.splice(bottomRow + 1, 0, newLine); + this.buffer.lines.splice(bottomRow + 1, 0, blank.clone()); } // Only adjust ybase and ydisp when the buffer is not trimmed @@ -1208,7 +1215,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // scrollback, instead we can just shift them in-place. const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); - this.buffer.lines.set(bottomRow, newLine); + this.buffer.lines.set(bottomRow, blank.clone()); } // Move the viewport to the bottom of the buffer unless the user is diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 542dbf12..9609960d 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -100,6 +100,24 @@ export class CircularList extends EventEmitter implements ICircularList { } } + /** + * Recycling push variant with a callback. + * The callback gets the value at the current position to be overwritten. + * Return the new value from the callback. + */ + public pushRecycling(callback: (el: T | undefined) => T): void { + this._array[this._getCyclicIndex(this._length)] = callback(this._array[this._getCyclicIndex(this._length)]); + if (this._length === this._maxLength) { + this._startIndex++; + if (this._startIndex === this._maxLength) { + this._startIndex = 0; + } + this.emit('trim', 1); + } else { + this._length++; + } + } + /** * Removes and returns the last value on the list. * @return The popped value. From 751e2efa755d7d5e7b45474725676bf0b2d0100c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 10 Oct 2018 15:00:38 +0200 Subject: [PATCH 2/9] better integration and experimentalPushRecycling option --- src/Terminal.ts | 34 ++++++++++++++++++++++++---------- src/Types.ts | 2 +- src/common/CircularList.ts | 2 +- src/common/Types.ts | 1 + typings/xterm.d.ts | 2 ++ 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 919b039e..ff820982 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -106,7 +106,8 @@ const DEFAULT_OPTIONS: ITerminalOptions = { theme: null, rightClickSelectsWord: Browser.isMac, rendererType: 'canvas', - experimentalBufferLineImpl: 'JsArray' + experimentalBufferLineImpl: 'JsArray', + experimentalPushRecycling: false }; export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal { @@ -208,6 +209,9 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II private _screenDprMonitor: ScreenDprMonitor; private _theme: ITheme; + // bufferline to clone/copy from for new blank lines + private _blankLine: IBufferLine = null; + public cols: number; public rows: number; @@ -496,6 +500,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II case 'experimentalBufferLineImpl': this.buffers.normal.setBufferLineFactory(value); this.buffers.alt.setBufferLineFactory(value); + this._blankLine = null; break; } // Inform renderer of changes @@ -1174,13 +1179,18 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { - // TODO: make blank a member - let blank: IBufferLine = (this as any)._blank; - if (!blank || blank.length !== this.cols) { - blank = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); - (this as any)._blank = blank; + let newLine: IBufferLine; + const useRecycling = this.options.experimentalPushRecycling; + if (useRecycling) { + newLine = this._blankLine; + if (!newLine || newLine.length !== this.cols) { + newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + this._blankLine = newLine; + } + newLine.isWrapped = !!(isWrapped); + } else { + newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); } - blank.isWrapped = !!(isWrapped); const topRow = this.buffer.ybase + this.buffer.scrollTop; const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; @@ -1191,9 +1201,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { - (this.buffer.lines as any).pushRecycling((line: IBufferLine | undefined) => (line) ? line.copyFrom(blank) : blank.clone()); + if (useRecycling) { + this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); + } else { + this.buffer.lines.push(newLine); + } } else { - this.buffer.lines.splice(bottomRow + 1, 0, blank.clone()); + this.buffer.lines.splice(bottomRow + 1, 0, (useRecycling) ? newLine.clone() : newLine); } // Only adjust ybase and ydisp when the buffer is not trimmed @@ -1215,7 +1229,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // scrollback, instead we can just shift them in-place. const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); - this.buffer.lines.set(bottomRow, blank.clone()); + this.buffer.lines.set(bottomRow, (useRecycling) ? newLine.clone() : newLine); } // Move the viewport to the bottom of the buffer unless the user is diff --git a/src/Types.ts b/src/Types.ts index e8578426..cafe60db 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): void; + copyFrom(line: IBufferLine): IBufferLine; clone(): IBufferLine; } diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 9609960d..cc0809ed 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -105,7 +105,7 @@ export class CircularList extends EventEmitter implements ICircularList { * The callback gets the value at the current position to be overwritten. * Return the new value from the callback. */ - public pushRecycling(callback: (el: T | undefined) => T): void { + public pushRecycling(callback: (item: T | undefined) => T): void { this._array[this._getCyclicIndex(this._length)] = callback(this._array[this._getCyclicIndex(this._length)]); if (this._length === this._maxLength) { this._startIndex++; diff --git a/src/common/Types.ts b/src/common/Types.ts index aabe721e..ec6fcfd5 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,6 +28,7 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; + pushRecycling(callback: (item: T | undefined) => T): void; 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 b5a8a109..7fb33a9b 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -112,6 +112,8 @@ declare module 'xterm' { */ experimentalBufferLineImpl?: 'JsArray' | 'TypedArray'; + experimentalPushRecycling?: boolean; + /** * The font size used to render text. */ From 82228ad8913d85faeb442c76a9da29ffd0895917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 22 Oct 2018 18:15:47 +0200 Subject: [PATCH 3/9] trimAndRecycle with external check --- src/Terminal.ts | 7 ++++++- src/common/CircularList.ts | 19 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index ff820982..516aa2a0 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1202,7 +1202,12 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { - this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); + // this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); + if (willBufferBeTrimmed) { + (this.buffer.lines as any).trimAndRecycle().copyFrom(newLine); + } else { + this.buffer.lines.push(newLine.clone()); + } } else { this.buffer.lines.push(newLine); } diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index cc0809ed..682b8775 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -118,6 +118,15 @@ export class CircularList extends EventEmitter implements ICircularList { } } + public trimAndRecycle(): T | undefined { + this._startIndex++; + if (this._startIndex === this._maxLength) { + this._startIndex = 0; + } + this.emit('trim', 1); + return this._array[this._getCyclicIndex(this._length - 1)]; + } + /** * Removes and returns the last value on the list. * @return The popped value. @@ -154,10 +163,10 @@ export class CircularList extends EventEmitter implements ICircularList { } // Adjust length as needed - if (this._length + items.length > this.maxLength) { - const countToTrim = (this._length + items.length) - this.maxLength; + if (this._length + items.length > this._maxLength) { + const countToTrim = (this._length + items.length) - this._maxLength; this._startIndex += countToTrim; - this._length = this.maxLength; + this._length = this._maxLength; this.emit('trim', countToTrim); } else { this._length += items.length; @@ -196,7 +205,7 @@ export class CircularList extends EventEmitter implements ICircularList { const expandListBy = (start + count + offset) - this._length; if (expandListBy > 0) { this._length += expandListBy; - while (this._length > this.maxLength) { + while (this._length > this._maxLength) { this._length--; this._startIndex++; this.emit('trim', 1); @@ -216,6 +225,6 @@ export class CircularList extends EventEmitter implements ICircularList { * @returns The cyclic index. */ private _getCyclicIndex(index: number): number { - return (this._startIndex + index) % this.maxLength; + return (this._startIndex + index) % this._maxLength; } } From 1585aba5b5c174a7f9dc88395df87ac2180c4b50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 22 Oct 2018 21:12:51 +0200 Subject: [PATCH 4/9] slightly faster trimAndRecycle --- src/common/CircularList.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 682b8775..48394c85 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -119,10 +119,7 @@ export class CircularList extends EventEmitter implements ICircularList { } public trimAndRecycle(): T | undefined { - this._startIndex++; - if (this._startIndex === this._maxLength) { - this._startIndex = 0; - } + this._startIndex = ++this._startIndex % this._maxLength; this.emit('trim', 1); return this._array[this._getCyclicIndex(this._length - 1)]; } From 93d7a3021d22f1940f1e3ecd58ff663854bfcec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 16:11:47 +0200 Subject: [PATCH 5/9] remove pushRecycling, test case for trimAndRecycle, docs --- src/Terminal.ts | 17 +++++++---------- src/common/CircularList.test.ts | 16 ++++++++++++++++ src/common/CircularList.ts | 23 +++++++---------------- src/common/Types.ts | 2 +- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 9e79f3a8..5b79750c 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -25,7 +25,7 @@ import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions import { IMouseZoneManager } from './ui/Types'; import { IRenderer } from './renderer/Types'; import { BufferSet } from './BufferSet'; -import { Buffer, MAX_BUFFER_SIZE, DEFAULT_ATTR, NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR } from './Buffer'; +import { Buffer, MAX_BUFFER_SIZE, DEFAULT_ATTR, NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR, CHAR_DATA_ATTR_INDEX } from './Buffer'; import { CompositionHelper } from './CompositionHelper'; import { EventEmitter } from './common/EventEmitter'; import { Viewport } from './Viewport'; @@ -1179,23 +1179,19 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { -<<<<<<< HEAD let newLine: IBufferLine; const useRecycling = this.options.experimentalPushRecycling; if (useRecycling) { newLine = this._blankLine; - if (!newLine || newLine.length !== this.cols) { - newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + 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); } else { - newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); } -======= - const newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); ->>>>>>> master const topRow = this.buffer.ybase + this.buffer.scrollTop; const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; @@ -1206,9 +1202,10 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { - // this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); if (willBufferBeTrimmed) { - (this.buffer.lines as any).trimAndRecycle().copyFrom(newLine); + // Warning: Never call .trimAndRecycle() without the + // willBufferBeTrimmed guard! + this.buffer.lines.trimAndRecycle().copyFrom(newLine); } else { this.buffer.lines.push(newLine.clone()); } diff --git a/src/common/CircularList.test.ts b/src/common/CircularList.test.ts index 4c07b16c..5f0419ab 100644 --- a/src/common/CircularList.test.ts +++ b/src/common/CircularList.test.ts @@ -257,4 +257,20 @@ 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 48394c85..345eb106 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -101,23 +101,14 @@ export class CircularList extends EventEmitter implements ICircularList { } /** - * Recycling push variant with a callback. - * The callback gets the value at the current position to be overwritten. - * Return the new value from the callback. + * 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. */ - public pushRecycling(callback: (item: T | undefined) => T): void { - this._array[this._getCyclicIndex(this._length)] = callback(this._array[this._getCyclicIndex(this._length)]); - if (this._length === this._maxLength) { - this._startIndex++; - if (this._startIndex === this._maxLength) { - this._startIndex = 0; - } - this.emit('trim', 1); - } else { - this._length++; - } - } - public trimAndRecycle(): T | undefined { this._startIndex = ++this._startIndex % this._maxLength; this.emit('trim', 1); diff --git a/src/common/Types.ts b/src/common/Types.ts index ec6fcfd5..a1c12adf 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; - pushRecycling(callback: (item: T | undefined) => T): void; + trimAndRecycle(): T | undefined; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; 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 6/9] 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. From e9906f9b9e73f2a8ffefcb6ceccb8dd4c20bd864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 7 Nov 2018 00:56:45 +0100 Subject: [PATCH 7/9] compromise between code safety and speed --- src/Terminal.ts | 8 ++------ src/common/CircularList.ts | 20 +++++++++++++++++--- src/common/Types.ts | 3 ++- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 420db146..13be90ef 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1198,17 +1198,13 @@ 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.pushWouldTrim(); + const willBufferBeTrimmed = this.buffer.lines.isFull(); // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { if (willBufferBeTrimmed) { - // 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); + this.buffer.lines.recycle().copyFrom(newLine); } else { this.buffer.lines.push(newLine.clone()); } diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 00d5a520..0a57ece0 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -98,10 +98,24 @@ export class CircularList extends EventEmitter implements ICircularList { } /** - * Whether a push would trim. - * True when the ringbuffer is full. + * Advance ringbuffer index and return current element for recycling. + * Note: If the ringbuffer is not full this method will return undefined, + * Either precheck with isFull() or handle the undefined return value accordingly. */ - public pushWouldTrim(): boolean { + public recycle(): T | undefined { + if (this._length === this._maxLength) { + this._startIndex = ++this._startIndex % this._maxLength; + this.emit('trim', 1); + } else { + this._length++; + } + return this._array[this._getCyclicIndex(this._length - 1)]; + } + + /** + * Ringbuffer is at max length. + */ + public isFull(): boolean { return this._length === this._maxLength; } diff --git a/src/common/Types.ts b/src/common/Types.ts index c866581d..841029ec 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,7 +28,8 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; - pushWouldTrim(): boolean; + recycle(): T | undefined; + isFull(): boolean; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; From 3964bb28b5c3fe061f3c4eba43c78635d4737c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 20 Nov 2018 21:49:31 +0100 Subject: [PATCH 8/9] throw error in recycle with buffer not full --- src/common/CircularList.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 0a57ece0..5a6a652f 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -99,17 +99,16 @@ export class CircularList extends EventEmitter implements ICircularList { /** * Advance ringbuffer index and return current element for recycling. - * Note: If the ringbuffer is not full this method will return undefined, - * Either precheck with isFull() or handle the undefined return value accordingly. + * Note: The buffer must be full for this method to work. + * @throws When the buffer is not full. */ - public recycle(): T | undefined { - if (this._length === this._maxLength) { - this._startIndex = ++this._startIndex % this._maxLength; - this.emit('trim', 1); - } else { - this._length++; + public recycle(): T { + if (this._length !== this._maxLength) { + throw new Error('Can only recycle when the buffer is full'); } - return this._array[this._getCyclicIndex(this._length - 1)]; + this._startIndex = ++this._startIndex % this._maxLength; + this.emit('trim', 1); + return this._array[this._getCyclicIndex(this._length - 1)]!; } /** From 15c4cc8a74c89a9aab02d5277ec66c39aa88664e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 20 Nov 2018 22:01:34 +0100 Subject: [PATCH 9/9] remove experimental flag for recycling, merge with TypedArray setting --- src/Terminal.ts | 7 +++---- src/common/CircularList.ts | 2 +- src/common/Types.ts | 2 +- typings/xterm.d.ts | 7 ------- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 70dc15d8..38eb3d5b 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -106,8 +106,7 @@ const DEFAULT_OPTIONS: ITerminalOptions = { theme: null, rightClickSelectsWord: Browser.isMac, rendererType: 'canvas', - experimentalBufferLineImpl: 'JsArray', - experimentalBufferLineRecycling: false + experimentalBufferLineImpl: 'JsArray' }; export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal { @@ -1181,7 +1180,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II */ public scroll(isWrapped: boolean = false): void { let newLine: IBufferLine; - const useRecycling = this.options.experimentalBufferLineRecycling; + const useRecycling = this.options.experimentalBufferLineImpl === 'TypedArray'; if (useRecycling) { newLine = this._blankLine; if (!newLine || newLine.length !== this.cols || newLine.get(0)[CHAR_DATA_ATTR_INDEX] !== this.eraseAttr()) { @@ -1198,7 +1197,7 @@ 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.isFull(); + const willBufferBeTrimmed = this.buffer.lines.isFull; // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 5a6a652f..9faf534a 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -114,7 +114,7 @@ export class CircularList extends EventEmitter implements ICircularList { /** * Ringbuffer is at max length. */ - public isFull(): boolean { + public get isFull(): boolean { return this._length === this._maxLength; } diff --git a/src/common/Types.ts b/src/common/Types.ts index 841029ec..8a416bf1 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -24,12 +24,12 @@ export interface IKeyboardEvent { export interface ICircularList extends IEventEmitter { length: number; maxLength: number; + isFull: boolean; get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; recycle(): T | undefined; - isFull(): 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 8b36dd75..c6b6b1e5 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -112,13 +112,6 @@ declare module 'xterm' { */ experimentalBufferLineImpl?: 'JsArray' | 'TypedArray'; - /** - * (EXPERIMENTAL) Enable recycling of buffer lines. - * - * This option will be removed in the future. - */ - experimentalBufferLineRecycling?: boolean; - /** * The font size used to render text. */