change output buffer to setTimeout instead of setInterval (#1123)

* change output buffer to setTimeout * remove unnec. code * dispose buffer on end, not 'finally' * revert core-service changes * refactor, disposable pattern * newline
arduino · Jul 5, 2022 · 0b0958c · 0b0958c
1 parent 06acd7f
commit 0b0958c
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 19 deletions.
diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts
@@ -28,7 +28,7 @@ import { ArduinoCoreServiceClient } from './cli-protocol/cc/arduino/cli/commands
 import { Port as GrpcPort } from './cli-protocol/cc/arduino/cli/commands/v1/port_pb';
 import { ApplicationError, Disposable, nls } from '@theia/core';
 import { MonitorManager } from './monitor-manager';
-import { SimpleBuffer } from './utils/simple-buffer';
+import { AutoFlushingBuffer } from './utils/buffers';
 import { tryParseError } from './cli-error-parser';
 import { Instance } from './cli-protocol/cc/arduino/cli/commands/v1/common_pb';
 import { firstToUpperCase, notEmpty } from '../common/utils';
@@ -290,7 +290,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     onData: (response: R) => void;
   } {
     const stderr: Buffer[] = [];
-    const buffer = new SimpleBuffer((chunks) => {
+    const buffer = new AutoFlushingBuffer((chunks) => {
       Array.from(chunks.entries()).forEach(([severity, chunk]) => {
         if (chunk) {
           this.sendResponse(chunk, severity);

diff --git a/...extension/src/node/utils/simple-buffer.ts → ...o-ide-extension/src/node/utils/buffers.ts b/...extension/src/node/utils/simple-buffer.ts → ...o-ide-extension/src/node/utils/buffers.ts
@@ -1,25 +1,33 @@
+import { DisposableCollection } from '@theia/core';
 import { Disposable } from '@theia/core/shared/vscode-languageserver-protocol';
 import { OutputMessage } from '../../common/protocol';
 
-const DEFAULT_FLUS_TIMEOUT_MS = 32;
-
-export class SimpleBuffer implements Disposable {
+export class AutoFlushingBuffer implements Disposable {
   private readonly chunks = Chunks.create();
-  private readonly flush: () => void;
-  private flushInterval?: NodeJS.Timeout;
+  private readonly toDispose;
+  private timer?: NodeJS.Timeout;
+  private disposed = false;
 
   constructor(
     onFlush: (chunks: Map<OutputMessage.Severity, string | undefined>) => void,
-    flushTimeout: number = DEFAULT_FLUS_TIMEOUT_MS
+    taskTimeout: number = AutoFlushingBuffer.DEFAULT_FLUSH_TIMEOUT_MS
   ) {
-    this.flush = () => {
+    const task = () => {
       if (!Chunks.isEmpty(this.chunks)) {
         const chunks = Chunks.toString(this.chunks);
-        this.clearChunks();
+        Chunks.clear(this.chunks);
         onFlush(chunks);
       }
+      if (!this.disposed) {
+        this.timer = setTimeout(task, taskTimeout);
+      }
     };
-    this.flushInterval = setInterval(this.flush, flushTimeout);
+    this.timer = setTimeout(task, taskTimeout);
+    this.toDispose = new DisposableCollection(
+      Disposable.create(() => (this.disposed = true)),
+      Disposable.create(() => clearTimeout(this.timer)),
+      Disposable.create(() => task())
+    );
   }
 
   addChunk(
@@ -29,17 +37,17 @@ export class SimpleBuffer implements Disposable {
     this.chunks.get(severity)?.push(chunk);
   }
 
-  private clearChunks(): void {
-    Chunks.clear(this.chunks);
-  }
-
   dispose(): void {
-    this.flush();
-    clearInterval(this.flushInterval);
-    this.clearChunks();
-    this.flushInterval = undefined;
+    this.toDispose.dispose();
   }
 }
+export namespace AutoFlushingBuffer {
+  /**
+   * _"chunking and sending every 16ms (60hz) is the best for small amount of data
+   * To be able to crunch more data without the cpu going to high, I opted for a 30fps refresh rate, hence the 32msec"_
+   */
+  export const DEFAULT_FLUSH_TIMEOUT_MS = 32;
+}
 
 type Chunks = Map<OutputMessage.Severity, Uint8Array[]>;
 namespace Chunks {