From 9c6765d5cf697b5cb2fcda3d21bb2c2985ecdaf9 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 5 Sep 2017 15:47:54 -0700 Subject: [PATCH 1/4] Document ThrottledOperations.schedule --- src/server/utilities.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/server/utilities.ts b/src/server/utilities.ts index 700a1e12feeb8..cde6329bf0764 100644 --- a/src/server/utilities.ts +++ b/src/server/utilities.ts @@ -179,6 +179,12 @@ namespace ts.server { constructor(private readonly host: ServerHost) { } + /** + * Wait `number` milliseconds and then invoke `cb`. If, while waiting, schedule + * is called again with the same `operationId`, cancel this operation in favor + * of the new one. (Note that the amount of time the canceled operation had been + * waiting does not affect the amount of time that the new operation waits.) + */ public schedule(operationId: string, delay: number, cb: () => void) { const pendingTimeout = this.pendingTimeouts.get(operationId); if (pendingTimeout) { From 482e802e83598da9bb3c02adb7af71cffa2331aa Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 5 Sep 2017 16:00:19 -0700 Subject: [PATCH 2/4] Limit the number of unanswered typings installer requests If we send them all at once, we (apparently) hit a buffer limit in the node IPC channel and both TS Server and the typings installer become unresponsive. --- src/server/server.ts | 62 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/src/server/server.ts b/src/server/server.ts index f70e2d0faf766..7f6daa92d5eda 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -236,25 +236,35 @@ namespace ts.server { return `${d.getHours()}:${d.getMinutes()}:${d.getSeconds()}.${d.getMilliseconds()}`; } + interface QueuedOperation { + operationId: string; + operation: () => void; + } + class NodeTypingsInstaller implements ITypingsInstaller { private installer: NodeChildProcess; private installerPidReported = false; private socket: NodeSocket; private projectService: ProjectService; - private throttledOperations: ThrottledOperations; private eventSender: EventSender; + private activeRequestCount = 0; + private requestQueue: QueuedOperation[] = []; + private requestMap = createMap(); // Maps operation ID to newest requestQueue entry with that ID + + private static readonly maxActiveRequestCount = 10; + private static readonly requestDelayMillis = 100; + constructor( private readonly telemetryEnabled: boolean, private readonly logger: server.Logger, - host: ServerHost, + private readonly host: ServerHost, eventPort: number, readonly globalTypingsCacheLocation: string, readonly typingSafeListLocation: string, readonly typesMapLocation: string, private readonly npmLocation: string | undefined, private newLine: string) { - this.throttledOperations = new ThrottledOperations(host); if (eventPort) { const s = net.connect({ port: eventPort }, () => { this.socket = s; @@ -338,12 +348,26 @@ namespace ts.server { this.logger.info(`Scheduling throttled operation: ${JSON.stringify(request)}`); } } - this.throttledOperations.schedule(project.getProjectName(), /*ms*/ 250, () => { + + const operationId = project.getProjectName(); + const operation = () => { if (this.logger.hasLevel(LogLevel.verbose)) { this.logger.info(`Sending request: ${JSON.stringify(request)}`); } this.installer.send(request); - }); + }; + const queuedRequest: QueuedOperation = { operationId, operation }; + + if (this.activeRequestCount < NodeTypingsInstaller.maxActiveRequestCount) { + this.scheduleRequest(queuedRequest); + } + else { + if (this.logger.hasLevel(LogLevel.verbose)) { + this.logger.info(`Deferring request for: ${operationId}`); + } + this.requestQueue.push(queuedRequest); + this.requestMap.set(operationId, queuedRequest); + } } private handleMessage(response: SetTypings | InvalidateCachedTypings | BeginInstallTypes | EndInstallTypes | InitializationFailedResponse) { @@ -404,11 +428,39 @@ namespace ts.server { return; } + if (this.activeRequestCount > 0) { + this.activeRequestCount--; + } + else { + Debug.fail("Received too many responses"); + } + + while (this.requestQueue.length > 0) { + const queuedRequest = this.requestQueue.shift(); + if (this.requestMap.get(queuedRequest.operationId) == queuedRequest) { + this.requestMap.delete(queuedRequest.operationId); + this.scheduleRequest(queuedRequest); + break; + } + + if (this.logger.hasLevel(LogLevel.verbose)) { + this.logger.info(`Skipping defunct request for: ${queuedRequest.operationId}`); + } + } + this.projectService.updateTypingsForProject(response); if (response.kind === ActionSet && this.socket) { this.sendEvent(0, "setTypings", response); } } + + private scheduleRequest(request: QueuedOperation) { + if(this.logger.hasLevel(LogLevel.verbose)) { + this.logger.info(`Scheduling request for: ${request.operationId}`); + } + this.activeRequestCount++; + this.host.setTimeout(request.operation, NodeTypingsInstaller.requestDelayMillis); + } } class IOSession extends Session { From 0b1bad8421c2a252e89731d056649fe7673414e3 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 6 Sep 2017 15:44:00 -0700 Subject: [PATCH 3/4] Fix lint issues --- src/server/server.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/server.ts b/src/server/server.ts index 7f6daa92d5eda..96368fb9e5447 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -437,7 +437,7 @@ namespace ts.server { while (this.requestQueue.length > 0) { const queuedRequest = this.requestQueue.shift(); - if (this.requestMap.get(queuedRequest.operationId) == queuedRequest) { + if (this.requestMap.get(queuedRequest.operationId) === queuedRequest) { this.requestMap.delete(queuedRequest.operationId); this.scheduleRequest(queuedRequest); break; @@ -455,7 +455,7 @@ namespace ts.server { } private scheduleRequest(request: QueuedOperation) { - if(this.logger.hasLevel(LogLevel.verbose)) { + if (this.logger.hasLevel(LogLevel.verbose)) { this.logger.info(`Scheduling request for: ${request.operationId}`); } this.activeRequestCount++; From 9692ce86db3fb81c31c64c7716b9afe2c6cd4128 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 6 Sep 2017 15:46:59 -0700 Subject: [PATCH 4/4] Add explanatory comment --- src/server/server.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/server/server.ts b/src/server/server.ts index 96368fb9e5447..6b6535c8cacdc 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -251,6 +251,11 @@ namespace ts.server { private requestQueue: QueuedOperation[] = []; private requestMap = createMap(); // Maps operation ID to newest requestQueue entry with that ID + // This number is essentially arbitrary. Processing more than one typings request + // at a time makes sense, but having too many in the pipe results in a hang + // (see https://github.com/nodejs/node/issues/7657). + // It would be preferable to base our limit on the amount of space left in the + // buffer, but we have yet to find a way to retrieve that value. private static readonly maxActiveRequestCount = 10; private static readonly requestDelayMillis = 100;