Skip to content

Commit 5367bf6

Browse files
author
Eric Amodio
authored
Merge pull request #82189 from microsoft/bug/#48513
Fixes #48513 - buffers terminal onData events
2 parents fdec4a3 + ebc27c4 commit 5367bf6

File tree

5 files changed

+302
-14
lines changed

5 files changed

+302
-14
lines changed

extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts

+34-12
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,27 @@ suite('window namespace tests', () => {
173173
const dataEvents: { name: string, data: string }[] = [];
174174
const closeEvents: string[] = [];
175175
const reg1 = window.onDidOpenTerminal(e => openEvents.push(e.name));
176-
const reg2 = window.onDidWriteTerminalData(e => dataEvents.push({ name: e.terminal.name, data: e.data }));
176+
177+
let resolveOnceDataWritten: (() => void) | undefined;
178+
179+
const reg2 = window.onDidWriteTerminalData(e => {
180+
dataEvents.push({ name: e.terminal.name, data: e.data });
181+
182+
resolveOnceDataWritten!();
183+
});
184+
177185
const reg3 = window.onDidCloseTerminal(e => {
178186
closeEvents.push(e.name);
179-
if (closeEvents.length === 2) {
180-
deepEqual(openEvents, [ 'test1', 'test2' ]);
181-
deepEqual(dataEvents, [ { name: 'test1', data: 'write1' }, { name: 'test2', data: 'write2' } ]);
182-
deepEqual(closeEvents, [ 'test1', 'test2' ]);
187+
188+
if (closeEvents.length === 1) {
189+
deepEqual(openEvents, ['test1']);
190+
deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }]);
191+
deepEqual(closeEvents, ['test1']);
192+
} else if (closeEvents.length === 2) {
193+
deepEqual(openEvents, ['test1', 'test2']);
194+
deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }, { name: 'test2', data: 'write2' }]);
195+
deepEqual(closeEvents, ['test1', 'test2']);
196+
183197
reg1.dispose();
184198
reg2.dispose();
185199
reg3.dispose();
@@ -192,22 +206,30 @@ suite('window namespace tests', () => {
192206
window.createTerminal({ name: 'test1', pty: {
193207
onDidWrite: term1Write.event,
194208
onDidClose: term1Close.event,
195-
open: () => {
209+
open: async () => {
196210
term1Write.fire('write1');
211+
212+
// Wait until the data is written
213+
await new Promise(resolve => { resolveOnceDataWritten = resolve; });
197214
term1Close.fire();
215+
198216
const term2Write = new EventEmitter<string>();
199217
const term2Close = new EventEmitter<void>();
200218
window.createTerminal({ name: 'test2', pty: {
201219
onDidWrite: term2Write.event,
202220
onDidClose: term2Close.event,
203-
open: () => {
221+
open: async () => {
204222
term2Write.fire('write2');
223+
224+
// Wait until the data is written
225+
await new Promise<void>(resolve => { resolveOnceDataWritten = resolve; });
226+
205227
term2Close.fire();
206228
},
207-
close: () => {}
229+
close: () => { }
208230
}});
209231
},
210-
close: () => {}
232+
close: () => { }
211233
}});
212234
});
213235
});
@@ -225,8 +247,8 @@ suite('window namespace tests', () => {
225247
});
226248
const pty: Pseudoterminal = {
227249
onDidWrite: new EventEmitter<string>().event,
228-
open: () => {},
229-
close: () => {}
250+
open: () => { },
251+
close: () => { }
230252
};
231253
window.createTerminal({ name: 'c', pty });
232254
});
@@ -303,7 +325,7 @@ suite('window namespace tests', () => {
303325
open: () => {
304326
overrideDimensionsEmitter.fire({ columns: 10, rows: 5 });
305327
},
306-
close: () => {}
328+
close: () => { }
307329
};
308330
const terminal = window.createTerminal({ name: 'foo', pty });
309331
});

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { StopWatch } from 'vs/base/common/stopwatch';
1212
import { ITerminalInstanceService, ITerminalService, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal';
1313
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
1414
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
15+
import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering';
1516

1617
@extHostNamedCustomer(MainContext.MainThreadTerminalService)
1718
export class MainThreadTerminalService implements MainThreadTerminalServiceShape {
@@ -327,16 +328,23 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
327328
* listeners are removed.
328329
*/
329330
class TerminalDataEventTracker extends Disposable {
331+
private readonly _bufferer: TerminalDataBufferer;
332+
330333
constructor(
331334
private readonly _callback: (id: number, data: string) => void,
332335
@ITerminalService private readonly _terminalService: ITerminalService
333336
) {
334337
super();
338+
339+
this._register(this._bufferer = new TerminalDataBufferer());
340+
335341
this._terminalService.terminalInstances.forEach(instance => this._registerInstance(instance));
336342
this._register(this._terminalService.onInstanceCreated(instance => this._registerInstance(instance)));
343+
this._register(this._terminalService.onInstanceDisposed(instance => this._bufferer.stopBuffering(instance.id)));
337344
}
338345

339346
private _registerInstance(instance: ITerminalInstance): void {
340-
this._register(instance.onData(e => this._callback(instance.id, e)));
347+
// Buffer data events to reduce the amount of messages going to the extension host
348+
this._register(this._bufferer.startBuffering(instance.id, instance.onData, this._callback));
341349
}
342350
}

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

+11-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { URI, UriComponents } from 'vs/base/common/uri';
1212
import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal';
1313
import { timeout } from 'vs/base/common/async';
1414
import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
15+
import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering';
1516

1617
export interface IExtHostTerminalService extends ExtHostTerminalServiceShape {
1718

@@ -286,9 +287,13 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
286287
protected readonly _onDidWriteTerminalData: Emitter<vscode.TerminalDataWriteEvent>;
287288
public get onDidWriteTerminalData(): Event<vscode.TerminalDataWriteEvent> { return this._onDidWriteTerminalData && this._onDidWriteTerminalData.event; }
288289

290+
private readonly _bufferer: TerminalDataBufferer;
291+
289292
constructor(
290293
@IExtHostRpcService extHostRpc: IExtHostRpcService
291294
) {
295+
this._bufferer = new TerminalDataBufferer();
296+
292297
this._proxy = extHostRpc.getProxy(MainContext.MainThreadTerminalService);
293298
this._onDidWriteTerminalData = new Emitter<vscode.TerminalDataWriteEvent>({
294299
onFirstListenerAdd: () => this._proxy.$startSendingDataEvents(),
@@ -464,8 +469,11 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
464469
protected _setupExtHostProcessListeners(id: number, p: ITerminalChildProcess): void {
465470
p.onProcessReady((e: { pid: number, cwd: string }) => this._proxy.$sendProcessReady(id, e.pid, e.cwd));
466471
p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title));
467-
p.onProcessData(data => this._proxy.$sendProcessData(id, data));
472+
473+
// Buffer data events to reduce the amount of messages going to the renderer
474+
this._bufferer.startBuffering(id, p.onProcessData, this._proxy.$sendProcessData);
468475
p.onProcessExit(exitCode => this._onProcessExit(id, exitCode));
476+
469477
if (p.onProcessOverrideDimensions) {
470478
p.onProcessOverrideDimensions(e => this._proxy.$sendOverrideDimensions(id, e));
471479
}
@@ -504,6 +512,8 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
504512
}
505513

506514
private _onProcessExit(id: number, exitCode: number): void {
515+
this._bufferer.stopBuffering(id);
516+
507517
// Remove process reference
508518
delete this._terminalProcesses[id];
509519

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { Event } from 'vs/base/common/event';
7+
import { IDisposable } from 'vs/base/common/lifecycle';
8+
9+
interface TerminalDataBuffer extends IDisposable {
10+
data: string[];
11+
timeoutId: any;
12+
}
13+
14+
export class TerminalDataBufferer implements IDisposable {
15+
private readonly _terminalBufferMap = new Map<number, TerminalDataBuffer>();
16+
17+
dispose() {
18+
for (const buffer of this._terminalBufferMap.values()) {
19+
buffer.dispose();
20+
}
21+
}
22+
23+
startBuffering(id: number, event: Event<string>, callback: (id: number, data: string) => void, throttleBy: number = 5): IDisposable {
24+
let disposable: IDisposable;
25+
disposable = event((e: string) => {
26+
let buffer = this._terminalBufferMap.get(id);
27+
if (buffer) {
28+
buffer.data.push(e);
29+
30+
return;
31+
}
32+
33+
const timeoutId = setTimeout(() => {
34+
this._terminalBufferMap.delete(id);
35+
callback(id, buffer!.data.join(''));
36+
}, throttleBy);
37+
buffer = {
38+
data: [e],
39+
timeoutId: timeoutId,
40+
dispose: () => {
41+
clearTimeout(timeoutId);
42+
this._terminalBufferMap.delete(id);
43+
disposable.dispose();
44+
}
45+
};
46+
this._terminalBufferMap.set(id, buffer);
47+
});
48+
return disposable;
49+
}
50+
51+
stopBuffering(id: number) {
52+
const buffer = this._terminalBufferMap.get(id);
53+
if (buffer) {
54+
buffer.dispose();
55+
}
56+
}
57+
}

0 commit comments

Comments
 (0)