Addressing CR comments

- Adding a throttle
- Refactor
- Navbar reset onCancel
This commit is contained in:
Jason Ramsay
2017-02-22 14:23:06 -08:00
parent 3f198e6751
commit a37053f780
8 changed files with 137 additions and 102 deletions
+28 -24
View File
@@ -72,6 +72,20 @@ namespace ts {
return getNormalizedPathFromPathComponents(commonPathComponents);
}
/* @internal */
export function runWithCancellationToken<T>(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;
@@ -873,29 +887,19 @@ namespace ts {
return sourceFile.parseDiagnostics;
}
function runWithCancellationToken<T>(func: () => T): T {
try {
return func();
}
catch (e) {
if (e instanceof OperationCanceledException) {
// We were canceled while performing the operation. Because our type checker
// might be a bad state, we need to throw it away.
//
// Note: we are overly aggressive here. We do not actually *have* to throw away
// the "noDiagnosticsTypeChecker". However, for simplicity, i'd like to keep
// the lifetimes of these two TypeCheckers the same. Also, we generally only
// cancel when the user has made a change anyways. And, in that case, we (the
// program instance) will get thrown away anyways. So trying to keep one of
// these type checkers alive doesn't serve much purpose.
noDiagnosticsTypeChecker = undefined;
diagnosticsProducingTypeChecker = undefined;
}
throw e;
}
function onCancel() {
// Because our type checker might be a bad state, we need to throw it away.
// Note: we are overly aggressive here. We do not actually *have* to throw away
// the "noDiagnosticsTypeChecker". However, for simplicity, i'd like to keep
// the lifetimes of these two TypeCheckers the same. Also, we generally only
// cancel when the user has made a change anyways. And, in that case, we (the
// program instance) will get thrown away anyways. So trying to keep one of
// these type checkers alive doesn't serve much purpose.
noDiagnosticsTypeChecker = undefined;
diagnosticsProducingTypeChecker = undefined;
}
function getSemanticDiagnosticsForFile(sourceFile: SourceFile, cancellationToken: CancellationToken): Diagnostic[] {
return runWithCancellationToken(() => {
const typeChecker = getDiagnosticsProducingTypeChecker();
@@ -910,7 +914,7 @@ namespace ts {
const programDiagnosticsInFile = programDiagnostics.getDiagnostics(sourceFile.fileName);
return bindDiagnostics.concat(checkDiagnostics, fileProcessingDiagnosticsInFile, programDiagnosticsInFile);
});
}, onCancel);
}
function getJavaScriptSyntacticDiagnosticsForFile(sourceFile: SourceFile): Diagnostic[] {
@@ -1089,7 +1093,7 @@ namespace ts {
function createDiagnosticForNode(node: Node, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number): Diagnostic {
return createDiagnosticForNodeInSourceFile(sourceFile, node, message, arg0, arg1, arg2);
}
});
}, onCancel);
}
function getDeclarationDiagnosticsWorker(sourceFile: SourceFile, cancellationToken: CancellationToken): Diagnostic[] {
@@ -1097,7 +1101,7 @@ namespace ts {
const resolver = getDiagnosticsProducingTypeChecker().getEmitResolver(sourceFile, cancellationToken);
// Don't actually write any files since we're just getting diagnostics.
return ts.getDeclarationDiagnostics(getEmitHost(noop), resolver, sourceFile);
});
}, onCancel);
}
function getDeclarationDiagnosticsForFile(sourceFile: SourceFile, cancellationToken: CancellationToken): Diagnostic[] {
+2
View File
@@ -2262,6 +2262,8 @@
/** @throws OperationCanceledException if isCancellationRequested is true */
throwIfCancellationRequested(): void;
throttleWaitMilliseconds?: number;
}
export interface Program extends ScriptReferenceHost {
+50 -44
View File
@@ -547,6 +547,45 @@ namespace ts.projectSystem {
readonly getEnvironmentVariable = notImplemented;
}
export class TestServerCancellationToken implements server.ServerCancellationToken {
private currentId = -1;
private requestToCancel = -1;
private isCancellationRequestedCount = 0;
constructor(private cancelAfterRequest = 0) {
}
get throttleWaitMilliseconds() {
// For testing purposes disable the throttle
return 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++;
return this.requestToCancel === this.currentId && this.isCancellationRequestedCount >= this.cancelAfterRequest;
}
resetToken() {
this.currentId = -1;
this.isCancellationRequestedCount = 0;
this.requestToCancel = -1;
}
}
export function makeSessionRequest<T>(command: string, args: T) {
const newRequest: protocol.Request = {
seq: 0,
@@ -3324,22 +3363,7 @@ namespace ts.projectSystem {
})
};
let requestToCancel = -1;
const cancellationToken: server.ServerCancellationToken = (function(){
let currentId: number;
return <server.ServerCancellationToken>{
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);
{
@@ -3374,13 +3398,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();
@@ -3397,12 +3421,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();
@@ -3425,7 +3449,7 @@ namespace ts.projectSystem {
assert.equal(e2.event, "semanticDiag");
verifyRequestCompleted(getErrId, 1);
requestToCancel = -1;
cancellationToken.resetToken();
}
{
const getErr1 = session.getNextSeq();
@@ -3472,26 +3496,8 @@ namespace ts.projectSystem {
})
};
let requestToCancel = -1;
let isCancellationRequestedCount = 0;
let cancelAfterRequest = 3;
let operationCanceledExceptionThrown = false;
const cancellationToken: server.ServerCancellationToken = (function () {
let currentId: number;
return <server.ServerCancellationToken>{
setRequest(requestId) {
currentId = requestId;
},
resetRequest(requestId) {
assert.equal(requestId, currentId, "unexpected request id in cancellation")
currentId = undefined;
},
isCancellationRequested() {
isCancellationRequestedCount++;
return requestToCancel === currentId && isCancellationRequestedCount >= cancelAfterRequest;
}
}
})();
const cancellationToken = new TestServerCancellationToken(/*cancelAfterRequest*/ 3);
const host = createServerHost([f1, config]);
const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken);
{
@@ -3529,17 +3535,17 @@ namespace ts.projectSystem {
// Set the next request to be cancellable
// The cancellation token will cancel the request the third time
// isCancellationRequested() is called.
requestToCancel = session.getNextSeq();
isCancellationRequestedCount = 0;
cancellationToken.setRequestToCancel(session.getNextSeq());
operationCanceledExceptionThrown = false;
try {
session.executeCommandSeq(request);
} catch (e) {
}
catch (e) {
assert(e instanceof OperationCanceledException);
operationCanceledExceptionThrown = true;
}
assert(operationCanceledExceptionThrown);
assert(operationCanceledExceptionThrown, "Operation Canceled Exception not thrown for request: " + JSON.stringify(request));
}
});
});
+10 -7
View File
@@ -14,21 +14,24 @@ namespace ts.NavigationBar {
indent: number; // # of parents
}
export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarItem[] {
export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: ThrottledCancellationToken): NavigationBarItem[] {
curCancellationToken = cancellationToken;
curSourceFile = sourceFile;
const result = map(topLevelItems(rootNavigationBarNode(sourceFile, cancellationToken)), convertToTopLevelItem);
const result = runWithCancellationToken(() => map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem), () => curSourceFile = undefined);
curSourceFile = undefined;
return result;
}
export function getNavigationTree(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationTree {
export function getNavigationTree(sourceFile: SourceFile, cancellationToken: ThrottledCancellationToken): NavigationTree {
curCancellationToken = cancellationToken;
curSourceFile = sourceFile;
const result = convertToTree(rootNavigationBarNode(sourceFile, cancellationToken));
const result = runWithCancellationToken(() => convertToTree(rootNavigationBarNode(sourceFile)), () => curSourceFile = undefined);
curSourceFile = undefined;
return result;
}
// Keep sourceFile handy so we don't have to search for it every time we need to call `getText`.
let curCancellationToken: ThrottledCancellationToken;
let curSourceFile: SourceFile;
function nodeText(node: Node): string {
return node.getText(curSourceFile);
@@ -55,12 +58,12 @@ namespace ts.NavigationBar {
const parentsStack: NavigationBarNode[] = [];
let parent: NavigationBarNode;
function rootNavigationBarNode(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarNode {
function rootNavigationBarNode(sourceFile: SourceFile): NavigationBarNode {
Debug.assert(!parentsStack.length);
const root: NavigationBarNode = { node: sourceFile, additionalNodes: undefined, parent: undefined, children: undefined, indent: 0 };
parent = root;
for (const statement of sourceFile.statements) {
addChildrenRecursively(statement, cancellationToken);
addChildrenRecursively(statement, curCancellationToken);
}
endNode();
Debug.assert(!parent && !parentsStack.length);
@@ -110,7 +113,7 @@ namespace ts.NavigationBar {
}
/** Look for navigation bar items in node's subtree, adding them to the current `parent`. */
function addChildrenRecursively(node: Node, cancellationToken: CancellationToken): void {
function addChildrenRecursively(node: Node, cancellationToken: ThrottledCancellationToken): void {
function addChildrenRecursively(node: Node): void {
cancellationToken.throwIfCancellationRequested();
+1 -1
View File
@@ -1,6 +1,6 @@
/* @internal */
namespace ts.OutliningElementsCollector {
export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] {
export function collectElements(sourceFile: SourceFile, cancellationToken: ThrottledCancellationToken): OutliningSpan[] {
const elements: OutliningSpan[] = [];
const collapseText = "...";
+45 -3
View File
@@ -958,7 +958,12 @@ namespace ts {
}
class CancellationTokenObject implements CancellationToken {
throttleWaitMilliseconds?: number;
constructor(private cancellationToken: HostCancellationToken) {
if (cancellationToken.throttleWaitMilliseconds !== undefined) {
this.throttleWaitMilliseconds = cancellationToken.throttleWaitMilliseconds;
}
}
public isCancellationRequested() {
@@ -972,6 +977,42 @@ 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 {
@@ -984,6 +1025,7 @@ 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
@@ -1541,11 +1583,11 @@ namespace ts {
}
function getNavigationBarItems(fileName: string): NavigationBarItem[] {
return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken);
return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName), throttledCancellationToken);
}
function getNavigationTree(fileName: string): NavigationTree {
return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken);
return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName), throttledCancellationToken);
}
function isTsOrTsxFile(fileName: string): boolean {
@@ -1584,7 +1626,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, cancellationToken);
return OutliningElementsCollector.collectElements(sourceFile, throttledCancellationToken);
}
function getBraceMatchingAtPosition(fileName: string, position: number) {
-23
View File
@@ -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;
+1
View File
@@ -121,6 +121,7 @@ namespace ts {
export interface HostCancellationToken {
isCancellationRequested(): boolean;
throttleWaitMilliseconds?: number;
}
//