Merge pull request #4115 from jerch/buffer_optimizations

some buffer handling optimizations

Author: Tyriar

src/common/TaskQueue.ts

@@ -8,8 +8,11 @@ import { isNode } from 'common/Platform';
 interface ITaskQueue {
   /**
    * Adds a task to the queue which will run in a future idle callback.
+   * To avoid perceivable stalls on the mainthread, tasks with heavy workload
+   * should split their work into smaller pieces and return `true` to get
+   * called again until the work is done (on falsy return value).
    */
-  enqueue(task: () => void): void;
+  enqueue(task: () => boolean | void): void;
 
   /**
    * Flushes the queue, running all remaining tasks synchronously.
@@ -28,21 +31,23 @@ interface ITaskDeadline {
 type CallbackWithDeadline = (deadline: ITaskDeadline) => void;
 
 abstract class TaskQueue implements ITaskQueue {
-  private _tasks: (() => void)[] = [];
+  private _tasks: (() => boolean | void)[] = [];
   private _idleCallback?: number;
   private _i = 0;
 
   protected abstract _requestCallback(callback: CallbackWithDeadline): number;
   protected abstract _cancelCallback(identifier: number): void;
 
-  public enqueue(task: () => void): void {
+  public enqueue(task: () => boolean | void): void {
     this._tasks.push(task);
     this._start();
   }
 
   public flush(): void {
     while (this._i < this._tasks.length) {
-      this._tasks[this._i++]();
+      if (!this._tasks[this._i]()) {
+        this._i++;
+      }
     }
     this.clear();
   }
@@ -69,9 +74,14 @@ abstract class TaskQueue implements ITaskQueue {
     let lastDeadlineRemaining = deadline.timeRemaining();
     let deadlineRemaining = 0;
     while (this._i < this._tasks.length) {
-      taskDuration = performance.now();
-      this._tasks[this._i++]();
-      taskDuration = performance.now() - taskDuration;
+      taskDuration = Date.now();
+      if (!this._tasks[this._i]()) {
+        this._i++;
+      }
+      // other than performance.now, Date.now might not be stable (changes on wall clock changes),
+      // this is not an issue here as a clock change during a short running task is very unlikely
+      // in case it still happened and leads to negative duration, simply assume 1 msec
+      taskDuration = Math.max(1, Date.now() - taskDuration);
       longestTask = Math.max(taskDuration, longestTask);
       // Guess the following task will take a similar time to the longest task in this batch, allow
       // additional room to try avoid exceeding the deadline
@@ -106,9 +116,9 @@ export class PriorityTaskQueue extends TaskQueue {
   }
 
   private _createDeadline(duration: number): ITaskDeadline {
-    const end = performance.now() + duration;
+    const end = Date.now() + duration;
     return {
-      timeRemaining: () => Math.max(0, end - performance.now())
+      timeRemaining: () => Math.max(0, end - Date.now())
     };
   }
 }
@@ -145,7 +155,7 @@ export class DebouncedIdleTask {
     this._queue = new IdleTaskQueue();
   }
 
-  public set(task: () => void): void {
+  public set(task: () => boolean | void): void {
     this._queue.clear();
     this._queue.enqueue(task);
   }

src/common/Types.d.ts

@@ -205,7 +205,8 @@ export interface IBufferLine {
   insertCells(pos: number, n: number, ch: ICellData, eraseAttr?: IAttributeData): void;
   deleteCells(pos: number, n: number, fill: ICellData, eraseAttr?: IAttributeData): void;
   replaceCells(start: number, end: number, fill: ICellData, eraseAttr?: IAttributeData, respectProtect?: boolean): void;
-  resize(cols: number, fill: ICellData): void;
+  resize(cols: number, fill: ICellData): boolean;
+  cleanupMemory(): number;
   fill(fillCellData: ICellData, respectProtect?: boolean): void;
   copyFrom(line: IBufferLine): void;
   clone(): IBufferLine;

src/common/buffer/Buffer.test.ts

@@ -9,6 +9,7 @@ import { CircularList } from 'common/CircularList';
 import { MockOptionsService, MockBufferService } from 'common/TestUtils.test';
 import { BufferLine, DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine';
 import { CellData } from 'common/buffer/CellData';
+import { ExtendedAttrs } from 'common/buffer/AttributeData';
 
 const INIT_COLS = 80;
 const INIT_ROWS = 24;
@@ -1177,4 +1178,33 @@ describe('Buffer', () => {
       assert.equal(str3, '😁a');
     });
   });
+
+  describe('memory cleanup after shrinking', () => {
+    it('should realign memory from idle task execution', async () => {
+      buffer.fillViewportRows();
+
+      // shrink more than 2 times to trigger lazy memory cleanup
+      buffer.resize(INIT_COLS / 2 - 1, INIT_ROWS);
+
+      // sync
+      for (let i = 0; i < INIT_ROWS; i++) {
+        const line = buffer.lines.get(i)!;
+        // line memory is still at old size from initialization
+        assert.equal((line as any)._data.buffer.byteLength, INIT_COLS * 3 * 4);
+        // array.length and .length get immediately adjusted
+        assert.equal((line as any)._data.length, (INIT_COLS / 2 - 1) * 3);
+        assert.equal(line.length, INIT_COLS / 2 - 1);
+      }
+
+      // wait for a bit to give IdleTaskQueue a chance to kick in
+      // and finish memory cleaning
+      await new Promise(r => setTimeout(r, 30));
+
+      // cleanup should have realigned memory with exact bytelength
+      for (let i = 0; i < INIT_ROWS; i++) {
+        const line = buffer.lines.get(i)!;
+        assert.equal((line as any)._data.buffer.byteLength, (INIT_COLS / 2 - 1) * 3 * 4);
+      }
+    });
+  });
 });

src/common/buffer/Buffer.ts

@@ -14,6 +14,7 @@ import { Marker } from 'common/buffer/Marker';
 import { IOptionsService, IBufferService } from 'common/services/Services';
 import { DEFAULT_CHARSET } from 'common/data/Charsets';
 import { ExtendedAttrs } from 'common/buffer/AttributeData';
+import { DebouncedIdleTask, IdleTaskQueue } from 'common/TaskQueue';
 
 export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1
 
@@ -150,6 +151,9 @@ export class Buffer implements IBuffer {
     // store reference to null cell with default attrs
     const nullCell = this.getNullCell(DEFAULT_ATTR_DATA);
 
+    // count bufferlines with overly big memory to be cleaned afterwards
+    let dirtyMemoryLines = 0;
+
     // Increase max length if needed before adjustments to allow space to fill
     // as required.
     const newMaxLength = this._getCorrectBufferLength(newRows);
@@ -163,7 +167,8 @@ export class Buffer implements IBuffer {
       // Deal with columns increasing (reducing needs to happen after reflow)
       if (this._cols < newCols) {
         for (let i = 0; i < this.lines.length; i++) {
-          this.lines.get(i)!.resize(newCols, nullCell);
+          // +boolean for fast 0 or 1 conversion
+          dirtyMemoryLines += +this.lines.get(i)!.resize(newCols, nullCell);
         }
       }
 
@@ -242,13 +247,46 @@ export class Buffer implements IBuffer {
       // Trim the end of the line off if cols shrunk
       if (this._cols > newCols) {
         for (let i = 0; i < this.lines.length; i++) {
-          this.lines.get(i)!.resize(newCols, nullCell);
+          // +boolean for fast 0 or 1 conversion
+          dirtyMemoryLines += +this.lines.get(i)!.resize(newCols, nullCell);
         }
       }
     }
 
     this._cols = newCols;
     this._rows = newRows;
+
+    this._memoryCleanupQueue.clear();
+    // schedule memory cleanup only, if more than 10% of the lines are affected
+    if (dirtyMemoryLines > 0.1 * this.lines.length) {
+      this._memoryCleanupPosition = 0;
+      this._memoryCleanupQueue.enqueue(() => this._batchedMemoryCleanup());
+    }
+  }
+
+  private _memoryCleanupQueue = new IdleTaskQueue();
+  private _memoryCleanupPosition = 0;
+
+  private _batchedMemoryCleanup(): boolean {
+    let normalRun = true;
+    if (this._memoryCleanupPosition >= this.lines.length) {
+      // cleanup made it once through all lines, thus rescan in loop below to also catch shifted lines,
+      // which should finish rather quick if there are no more cleanups pending
+      this._memoryCleanupPosition = 0;
+      normalRun = false;
+    }
+    let counted = 0;
+    while (this._memoryCleanupPosition < this.lines.length) {
+      counted += this.lines.get(this._memoryCleanupPosition++)!.cleanupMemory();
+      // cleanup max 100 lines per batch
+      if (counted > 100) {
+        return true;
+      }
+    }
+    // normal runs always need another rescan afterwards
+    // if we made it here with normalRun=false, we are in a final run
+    // and can end the cleanup task for sure
+    return normalRun;
   }
 
   private get _isReflowEnabled(): boolean {

src/common/buffer/BufferLine.ts

@@ -40,6 +40,9 @@ export const DEFAULT_ATTR_DATA = Object.freeze(new AttributeData());
 // Work variables to avoid garbage collection
 let $startIndex = 0;
 
+/** Factor when to cleanup underlying array buffer after shrinking. */
+const CLEANUP_THRESHOLD = 2;
+
 /**
  * Typed array based bufferline implementation.
  *
@@ -333,42 +336,69 @@ export class BufferLine implements IBufferLine {
     }
   }
 
-  public resize(cols: number, fillCellData: ICellData): void {
+  /**
+   * Resize BufferLine to `cols` filling excess cells with `fillCellData`.
+   * The underlying array buffer will not change if there is still enough space
+   * to hold the new buffer line data.
+   * Returns a boolean indicating, whether a `cleanupMemory` call would free
+   * excess memory (true after shrinking > CLEANUP_THRESHOLD).
+   */
+  public resize(cols: number, fillCellData: ICellData): boolean {
     if (cols === this.length) {
-      return;
+      return this._data.length * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength;
     }
+    const uint32Cells = cols * CELL_SIZE;
     if (cols > this.length) {
-      const data = new Uint32Array(cols * CELL_SIZE);
-      if (this.length) {
-        if (cols * CELL_SIZE < this._data.length) {
-          data.set(this._data.subarray(0, cols * CELL_SIZE));
-        } else {
-          data.set(this._data);
-        }
+      if (this._data.buffer.byteLength >= uint32Cells * 4) {
+        // optimization: avoid alloc and data copy if buffer has enough room
+        this._data = new Uint32Array(this._data.buffer, 0, uint32Cells);
+      } else {
+        // slow path: new alloc and full data copy
+        const data = new Uint32Array(uint32Cells);
+        data.set(this._data);
+        this._data = data;
       }
-      this._data = data;
       for (let i = this.length; i < cols; ++i) {
         this.setCell(i, fillCellData);
       }
     } else {
-      if (cols) {
-        const data = new Uint32Array(cols * CELL_SIZE);
-        data.set(this._data.subarray(0, cols * CELL_SIZE));
-        this._data = data;
-        // Remove any cut off combined data, FIXME: repeat this for extended attrs
-        const keys = Object.keys(this._combined);
-        for (let i = 0; i < keys.length; i++) {
-          const key = parseInt(keys[i], 10);
-          if (key >= cols) {
-            delete this._combined[key];
-          }
+      // optimization: just shrink the view on existing buffer
+      this._data = this._data.subarray(0, uint32Cells);
+      // Remove any cut off combined data
+      const keys = Object.keys(this._combined);
+      for (let i = 0; i < keys.length; i++) {
+        const key = parseInt(keys[i], 10);
+        if (key >= cols) {
+          delete this._combined[key];
+        }
+      }
+      // remove any cut off extended attributes
+      const extKeys = Object.keys(this._extendedAttrs);
+      for (let i = 0; i < extKeys.length; i++) {
+        const key = parseInt(extKeys[i], 10);
+        if (key >= cols) {
+          delete this._extendedAttrs[key];
         }
-      } else {
-        this._data = new Uint32Array(0);
-        this._combined = {};
       }
     }
     this.length = cols;
+    return uint32Cells * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength;
+  }
+
+  /**
+   * Cleanup underlying array buffer.
+   * A cleanup will be triggered if the array buffer exceeds the actual used
+   * memory by a factor of CLEANUP_THRESHOLD.
+   * Returns 0 or 1 indicating whether a cleanup happened.
+   */
+  public cleanupMemory(): number {
+    if (this._data.length * 4 * CLEANUP_THRESHOLD < this._data.buffer.byteLength) {
+      const data = new Uint32Array(this._data.length);
+      data.set(this._data);
+      this._data = data;
+      return 1;
+    }
+    return 0;
   }
 
   /** fill a line with fillCharData */