From 2e7b125abf47f935fd2d2c13030da7353f0d3603 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Tue, 28 Jun 2022 11:59:16 +0200 Subject: [PATCH 1/6] change output buffer to setTimeout --- .../src/node/core-service-impl.ts | 18 +++++--- .../src/node/utils/simple-buffer.ts | 43 ++++++++++++++++--- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts index 6d55b307b..0aaabe542 100644 --- a/arduino-ide-extension/src/node/core-service-impl.ts +++ b/arduino-ide-extension/src/node/core-service-impl.ts @@ -76,10 +76,11 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { .filter(notEmpty) .shift() ?? error.details ); - this.sendResponse( - error.details + '\n\n' + message, - OutputMessage.Severity.Error + const chunk = new TextEncoder().encode( + `${error.details}'\n\n'${message}` ); + handler.addChunk(chunk, OutputMessage.Severity.Error); + reject(CoreError.VerifyFailed(message, compilerErrors)); } }) @@ -181,7 +182,8 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { firstToUpperCase(task), error.details ); - this.sendResponse(error.details, OutputMessage.Severity.Error); + const chunk = new TextEncoder().encode(error.details); + handler.addChunk(chunk, OutputMessage.Severity.Error); reject( errorHandler( message, @@ -245,7 +247,8 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { ); reject(error); } else { - this.sendResponse(error.details, OutputMessage.Severity.Error); + const chunk = new TextEncoder().encode(error.details); + handler.addChunk(chunk, OutputMessage.Severity.Error); reject( CoreError.BurnBootloaderFailed( nls.localize( @@ -288,6 +291,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { private createOnDataHandler(): Disposable & { stderr: Buffer[]; onData: (response: R) => void; + addChunk: (chunk: Uint8Array, severity?: OutputMessage.Severity) => void; } { const stderr: Buffer[] = []; const buffer = new SimpleBuffer((chunks) => { @@ -301,10 +305,14 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { buffer.addChunk(out); buffer.addChunk(err, OutputMessage.Severity.Error); }); + const addChunk = (chunk: Uint8Array, severity?: OutputMessage.Severity) => + buffer.addChunk(chunk, severity); + return { dispose: () => buffer.dispose(), stderr, onData, + addChunk, }; } diff --git a/arduino-ide-extension/src/node/utils/simple-buffer.ts b/arduino-ide-extension/src/node/utils/simple-buffer.ts index 1e3a8293d..5d3eb7d83 100644 --- a/arduino-ide-extension/src/node/utils/simple-buffer.ts +++ b/arduino-ide-extension/src/node/utils/simple-buffer.ts @@ -6,7 +6,8 @@ const DEFAULT_FLUS_TIMEOUT_MS = 32; export class SimpleBuffer implements Disposable { private readonly chunks = Chunks.create(); private readonly flush: () => void; - private flushInterval?: NodeJS.Timeout; + private flushTimeout?: NodeJS.Timeout; + private disposed = false; constructor( onFlush: (chunks: Map) => void, @@ -19,7 +20,8 @@ export class SimpleBuffer implements Disposable { onFlush(chunks); } }; - this.flushInterval = setInterval(this.flush, flushTimeout); + + this.setTimeoutVariable(flushTimeout); } addChunk( @@ -33,11 +35,40 @@ export class SimpleBuffer implements Disposable { Chunks.clear(this.chunks); } + private setTimeoutVariable(flushTimeout: number): void { + let isDisposed = this.disposed; + if (isDisposed) { + // once "isDisposed" is true we stop + // creating timeouts and do one more + // flush AFTER any setTimeout + // callback that may be in progress + this.flush(); + isDisposed = false; + return; + } + + if (!this.flushTimeout) { + const onTimeout = () => { + this.flush(); + this.clearTimeoutVariable(); + }; + + this.flushTimeout = setTimeout(() => { + onTimeout(); + this.setTimeoutVariable(flushTimeout); + }, flushTimeout); + } + } + + private clearTimeoutVariable(): void { + if (this.flushTimeout) { + clearTimeout(this.flushTimeout); + this.flushTimeout = undefined; + } + } + dispose(): void { - this.flush(); - clearInterval(this.flushInterval); - this.clearChunks(); - this.flushInterval = undefined; + this.disposed = true; } } From fad7740da86b41a1daaafffbf33ba4854e9ae763 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Tue, 28 Jun 2022 13:11:46 +0200 Subject: [PATCH 2/6] remove unnec. code --- arduino-ide-extension/src/node/utils/simple-buffer.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arduino-ide-extension/src/node/utils/simple-buffer.ts b/arduino-ide-extension/src/node/utils/simple-buffer.ts index 5d3eb7d83..621078c20 100644 --- a/arduino-ide-extension/src/node/utils/simple-buffer.ts +++ b/arduino-ide-extension/src/node/utils/simple-buffer.ts @@ -36,14 +36,13 @@ export class SimpleBuffer implements Disposable { } private setTimeoutVariable(flushTimeout: number): void { - let isDisposed = this.disposed; + const isDisposed = this.disposed; if (isDisposed) { // once "isDisposed" is true we stop // creating timeouts and do one more // flush AFTER any setTimeout // callback that may be in progress this.flush(); - isDisposed = false; return; } From 76315b56869e4e528e53ec7a01d547b88b63f14f Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Tue, 28 Jun 2022 13:34:06 +0200 Subject: [PATCH 3/6] dispose buffer on end, not 'finally' --- .../src/node/core-service-impl.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts index 0aaabe542..3c1218228 100644 --- a/arduino-ide-extension/src/node/core-service-impl.ts +++ b/arduino-ide-extension/src/node/core-service-impl.ts @@ -84,8 +84,11 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { reject(CoreError.VerifyFailed(message, compilerErrors)); } }) - .on('end', resolve); - }).finally(() => handler.dispose()); + .on('end', () => { + handler.dispose(); + resolve(); + }); + }); } private compileRequest( @@ -195,9 +198,11 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { ); } }) - .on('end', resolve); + .on('end', () => { + handler.dispose(); + resolve(); + }); }).finally(async () => { - handler.dispose(); await this.notifyUploadDidFinish(options); }) ); @@ -261,9 +266,11 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { ); } }) - .on('end', resolve); + .on('end', () => { + handler.dispose(); + resolve(); + }); }).finally(async () => { - handler.dispose(); await this.notifyUploadDidFinish(options); }) ); From 3f68557912b06e25217ad87d06a85845795cd4be Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Wed, 29 Jun 2022 15:38:41 +0200 Subject: [PATCH 4/6] revert core-service changes --- .../src/node/core-service-impl.ts | 37 ++++++------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts index 3c1218228..6d55b307b 100644 --- a/arduino-ide-extension/src/node/core-service-impl.ts +++ b/arduino-ide-extension/src/node/core-service-impl.ts @@ -76,19 +76,15 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { .filter(notEmpty) .shift() ?? error.details ); - const chunk = new TextEncoder().encode( - `${error.details}'\n\n'${message}` + this.sendResponse( + error.details + '\n\n' + message, + OutputMessage.Severity.Error ); - handler.addChunk(chunk, OutputMessage.Severity.Error); - reject(CoreError.VerifyFailed(message, compilerErrors)); } }) - .on('end', () => { - handler.dispose(); - resolve(); - }); - }); + .on('end', resolve); + }).finally(() => handler.dispose()); } private compileRequest( @@ -185,8 +181,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { firstToUpperCase(task), error.details ); - const chunk = new TextEncoder().encode(error.details); - handler.addChunk(chunk, OutputMessage.Severity.Error); + this.sendResponse(error.details, OutputMessage.Severity.Error); reject( errorHandler( message, @@ -198,11 +193,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { ); } }) - .on('end', () => { - handler.dispose(); - resolve(); - }); + .on('end', resolve); }).finally(async () => { + handler.dispose(); await this.notifyUploadDidFinish(options); }) ); @@ -252,8 +245,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { ); reject(error); } else { - const chunk = new TextEncoder().encode(error.details); - handler.addChunk(chunk, OutputMessage.Severity.Error); + this.sendResponse(error.details, OutputMessage.Severity.Error); reject( CoreError.BurnBootloaderFailed( nls.localize( @@ -266,11 +258,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { ); } }) - .on('end', () => { - handler.dispose(); - resolve(); - }); + .on('end', resolve); }).finally(async () => { + handler.dispose(); await this.notifyUploadDidFinish(options); }) ); @@ -298,7 +288,6 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { private createOnDataHandler(): Disposable & { stderr: Buffer[]; onData: (response: R) => void; - addChunk: (chunk: Uint8Array, severity?: OutputMessage.Severity) => void; } { const stderr: Buffer[] = []; const buffer = new SimpleBuffer((chunks) => { @@ -312,14 +301,10 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { buffer.addChunk(out); buffer.addChunk(err, OutputMessage.Severity.Error); }); - const addChunk = (chunk: Uint8Array, severity?: OutputMessage.Severity) => - buffer.addChunk(chunk, severity); - return { dispose: () => buffer.dispose(), stderr, onData, - addChunk, }; } From e5f922abfe2b2e2ace3f813d40e560274d4ae892 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Mon, 4 Jul 2022 15:18:58 +0200 Subject: [PATCH 5/6] refactor, disposable pattern --- .../src/node/core-service-impl.ts | 4 +- .../utils/{simple-buffer.ts => buffers.ts} | 72 +++++++------------ 2 files changed, 27 insertions(+), 49 deletions(-) rename arduino-ide-extension/src/node/utils/{simple-buffer.ts => buffers.ts} (56%) diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts index 6d55b307b..c1919d6db 100644 --- 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/arduino-ide-extension/src/node/utils/simple-buffer.ts b/arduino-ide-extension/src/node/utils/buffers.ts similarity index 56% rename from arduino-ide-extension/src/node/utils/simple-buffer.ts rename to arduino-ide-extension/src/node/utils/buffers.ts index 621078c20..d84935b05 100644 --- a/arduino-ide-extension/src/node/utils/simple-buffer.ts +++ b/arduino-ide-extension/src/node/utils/buffers.ts @@ -1,27 +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 flushTimeout?: NodeJS.Timeout; + private readonly toDispose; + private timer?: NodeJS.Timeout; private disposed = false; constructor( onFlush: (chunks: Map) => 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.setTimeoutVariable(flushTimeout); + this.timer = setTimeout(task, taskTimeout); + this.toDispose = new DisposableCollection( + Disposable.create(() => (this.disposed = true)), + Disposable.create(() => clearTimeout(this.timer)), + Disposable.create(() => task()) + ); } addChunk( @@ -31,45 +37,17 @@ export class SimpleBuffer implements Disposable { this.chunks.get(severity)?.push(chunk); } - private clearChunks(): void { - Chunks.clear(this.chunks); - } - - private setTimeoutVariable(flushTimeout: number): void { - const isDisposed = this.disposed; - if (isDisposed) { - // once "isDisposed" is true we stop - // creating timeouts and do one more - // flush AFTER any setTimeout - // callback that may be in progress - this.flush(); - return; - } - - if (!this.flushTimeout) { - const onTimeout = () => { - this.flush(); - this.clearTimeoutVariable(); - }; - - this.flushTimeout = setTimeout(() => { - onTimeout(); - this.setTimeoutVariable(flushTimeout); - }, flushTimeout); - } - } - - private clearTimeoutVariable(): void { - if (this.flushTimeout) { - clearTimeout(this.flushTimeout); - this.flushTimeout = undefined; - } - } - dispose(): void { - this.disposed = true; + 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; namespace Chunks { @@ -99,4 +77,4 @@ namespace Chunks { ]) ); } -} +} \ No newline at end of file From 758a90ac272cb332bedd8f9a18159c009db8cb73 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Mon, 4 Jul 2022 15:34:37 +0200 Subject: [PATCH 6/6] newline --- arduino-ide-extension/src/node/utils/buffers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino-ide-extension/src/node/utils/buffers.ts b/arduino-ide-extension/src/node/utils/buffers.ts index d84935b05..e703e1bce 100644 --- a/arduino-ide-extension/src/node/utils/buffers.ts +++ b/arduino-ide-extension/src/node/utils/buffers.ts @@ -77,4 +77,4 @@ namespace Chunks { ]) ); } -} \ No newline at end of file +}