From f0942786b4ee338f02449e2e5751091e028eb898 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 6 Feb 2020 15:15:20 -0800 Subject: [PATCH] Add experimental setting to use separate server to compute project level diagnostics For #13953 **Problem** We'd like to show project wide diagnostics, however at the moment TS server is single threaded. This means that computing all these diagnostics would interrupt other user operations such as completions. Right now, our advice is to use tasks to get around this limitation (since tasks always run as separate process) however few people actually use tasks. **Change** This change adds an experimental `tsserver.experimental.enableProjectDiagnostics` setting (default false) that makes VS Code spawn a separate TS Server that is only used for computing diagnostics. This should help keep the primary syntax server responsive while letting the diagnostics server churn away at project level diagnostics **Why experimental?** - We are comporting too many diagnostics. This is bad for larger projects. I don't think TS provides the right APIs to know which files we actually need to request diagnostics on when a file changes. - This hasn't been fully extensively tested to make sure it plays nicely with feature such as automatic type acquisition or in complex workspace with multiple projects --- .../typescript-language-features/package.json | 6 + .../package.nls.json | 1 + .../src/features/bufferSyncSupport.ts | 22 +- .../src/tsServer/server.ts | 263 ++++++++++++------ .../src/tsServer/spawner.ts | 41 ++- .../src/typescriptService.ts | 7 +- .../src/utils/configuration.ts | 7 + 7 files changed, 245 insertions(+), 102 deletions(-) diff --git a/extensions/typescript-language-features/package.json b/extensions/typescript-language-features/package.json index 7f2f202e926..f6e397cb268 100644 --- a/extensions/typescript-language-features/package.json +++ b/extensions/typescript-language-features/package.json @@ -730,6 +730,12 @@ "default": 3072, "description": "%configuration.tsserver.maxTsServerMemory%", "scope": "window" + }, + "typescript.tsserver.experimental.enableProjectDiagnostics": { + "type": "boolean", + "default": false, + "description": "%configuration.tsserver.experimental.enableProjectDiagnostics%", + "scope": "window" } } }, diff --git a/extensions/typescript-language-features/package.nls.json b/extensions/typescript-language-features/package.nls.json index adad367759b..e0dff4cac70 100644 --- a/extensions/typescript-language-features/package.nls.json +++ b/extensions/typescript-language-features/package.nls.json @@ -58,6 +58,7 @@ "configuration.suggest.paths": "Enable/disable suggestions for paths in import statements and require calls.", "configuration.tsserver.useSeparateSyntaxServer": "Enable/disable spawning a separate TypeScript server that can more quickly respond to syntax related operations, such as calculating folding or computing document symbols. Requires using TypeScript 3.4.0 or newer in the workspace.", "configuration.tsserver.maxTsServerMemory": "Set the maximum amount of memory (in MB) to allocate to the TypeScript server process", + "configuration.tsserver.experimental.enableProjectDiagnostics": "(Experimental) Enables project wide error reporting. Requires using TypeScript 3.8 or newer in the workspace.", "typescript.locale": "Sets the locale used to report JavaScript and TypeScript errors. Requires using TypeScript 2.6.0 or newer in the workspace. Default of `null` uses VS Code's locale.", "javascript.implicitProjectConfig.experimentalDecorators": "Enable/disable `experimentalDecorators` for JavaScript files that are not part of a project. Existing jsconfig.json or tsconfig.json files override this setting. Requires using TypeScript 2.3.1 or newer in the workspace.", "configuration.suggest.autoImports": "Enable/disable auto import suggestions. Requires using TypeScript 2.6.1 or newer in the workspace.", diff --git a/extensions/typescript-language-features/src/features/bufferSyncSupport.ts b/extensions/typescript-language-features/src/features/bufferSyncSupport.ts index 7cd17426fc8..7110e69c696 100644 --- a/extensions/typescript-language-features/src/features/bufferSyncSupport.ts +++ b/extensions/typescript-language-features/src/features/bufferSyncSupport.ts @@ -289,19 +289,25 @@ class GetErrRequest { public readonly files: ResourceMap, onDone: () => void ) { - const args: Proto.GeterrRequestArgs = { - delay: 0, - files: coalesce(Array.from(files.entries).map(entry => client.normalizedPath(entry.resource))) - }; + const allFiles = coalesce(Array.from(files.entries).map(entry => client.normalizedPath(entry.resource))); + if (!allFiles.length) { + this._done = true; + onDone(); + } else { + const request = client.configuration.enableProjectDiagnostics + // Note that geterrForProject is almost certainly not the api we want here as it ends up computing far + // too many diagnostics + ? client.executeAsync('geterrForProject', { delay: 0, file: allFiles[0] }, this._token.token) + : client.executeAsync('geterr', { delay: 0, files: allFiles }, this._token.token); - client.executeAsync('geterr', args, this._token.token) - .finally(() => { + request.finally(() => { if (this._done) { return; } this._done = true; onDone(); }); + } } public cancel(): any { @@ -454,7 +460,9 @@ export default class BufferSyncSupport extends Disposable { } public interuptGetErr(f: () => R): R { - if (!this.pendingGetErr) { + if (!this.pendingGetErr + || this.client.configuration.enableProjectDiagnostics // `geterr` happens on seperate server so no need to cancel it. + ) { return f(); } diff --git a/extensions/typescript-language-features/src/tsServer/server.ts b/extensions/typescript-language-features/src/tsServer/server.ts index 33bc66097d3..9eef0fc7b62 100644 --- a/extensions/typescript-language-features/src/tsServer/server.ts +++ b/extensions/typescript-language-features/src/tsServer/server.ts @@ -297,19 +297,120 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe } +class RequestRouter { + + private static readonly sharedCommands = new Set([ + 'change', + 'close', + 'open', + 'updateOpen', + 'configure', + 'configurePlugin', + ]); + + constructor( + private readonly servers: ReadonlyArray<{ readonly server: ITypeScriptServer, readonly preferredCommands?: ReadonlySet }>, + private readonly delegate: TsServerDelegate, + ) { } + + public execute(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean, lowPriority?: boolean }): Promise> | undefined { + if (RequestRouter.sharedCommands.has(command)) { + // Dispatch shared commands to all server but only return from first one one + + const requestStates: RequestState.State[] = this.servers.map(() => RequestState.Unresolved); + + // Also make sure we never cancel requests to just one server + let token: vscode.CancellationToken | undefined = undefined; + if (executeInfo.token) { + const source = new vscode.CancellationTokenSource(); + executeInfo.token.onCancellationRequested(() => { + if (requestStates.some(state => state === RequestState.Resolved)) { + // Don't cancel. + // One of the servers completed this request so we don't want to leave the other + // in a different state. + return; + } + source.cancel(); + }); + token = source.token; + } + + let firstRequest: Promise> | undefined; + + for (let serverIndex = 0; serverIndex < this.servers.length; ++serverIndex) { + const server = this.servers[serverIndex].server; + + const request = server.executeImpl(command, args, { ...executeInfo, token }); + if (serverIndex === 0) { + firstRequest = request; + } + if (request) { + request + .then(result => { + requestStates[serverIndex] = RequestState.Resolved; + const erroredRequest = requestStates.find(state => state.type === RequestState.Type.Errored) as RequestState.Errored | undefined; + if (erroredRequest) { + // We've gone out of sync + this.delegate.onFatalError(command, erroredRequest.err); + } + return result; + }, err => { + requestStates[serverIndex] = new RequestState.Errored(err); + if (requestStates.some(state => state === RequestState.Resolved)) { + // We've gone out of sync + this.delegate.onFatalError(command, err); + } + throw err; + }); + } + } + + return firstRequest; + } + + for (const { preferredCommands, server } of this.servers) { + if (!preferredCommands || preferredCommands.has(command)) { + return server.executeImpl(command, args, executeInfo); + } + } + + throw new Error(`Could not find server for command: '${command}'`); + } +} + + export class SyntaxRoutingTsServer extends Disposable implements ITypeScriptServer { + + private static readonly syntaxCommands = new Set([ + 'navtree', + 'getOutliningSpans', + 'jsxClosingTag', + 'selectionRange', + 'format', + 'formatonkey', + 'docCommentTemplate', + ]); + private readonly syntaxServer: ITypeScriptServer; private readonly semanticServer: ITypeScriptServer; + private readonly router: RequestRouter; public constructor( servers: { syntax: ITypeScriptServer, semantic: ITypeScriptServer }, - private readonly _delegate: TsServerDelegate, + delegate: TsServerDelegate, ) { super(); this.syntaxServer = servers.syntax; this.semanticServer = servers.semantic; + this.router = new RequestRouter( + [ + { server: this.syntaxServer, preferredCommands: SyntaxRoutingTsServer.syntaxCommands }, + { server: this.semanticServer, preferredCommands: undefined /* gets all other commands */ } + ], + delegate); + this._register(this.syntaxServer.onEvent(e => this._onEvent.fire(e))); this._register(this.semanticServer.onEvent(e => this._onEvent.fire(e))); @@ -338,95 +439,87 @@ export class SyntaxRoutingTsServer extends Disposable implements ITypeScriptServ this.semanticServer.kill(); } - private static readonly syntaxCommands = new Set([ - 'navtree', - 'getOutliningSpans', - 'jsxClosingTag', - 'selectionRange', - 'format', - 'formatonkey', - 'docCommentTemplate', - ]); - private static readonly sharedCommands = new Set([ - 'change', - 'close', - 'open', - 'updateOpen', - 'configure', - 'configurePlugin', + public executeImpl(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: false, lowPriority?: boolean }): undefined; + public executeImpl(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean, lowPriority?: boolean }): Promise>; + public executeImpl(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean, lowPriority?: boolean }): Promise> | undefined { + return this.router.execute(command, args, executeInfo); + } +} + + +export class GetErrRoutingTsServer extends Disposable implements ITypeScriptServer { + + private static readonly diagnosticEvents = new Set([ + 'configFileDiag', + 'syntaxDiag', + 'semanticDiag', + 'suggestionDiag' ]); + private readonly getErrServer: ITypeScriptServer; + private readonly mainServer: ITypeScriptServer; + private readonly router: RequestRouter; + + public constructor( + servers: { getErr: ITypeScriptServer, primary: ITypeScriptServer }, + delegate: TsServerDelegate, + ) { + super(); + + this.getErrServer = servers.getErr; + this.mainServer = servers.primary; + + this.router = new RequestRouter( + [ + { server: this.getErrServer, preferredCommands: new Set(['geterr', 'geterrForProject']) }, + { server: this.mainServer, preferredCommands: undefined /* gets all other commands */ } + ], + delegate); + + this._register(this.getErrServer.onEvent(e => { + if (GetErrRoutingTsServer.diagnosticEvents.has(e.event)) { + this._onEvent.fire(e); + } + // Ignore all other events + })); + this._register(this.mainServer.onEvent(e => { + if (!GetErrRoutingTsServer.diagnosticEvents.has(e.event)) { + this._onEvent.fire(e); + } + // Ignore all other events + })); + + this._register(this.getErrServer.onError(e => this._onError.fire(e))); + this._register(this.mainServer.onError(e => this._onError.fire(e))); + + this._register(this.mainServer.onExit(e => { + this._onExit.fire(e); + this.getErrServer.kill(); + })); + } + + private readonly _onEvent = this._register(new vscode.EventEmitter()); + public readonly onEvent = this._onEvent.event; + + private readonly _onExit = this._register(new vscode.EventEmitter()); + public readonly onExit = this._onExit.event; + + private readonly _onError = this._register(new vscode.EventEmitter()); + public readonly onError = this._onError.event; + + public get onReaderError() { return this.mainServer.onReaderError; } + + public get tsServerLogFile() { return this.mainServer.tsServerLogFile; } + + public kill(): void { + this.getErrServer.kill(); + this.mainServer.kill(); + } + public executeImpl(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: false, lowPriority?: boolean }): undefined; public executeImpl(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean, lowPriority?: boolean }): Promise>; public executeImpl(command: keyof TypeScriptRequests, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean, lowPriority?: boolean }): Promise> | undefined { - if (SyntaxRoutingTsServer.syntaxCommands.has(command)) { - return this.syntaxServer.executeImpl(command, args, executeInfo); - } else if (SyntaxRoutingTsServer.sharedCommands.has(command)) { - // Dispatch to both server but only return from syntax one - - let syntaxRequestState: RequestState.State = RequestState.Unresolved; - let semanticRequestState: RequestState.State = RequestState.Unresolved; - - // Also make sure we never cancel requests to just one server - let token: vscode.CancellationToken | undefined = undefined; - if (executeInfo.token) { - const source = new vscode.CancellationTokenSource(); - executeInfo.token.onCancellationRequested(() => { - if (syntaxRequestState !== RequestState.Unresolved && semanticRequestState === RequestState.Unresolved - || syntaxRequestState === RequestState.Unresolved && semanticRequestState !== RequestState.Unresolved - ) { - // Don't cancel. - // One of the servers completed this request so we don't want to leave the other - // in a different state - return; - } - source.cancel(); - }); - token = source.token; - } - - const semanticRequest = this.semanticServer.executeImpl(command, args, { ...executeInfo, token }); - if (semanticRequest) { - semanticRequest - .then(result => { - semanticRequestState = RequestState.Resolved; - if (syntaxRequestState.type === RequestState.Type.Errored) { - // We've gone out of sync - this._delegate.onFatalError(command, syntaxRequestState.err); - } - return result; - }, err => { - semanticRequestState = new RequestState.Errored(err); - if (syntaxRequestState === RequestState.Resolved) { - // We've gone out of sync - this._delegate.onFatalError(command, err); - } - throw err; - }); - } - const syntaxRequest = this.syntaxServer.executeImpl(command, args, { ...executeInfo, token }); - if (syntaxRequest) { - syntaxRequest - .then(result => { - syntaxRequestState = RequestState.Resolved; - if (semanticRequestState.type === RequestState.Type.Errored) { - // We've gone out of sync - this._delegate.onFatalError(command, semanticRequestState.err); - } - return result; - }, err => { - syntaxRequestState = new RequestState.Errored(err); - if (semanticRequestState === RequestState.Resolved) { - // We've gone out of sync - this._delegate.onFatalError(command, err); - } - throw err; - }); - } - return syntaxRequest; - } else { - return this.semanticServer.executeImpl(command, args, executeInfo); - } + return this.router.execute(command, args, executeInfo); } } diff --git a/extensions/typescript-language-features/src/tsServer/spawner.ts b/extensions/typescript-language-features/src/tsServer/spawner.ts index fea68951bfd..307c0161096 100644 --- a/extensions/typescript-language-features/src/tsServer/spawner.ts +++ b/extensions/typescript-language-features/src/tsServer/spawner.ts @@ -18,9 +18,14 @@ import { PluginManager } from '../utils/plugins'; import { TelemetryReporter } from '../utils/telemetry'; import Tracer from '../utils/tracer'; import { TypeScriptVersion, TypeScriptVersionProvider } from '../utils/versionProvider'; -import { ITypeScriptServer, PipeRequestCanceller, ProcessBasedTsServer, SyntaxRoutingTsServer, TsServerProcess, TsServerDelegate } from './server'; +import { ITypeScriptServer, PipeRequestCanceller, ProcessBasedTsServer, SyntaxRoutingTsServer, TsServerProcess, TsServerDelegate, GetErrRoutingTsServer } from './server'; -type ServerKind = 'main' | 'syntax' | 'semantic'; +const enum ServerKind { + Main = 'main', + Syntax = 'syntax', + Semantic = 'semantic', + Diagnostics = 'diagnostics' +} export class TypeScriptServerSpawner { public constructor( @@ -38,13 +43,24 @@ export class TypeScriptServerSpawner { pluginManager: PluginManager, delegate: TsServerDelegate, ): ITypeScriptServer { + let primaryServer: ITypeScriptServer; if (this.shouldUseSeparateSyntaxServer(version, configuration)) { - const syntaxServer = this.spawnTsServer('syntax', version, configuration, pluginManager); - const semanticServer = this.spawnTsServer('semantic', version, configuration, pluginManager); - return new SyntaxRoutingTsServer({ syntax: syntaxServer, semantic: semanticServer }, delegate); + primaryServer = new SyntaxRoutingTsServer({ + syntax: this.spawnTsServer(ServerKind.Syntax, version, configuration, pluginManager), + semantic: this.spawnTsServer(ServerKind.Semantic, version, configuration, pluginManager) + }, delegate); + } else { + primaryServer = this.spawnTsServer(ServerKind.Main, version, configuration, pluginManager); } - return this.spawnTsServer('main', version, configuration, pluginManager); + if (this.shouldUseSeparateDiagnosticsServer(version, configuration)) { + return new GetErrRoutingTsServer({ + getErr: this.spawnTsServer(ServerKind.Diagnostics, version, configuration, pluginManager), + primary: primaryServer, + }, delegate); + } + + return primaryServer; } private shouldUseSeparateSyntaxServer( @@ -54,6 +70,13 @@ export class TypeScriptServerSpawner { return configuration.useSeparateSyntaxServer && !!version.apiVersion && version.apiVersion.gte(API.v340); } + private shouldUseSeparateDiagnosticsServer( + version: TypeScriptVersion, + configuration: TypeScriptServiceConfiguration, + ): boolean { + return configuration.enableProjectDiagnostics && !!version.apiVersion && version.apiVersion.gte(API.v380); + } + private spawnTsServer( kind: ServerKind, version: TypeScriptVersion, @@ -107,7 +130,7 @@ export class TypeScriptServerSpawner { const args: string[] = []; let tsServerLogFile: string | undefined; - if (kind === 'syntax') { + if (kind === ServerKind.Syntax) { args.push('--syntaxOnly'); } @@ -117,11 +140,11 @@ export class TypeScriptServerSpawner { args.push('--useSingleInferredProject'); } - if (configuration.disableAutomaticTypeAcquisition || kind === 'syntax') { + if (configuration.disableAutomaticTypeAcquisition || kind === ServerKind.Syntax || kind === ServerKind.Diagnostics) { args.push('--disableAutomaticTypingAcquisition'); } - if (kind !== 'syntax') { + if (kind === ServerKind.Semantic || kind === ServerKind.Main) { args.push('--enableTelemetry'); } diff --git a/extensions/typescript-language-features/src/typescriptService.ts b/extensions/typescript-language-features/src/typescriptService.ts index 043284c1244..51e5cab5c5b 100644 --- a/extensions/typescript-language-features/src/typescriptService.ts +++ b/extensions/typescript-language-features/src/typescriptService.ts @@ -74,6 +74,7 @@ interface NoResponseTsServerRequests { interface AsyncTsServerRequests { 'geterr': [Proto.GeterrRequestArgs, Proto.Response]; + 'geterrForProject': [Proto.GeterrForProjectRequestArgs, Proto.Response]; } export type TypeScriptRequests = StandardTsServerRequests & NoResponseTsServerRequests & AsyncTsServerRequests; @@ -137,7 +138,11 @@ export interface ITypeScriptServiceClient { args: NoResponseTsServerRequests[K][0] ): void; - executeAsync(command: 'geterr', args: Proto.GeterrRequestArgs, token: vscode.CancellationToken): Promise>; + executeAsync( + command: K, + args: AsyncTsServerRequests[K][0], + token: vscode.CancellationToken + ): Promise>; /** * Cancel on going geterr requests and re-queue them after `f` has been evaluated. diff --git a/extensions/typescript-language-features/src/utils/configuration.ts b/extensions/typescript-language-features/src/utils/configuration.ts index 54c3a19b001..25315feab3c 100644 --- a/extensions/typescript-language-features/src/utils/configuration.ts +++ b/extensions/typescript-language-features/src/utils/configuration.ts @@ -55,6 +55,7 @@ export class TypeScriptServiceConfiguration { public readonly experimentalDecorators: boolean; public readonly disableAutomaticTypeAcquisition: boolean; public readonly useSeparateSyntaxServer: boolean; + public readonly enableProjectDiagnostics: boolean; public readonly maxTsServerMemory: number; public static loadFromWorkspace(): TypeScriptServiceConfiguration { @@ -74,6 +75,7 @@ export class TypeScriptServiceConfiguration { this.experimentalDecorators = TypeScriptServiceConfiguration.readExperimentalDecorators(configuration); this.disableAutomaticTypeAcquisition = TypeScriptServiceConfiguration.readDisableAutomaticTypeAcquisition(configuration); this.useSeparateSyntaxServer = TypeScriptServiceConfiguration.readUseSeparateSyntaxServer(configuration); + this.enableProjectDiagnostics = TypeScriptServiceConfiguration.readEnableProjectDiagnostics(configuration); this.maxTsServerMemory = TypeScriptServiceConfiguration.readMaxTsServerMemory(configuration); } @@ -88,6 +90,7 @@ export class TypeScriptServiceConfiguration { && this.disableAutomaticTypeAcquisition === other.disableAutomaticTypeAcquisition && arrays.equals(this.tsServerPluginPaths, other.tsServerPluginPaths) && this.useSeparateSyntaxServer === other.useSeparateSyntaxServer + && this.enableProjectDiagnostics === other.enableProjectDiagnostics && this.maxTsServerMemory === other.maxTsServerMemory; } @@ -150,6 +153,10 @@ export class TypeScriptServiceConfiguration { return configuration.get('typescript.tsserver.useSeparateSyntaxServer', true); } + private static readEnableProjectDiagnostics(configuration: vscode.WorkspaceConfiguration): boolean { + return configuration.get('typescript.tsserver.experimental.enableProjectDiagnostics', false); + } + private static readMaxTsServerMemory(configuration: vscode.WorkspaceConfiguration): number { const defaultMaxMemory = 3072; const minimumMaxMemory = 128;