Merge pull request #3723 from Tyriar/3703_ruler_perf

Improve performance of overview ruler by computing and rendering color zones

demo/client.ts

@@ -556,8 +556,8 @@ function loadTest() {
 function addDecoration() {
   term.options['overviewRulerWidth'] = 15;
   const marker = term.addMarker(1);
-  const decoration = term.registerDecoration({ marker, overviewRulerOptions: { color: '#ef2929'} });
-  decoration.onRender((e) => e.style.backgroundColor = '#ef2929');
+  const decoration = term.registerDecoration({ marker, overviewRulerOptions: { color: '#ef292980', position: 'left' } });
+  decoration.onRender((e) => e.style.backgroundColor = '#ef292980');
 }
 
 function addOverviewRuler() {

src/browser/Decorations/ColorZoneStore.test.ts

@@ -0,0 +1,88 @@
+/**
+ * Copyright (c) 2017 The xterm.js authors. All rights reserved.
+ * @license MIT
+ */
+
+import { assert } from 'chai';
+import { ColorZoneStore } from 'browser/Decorations/ColorZoneStore';
+
+const optionsRedFull = {
+  overviewRulerOptions: {
+    color: 'red',
+    position: 'full' as 'full'
+  }
+};
+
+describe('ColorZoneStore', () => {
+  let store: ColorZoneStore;
+
+  beforeEach(() => {
+    store = new ColorZoneStore();
+    store.setPadding({
+      full: 1,
+      left: 1,
+      center: 1,
+      right: 1
+    });
+  });
+
+  it('should merge adjacent zones', () => {
+    store.addDecoration({
+      marker: { line: 0 },
+      options: optionsRedFull
+    });
+    store.addDecoration({
+      marker: { line: 1 },
+      options: optionsRedFull
+    });
+    assert.deepStrictEqual(store.zones, [
+      {
+        color: 'red',
+        position: 'full',
+        startBufferLine: 0,
+        endBufferLine: 1
+      }
+    ]);
+  });
+
+  it('should not merge non-adjacent zones', () => {
+    store.addDecoration({
+      marker: { line: 0 },
+      options: optionsRedFull
+    });
+    store.addDecoration({
+      marker: { line: 2 },
+      options: optionsRedFull
+    });
+    assert.deepStrictEqual(store.zones, [
+      {
+        color: 'red',
+        position: 'full',
+        startBufferLine: 0,
+        endBufferLine: 0
+      },
+      {
+        color: 'red',
+        position: 'full',
+        startBufferLine: 2,
+        endBufferLine: 2
+      }
+    ]);
+  });
+
+  it('should reuse zone objects', () => {
+    const obj = {
+      marker: { line: 0 },
+      options: optionsRedFull
+    };
+    store.addDecoration(obj);
+    const zone = store.zones[0];
+    store.clear();
+    store.addDecoration({
+      marker: { line: 1 },
+      options: optionsRedFull
+    });
+    // The object reference should be the same
+    assert.equal(zone, store.zones[0]);
+  });
+});

src/browser/Decorations/ColorZoneStore.ts

@@ -0,0 +1,117 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { IInternalDecoration } from 'common/services/Services';
+
+export interface IColorZoneStore {
+  readonly zones: IColorZone[];
+  clear(): void;
+  addDecoration(decoration: IInternalDecoration): void;
+  /**
+   * Sets the amount of padding in lines that will be added between zones, if new lines intersect
+   * the padding they will be merged into the same zone.
+   */
+  setPadding(padding: { [position: string]: number }): void;
+}
+
+export interface IColorZone {
+  /** Color in a format supported by canvas' fillStyle. */
+  color: string;
+  position: 'full' | 'left' | 'center' | 'right' | undefined;
+  startBufferLine: number;
+  endBufferLine: number;
+}
+
+interface IMinimalDecorationForColorZone {
+  marker: Pick<IInternalDecoration['marker'], 'line'>;
+  options: Pick<IInternalDecoration['options'], 'overviewRulerOptions'>;
+}
+
+export class ColorZoneStore implements IColorZoneStore {
+  private _zones: IColorZone[] = [];
+
+  // The zone pool is used to keep zone objects from being freed between clearing the color zone
+  // store and fetching the zones. This helps reduce GC pressure since the color zones are
+  // accumulated on potentially every scroll event.
+  private _zonePool: IColorZone[] = [];
+  private _zonePoolIndex = 0;
+
+  private _linePadding: { [position: string]: number } = {
+    full: 0,
+    left: 0,
+    center: 0,
+    right: 0
+  };
+
+  public get zones(): IColorZone[] {
+    // Trim the zone pool to free unused memory
+    this._zonePool.length = Math.min(this._zonePool.length, this._zones.length);
+    return this._zones;
+  }
+
+  public clear(): void {
+    this._zones.length = 0;
+    this._zonePoolIndex = 0;
+  }
+
+  public addDecoration(decoration: IMinimalDecorationForColorZone): void {
+    if (!decoration.options.overviewRulerOptions) {
+      return;
+    }
+    for (const z of this._zones) {
+      if (z.color === decoration.options.overviewRulerOptions.color &&
+          z.position === decoration.options.overviewRulerOptions.position) {
+        if (this._lineIntersectsZone(z, decoration.marker.line)) {
+          return;
+        }
+        if (this._lineAdjacentToZone(z, decoration.marker.line, decoration.options.overviewRulerOptions.position)) {
+          this._addLineToZone(z, decoration.marker.line);
+          return;
+        }
+      }
+    }
+    // Create using zone pool if possible
+    if (this._zonePoolIndex < this._zonePool.length) {
+      this._zonePool[this._zonePoolIndex].color = decoration.options.overviewRulerOptions.color;
+      this._zonePool[this._zonePoolIndex].position = decoration.options.overviewRulerOptions.position;
+      this._zonePool[this._zonePoolIndex].startBufferLine = decoration.marker.line;
+      this._zonePool[this._zonePoolIndex].endBufferLine = decoration.marker.line;
+      this._zones.push(this._zonePool[this._zonePoolIndex++]);
+      return;
+    }
+    // Create
+    this._zones.push({
+      color: decoration.options.overviewRulerOptions.color,
+      position: decoration.options.overviewRulerOptions.position,
+      startBufferLine: decoration.marker.line,
+      endBufferLine: decoration.marker.line
+    });
+    this._zonePool.push(this._zones[this._zones.length - 1]);
+    this._zonePoolIndex++;
+  }
+
+  public setPadding(padding: { [position: string]: number }): void {
+    this._linePadding = padding;
+  }
+
+  private _lineIntersectsZone(zone: IColorZone, line: number): boolean {
+    return (
+      line >= zone.startBufferLine &&
+      line <= zone.endBufferLine
+    );
+  }
+
+  private _lineAdjacentToZone(zone: IColorZone, line: number, position: IColorZone['position']): boolean {
+    return (
+      (line >= zone.startBufferLine - this._linePadding[position || 'full']) &&
+      (line <= zone.endBufferLine + this._linePadding[position || 'full'])
+    );
+  }
+
+  private _addLineToZone(zone: IColorZone, line: number): void {
+    zone.startBufferLine = Math.min(zone.startBufferLine, line);
+    zone.endBufferLine = Math.max(zone.endBufferLine, line);
+  }
+}

src/browser/Decorations/OverviewRulerRenderer.ts

@@ -3,6 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { ColorZoneStore, IColorZone, IColorZoneStore } from 'browser/Decorations/ColorZoneStore';
 import { addDisposableDomListener } from 'browser/Lifecycle';
 import { IRenderService } from 'browser/services/Services';
 import { Disposable } from 'common/Lifecycle';
@@ -32,7 +33,7 @@ const drawX = {
 export class OverviewRulerRenderer extends Disposable {
   private readonly _canvas: HTMLCanvasElement;
   private readonly _ctx: CanvasRenderingContext2D;
-  private readonly _decorationElements: Map<IInternalDecoration, HTMLElement> = new Map();
+  private readonly _colorZoneStore: IColorZoneStore = new ColorZoneStore();
   private get _width(): number {
     return this._optionsService.options.overviewRulerWidth || 0;
   }
@@ -40,6 +41,7 @@ export class OverviewRulerRenderer extends Disposable {
 
   private _shouldUpdateDimensions: boolean | undefined = true;
   private _shouldUpdateAnchor: boolean | undefined = true;
+  private _lastKnownBufferLength: number = 0;
 
   private _containerHeight: number | undefined;
 
@@ -72,7 +74,6 @@ export class OverviewRulerRenderer extends Disposable {
    */
   private _registerDecorationListeners(): void {
     this.register(this._decorationService.onDecorationRegistered(() => this._queueRefresh(undefined, true)));
-    this.register(this._decorationService.onDecorationRemoved(decoration => this._removeDecoration(decoration)));
   }
 
   /**
@@ -84,6 +85,11 @@ export class OverviewRulerRenderer extends Disposable {
     this.register(this._bufferService.buffers.onBufferActivate(() => {
       this._canvas!.style.display = this._bufferService.buffer === this._bufferService.buffers.alt ? 'none' : 'block';
     }));
+    this.register(this._bufferService.onScroll(() => {
+      if (this._lastKnownBufferLength !== this._bufferService.buffers.normal.lines.length) {
+        this._refreshColorZonePadding();
+      }
+    }));
   }
   /**
    * On dimension change, update canvas dimensions
@@ -112,10 +118,6 @@ export class OverviewRulerRenderer extends Disposable {
   }
 
   public override dispose(): void {
-    for (const decoration of this._decorationElements) {
-      decoration[0].dispose();
-    }
-    this._decorationElements.clear();
     this._canvas?.remove();
     super.dispose();
   }
@@ -140,22 +142,14 @@ export class OverviewRulerRenderer extends Disposable {
     drawX.right = drawWidth.left + drawWidth.center;
   }
 
-  private _refreshStyle(decoration: IInternalDecoration): void {
-    if (!decoration.options.overviewRulerOptions) {
-      this._decorationElements.delete(decoration);
-      return;
-    }
-    this._ctx.lineWidth = 1;
-    this._ctx.fillStyle = decoration.options.overviewRulerOptions.color;
-    this._ctx.fillRect(
-      /* x */ drawX[decoration.options.overviewRulerOptions.position!],
-      /* y */ Math.round(
-        (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line
-      (decoration.options.marker.line / this._bufferService.buffers.active.lines.length) - drawHeight[decoration.options.overviewRulerOptions.position!] / 2
-      ),
-      /* w */ drawWidth[decoration.options.overviewRulerOptions.position!],
-      /* h */ drawHeight[decoration.options.overviewRulerOptions.position!]
-    );
+  private _refreshColorZonePadding(): void {
+    this._colorZoneStore.setPadding({
+      full: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.full),
+      left: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.left),
+      center: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.center),
+      right: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.right)
+    });
+    this._lastKnownBufferLength = this._bufferService.buffers.normal.lines.length;
   }
 
   private _refreshCanvasDimensions(): void {
@@ -164,6 +158,7 @@ export class OverviewRulerRenderer extends Disposable {
     this._canvas.style.height = `${this._screenElement.clientHeight}px`;
     this._canvas.height = Math.round(this._screenElement.clientHeight * window.devicePixelRatio);
     this._refreshDrawConstants();
+    this._refreshColorZonePadding();
   }
 
   private _refreshDecorations(): void {
@@ -171,27 +166,42 @@ export class OverviewRulerRenderer extends Disposable {
       this._refreshCanvasDimensions();
     }
     this._ctx.clearRect(0, 0, this._canvas.width, this._canvas.height);
+    this._colorZoneStore.clear();
     for (const decoration of this._decorationService.decorations) {
-      if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position !== 'full') {
-        this._renderDecoration(decoration);
+      this._colorZoneStore.addDecoration(decoration);
+    }
+    this._ctx.lineWidth = 1;
+    const zones = this._colorZoneStore.zones;
+    for (const zone of zones) {
+      if (zone.position !== 'full') {
+        this._renderColorZone(zone);
       }
     }
-    for (const decoration of this._decorationService.decorations) {
-      if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position === 'full') {
-        this._renderDecoration(decoration);
+    for (const zone of zones) {
+      if (zone.position === 'full') {
+        this._renderColorZone(zone);
       }
     }
     this._shouldUpdateDimensions = false;
     this._shouldUpdateAnchor = false;
   }
 
-  private _renderDecoration(decoration: IInternalDecoration): void {
-    const element = this._decorationElements.get(decoration);
-    if (!element) {
-      this._decorationElements.set(decoration, this._canvas);
-      decoration.onDispose(() => this._queueRefresh());
-    }
-    this._refreshStyle(decoration);
+  private _renderColorZone(zone: IColorZone): void {
+    // TODO: Is _decorationElements needed?
+
+    this._ctx.fillStyle = zone.color;
+    this._ctx.fillRect(
+      /* x */ drawX[zone.position || 'full'],
+      /* y */ Math.round(
+        (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line
+        (zone.startBufferLine / this._bufferService.buffers.active.lines.length) - drawHeight[zone.position || 'full'] / 2
+      ),
+      /* w */ drawWidth[zone.position || 'full'],
+      /* h */ Math.round(
+        (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line
+        ((zone.endBufferLine - zone.startBufferLine) / this._bufferService.buffers.active.lines.length) + drawHeight[zone.position || 'full']
+      )
+    );
   }
 
   private _queueRefresh(updateCanvasDimensions?: boolean, updateAnchor?: boolean): void {
@@ -205,9 +215,4 @@ export class OverviewRulerRenderer extends Disposable {
       this._animationFrame = undefined;
     });
   }
-
-  private _removeDecoration(decoration: IInternalDecoration): void {
-    this._decorationElements.get(decoration)?.remove();
-    this._decorationElements.delete(decoration);
-  }
 }

src/browser/Terminal.test.ts

@@ -231,7 +231,7 @@ describe('Terminal', () => {
       });
       term.paste('foo');
     });
-    it('should sanitize \n chars', done => {
+    it('should sanitize \\n chars', done => {
       term.onData(e => {
         assert.equal(e, '\rfoo\rbar\r');
         done();