Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log latency stats and remove old unused latency mechanism #187281

Merged
merged 2 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/vs/platform/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,10 @@ export interface IPtyService {
*/
listProcesses(): Promise<IProcessDetails[]>;
getPerformanceMarks(): Promise<performance.PerformanceMark[]>;
/**
* Measures and returns the latency of the current and all other processes to the pty host.
*/
getLatency(): Promise<IPtyHostLatencyMeasurement[]>;

start(id: number): Promise<ITerminalLaunchError | { injectedArgs: string[] } | undefined>;
shutdown(id: number, immediate: boolean): Promise<void>;
Expand All @@ -308,7 +312,6 @@ export interface IPtyService {
clearBuffer(id: number): Promise<void>;
getInitialCwd(id: number): Promise<string>;
getCwd(id: number): Promise<string>;
getLatency(id: number): Promise<number>;
acknowledgeDataEvent(id: number, charCount: number): Promise<void>;
setUnicodeVersion(id: number, version: '6' | '11'): Promise<void>;
processBinary(id: number, data: string): Promise<void>;
Expand Down Expand Up @@ -367,6 +370,11 @@ export interface IPtyHostController {
export interface IPtyHostService extends IPtyService, IPtyHostController {
}

export interface IPtyHostLatencyMeasurement {
label: string;
latency: number;
}

/**
* Serialized terminal state matching the interface that can be used across versions, the version
* should be verified before using the state payload.
Expand Down Expand Up @@ -739,7 +747,6 @@ export interface ITerminalChildProcess {

getInitialCwd(): Promise<string>;
getCwd(): Promise<string>;
getLatency(): Promise<number>;
refreshProperty<T extends ProcessPropertyType>(property: T): Promise<IProcessPropertyMap[T]>;
updateProperty<T extends ProcessPropertyType>(property: T, value: IProcessPropertyMap[T]): Promise<void>;
}
Expand Down Expand Up @@ -992,6 +999,7 @@ export interface ITerminalBackend {
attachToProcess(id: number): Promise<ITerminalChildProcess | undefined>;
attachToRevivedProcess(id: number): Promise<ITerminalChildProcess | undefined>;
listProcesses(): Promise<IProcessDetails[]>;
getLatency(): Promise<IPtyHostLatencyMeasurement[]>;
getDefaultSystemShell(osOverride?: OperatingSystem): Promise<string>;
getProfiles(profiles: unknown, defaultProfile: unknown, includeDetectedProfiles?: boolean): Promise<ITerminalProfile[]>;
getWslPath(original: string, direction: 'unix-to-win' | 'win-to-unix'): Promise<string>;
Expand Down
16 changes: 13 additions & 3 deletions src/vs/platform/terminal/node/ptyHostService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import { RemoteLoggerChannelClient } from 'vs/platform/log/common/logIpc';
import { getResolvedShellEnv } from 'vs/platform/shell/node/shellEnv';
import { IPtyHostProcessReplayEvent } from 'vs/platform/terminal/common/capabilities/capabilities';
import { RequestStore } from 'vs/platform/terminal/common/requestStore';
import { HeartbeatConstants, IHeartbeatService, IProcessDataEvent, IProcessProperty, IProcessPropertyMap, IProcessReadyEvent, IPtyHostService, IPtyService, IRequestResolveVariablesEvent, ISerializedTerminalState, IShellLaunchConfig, ITerminalLaunchError, ITerminalProcessOptions, ITerminalProfile, ITerminalsLayoutInfo, ProcessPropertyType, TerminalIcon, TerminalIpcChannels, TerminalSettingId, TitleEventSource } from 'vs/platform/terminal/common/terminal';
import { HeartbeatConstants, IHeartbeatService, IProcessDataEvent, IProcessProperty, IProcessPropertyMap, IProcessReadyEvent, IPtyHostLatencyMeasurement, IPtyHostService, IPtyService, IRequestResolveVariablesEvent, ISerializedTerminalState, IShellLaunchConfig, ITerminalLaunchError, ITerminalProcessOptions, ITerminalProfile, ITerminalsLayoutInfo, ProcessPropertyType, TerminalIcon, TerminalIpcChannels, TerminalSettingId, TitleEventSource } from 'vs/platform/terminal/common/terminal';
import { registerTerminalPlatformConfiguration } from 'vs/platform/terminal/common/terminalPlatformConfiguration';
import { IGetTerminalLayoutInfoArgs, IProcessDetails, ISetTerminalLayoutInfoArgs } from 'vs/platform/terminal/common/terminalProcess';
import { IPtyHostConnection, IPtyHostStarter } from 'vs/platform/terminal/node/ptyHost';
import { detectAvailableProfiles } from 'vs/platform/terminal/node/terminalProfiles';
import * as performance from 'vs/base/common/performance';
import { getSystemShell } from 'vs/base/node/shell';
import { StopWatch } from 'vs/base/common/stopwatch';

enum Constants {
MaxRestarts = 5
Expand Down Expand Up @@ -272,8 +273,17 @@ export class PtyHostService extends Disposable implements IPtyHostService {
getCwd(id: number): Promise<string> {
return this._proxy.getCwd(id);
}
getLatency(id: number): Promise<number> {
return this._proxy.getLatency(id);
async getLatency(): Promise<IPtyHostLatencyMeasurement[]> {
const sw = new StopWatch();
const results = await this._proxy.getLatency();
sw.stop();
return [
{
label: 'ptyhostservice<->ptyhost',
latency: sw.elapsed()
},
...results
];
}
orphanQuestionReply(id: number): Promise<void> {
return this._proxy.orphanQuestionReply(id);
Expand Down
9 changes: 3 additions & 6 deletions src/vs/platform/terminal/node/ptyService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { URI } from 'vs/base/common/uri';
import { getSystemShell } from 'vs/base/node/shell';
import { ILogService, LogLevel } from 'vs/platform/log/common/log';
import { RequestStore } from 'vs/platform/terminal/common/requestStore';
import { IProcessDataEvent, IProcessReadyEvent, IPtyService, IRawTerminalInstanceLayoutInfo, IReconnectConstants, IShellLaunchConfig, ITerminalInstanceLayoutInfoById, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalTabLayoutInfoById, TerminalIcon, IProcessProperty, TitleEventSource, ProcessPropertyType, IProcessPropertyMap, IFixedTerminalDimensions, IPersistentTerminalProcessLaunchConfig, ICrossVersionSerializedTerminalState, ISerializedTerminalState, ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
import { IProcessDataEvent, IProcessReadyEvent, IPtyService, IRawTerminalInstanceLayoutInfo, IReconnectConstants, IShellLaunchConfig, ITerminalInstanceLayoutInfoById, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalTabLayoutInfoById, TerminalIcon, IProcessProperty, TitleEventSource, ProcessPropertyType, IProcessPropertyMap, IFixedTerminalDimensions, IPersistentTerminalProcessLaunchConfig, ICrossVersionSerializedTerminalState, ISerializedTerminalState, ITerminalProcessOptions, IPtyHostLatencyMeasurement } from 'vs/platform/terminal/common/terminal';
import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
import { escapeNonWindowsPath } from 'vs/platform/terminal/common/terminalEnvironment';
import { Terminal as XtermTerminal } from 'xterm-headless';
Expand Down Expand Up @@ -399,8 +399,8 @@ export class PtyService extends Disposable implements IPtyService {
return this._throwIfNoPty(id).setUnicodeVersion(version);
}
@traceRpc
async getLatency(id: number): Promise<number> {
return 0;
async getLatency(): Promise<IPtyHostLatencyMeasurement[]> {
return [];
}
@traceRpc
async orphanQuestionReply(id: number): Promise<void> {
Expand Down Expand Up @@ -874,9 +874,6 @@ class PersistentTerminalProcess extends Disposable {
getCwd(): Promise<string> {
return this._terminalProcess.getCwd();
}
getLatency(): Promise<number> {
return this._terminalProcess.getLatency();
}

async triggerReplay(): Promise<void> {
if (this._interactionState.value === InteractionState.None) {
Expand Down
4 changes: 0 additions & 4 deletions src/vs/platform/terminal/node/terminalProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,10 +627,6 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess
return this._initialCwd;
}

getLatency(): Promise<number> {
return Promise.resolve(0);
}

getWindowsPty(): IProcessReadyWindowsPty | undefined {
return isWindows ? {
backend: 'useConpty' in this._ptyOptions && this._ptyOptions.useConpty ? 'conpty' : 'winpty',
Expand Down
1 change: 1 addition & 0 deletions src/vs/server/node/remoteTerminalChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class RemoteTerminalChannel extends Disposable implements IServerChannel<
case RemoteTerminalChannelRequest.DetachFromProcess: return this._ptyHostService.detachFromProcess.apply(this._ptyHostService, args);

case RemoteTerminalChannelRequest.ListProcesses: return this._ptyHostService.listProcesses.apply(this._ptyHostService, args);
case RemoteTerminalChannelRequest.GetLatency: return this._ptyHostService.getLatency.apply(this._ptyHostService, args);
case RemoteTerminalChannelRequest.GetPerformanceMarks: return this._ptyHostService.getPerformanceMarks.apply(this._ptyHostService, args);
case RemoteTerminalChannelRequest.OrphanQuestionReply: return this._ptyHostService.orphanQuestionReply.apply(this._ptyHostService, args);
case RemoteTerminalChannelRequest.AcceptPtyHostResolvedVariables: return this._ptyHostService.acceptPtyHostResolvedVariables.apply(this._ptyHostService, args);
Expand Down
23 changes: 0 additions & 23 deletions src/vs/workbench/api/browser/mainThreadTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { DisposableStore, Disposable, IDisposable, MutableDisposable } from 'vs/
import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, TerminalLaunchConfig, ITerminalDimensionsDto, ExtHostTerminalIdentifier, TerminalQuickFix } from 'vs/workbench/api/common/extHost.protocol';
import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
import { URI } from 'vs/base/common/uri';
import { StopWatch } from 'vs/base/common/stopwatch';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ILogService } from 'vs/platform/log/common/log';
import { IProcessProperty, IProcessReadyWindowsPty, IShellLaunchConfig, IShellLaunchConfigDto, ITerminalOutputMatch, ITerminalOutputMatcher, ProcessPropertyType, TerminalExitReason, TerminalLocation } from 'vs/platform/terminal/common/terminal';
Expand Down Expand Up @@ -368,7 +367,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(proxy.instanceId, immediate));
proxy.onRequestCwd(() => this._proxy.$acceptProcessRequestCwd(proxy.instanceId));
proxy.onRequestInitialCwd(() => this._proxy.$acceptProcessRequestInitialCwd(proxy.instanceId));
proxy.onRequestLatency(() => this._onRequestLatency(proxy.instanceId));
}

public $sendProcessData(terminalId: number, data: string): void {
Expand All @@ -387,27 +385,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
this._terminalProcessProxies.get(terminalId)?.emitProcessProperty(property);
}

private async _onRequestLatency(terminalId: number): Promise<void> {
const COUNT = 2;
let sum = 0;
for (let i = 0; i < COUNT; i++) {
const sw = StopWatch.create();
await this._proxy.$acceptProcessRequestLatency(terminalId);
sw.stop();
sum += sw.elapsed();
}
this._getTerminalProcess(terminalId)?.emitLatency(sum / COUNT);
}

private _getTerminalProcess(terminalId: number): ITerminalProcessExtHostProxy | undefined {
const terminal = this._terminalProcessProxies.get(terminalId);
if (!terminal) {
this._logService.error(`Unknown terminal: ${terminalId}`);
return undefined;
}
return terminal;
}

$setEnvironmentVariableCollection(extensionIdentifier: string, persistent: boolean, collection: ISerializableEnvironmentVariableCollection | undefined, descriptionMap: ISerializableEnvironmentDescriptionMap): void {
if (collection) {
const translatedCollection = {
Expand Down
4 changes: 0 additions & 4 deletions src/vs/workbench/api/common/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,6 @@ class ExtHostPseudoterminal implements ITerminalChildProcess {
return Promise.resolve('');
}

getLatency(): Promise<number> {
return Promise.resolve(0);
}

startSendingEvents(initialDimensions: ITerminalDimensionsDto | undefined): void {
// Attach the listeners
this._pty.onDidWrite(e => this._onProcessData.fire(e));
Expand Down
4 changes: 0 additions & 4 deletions src/vs/workbench/contrib/terminal/browser/remotePty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,4 @@ export class RemotePty extends Disposable implements ITerminalChildProcess {
handleOrphanQuestion() {
this._remoteTerminalChannel.orphanQuestionReply(this.id);
}

async getLatency(): Promise<number> {
return 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import { Emitter } from 'vs/base/common/event';
import { revive } from 'vs/base/common/marshalling';
import { PerformanceMark, mark } from 'vs/base/common/performance';
import { IProcessEnvironment, OperatingSystem } from 'vs/base/common/platform';
import { StopWatch } from 'vs/base/common/stopwatch';
import { ICommandService } from 'vs/platform/commands/common/commands';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { Registry } from 'vs/platform/registry/common/platform';
import { IRemoteAuthorityResolverService } from 'vs/platform/remote/common/remoteAuthorityResolver';
import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
import { ISerializedTerminalCommand } from 'vs/platform/terminal/common/capabilities/capabilities';
import { IShellLaunchConfig, IShellLaunchConfigDto, ITerminalBackend, ITerminalBackendRegistry, ITerminalChildProcess, ITerminalEnvironment, ITerminalLogService, ITerminalProcessOptions, ITerminalProfile, ITerminalsLayoutInfo, ITerminalsLayoutInfoById, ProcessPropertyType, TerminalExtensions, TerminalIcon, TerminalSettingId, TitleEventSource } from 'vs/platform/terminal/common/terminal';
import { IPtyHostLatencyMeasurement, IShellLaunchConfig, IShellLaunchConfigDto, ITerminalBackend, ITerminalBackendRegistry, ITerminalChildProcess, ITerminalEnvironment, ITerminalLogService, ITerminalProcessOptions, ITerminalProfile, ITerminalsLayoutInfo, ITerminalsLayoutInfoById, ProcessPropertyType, TerminalExtensions, TerminalIcon, TerminalSettingId, TitleEventSource } from 'vs/platform/terminal/common/terminal';
import { IProcessDetails } from 'vs/platform/terminal/common/terminalProcess';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
Expand Down Expand Up @@ -260,6 +261,19 @@ class RemoteTerminalBackend extends BaseTerminalBackend implements ITerminalBack
return this._remoteTerminalChannel.listProcesses();
}

async getLatency(): Promise<IPtyHostLatencyMeasurement[]> {
const sw = new StopWatch();
const results = await this._remoteTerminalChannel.getLatency();
sw.stop();
return [
{
label: 'window<->ptyhostservice<->ptyhost',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this label include remote?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latency: sw.elapsed()
},
...results
];
}

async updateProperty<T extends ProcessPropertyType>(id: number, property: T, value: any): Promise<void> {
await this._remoteTerminalChannel.updateProperty(id, property, value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,13 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal
readonly onRequestInitialCwd: Event<void> = this._onRequestInitialCwd.event;
private readonly _onRequestCwd = this._register(new Emitter<void>());
readonly onRequestCwd: Event<void> = this._onRequestCwd.event;
private readonly _onRequestLatency = this._register(new Emitter<void>());
readonly onRequestLatency: Event<void> = this._onRequestLatency.event;
private readonly _onDidChangeProperty = this._register(new Emitter<IProcessProperty<any>>());
readonly onDidChangeProperty = this._onDidChangeProperty.event;
private readonly _onProcessExit = this._register(new Emitter<number | undefined>());
readonly onProcessExit: Event<number | undefined> = this._onProcessExit.event;


private _pendingInitialCwdRequests: ((value: string | PromiseLike<string>) => void)[] = [];
private _pendingCwdRequests: ((value: string | PromiseLike<string>) => void)[] = [];
private _pendingLatencyRequests: ((value: number | PromiseLike<number>) => void)[] = [];

constructor(
public instanceId: number,
Expand Down Expand Up @@ -112,12 +108,6 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal
}
}

emitLatency(latency: number): void {
while (this._pendingLatencyRequests.length > 0) {
this._pendingLatencyRequests.pop()!(latency);
}
}

async start(): Promise<ITerminalLaunchError | undefined> {
return this._terminalService.requestStartExtensionTerminal(this, this._cols, this._rows);
}
Expand Down Expand Up @@ -165,13 +155,6 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal
});
}

getLatency(): Promise<number> {
return new Promise<number>(resolve => {
this._onRequestLatency.fire();
this._pendingLatencyRequests.push(resolve);
});
}

async refreshProperty<T extends ProcessPropertyType>(type: T): Promise<any> {
// throws if called in extHostTerminalService
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import Severity from 'vs/base/common/severity';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { IEnvironmentVariableCollection, IMergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
import { generateUuid } from 'vs/base/common/uuid';
import { runWhenIdle } from 'vs/base/common/async';

const enum ProcessConstants {
/**
Expand Down Expand Up @@ -79,8 +80,6 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
private _process: ITerminalChildProcess | null = null;
private _processType: ProcessType = ProcessType.Process;
private _preLaunchInputQueue: string[] = [];
private _latency: number = -1;
private _latencyLastMeasured: number = 0;
private _initialCwd: string | undefined;
private _extEnvironmentVariableCollection: IMergedEnvironmentVariableCollection | undefined;
private _ackDataBufferer: AckDataBufferer;
Expand Down Expand Up @@ -152,7 +151,6 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
super();
this._cwdWorkspaceFolder = terminalEnvironment.getWorkspaceForTerminal(cwd, this._workspaceContextService, this._historyService);
this.ptyProcessReady = this._createPtyProcessReadyPromise();
this.getLatency();
this._ackDataBufferer = new AckDataBufferer(e => this._process?.acknowledgeDataEvent(e));
this._dataFilter = this._instantiationService.createInstance(SeamlessRelaunchDataFilter);
this._dataFilter.onProcessData(ev => {
Expand Down Expand Up @@ -394,6 +392,14 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
// Error
return result;
}

// Report the latency to the pty host when idle
runWhenIdle(() => {
this.backend?.getLatency().then(measurements => {
this._logService.info(`Latency measurements for ${this.remoteAuthority ?? 'local'} backend\n${measurements.map(e => `${e.label}: ${e.latency.toFixed(2)}ms`).join('\n')}`);
});
});

return undefined;
}

Expand Down Expand Up @@ -605,19 +611,6 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
return this._initialCwd ?? '';
}

async getLatency(): Promise<number> {
await this.ptyProcessReady;
if (!this._process) {
return Promise.resolve(0);
}
if (this._latencyLastMeasured === 0 || this._latencyLastMeasured + ProcessConstants.LatencyMeasuringInterval < Date.now()) {
const latencyRequest = this._process.getLatency();
this._latency = await latencyRequest;
this._latencyLastMeasured = Date.now();
}
return Promise.resolve(this._latency);
}

async refreshProperty<T extends ProcessPropertyType>(type: T): Promise<IProcessPropertyMap[T]> {
if (!this._process) {
throw new Error('Cannot refresh property when process is not set');
Expand Down