Skip to content

Commit

Permalink
Nonresponsive editor with large project when `typescript.tsserver.wat…
Browse files Browse the repository at this point in the history 
…chOptions: vscode` (fix #237351) (#237459)
  • Loading branch information
@bpasero
bpasero authored Jan 8, 2025
1 parent 9b0b13d commit b0d6d34
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 55 deletions.
35 changes: 11 additions & 24 deletions src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -28,7 +27,8 @@ export class NodeJSWatcher extends BaseWatcher implements INonRecursiveWatcher {

readonly onDidError = Event.None;

readonly watchers = new Set<INodeJSWatcherInstance>();
private readonly _watchers = new Map<string /* path */ | number /* correlation ID */, INodeJSWatcherInstance>();
get watchers() { return this._watchers.values(); }

constructor(protected readonly recursiveWatcher: IRecursiveWatcherWithSubscribe | undefined) {
super();
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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<void> {
Expand All @@ -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();
}
Expand All @@ -124,14 +111,14 @@ 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) {
requestsForCorrelation = new Map<string, INonRecursiveWatchRequest>();
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)}`);
}
Expand Down
36 changes: 12 additions & 24 deletions src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
private readonly _onDidError = this._register(new Emitter<IWatcherErrorEvent>());
readonly onDidError = this._onDidError.event;

readonly watchers = new Set<ParcelWatcherInstance>();
private readonly _watchers = new Map<string /* path */ | number /* correlation ID */, ParcelWatcherInstance>();
get watchers() { return this._watchers.values(); }

// A delay for collecting file changes from Parcel
// before collecting them for coalescing and emitting.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -643,7 +631,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
private async stopWatching(watcher: ParcelWatcherInstance, joinRestart?: Promise<void>): Promise<void> {
this.trace(`stopping file watcher`, watcher);

this.watchers.delete(watcher);
this._watchers.delete(this.requestToWatcherKey(watcher.request));

try {
await watcher.stop(joinRestart);
Expand All @@ -666,14 +654,14 @@ 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) {
requestsForCorrelation = new Map<string, IRecursiveWatchRequest>();
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)}`);
}
Expand Down
10 changes: 5 additions & 5 deletions src/vs/platform/files/node/watcher/watcherStats.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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})]:`);
Expand Down Expand Up @@ -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++;
}
Expand Down Expand Up @@ -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})]:`);
Expand All @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/vs/platform/files/test/node/parcelWatcher.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)(() => {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b0d6d34

Please sign in to comment.