encapsulate deferred writing in WriteBuffer class

src/InputHandler.ts

@@ -344,6 +344,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     if (buffer.x !== cursorStartX || buffer.y !== cursorStartY) {
       this._onCursorMove.fire();
     }
+    this._terminal.refresh(this._dirtyRowService.start, this._dirtyRowService.end);
   }
 
   public print(data: Uint32Array, start: number, end: number): void {

src/Terminal.ts

@@ -62,35 +62,11 @@ import { ILinkifier, IMouseZoneManager, LinkMatcherHandler, ILinkMatcherOptions,
 import { DirtyRowService } from 'common/services/DirtyRowService';
 import { InstantiationService } from 'common/services/InstantiationService';
 import { CoreMouseService } from 'common/services/CoreMouseService';
+import { WriteBuffer } from 'common/input/WriteBuffer';
 
 // Let it work inside Node.js for automated testing purposes.
 const document = (typeof window !== 'undefined') ? window.document : null;
 
-/**
- * Safety watermark to avoid memory exhaustion and browser engine crash on fast data input.
- * Enable flow control to avoid this limit and make sure that your backend correctly
- * propagates this to the underlying pty. (see docs for further instructions)
- * Since this limit is meant as a safety parachute to prevent browser crashs,
- * it is set to a very high number. Typically xterm.js gets unresponsive with
- * a 100 times lower number (>500 kB).
- */
-const DISCARD_WATERMARK = 50000000; // ~50 MB
-
-/**
- * The max number of ms to spend on writes before allowing the renderer to
- * catch up with a 0ms setTimeout. A value of < 33 to keep us close to
- * 30fps, and a value of < 16 to try to run at 60fps. Of course, the real FPS
- * depends on the time it takes for the renderer to draw the frame.
- */
-const WRITE_TIMEOUT_MS = 12;
-
-/**
- * Threshold of max held chunks in the write buffer, that were already processed.
- * This is a tradeoff between extensive write buffer shifts (bad runtime) and high
- * memory consumption by data thats not used anymore.
- */
-const WRITE_BUFFER_LENGTH_THRESHOLD = 50;
-
 
 export class Terminal extends Disposable implements ITerminal, IDisposable, IInputHandlingTerminal {
   public textarea: HTMLTextAreaElement;
@@ -163,11 +139,8 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp
   public params: (string | number)[];
   public currentParam: string | number;
 
-  // write data related containers
-  protected _writeBuffer: (Uint8Array | string)[] = [];
-  private _pendingWriteDataSize: number = 0;
-  private _writeChunkCallbacks: ((() => void) | undefined)[] = [];
-  private _writeInProgress = false;
+  // write buffer
+  private _deferredWriteBuffer: WriteBuffer;
 
   // Store if user went browsing history in scrollback
   private _userScrolling: boolean;
@@ -258,6 +231,8 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp
 
     this._setupOptionsListeners();
     this._setup();
+
+    this._deferredWriteBuffer = new WriteBuffer(data => this._inputHandler.parse(data));
   }
 
   public dispose(): void {
@@ -1137,60 +1112,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp
     }
   }
 
-  public write(data: string | Uint8Array, callback?: () => void): void {
-    // Ensure the terminal isn't disposed
-    // NOOP on empty data
-    if (this._isDisposed || !data.length) {
-      return;
-    }
-
-    if (this._pendingWriteDataSize > DISCARD_WATERMARK) {
-      throw new Error('write data discarded, use flow control to avoid losing data');
-    }
-
-    this._pendingWriteDataSize += data.length;
-    this._writeBuffer.push(data);
-    this._writeChunkCallbacks.push(callback);
-
-    if (!this._writeInProgress) {
-      this._writeInProgress = true;
-      setTimeout(() => this._innerWrite());
-    }
-  }
-
-  protected _innerWrite(bufferOffset: number = 0): void {
-    const startTime = Date.now();
-    while (this._writeBuffer.length > bufferOffset) {
-      const data = this._writeBuffer[bufferOffset];
-      const cb = this._writeChunkCallbacks[bufferOffset];
-      bufferOffset++;
-
-      this._inputHandler.parse(data);
-      this._pendingWriteDataSize -= data.length;
-      if (cb) cb();
-
-      this.refresh(this._dirtyRowService.start, this._dirtyRowService.end);
-
-      if (Date.now() - startTime >= WRITE_TIMEOUT_MS) {
-        break;
-      }
-    }
-    if (this._writeBuffer.length > bufferOffset) {
-      // Allow renderer to catch up before processing the next batch
-      // trim already processed chunks if we are above threshold
-      if (bufferOffset > WRITE_BUFFER_LENGTH_THRESHOLD) {
-        this._writeBuffer = this._writeBuffer.slice(bufferOffset);
-        this._writeChunkCallbacks = this._writeChunkCallbacks.slice(bufferOffset);
-        bufferOffset = 0;
-      }
-      setTimeout(() => this._innerWrite(bufferOffset), 0);
-    } else {
-      this._writeInProgress = false;
-      this._writeBuffer = [];
-      this._writeChunkCallbacks = [];
-    }
-  }
-
   public paste(data: string): void {
     paste(data, this.textarea, this.bracketedPasteMode, this._coreService);
   }
@@ -1628,8 +1549,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp
     const customKeyEventHandler = this._customKeyEventHandler;
     const inputHandler = this._inputHandler;
     const cursorState = this.cursorState;
-    const writeBuffer = this._writeBuffer;
-    const writeInProgress = this._writeInProgress;
     const userScrolling = this._userScrolling;
 
     this._setup();
@@ -1644,8 +1563,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp
     this._customKeyEventHandler = customKeyEventHandler;
     this._inputHandler = inputHandler;
     this.cursorState = cursorState;
-    this._writeBuffer = writeBuffer;
-    this._writeInProgress = writeInProgress;
     this._userScrolling = userScrolling;
 
     // do a full screen refresh
@@ -1676,6 +1593,13 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp
     // return this.options.bellStyle === 'sound' ||
     //     this.options.bellStyle === 'both';
   }
+
+  public write(data: string | Uint8Array, callback?: () => void): void {
+    this._deferredWriteBuffer.write(data, callback);
+  }
+  public writeSync(data: string | Uint8Array): void {
+    this._deferredWriteBuffer.writeSync(data);
+  }
 }
 
 /**

src/TestUtils.test.ts

@@ -19,10 +19,6 @@ import { IParams, IFunctionIdentifier } from 'common/parser/Types';
 import { ISelectionService } from 'browser/services/Services';
 
 export class TestTerminal extends Terminal {
-  writeSync(data: string): void {
-    this._writeBuffer.push(data);
-    this._innerWrite();
-  }
   keyDown(ev: any): boolean { return this._keyDown(ev); }
   keyPress(ev: any): boolean { return this._keyPress(ev); }
 }

src/common/input/WriteBuffer.ts

@@ -0,0 +1,110 @@
+
+/**
+ * Copyright (c) 2019 The xterm.js authors. All rights reserved.
+ * @license MIT
+ */
+
+declare const setTimeout: (handler: () => void, timeout?: number) => void;
+
+/**
+ * Safety watermark to avoid memory exhaustion and browser engine crash on fast data input.
+ * Enable flow control to avoid this limit and make sure that your backend correctly
+ * propagates this to the underlying pty. (see docs for further instructions)
+ * Since this limit is meant as a safety parachute to prevent browser crashs,
+ * it is set to a very high number. Typically xterm.js gets unresponsive with
+ * a 100 times lower number (>500 kB).
+ */
+const DISCARD_WATERMARK = 50000000; // ~50 MB
+
+/**
+ * The max number of ms to spend on writes before allowing the renderer to
+ * catch up with a 0ms setTimeout. A value of < 33 to keep us close to
+ * 30fps, and a value of < 16 to try to run at 60fps. Of course, the real FPS
+ * depends on the time it takes for the renderer to draw the frame.
+ */
+const WRITE_TIMEOUT_MS = 12;
+
+/**
+ * Threshold of max held chunks in the write buffer, that were already processed.
+ * This is a tradeoff between extensive write buffer shifts (bad runtime) and high
+ * memory consumption by data thats not used anymore.
+ */
+const WRITE_BUFFER_LENGTH_THRESHOLD = 50;
+
+export class WriteBuffer {
+  private _writeBuffer: (string | Uint8Array)[] = [];
+  private _callbacks: ((() => void) | undefined)[] = [];
+  private _pendingData = 0;
+  private _bufferOffset = 0;
+
+  constructor(private _action: (data: string | Uint8Array) => void) { }
+
+  public writeSync(data: string | Uint8Array): void {
+    // force sync processing on pending data chunks to avoid in-band data scrambling
+    // does the same as innerWrite but without event loop
+    if (this._writeBuffer.length) {
+      for (let i = this._bufferOffset; i < this._writeBuffer.length; ++i) {
+        const data = this._writeBuffer[i];
+        const cb = this._callbacks[i];
+        this._action(data);
+        if (cb) cb();
+      }
+      // reset all to avoid reprocessing of chunks with scheduled innerWrite call
+      this._writeBuffer = [];
+      this._callbacks = [];
+      this._pendingData = 0;
+      // stop scheduled innerWrite by offset > length condition
+      this._bufferOffset = 0x7FFFFFFF;
+    }
+    // handle current data chunk
+    this._action(data);
+  }
+
+  public write(data: string | Uint8Array, callback?: () => void): void {
+    if (this._pendingData > DISCARD_WATERMARK) {
+      throw new Error('write data discarded, use flow control to avoid losing data');
+    }
+
+    // schedule chunk processing for next event loop run
+    if (!this._writeBuffer.length) {
+      this._bufferOffset = 0;
+      setTimeout(() => this._innerWrite());
+    }
+
+    this._pendingData += data.length;
+    this._writeBuffer.push(data);
+    this._callbacks.push(callback);
+  }
+
+  protected _innerWrite(): void {
+    const startTime = Date.now();
+    while (this._writeBuffer.length > this._bufferOffset) {
+      const data = this._writeBuffer[this._bufferOffset];
+      const cb = this._callbacks[this._bufferOffset];
+      this._bufferOffset++;
+
+      this._action(data);
+      this._pendingData -= data.length;
+      if (cb) cb();
+
+      if (Date.now() - startTime >= WRITE_TIMEOUT_MS) {
+        break;
+      }
+    }
+    if (this._writeBuffer.length > this._bufferOffset) {
+      // Allow renderer to catch up before processing the next batch
+      // trim already processed chunks if we are above threshold
+      if (this._bufferOffset > WRITE_BUFFER_LENGTH_THRESHOLD) {
+        this._writeBuffer = this._writeBuffer.slice(this._bufferOffset);
+        this._callbacks = this._callbacks.slice(this._bufferOffset);
+        this._bufferOffset = 0;
+      }
+      setTimeout(() => this._innerWrite(), 0);
+    } else {
+      this._writeBuffer = [];
+      this._callbacks = [];
+      this._pendingData = 0;
+      this._bufferOffset = 0;
+    }
+  }
+}

test/benchmark/Terminal.benchmark.ts

@@ -49,8 +49,8 @@ perfContext('Terminal: ls -lR /usr/lib', () => {
     before(() => {
       terminal = new Terminal({cols: 80, rows: 25, scrollback: 1000});
     });
-    new ThroughputRuntimeCase('', async () => {
-      await new Promise(resolve => terminal.write(content, resolve));
+    new ThroughputRuntimeCase('', () => {
+      terminal.writeSync(content);
       return {payloadSize: contentUtf8.length};
     }, {fork: false}).showAverageThroughput();
   });
@@ -60,8 +60,8 @@ perfContext('Terminal: ls -lR /usr/lib', () => {
     before(() => {
       terminal = new Terminal({cols: 80, rows: 25, scrollback: 1000});
     });
-    new ThroughputRuntimeCase('', async () => {
-      await new Promise(resolve => terminal.write(content, resolve));
+    new ThroughputRuntimeCase('', () => {
+      terminal.writeSync(content);
       return {payloadSize: contentUtf8.length};
     }, {fork: false}).showAverageThroughput();
   });