From e379e1f1893ab840bd2f35d0aecf08cb4dc2e819 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 8 Jan 2025 07:49:22 +0100 Subject: [PATCH] Nonresponsive editor with large project when `typescript.tsserver.watchOptions: vscode` (fix #237351) --- .../node/watcher/nodejs/nodejsWatcher.ts | 35 ++++++------------ .../node/watcher/parcel/parcelWatcher.ts | 36 +++++++------------ .../files/node/watcher/watcherStats.ts | 10 +++--- .../files/test/node/parcelWatcher.test.ts | 3 +- 4 files changed, 29 insertions(+), 55 deletions(-) diff --git a/src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts b/src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts index 0c60edd91a97f..6b40b9d4ba382 100644 --- a/src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts +++ b/src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts @@ -9,7 +9,6 @@ import { BaseWatcher } from '../baseWatcher.js'; import { isLinux } from '../../../../../base/common/platform.js'; import { INonRecursiveWatchRequest, INonRecursiveWatcher, IRecursiveWatcherWithSubscribe } from '../../../common/watcher.js'; import { NodeJSFileWatcherLibrary } from './nodejsWatcherLib.js'; -import { isEqual } from '../../../../../base/common/extpath.js'; export interface INodeJSWatcherInstance { @@ -28,7 +27,8 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { readonly onDidError = Event.None; - readonly watchers = new Set(); + private readonly _watchers = new Map(); + get watchers() { return this._watchers.values(); } constructor(protected readonly recursiveWatcher: IRecursiveWatcherWithSubscribe | undefined) { super(); @@ -43,7 +43,7 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { const requestsToStart: INonRecursiveWatchRequest[] = []; const watchersToStop = new Set(Array.from(this.watchers)); for (const request of requests) { - const watcher = this.findWatcher(request); + const watcher = this._watchers.get(this.requestToWatcherKey(request)); if (watcher && patternsEquals(watcher.request.excludes, request.excludes) && patternsEquals(watcher.request.includes, request.includes)) { watchersToStop.delete(watcher); // keep watcher } else { @@ -72,25 +72,12 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { } } - private findWatcher(request: INonRecursiveWatchRequest): INodeJSWatcherInstance | undefined { - for (const watcher of this.watchers) { - - // Requests or watchers with correlation always match on that - if (typeof request.correlationId === 'number' || typeof watcher.request.correlationId === 'number') { - if (watcher.request.correlationId === request.correlationId) { - return watcher; - } - } - - // Non-correlated requests or watchers match on path - else { - if (isEqual(watcher.request.path, request.path, !isLinux /* ignorecase */)) { - return watcher; - } - } - } + private requestToWatcherKey(request: INonRecursiveWatchRequest): string | number { + return typeof request.correlationId === 'number' ? request.correlationId : this.pathToWatcherKey(request.path); + } - return undefined; + private pathToWatcherKey(path: string): string { + return isLinux ? path : path.toLowerCase() /* ignore path casing */; } private startWatching(request: INonRecursiveWatchRequest): void { @@ -100,7 +87,7 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { // Remember as watcher instance const watcher: INodeJSWatcherInstance = { request, instance }; - this.watchers.add(watcher); + this._watchers.set(this.requestToWatcherKey(request), watcher); } override async stop(): Promise { @@ -114,7 +101,7 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { private stopWatching(watcher: INodeJSWatcherInstance): void { this.trace(`stopping file watcher`, watcher); - this.watchers.delete(watcher); + this._watchers.delete(this.requestToWatcherKey(watcher.request)); watcher.instance.dispose(); } @@ -124,7 +111,6 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { // Ignore requests for the same paths that have the same correlation for (const request of requests) { - const path = isLinux ? request.path : request.path.toLowerCase(); // adjust for case sensitivity let requestsForCorrelation = mapCorrelationtoRequests.get(request.correlationId); if (!requestsForCorrelation) { @@ -132,6 +118,7 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher { mapCorrelationtoRequests.set(request.correlationId, requestsForCorrelation); } + const path = this.pathToWatcherKey(request.path); if (requestsForCorrelation.has(path)) { this.trace(`ignoring a request for watching who's path is already watched: ${this.requestToString(request)}`); } diff --git a/src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts b/src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts index 3b47a55d69154..fbc46fb9f8c92 100644 --- a/src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts +++ b/src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts @@ -157,7 +157,8 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS private readonly _onDidError = this._register(new Emitter()); readonly onDidError = this._onDidError.event; - readonly watchers = new Set(); + private readonly _watchers = new Map(); + get watchers() { return this._watchers.values(); } // A delay for collecting file changes from Parcel // before collecting them for coalescing and emitting. @@ -206,7 +207,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS const requestsToStart: IRecursiveWatchRequest[] = []; const watchersToStop = new Set(Array.from(this.watchers)); for (const request of requests) { - const watcher = this.findWatcher(request); + const watcher = this._watchers.get(this.requestToWatcherKey(request)); if (watcher && patternsEquals(watcher.request.excludes, request.excludes) && patternsEquals(watcher.request.includes, request.includes) && watcher.request.pollingInterval === request.pollingInterval) { watchersToStop.delete(watcher); // keep watcher } else { @@ -238,25 +239,12 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS } } - private findWatcher(request: IRecursiveWatchRequest): ParcelWatcherInstance | undefined { - for (const watcher of this.watchers) { - - // Requests or watchers with correlation always match on that - if (this.isCorrelated(request) || this.isCorrelated(watcher.request)) { - if (watcher.request.correlationId === request.correlationId) { - return watcher; - } - } - - // Non-correlated requests or watchers match on path - else { - if (isEqual(watcher.request.path, request.path, !isLinux /* ignorecase */)) { - return watcher; - } - } - } + private requestToWatcherKey(request: IRecursiveWatchRequest): string | number { + return typeof request.correlationId === 'number' ? request.correlationId : this.pathToWatcherKey(request.path); + } - return undefined; + private pathToWatcherKey(path: string): string { + return isLinux ? path : path.toLowerCase() /* ignore path casing */; } private startPolling(request: IRecursiveWatchRequest, pollingInterval: number, restarts = 0): void { @@ -283,7 +271,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS unlinkSync(snapshotFile); } ); - this.watchers.add(watcher); + this._watchers.set(this.requestToWatcherKey(request), watcher); // Path checks for symbolic links / wrong casing const { realPath, realPathDiffers, realPathLength } = this.normalizePath(request); @@ -352,7 +340,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS await watcherInstance?.unsubscribe(); } ); - this.watchers.add(watcher); + this._watchers.set(this.requestToWatcherKey(request), watcher); // Path checks for symbolic links / wrong casing const { realPath, realPathDiffers, realPathLength } = this.normalizePath(request); @@ -643,7 +631,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS private async stopWatching(watcher: ParcelWatcherInstance, joinRestart?: Promise): Promise { this.trace(`stopping file watcher`, watcher); - this.watchers.delete(watcher); + this._watchers.delete(this.requestToWatcherKey(watcher.request)); try { await watcher.stop(joinRestart); @@ -666,7 +654,6 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS continue; // path is ignored entirely (via `**` glob exclude) } - const path = isLinux ? request.path : request.path.toLowerCase(); // adjust for case sensitivity let requestsForCorrelation = mapCorrelationtoRequests.get(request.correlationId); if (!requestsForCorrelation) { @@ -674,6 +661,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS mapCorrelationtoRequests.set(request.correlationId, requestsForCorrelation); } + const path = this.pathToWatcherKey(request.path); if (requestsForCorrelation.has(path)) { this.trace(`ignoring a request for watching who's path is already watched: ${this.requestToString(request)}`); } diff --git a/src/vs/platform/files/node/watcher/watcherStats.ts b/src/vs/platform/files/node/watcher/watcherStats.ts index b13a2187fb4ce..eca99eddef90f 100644 --- a/src/vs/platform/files/node/watcher/watcherStats.ts +++ b/src/vs/platform/files/node/watcher/watcherStats.ts @@ -34,8 +34,8 @@ export function computeStats( lines.push('[Summary]'); lines.push(`- Recursive Requests: total: ${allRecursiveRequests.length}, suspended: ${recursiveRequestsStatus.suspended}, polling: ${recursiveRequestsStatus.polling}, failed: ${failedRecursiveRequests}`); lines.push(`- Non-Recursive Requests: total: ${allNonRecursiveRequests.length}, suspended: ${nonRecursiveRequestsStatus.suspended}, polling: ${nonRecursiveRequestsStatus.polling}`); - lines.push(`- Recursive Watchers: total: ${recursiveWatcher.watchers.size}, active: ${recursiveWatcherStatus.active}, failed: ${recursiveWatcherStatus.failed}, stopped: ${recursiveWatcherStatus.stopped}`); - lines.push(`- Non-Recursive Watchers: total: ${nonRecursiveWatcher.watchers.size}, active: ${nonRecursiveWatcherStatus.active}, failed: ${nonRecursiveWatcherStatus.failed}, reusing: ${nonRecursiveWatcherStatus.reusing}`); + lines.push(`- Recursive Watchers: total: ${Array.from(recursiveWatcher.watchers).length}, active: ${recursiveWatcherStatus.active}, failed: ${recursiveWatcherStatus.failed}, stopped: ${recursiveWatcherStatus.stopped}`); + lines.push(`- Non-Recursive Watchers: total: ${Array.from(nonRecursiveWatcher.watchers).length}, active: ${nonRecursiveWatcherStatus.active}, failed: ${nonRecursiveWatcherStatus.failed}, reusing: ${nonRecursiveWatcherStatus.reusing}`); lines.push(`- I/O Handles Impact: total: ${recursiveRequestsStatus.polling + nonRecursiveRequestsStatus.polling + recursiveWatcherStatus.active + nonRecursiveWatcherStatus.active}`); lines.push(`\n[Recursive Requests (${allRecursiveRequests.length}, suspended: ${recursiveRequestsStatus.suspended}, polling: ${recursiveRequestsStatus.polling})]:`); @@ -106,7 +106,7 @@ function computeRecursiveWatchStatus(recursiveWatcher: ParcelWatcher): { active: let failed = 0; let stopped = 0; - for (const watcher of recursiveWatcher.watchers.values()) { + for (const watcher of recursiveWatcher.watchers) { if (!watcher.failed && !watcher.stopped) { active++; } @@ -189,7 +189,7 @@ function requestDetailsToString(request: IUniversalWatchRequest): string { } function fillRecursiveWatcherStats(lines: string[], recursiveWatcher: ParcelWatcher): void { - const watchers = sortByPathPrefix(Array.from(recursiveWatcher.watchers.values())); + const watchers = sortByPathPrefix(Array.from(recursiveWatcher.watchers)); const { active, failed, stopped } = computeRecursiveWatchStatus(recursiveWatcher); lines.push(`\n[Recursive Watchers (${watchers.length}, active: ${active}, failed: ${failed}, stopped: ${stopped})]:`); @@ -213,7 +213,7 @@ function fillRecursiveWatcherStats(lines: string[], recursiveWatcher: ParcelWatc } function fillNonRecursiveWatcherStats(lines: string[], nonRecursiveWatcher: NodeJSWatcher): void { - const allWatchers = sortByPathPrefix(Array.from(nonRecursiveWatcher.watchers.values())); + const allWatchers = sortByPathPrefix(Array.from(nonRecursiveWatcher.watchers)); const activeWatchers = allWatchers.filter(watcher => !watcher.instance.failed && !watcher.instance.isReusingRecursiveWatcher); const failedWatchers = allWatchers.filter(watcher => watcher.instance.failed); const reusingWatchers = allWatchers.filter(watcher => watcher.instance.isReusingRecursiveWatcher); diff --git a/src/vs/platform/files/test/node/parcelWatcher.test.ts b/src/vs/platform/files/test/node/parcelWatcher.test.ts index 558b81e19a9ad..8b1824af3bd83 100644 --- a/src/vs/platform/files/test/node/parcelWatcher.test.ts +++ b/src/vs/platform/files/test/node/parcelWatcher.test.ts @@ -105,7 +105,7 @@ suite.skip('File Watcher (parcel)', function () { }); teardown(async () => { - const watchers = watcher.watchers.size; + const watchers = Array.from(watcher.watchers).length; let stoppedInstances = 0; for (const instance of watcher.watchers) { Event.once(instance.onDidStop)(() => { @@ -190,7 +190,6 @@ suite.skip('File Watcher (parcel)', function () { test('basics', async function () { const request = { path: testDir, excludes: [], recursive: true }; await watcher.watch([request]); - assert.strictEqual(watcher.watchers.size, watcher.watchers.size); const instance = Array.from(watcher.watchers)[0]; assert.strictEqual(request, instance.request);