From 21ef9078adcdbee99c5ebf2041850ea7562cb55c Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Thu, 23 Feb 2017 15:52:38 -0800 Subject: [PATCH] Wrapping LSHost's cancellationtoken with a throttle --- .../unittests/tsserverProjectSystem.ts | 7 +- src/server/editorServices.ts | 3 +- src/server/lsHost.ts | 1 + src/server/session.ts | 5 +- src/services/navigationBar.ts | 73 ++++++++++--------- src/services/services.ts | 30 ++++++++ src/services/shims.ts | 23 ------ 7 files changed, 80 insertions(+), 62 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index d9abb0a9078..353d0e2e187 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -176,11 +176,11 @@ namespace ts.projectSystem { } }; - export function createSession(host: server.ServerHost, typingsInstaller?: server.ITypingsInstaller, projectServiceEventHandler?: server.ProjectServiceEventHandler, cancellationToken?: server.ServerCancellationToken) { + export function createSession(host: server.ServerHost, typingsInstaller?: server.ITypingsInstaller, projectServiceEventHandler?: server.ProjectServiceEventHandler, cancellationToken?: server.ServerCancellationToken, throttleWaitMilliseconds?: number) { if (typingsInstaller === undefined) { typingsInstaller = new TestTypingsInstaller("/a/data/", /*throttleLimit*/5, host); } - return new TestSession(host, cancellationToken || server.nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ projectServiceEventHandler !== undefined, projectServiceEventHandler); + return new TestSession(host, cancellationToken || server.nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ projectServiceEventHandler !== undefined, projectServiceEventHandler, throttleWaitMilliseconds); } export interface CreateProjectServiceParameters { @@ -3320,6 +3320,7 @@ namespace ts.projectSystem { }, resetRequest: noop } + const session = createSession(host, /*typingsInstaller*/ undefined, /*projectServiceEventHandler*/ undefined, cancellationToken); expectedRequestId = session.getNextSeq(); @@ -3492,7 +3493,7 @@ namespace ts.projectSystem { }; const cancellationToken = new TestServerCancellationToken(/*cancelAfterRequest*/ 3); const host = createServerHost([f1, config]); - const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken); + const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken, /*throttleWaitMilliseconds*/ 0); { session.executeCommandSeq({ command: "open", diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 36aa3939c83..4067d85fb3f 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -270,7 +270,8 @@ namespace ts.server { public readonly cancellationToken: HostCancellationToken, public readonly useSingleInferredProject: boolean, readonly typingsInstaller: ITypingsInstaller = nullTypingsInstaller, - private readonly eventHandler?: ProjectServiceEventHandler) { + private readonly eventHandler?: ProjectServiceEventHandler, + public readonly throttleWaitMilliseconds?: number) { Debug.assert(!!host.createHash, "'ServerHost.createHash' is required for ProjectService"); diff --git a/src/server/lsHost.ts b/src/server/lsHost.ts index eee80d82e7f..ec655c545ea 100644 --- a/src/server/lsHost.ts +++ b/src/server/lsHost.ts @@ -16,6 +16,7 @@ namespace ts.server { readonly realpath?: (path: string) => string; constructor(private readonly host: ServerHost, private readonly project: Project, private readonly cancellationToken: HostCancellationToken) { + this.cancellationToken = new ThrottledCancellationToken(cancellationToken, project.projectService.throttleWaitMilliseconds); this.getCanonicalFileName = ts.createGetCanonicalFileName(this.host.useCaseSensitiveFileNames); if (host.trace) { diff --git a/src/server/session.ts b/src/server/session.ts index 262ba8da1da..e7b4fc4bd47 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -338,7 +338,8 @@ namespace ts.server { private hrtime: (start?: number[]) => number[], protected logger: Logger, protected readonly canUseEvents: boolean, - eventHandler?: ProjectServiceEventHandler) { + eventHandler?: ProjectServiceEventHandler, + private readonly throttleWaitMilliseconds?: number) { this.eventHander = canUseEvents ? eventHandler || (event => this.defaultEventHandler(event)) @@ -353,7 +354,7 @@ namespace ts.server { isCancellationRequested: () => cancellationToken.isCancellationRequested() } this.errorCheck = new MultistepOperation(multistepOperationHost); - this.projectService = new ProjectService(host, logger, cancellationToken, useSingleInferredProject, typingsInstaller, this.eventHander); + this.projectService = new ProjectService(host, logger, cancellationToken, useSingleInferredProject, typingsInstaller, this.eventHander, this.throttleWaitMilliseconds); this.gcTimer = new GcTimer(host, /*delay*/ 7000, logger); } diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 811762fb736..7a2508fcfd6 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -2,6 +2,36 @@ /* @internal */ namespace ts.NavigationBar { + /** + * Matches all whitespace characters in a string. Eg: + * + * "app. + * + * onactivated" + * + * matches because of the newline, whereas + * + * "app.onactivated" + * + * does not match. + */ + const whiteSpaceRegex = /\s+/g; + + // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`. + let curCancellationToken: CancellationToken; + let curSourceFile: SourceFile; + + /** + * For performance, we keep navigation bar parents on a stack rather than passing them through each recursion. + * `parent` is the current parent and is *not* stored in parentsStack. + * `startNode` sets a new parent and `endNode` returns to the previous parent. + */ + let parentsStack: NavigationBarNode[] = []; + let parent: NavigationBarNode; + + // NavigationBarItem requires an array, but will not mutate it, so just give it this for performance. + let emptyChildItemArray: NavigationBarItem[] = []; + /** * Represents a navigation bar item and its children. * The returned NavigationBarItem is more complicated and doesn't include 'parent', so we use these to do work before converting. @@ -21,8 +51,7 @@ namespace ts.NavigationBar { return map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem); } finally { - curSourceFile = undefined; - curCancellationToken = undefined; + reset(); } } @@ -33,14 +62,18 @@ namespace ts.NavigationBar { return convertToTree(rootNavigationBarNode(sourceFile)); } finally { - curSourceFile = undefined; - curCancellationToken = undefined; + reset(); } } - // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`. - let curCancellationToken: CancellationToken; - let curSourceFile: SourceFile; + function reset() { + curSourceFile = undefined; + curCancellationToken = undefined; + parentsStack = []; + parent = undefined; + emptyChildItemArray = []; + } + function nodeText(node: Node): string { return node.getText(curSourceFile); } @@ -58,14 +91,6 @@ namespace ts.NavigationBar { } } - /* - For performance, we keep navigation bar parents on a stack rather than passing them through each recursion. - `parent` is the current parent and is *not* stored in parentsStack. - `startNode` sets a new parent and `endNode` returns to the previous parent. - */ - const parentsStack: NavigationBarNode[] = []; - let parent: NavigationBarNode; - function rootNavigationBarNode(sourceFile: SourceFile): NavigationBarNode { Debug.assert(!parentsStack.length); const root: NavigationBarNode = { node: sourceFile, additionalNodes: undefined, parent: undefined, children: undefined, indent: 0 }; @@ -500,9 +525,6 @@ namespace ts.NavigationBar { } } - // NavigationBarItem requires an array, but will not mutate it, so just give it this for performance. - const emptyChildItemArray: NavigationBarItem[] = []; - function convertToTree(n: NavigationBarNode): NavigationTree { return { text: getItemName(n.node), @@ -623,19 +645,4 @@ namespace ts.NavigationBar { function isFunctionOrClassExpression(node: Node): boolean { return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.ClassExpression; } - - /** - * Matches all whitespace characters in a string. Eg: - * - * "app. - * - * onactivated" - * - * matches because of the newline, whereas - * - * "app.onactivated" - * - * does not match. - */ - const whiteSpaceRegex = /\s+/g; } diff --git a/src/services/services.ts b/src/services/services.ts index cef307eb8fd..d808ed28b72 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -972,6 +972,36 @@ namespace ts { } } + /* @internal */ + /** A cancellation that throttles calls to the host */ + export class ThrottledCancellationToken implements CancellationToken { + // Store when we last tried to cancel. Checking cancellation can be expensive (as we have + // to marshall over to the host layer). So we only bother actually checking once enough + // time has passed. + private lastCancellationCheckTime = 0; + + constructor(private hostCancellationToken: HostCancellationToken, private readonly throttleWaitMilliseconds = 20) { + } + + public isCancellationRequested(): boolean { + const time = timestamp(); + const duration = Math.abs(time - this.lastCancellationCheckTime); + if (duration >= this.throttleWaitMilliseconds) { + // Check no more than once every throttle wait milliseconds + this.lastCancellationCheckTime = time; + return this.hostCancellationToken.isCancellationRequested(); + } + + return false; + } + + public throwIfCancellationRequested(): void { + if (this.isCancellationRequested()) { + throw new OperationCanceledException(); + } + } + } + export function createLanguageService(host: LanguageServiceHost, documentRegistry: DocumentRegistry = createDocumentRegistry(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames(), host.getCurrentDirectory())): LanguageService { diff --git a/src/services/shims.ts b/src/services/shims.ts index 6fe9aed144e..de2fd39b8a8 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -469,29 +469,6 @@ namespace ts { } } - /** A cancellation that throttles calls to the host */ - class ThrottledCancellationToken implements HostCancellationToken { - // Store when we last tried to cancel. Checking cancellation can be expensive (as we have - // to marshall over to the host layer). So we only bother actually checking once enough - // time has passed. - private lastCancellationCheckTime = 0; - - constructor(private hostCancellationToken: HostCancellationToken) { - } - - public isCancellationRequested(): boolean { - const time = timestamp(); - const duration = Math.abs(time - this.lastCancellationCheckTime); - if (duration > 10) { - // Check no more than once every 10 ms. - this.lastCancellationCheckTime = time; - return this.hostCancellationToken.isCancellationRequested(); - } - - return false; - } - } - export class CoreServicesShimHostAdapter implements ParseConfigHost, ModuleResolutionHost { public directoryExists: (directoryName: string) => boolean;