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/5] 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 53f5d4d4a8..4134673e62 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 c995af6220..919b039e12 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 542dbf12ab..9609960d3a 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/5] 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 919b039e12..ff82098273 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 e857842616..cafe60dbfb 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 9609960d3a..cc0809ed48 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 aabe721eb5..ec6fcfd513 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 b5a8a10900..7fb33a9b30 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/5] 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 ff82098273..516aa2a06f 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 cc0809ed48..682b8775d2 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/5] 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 682b8775d2..48394c8561 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/5] 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 9e79f3a863..5b79750c96 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 4c07b16c33..5f0419ab91 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 48394c8561..345eb106c9 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 ec6fcfd513..a1c12adfcf 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;