From e62108cf9b5dbf678bf596def315ab766b82fa25 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Wed, 22 Feb 2017 17:33:11 -0800 Subject: [PATCH] Removing throttling until tests prove it is required --- src/compiler/program.ts | 14 - src/compiler/types.ts | 2 - .../unittests/tsserverProjectSystem.ts | 9 +- src/services/navigationBar.ts | 258 +++++++++--------- src/services/outliningElementsCollector.ts | 2 +- src/services/services.ts | 48 +--- src/services/shims.ts | 23 ++ src/services/types.ts | 1 - 8 files changed, 156 insertions(+), 201 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index b94efe18222..62f3c066442 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -72,20 +72,6 @@ namespace ts { return getNormalizedPathFromPathComponents(commonPathComponents); } - /* @internal */ - export function runWithCancellationToken(func: () => T, onCancel?: () => void): T { - try { - return func(); - } - catch (e) { - if (e instanceof OperationCanceledException && onCancel) { - // We were canceled while performing the operation. - onCancel(); - } - throw e; - } - } - interface OutputFingerprint { hash: string; byteOrderMark: boolean; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 17adbd2ea3f..7fc77b48708 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2262,8 +2262,6 @@ /** @throws OperationCanceledException if isCancellationRequested is true */ throwIfCancellationRequested(): void; - - throttleWaitMilliseconds?: number; } export interface Program extends ScriptReferenceHost { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index ee4273c3d2e..d9abb0a9078 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -555,11 +555,6 @@ namespace ts.projectSystem { constructor(private cancelAfterRequest = 0) { } - get throttleWaitMilliseconds() { - // For testing purposes disable the throttle - return 0; - } - setRequest(requestId: number) { this.currentId = requestId; } @@ -3495,8 +3490,6 @@ namespace ts.projectSystem { compilerOptions: {} }) }; - - let operationCanceledExceptionThrown = false; const cancellationToken = new TestServerCancellationToken(/*cancelAfterRequest*/ 3); const host = createServerHost([f1, config]); const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken); @@ -3536,7 +3529,7 @@ namespace ts.projectSystem { // The cancellation token will cancel the request the third time // isCancellationRequested() is called. cancellationToken.setRequestToCancel(session.getNextSeq()); - operationCanceledExceptionThrown = false; + let operationCanceledExceptionThrown = false; try { session.executeCommandSeq(request); diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 62ead677d8b..811762fb736 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -14,7 +14,7 @@ namespace ts.NavigationBar { indent: number; // # of parents } - export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: ThrottledCancellationToken): NavigationBarItem[] { + export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarItem[] { curCancellationToken = cancellationToken; curSourceFile = sourceFile; try { @@ -22,10 +22,11 @@ namespace ts.NavigationBar { } finally { curSourceFile = undefined; + curCancellationToken = undefined; } } - export function getNavigationTree(sourceFile: SourceFile, cancellationToken: ThrottledCancellationToken): NavigationTree { + export function getNavigationTree(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationTree { curCancellationToken = cancellationToken; curSourceFile = sourceFile; try { @@ -33,6 +34,7 @@ namespace ts.NavigationBar { } finally { curSourceFile = undefined; + curCancellationToken = undefined; } } @@ -112,7 +114,7 @@ namespace ts.NavigationBar { parent = parentsStack.pop(); } - function addNodeWithRecursiveChild(node: Node, child: Node, addChildrenRecursively: (node: Node) => void): void { + function addNodeWithRecursiveChild(node: Node, child: Node): void { startNode(node); addChildrenRecursively(child); endNode(); @@ -120,136 +122,132 @@ namespace ts.NavigationBar { /** Look for navigation bar items in node's subtree, adding them to the current `parent`. */ function addChildrenRecursively(node: Node): void { - function addChildrenRecursively(node: Node): void { - curCancellationToken.throwIfCancellationRequested(); + curCancellationToken.throwIfCancellationRequested(); - if (!node || isToken(node)) { - return; - } - - switch (node.kind) { - case SyntaxKind.Constructor: - // Get parameter properties, and treat them as being on the *same* level as the constructor, not under it. - const ctr = node; - addNodeWithRecursiveChild(ctr, ctr.body, addChildrenRecursively); - - // Parameter properties are children of the class, not the constructor. - for (const param of ctr.parameters) { - if (isParameterPropertyDeclaration(param)) { - addLeafNode(param); - } - } - break; - - case SyntaxKind.MethodDeclaration: - case SyntaxKind.GetAccessor: - case SyntaxKind.SetAccessor: - case SyntaxKind.MethodSignature: - if (!hasDynamicName((node))) { - addNodeWithRecursiveChild(node, (node).body, addChildrenRecursively); - } - break; - - case SyntaxKind.PropertyDeclaration: - case SyntaxKind.PropertySignature: - if (!hasDynamicName((node))) { - addLeafNode(node); - } - break; - - case SyntaxKind.ImportClause: - const importClause = node; - // Handle default import case e.g.: - // import d from "mod"; - if (importClause.name) { - addLeafNode(importClause); - } - - // Handle named bindings in imports e.g.: - // import * as NS from "mod"; - // import {a, b as B} from "mod"; - const {namedBindings} = importClause; - if (namedBindings) { - if (namedBindings.kind === SyntaxKind.NamespaceImport) { - addLeafNode(namedBindings); - } - else { - for (const element of (namedBindings).elements) { - addLeafNode(element); - } - } - } - break; - - case SyntaxKind.BindingElement: - case SyntaxKind.VariableDeclaration: - const decl = node; - const name = decl.name; - if (isBindingPattern(name)) { - addChildrenRecursively(name); - } - else if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { - // For `const x = function() {}`, just use the function node, not the const. - addChildrenRecursively(decl.initializer); - } - else { - addNodeWithRecursiveChild(decl, decl.initializer, addChildrenRecursively); - } - break; - - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.FunctionExpression: - addNodeWithRecursiveChild(node, (node).body, addChildrenRecursively); - break; - - case SyntaxKind.EnumDeclaration: - startNode(node); - for (const member of (node).members) { - if (!isComputedProperty(member)) { - addLeafNode(member); - } - } - endNode(); - break; - - case SyntaxKind.ClassDeclaration: - case SyntaxKind.ClassExpression: - case SyntaxKind.InterfaceDeclaration: - startNode(node); - for (const member of (node).members) { - addChildrenRecursively(member); - } - endNode(); - break; - - case SyntaxKind.ModuleDeclaration: - addNodeWithRecursiveChild(node, getInteriorModule(node).body, addChildrenRecursively); - break; - - case SyntaxKind.ExportSpecifier: - case SyntaxKind.ImportEqualsDeclaration: - case SyntaxKind.IndexSignature: - case SyntaxKind.CallSignature: - case SyntaxKind.ConstructSignature: - case SyntaxKind.TypeAliasDeclaration: - addLeafNode(node); - break; - - default: - forEach(node.jsDoc, jsDoc => { - forEach(jsDoc.tags, tag => { - if (tag.kind === SyntaxKind.JSDocTypedefTag) { - addLeafNode(tag); - } - }); - }); - - forEachChild(node, addChildrenRecursively); - } + if (!node || isToken(node)) { + return; } - addChildrenRecursively(node); + switch (node.kind) { + case SyntaxKind.Constructor: + // Get parameter properties, and treat them as being on the *same* level as the constructor, not under it. + const ctr = node; + addNodeWithRecursiveChild(ctr, ctr.body); + + // Parameter properties are children of the class, not the constructor. + for (const param of ctr.parameters) { + if (isParameterPropertyDeclaration(param)) { + addLeafNode(param); + } + } + break; + + case SyntaxKind.MethodDeclaration: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + case SyntaxKind.MethodSignature: + if (!hasDynamicName((node))) { + addNodeWithRecursiveChild(node, (node).body); + } + break; + + case SyntaxKind.PropertyDeclaration: + case SyntaxKind.PropertySignature: + if (!hasDynamicName((node))) { + addLeafNode(node); + } + break; + + case SyntaxKind.ImportClause: + const importClause = node; + // Handle default import case e.g.: + // import d from "mod"; + if (importClause.name) { + addLeafNode(importClause); + } + + // Handle named bindings in imports e.g.: + // import * as NS from "mod"; + // import {a, b as B} from "mod"; + const {namedBindings} = importClause; + if (namedBindings) { + if (namedBindings.kind === SyntaxKind.NamespaceImport) { + addLeafNode(namedBindings); + } + else { + for (const element of (namedBindings).elements) { + addLeafNode(element); + } + } + } + break; + + case SyntaxKind.BindingElement: + case SyntaxKind.VariableDeclaration: + const decl = node; + const name = decl.name; + if (isBindingPattern(name)) { + addChildrenRecursively(name); + } + else if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { + // For `const x = function() {}`, just use the function node, not the const. + addChildrenRecursively(decl.initializer); + } + else { + addNodeWithRecursiveChild(decl, decl.initializer); + } + break; + + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.FunctionExpression: + addNodeWithRecursiveChild(node, (node).body); + break; + + case SyntaxKind.EnumDeclaration: + startNode(node); + for (const member of (node).members) { + if (!isComputedProperty(member)) { + addLeafNode(member); + } + } + endNode(); + break; + + case SyntaxKind.ClassDeclaration: + case SyntaxKind.ClassExpression: + case SyntaxKind.InterfaceDeclaration: + startNode(node); + for (const member of (node).members) { + addChildrenRecursively(member); + } + endNode(); + break; + + case SyntaxKind.ModuleDeclaration: + addNodeWithRecursiveChild(node, getInteriorModule(node).body); + break; + + case SyntaxKind.ExportSpecifier: + case SyntaxKind.ImportEqualsDeclaration: + case SyntaxKind.IndexSignature: + case SyntaxKind.CallSignature: + case SyntaxKind.ConstructSignature: + case SyntaxKind.TypeAliasDeclaration: + addLeafNode(node); + break; + + default: + forEach(node.jsDoc, jsDoc => { + forEach(jsDoc.tags, tag => { + if (tag.kind === SyntaxKind.JSDocTypedefTag) { + addLeafNode(tag); + } + }); + }); + + forEachChild(node, addChildrenRecursively); + } } /** Merge declarations of the same kind. */ diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 1b7618c0327..96a7c9a3c4a 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -1,6 +1,6 @@ /* @internal */ namespace ts.OutliningElementsCollector { - export function collectElements(sourceFile: SourceFile, cancellationToken: ThrottledCancellationToken): OutliningSpan[] { + export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; const collapseText = "..."; diff --git a/src/services/services.ts b/src/services/services.ts index 6cf50940fb7..cef307eb8fd 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -958,12 +958,7 @@ namespace ts { } class CancellationTokenObject implements CancellationToken { - throttleWaitMilliseconds?: number; - constructor(private cancellationToken: HostCancellationToken) { - if (cancellationToken.throttleWaitMilliseconds !== undefined) { - this.throttleWaitMilliseconds = cancellationToken.throttleWaitMilliseconds; - } } public isCancellationRequested() { @@ -977,42 +972,6 @@ namespace ts { } } - /** 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; - // The minuimum duration to wait in milliseconds before querying host. - // The default of 10 milliseconds will be used unless throttleWaitMilliseconds - // is specified in the host cancellation token. - throttleWaitMilliseconds: number = 10; - - constructor(private hostCancellationToken: HostCancellationToken) { - if (hostCancellationToken.throttleWaitMilliseconds !== undefined) { - this.throttleWaitMilliseconds = hostCancellationToken.throttleWaitMilliseconds; - } - } - - public isCancellationRequested(): boolean { - const time = timestamp(); - const duration = Math.abs(time - this.lastCancellationCheckTime); - if (duration >= this.throttleWaitMilliseconds) { - // Check no more than the min wait in 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 { @@ -1025,7 +984,6 @@ namespace ts { const useCaseSensitivefileNames = host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames(); const cancellationToken = new CancellationTokenObject(host.getCancellationToken && host.getCancellationToken()); - const throttledCancellationToken = new ThrottledCancellationToken(cancellationToken); const currentDirectory = host.getCurrentDirectory(); // Check if the localized messages json is set, otherwise query the host for it @@ -1583,11 +1541,11 @@ namespace ts { } function getNavigationBarItems(fileName: string): NavigationBarItem[] { - return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName), throttledCancellationToken); + return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken); } function getNavigationTree(fileName: string): NavigationTree { - return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName), throttledCancellationToken); + return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken); } function isTsOrTsxFile(fileName: string): boolean { @@ -1626,7 +1584,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, throttledCancellationToken); + return OutliningElementsCollector.collectElements(sourceFile, cancellationToken); } function getBraceMatchingAtPosition(fileName: string, position: number) { diff --git a/src/services/shims.ts b/src/services/shims.ts index de2fd39b8a8..6fe9aed144e 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -469,6 +469,29 @@ 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; diff --git a/src/services/types.ts b/src/services/types.ts index f9c33e15dfe..a6ca3852597 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -121,7 +121,6 @@ namespace ts { export interface HostCancellationToken { isCancellationRequested(): boolean; - throttleWaitMilliseconds?: number; } //