Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nonresponsive editor with large project when typescript.tsserver.watchOptions: vscode (fix #237351) #237459

Merged
merged 1 commit into from
Jan 8, 2025
+29 −55
Merged

Nonresponsive editor with large project when typescript.tsserver.watchOptions: vscode (fix #237351) #237459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 11 additions & 24 deletions src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading