Fixes #48513 - updated from review comments

Author: eamodio

src/vs/workbench/api/browser/mainThreadTerminalService.ts

@@ -4,14 +4,15 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { DisposableStore, Disposable } from 'vs/base/common/lifecycle';
-import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest, getTerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminal';
+import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest } from 'vs/workbench/contrib/terminal/common/terminal';
 import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, IShellLaunchConfigDto, TerminalLaunchConfig, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol';
 import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
 import { URI } from 'vs/base/common/uri';
 import { StopWatch } from 'vs/base/common/stopwatch';
 import { ITerminalInstanceService, ITerminalService, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal';
 import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
+import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering';
 
 @extHostNamedCustomer(MainContext.MainThreadTerminalService)
 export class MainThreadTerminalService implements MainThreadTerminalServiceShape {
@@ -327,29 +328,23 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
  * listeners are removed.
  */
 class TerminalDataEventTracker extends Disposable {
+	private readonly _bufferer: TerminalDataBufferer;
+
 	constructor(
 		private readonly _callback: (id: number, data: string) => void,
-		@ITerminalService private readonly _terminalService: ITerminalService
+		@ITerminalService private readonly _terminalService: ITerminalService,
 	) {
 		super();
+
+		this._register(this._bufferer = new TerminalDataBufferer());
+
 		this._terminalService.terminalInstances.forEach(instance => this._registerInstance(instance));
 		this._register(this._terminalService.onInstanceCreated(instance => this._registerInstance(instance)));
+		this._register(this._terminalService.onInstanceDisposed(instance => this._bufferer.stopBuffering(instance.id)));
 	}
 
 	private _registerInstance(instance: ITerminalInstance): void {
-		const bufferer = getTerminalDataBufferer(instance.id, this._callback);
-		const disposables = [bufferer, instance.onData(bufferer.onData)];
-
-		for (const d of disposables) {
-			this._register(d);
-		}
-
-		this._register(this._terminalService.onInstanceDisposed(i => {
-			if (i.id === instance.id) {
-				for (const d of disposables) {
-					d.dispose();
-				}
-			}
-		}));
+		// Buffer data events to reduce the amount of messages going to the extension host
+		this._register(this._bufferer.startBuffering(instance.id, instance.onData, this._callback));
 	}
 }

src/vs/workbench/api/common/extHostTerminalService.ts

@@ -4,15 +4,15 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as vscode from 'vscode';
-import { IDisposable } from 'vs/base/common/lifecycle';
 import { Event, Emitter } from 'vs/base/common/event';
 import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IShellLaunchConfigDto, IShellDefinitionDto, IShellAndArgsDto, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol';
 import { ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { URI, UriComponents } from 'vs/base/common/uri';
-import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions, getTerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminal';
+import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal';
 import { timeout } from 'vs/base/common/async';
 import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
+import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering';
 
 export interface IExtHostTerminalService extends ExtHostTerminalServiceShape {
 
@@ -287,9 +287,13 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
 	protected readonly _onDidWriteTerminalData: Emitter<vscode.TerminalDataWriteEvent>;
 	public get onDidWriteTerminalData(): Event<vscode.TerminalDataWriteEvent> { return this._onDidWriteTerminalData && this._onDidWriteTerminalData.event; }
 
+	private readonly _bufferer: TerminalDataBufferer;
+
 	constructor(
 		@IExtHostRpcService extHostRpc: IExtHostRpcService
 	) {
+		this._bufferer = new TerminalDataBufferer();
+
 		this._proxy = extHostRpc.getProxy(MainContext.MainThreadTerminalService);
 		this._onDidWriteTerminalData = new Emitter<vscode.TerminalDataWriteEvent>({
 			onFirstListenerAdd: () => this._proxy.$startSendingDataEvents(),
@@ -466,10 +470,9 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
 		p.onProcessReady((e: { pid: number, cwd: string }) => this._proxy.$sendProcessReady(id, e.pid, e.cwd));
 		p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title));
 
-		const bufferer = getTerminalDataBufferer(id, this._proxy.$sendProcessData);
-		const disposables = [bufferer, p.onProcessData(bufferer.onData)];
-
-		p.onProcessExit(exitCode => this._onProcessExit(id, exitCode, disposables));
+		// Buffer data events to reduce the amount of messages going to the renderer
+		this._bufferer.startBuffering(id, p.onProcessData, this._proxy.$sendProcessData);
+		p.onProcessExit(exitCode => this._onProcessExit(id, exitCode));
 
 		if (p.onProcessOverrideDimensions) {
 			p.onProcessOverrideDimensions(e => this._proxy.$sendOverrideDimensions(id, e));
@@ -508,10 +511,8 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
 		return id;
 	}
 
-	private _onProcessExit(id: number, exitCode: number, disposables: IDisposable[]): void {
-		for (const d of disposables) {
-			d.dispose();
-		}
+	private _onProcessExit(id: number, exitCode: number): void {
+		this._bufferer.stopBuffering(id);
 
 		// Remove process reference
 		delete this._terminalProcesses[id];

src/vs/workbench/contrib/terminal/common/terminal.ts

@@ -475,43 +475,3 @@ export const enum TERMINAL_COMMAND_ID {
 	NAVIGATION_MODE_FOCUS_NEXT = 'workbench.action.terminal.navigationModeFocusNext',
 	NAVIGATION_MODE_FOCUS_PREVIOUS = 'workbench.action.terminal.navigationModeFocusPrevious'
 }
-
-interface TerminalDataBuffer {
-	timeoutId: any;
-	data: string[];
-}
-
-export interface TerminalDataBufferer extends IDisposable {
-	onData: (e: string) => void;
-}
-
-const terminalBufferMap = new Map<number, TerminalDataBuffer>();
-
-export function getTerminalDataBufferer(id: number, callback: (id: number, data: string) => void): TerminalDataBufferer {
-	return {
-		dispose: () => {
-			const buffer = terminalBufferMap.get(id);
-			if (buffer) {
-				clearTimeout(buffer.timeoutId);
-				terminalBufferMap.delete(id);
-			}
-		},
-		onData: (e: string) => {
-			let buffer = terminalBufferMap.get(id);
-			if (buffer) {
-				buffer.data.push(e);
-
-				return;
-			}
-
-			buffer = {
-				data: [e],
-				timeoutId: setTimeout(() => {
-					terminalBufferMap.delete(id);
-					callback(id, buffer!.data.join(''));
-				}, 5)
-			};
-			terminalBufferMap.set(id, buffer);
-		}
-	};
-}

src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts

@@ -0,0 +1,57 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { Event } from 'vs/base/common/event';
+import { IDisposable } from 'vs/base/common/lifecycle';
+
+interface TerminalDataBuffer extends IDisposable {
+	data: string[];
+	timeoutId: any;
+}
+
+export class TerminalDataBufferer implements IDisposable {
+	private readonly _terminalBufferMap = new Map<number, TerminalDataBuffer>();
+
+	dispose() {
+		for (const buffer of this._terminalBufferMap.values()) {
+			buffer.dispose();
+		}
+	}
+
+	startBuffering(id: number, event: Event<string>, callback: (id: number, data: string) => void, throttleBy: number = 5): IDisposable {
+		let disposable: IDisposable;
+		disposable = event((e: string) => {
+			let buffer = this._terminalBufferMap.get(id);
+			if (buffer) {
+				buffer.data.push(e);
+
+				return;
+			}
+
+			const timeoutId = setTimeout(() => {
+				this._terminalBufferMap.delete(id);
+				callback(id, buffer!.data.join(''));
+			}, throttleBy);
+			buffer = {
+				data: [e],
+				timeoutId: timeoutId,
+				dispose: () => {
+					clearTimeout(timeoutId);
+					this._terminalBufferMap.delete(id);
+					disposable.dispose();
+				}
+			};
+			this._terminalBufferMap.set(id, buffer);
+		});
+		return disposable;
+	}
+
+	stopBuffering(id: number) {
+		const buffer = this._terminalBufferMap.get(id);
+		if (buffer) {
+			buffer.dispose();
+		}
+	}
+}