diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 356a7a8962..b2fc432224 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 { @@ -547,6 +547,49 @@ namespace ts.projectSystem { readonly getEnvironmentVariable = notImplemented; } + /** + * Test server cancellation token used to mock host token cancellation requests. + * The cancelAfterRequest constructor param specifies how many isCancellationRequested() calls + * should be made before canceling the token. The id of the request to cancel should be set with + * setRequestToCancel(); + */ + export class TestServerCancellationToken implements server.ServerCancellationToken { + private currentId = -1; + private requestToCancel = -1; + private isCancellationRequestedCount = 0; + + constructor(private cancelAfterRequest = 0) { + } + + setRequest(requestId: number) { + this.currentId = requestId; + } + + setRequestToCancel(requestId: number) { + this.resetToken(); + this.requestToCancel = requestId; + } + + resetRequest(requestId: number) { + assert.equal(requestId, this.currentId, "unexpected request id in cancellation"); + this.currentId = undefined; + } + + isCancellationRequested() { + this.isCancellationRequestedCount++; + // If the request id is the request to cancel and isCancellationRequestedCount + // has been met then cancel the request. Ex: cancel the request if it is a + // nav bar request & isCancellationRequested() has already been called three times. + return this.requestToCancel === this.currentId && this.isCancellationRequestedCount >= this.cancelAfterRequest; + } + + resetToken() { + this.currentId = -1; + this.isCancellationRequestedCount = 0; + this.requestToCancel = -1; + } + } + export function makeSessionRequest(command: string, args: T) { const newRequest: protocol.Request = { seq: 0, @@ -3384,6 +3427,7 @@ namespace ts.projectSystem { }, resetRequest: noop }; + const session = createSession(host, /*typingsInstaller*/ undefined, /*projectServiceEventHandler*/ undefined, cancellationToken); expectedRequestId = session.getNextSeq(); @@ -3422,22 +3466,7 @@ namespace ts.projectSystem { }) }; - let requestToCancel = -1; - const cancellationToken: server.ServerCancellationToken = (function(){ - let currentId: number; - return { - setRequest(requestId) { - currentId = requestId; - }, - resetRequest(requestId) { - assert.equal(requestId, currentId, "unexpected request id in cancellation"); - currentId = undefined; - }, - isCancellationRequested() { - return requestToCancel === currentId; - } - }; - })(); + const cancellationToken = new TestServerCancellationToken(); const host = createServerHost([f1, config]); const session = createSession(host, /*typingsInstaller*/ undefined, () => {}, cancellationToken); { @@ -3472,13 +3501,13 @@ namespace ts.projectSystem { host.clearOutput(); // cancel previously issued Geterr - requestToCancel = getErrId; + cancellationToken.setRequestToCancel(getErrId); host.runQueuedTimeoutCallbacks(); assert.equal(host.getOutput().length, 1, "expect 1 message"); verifyRequestCompleted(getErrId, 0); - requestToCancel = -1; + cancellationToken.resetToken(); } { const getErrId = session.getNextSeq(); @@ -3495,12 +3524,12 @@ namespace ts.projectSystem { assert.equal(e1.event, "syntaxDiag"); host.clearOutput(); - requestToCancel = getErrId; + cancellationToken.setRequestToCancel(getErrId); host.runQueuedImmediateCallbacks(); assert.equal(host.getOutput().length, 1, "expect 1 message"); verifyRequestCompleted(getErrId, 0); - requestToCancel = -1; + cancellationToken.resetToken(); } { const getErrId = session.getNextSeq(); @@ -3523,7 +3552,7 @@ namespace ts.projectSystem { assert.equal(e2.event, "semanticDiag"); verifyRequestCompleted(getErrId, 1); - requestToCancel = -1; + cancellationToken.resetToken(); } { const getErr1 = session.getNextSeq(); @@ -3558,6 +3587,68 @@ namespace ts.projectSystem { return JSON.parse(server.extractMessage(host.getOutput()[n])); } }); + it("Lower priority tasks are cancellable", () => { + const f1 = { + path: "/a/app.ts", + content: `{ let x = 1; } var foo = "foo"; var bar = "bar"; var fooBar = "fooBar";` + }; + const config = { + path: "/a/tsconfig.json", + content: JSON.stringify({ + compilerOptions: {} + }) + }; + const cancellationToken = new TestServerCancellationToken(/*cancelAfterRequest*/ 3); + const host = createServerHost([f1, config]); + const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken, /*throttleWaitMilliseconds*/ 0); + { + session.executeCommandSeq({ + command: "open", + arguments: { file: f1.path } + }); + + // send navbar request (normal priority) + session.executeCommandSeq({ + command: "navbar", + arguments: { file: f1.path } + }); + + // ensure the nav bar request can be canceled + verifyExecuteCommandSeqIsCancellable({ + command: "navbar", + arguments: { file: f1.path } + }); + + // send outlining spans request (normal priority) + session.executeCommandSeq({ + command: "outliningSpans", + arguments: { file: f1.path } + }); + + // ensure the outlining spans request can be canceled + verifyExecuteCommandSeqIsCancellable({ + command: "outliningSpans", + arguments: { file: f1.path } + }); + } + + function verifyExecuteCommandSeqIsCancellable(request: Partial) { + // Set the next request to be cancellable + // The cancellation token will cancel the request the third time + // isCancellationRequested() is called. + cancellationToken.setRequestToCancel(session.getNextSeq()); + let operationCanceledExceptionThrown = false; + + try { + session.executeCommandSeq(request); + } + catch (e) { + assert(e instanceof OperationCanceledException); + operationCanceledExceptionThrown = true; + } + assert(operationCanceledExceptionThrown, "Operation Canceled Exception not thrown for request: " + JSON.stringify(request)); + } + }); }); describe("occurence highlight on string", () => { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 8af6fe5d34..30c7a87dc7 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -271,7 +271,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 eee80d82e7..ec655c545e 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 11c99dfe46..de3149ccf1 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -337,7 +337,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)) @@ -352,7 +353,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 25059d1f57..7a2508fcfd 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. @@ -14,22 +44,36 @@ namespace ts.NavigationBar { indent: number; // # of parents } - export function getNavigationBarItems(sourceFile: SourceFile): NavigationBarItem[] { + export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarItem[] { + curCancellationToken = cancellationToken; curSourceFile = sourceFile; - const result = map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem); - curSourceFile = undefined; - return result; + try { + return map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem); + } + finally { + reset(); + } } - export function getNavigationTree(sourceFile: SourceFile): NavigationTree { + export function getNavigationTree(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationTree { + curCancellationToken = cancellationToken; curSourceFile = sourceFile; - const result = convertToTree(rootNavigationBarNode(sourceFile)); - curSourceFile = undefined; - return result; + try { + return convertToTree(rootNavigationBarNode(sourceFile)); + } + finally { + reset(); + } + } + + function reset() { + curSourceFile = undefined; + curCancellationToken = undefined; + parentsStack = []; + parent = undefined; + emptyChildItemArray = []; } - // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`. - let curSourceFile: SourceFile; function nodeText(node: Node): string { return node.getText(curSourceFile); } @@ -47,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 }; @@ -111,6 +147,8 @@ namespace ts.NavigationBar { /** Look for navigation bar items in node's subtree, adding them to the current `parent`. */ function addChildrenRecursively(node: Node): void { + curCancellationToken.throwIfCancellationRequested(); + if (!node || isToken(node)) { return; } @@ -487,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), @@ -610,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/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 43054a10f5..510f78c3f3 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -1,6 +1,6 @@ /* @internal */ namespace ts.OutliningElementsCollector { - export function collectElements(sourceFile: SourceFile): OutliningSpan[] { + export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; const collapseText = "..."; @@ -38,6 +38,7 @@ namespace ts.OutliningElementsCollector { let singleLineCommentCount = 0; for (const currentComment of comments) { + cancellationToken.throwIfCancellationRequested(); // For single line comments, combine consecutive ones (2 or more) into // a single span from the start of the first till the end of the last @@ -84,6 +85,7 @@ namespace ts.OutliningElementsCollector { let depth = 0; const maxDepth = 20; function walk(n: Node): void { + cancellationToken.throwIfCancellationRequested(); if (depth > maxDepth) { return; } diff --git a/src/services/services.ts b/src/services/services.ts index d8a4cbeae6..e2fbf74677 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -981,6 +981,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 { @@ -1552,11 +1582,11 @@ namespace ts { } function getNavigationBarItems(fileName: string): NavigationBarItem[] { - return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName)); + return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken); } function getNavigationTree(fileName: string): NavigationTree { - return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName)); + return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken); } function isTsOrTsxFile(fileName: string): boolean { @@ -1595,7 +1625,7 @@ namespace ts { function getOutliningSpans(fileName: string): OutliningSpan[] { // doesn't use compiler - no need to synchronize with host const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - return OutliningElementsCollector.collectElements(sourceFile); + return OutliningElementsCollector.collectElements(sourceFile, cancellationToken); } function getBraceMatchingAtPosition(fileName: string, position: number) { diff --git a/src/services/shims.ts b/src/services/shims.ts index 6fe9aed144..de2fd39b8a 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;