Skip to content

Commit 8d1eb29

Browse files
authored
Merge pull request #18265 from amcasey/ThrottledOperations
Limit the number of unanswered Typings Installer requests
2 parents de313ff + 9692ce8 commit 8d1eb29

File tree

2 files changed

+68
-5
lines changed

2 files changed

+68
-5
lines changed

src/server/server.ts

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,25 +236,40 @@ namespace ts.server {
236236
return `${d.getHours()}:${d.getMinutes()}:${d.getSeconds()}.${d.getMilliseconds()}`;
237237
}
238238

239+
interface QueuedOperation {
240+
operationId: string;
241+
operation: () => void;
242+
}
243+
239244
class NodeTypingsInstaller implements ITypingsInstaller {
240245
private installer: NodeChildProcess;
241246
private installerPidReported = false;
242247
private socket: NodeSocket;
243248
private projectService: ProjectService;
244-
private throttledOperations: ThrottledOperations;
245249
private eventSender: EventSender;
250+
private activeRequestCount = 0;
251+
private requestQueue: QueuedOperation[] = [];
252+
private requestMap = createMap<QueuedOperation>(); // Maps operation ID to newest requestQueue entry with that ID
253+
254+
// This number is essentially arbitrary. Processing more than one typings request
255+
// at a time makes sense, but having too many in the pipe results in a hang
256+
// (see https://github.com/nodejs/node/issues/7657).
257+
// It would be preferable to base our limit on the amount of space left in the
258+
// buffer, but we have yet to find a way to retrieve that value.
259+
private static readonly maxActiveRequestCount = 10;
260+
private static readonly requestDelayMillis = 100;
261+
246262

247263
constructor(
248264
private readonly telemetryEnabled: boolean,
249265
private readonly logger: server.Logger,
250-
host: ServerHost,
266+
private readonly host: ServerHost,
251267
eventPort: number,
252268
readonly globalTypingsCacheLocation: string,
253269
readonly typingSafeListLocation: string,
254270
readonly typesMapLocation: string,
255271
private readonly npmLocation: string | undefined,
256272
private newLine: string) {
257-
this.throttledOperations = new ThrottledOperations(host);
258273
if (eventPort) {
259274
const s = net.connect({ port: eventPort }, () => {
260275
this.socket = s;
@@ -338,12 +353,26 @@ namespace ts.server {
338353
this.logger.info(`Scheduling throttled operation: ${JSON.stringify(request)}`);
339354
}
340355
}
341-
this.throttledOperations.schedule(project.getProjectName(), /*ms*/ 250, () => {
356+
357+
const operationId = project.getProjectName();
358+
const operation = () => {
342359
if (this.logger.hasLevel(LogLevel.verbose)) {
343360
this.logger.info(`Sending request: ${JSON.stringify(request)}`);
344361
}
345362
this.installer.send(request);
346-
});
363+
};
364+
const queuedRequest: QueuedOperation = { operationId, operation };
365+
366+
if (this.activeRequestCount < NodeTypingsInstaller.maxActiveRequestCount) {
367+
this.scheduleRequest(queuedRequest);
368+
}
369+
else {
370+
if (this.logger.hasLevel(LogLevel.verbose)) {
371+
this.logger.info(`Deferring request for: ${operationId}`);
372+
}
373+
this.requestQueue.push(queuedRequest);
374+
this.requestMap.set(operationId, queuedRequest);
375+
}
347376
}
348377

349378
private handleMessage(response: SetTypings | InvalidateCachedTypings | BeginInstallTypes | EndInstallTypes | InitializationFailedResponse) {
@@ -404,11 +433,39 @@ namespace ts.server {
404433
return;
405434
}
406435

436+
if (this.activeRequestCount > 0) {
437+
this.activeRequestCount--;
438+
}
439+
else {
440+
Debug.fail("Received too many responses");
441+
}
442+
443+
while (this.requestQueue.length > 0) {
444+
const queuedRequest = this.requestQueue.shift();
445+
if (this.requestMap.get(queuedRequest.operationId) === queuedRequest) {
446+
this.requestMap.delete(queuedRequest.operationId);
447+
this.scheduleRequest(queuedRequest);
448+
break;
449+
}
450+
451+
if (this.logger.hasLevel(LogLevel.verbose)) {
452+
this.logger.info(`Skipping defunct request for: ${queuedRequest.operationId}`);
453+
}
454+
}
455+
407456
this.projectService.updateTypingsForProject(response);
408457
if (response.kind === ActionSet && this.socket) {
409458
this.sendEvent(0, "setTypings", response);
410459
}
411460
}
461+
462+
private scheduleRequest(request: QueuedOperation) {
463+
if (this.logger.hasLevel(LogLevel.verbose)) {
464+
this.logger.info(`Scheduling request for: ${request.operationId}`);
465+
}
466+
this.activeRequestCount++;
467+
this.host.setTimeout(request.operation, NodeTypingsInstaller.requestDelayMillis);
468+
}
412469
}
413470

414471
class IOSession extends Session {

src/server/utilities.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@ namespace ts.server {
179179
constructor(private readonly host: ServerHost) {
180180
}
181181

182+
/**
183+
* Wait `number` milliseconds and then invoke `cb`. If, while waiting, schedule
184+
* is called again with the same `operationId`, cancel this operation in favor
185+
* of the new one. (Note that the amount of time the canceled operation had been
186+
* waiting does not affect the amount of time that the new operation waits.)
187+
*/
182188
public schedule(operationId: string, delay: number, cb: () => void) {
183189
const pendingTimeout = this.pendingTimeouts.get(operationId);
184190
if (pendingTimeout) {

0 commit comments

Comments
 (0)