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

watcher - more perf improvements for large workspaces #237640

Merged
merged 1 commit into from
Jan 10, 2025
Merged

watcher - more perf improvements for large workspaces #237640

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
44 changes: 2 additions & 42 deletions src/vs/base/node/extpath.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { CancellationToken } from '../common/cancellation.js';
import { basename, dirname, join, normalize, sep } from '../common/path.js';
import { isLinux } from '../common/platform.js';
import { rtrim } from '../common/strings.js';
import { Promises, readdirSync } from './pfs.js';
import { Promises } from './pfs.js';

/**
* Copied from: https://github.com/microsoft/vscode-node-debug/blob/master/src/node/pathUtilities.ts#L83
Expand All @@ -17,48 +17,8 @@ import { Promises, readdirSync } from './pfs.js';
* On a case insensitive file system, the returned path might differ from the original path by character casing.
* On a case sensitive file system, the returned path will always be identical to the original path.
* In case of errors, null is returned. But you cannot use this function to verify that a path exists.
* realcaseSync does not handle '..' or '.' path segments and it does not take the locale into account.
* realcase does not handle '..' or '.' path segments and it does not take the locale into account.
*/
export function realcaseSync(path: string): string | null {
if (isLinux) {
// This method is unsupported on OS that have case sensitive
// file system where the same path can exist in different forms
// (see also https://github.com/microsoft/vscode/issues/139709)
return path;
}

const dir = dirname(path);
if (path === dir) { // end recursion
return path;
}

const name = (basename(path) /* can be '' for windows drive letters */ || path).toLowerCase();
try {
const entries = readdirSync(dir);
const found = entries.filter(e => e.toLowerCase() === name); // use a case insensitive search
if (found.length === 1) {
// on a case sensitive filesystem we cannot determine here, whether the file exists or not, hence we need the 'file exists' precondition
const prefix = realcaseSync(dir); // recurse
if (prefix) {
return join(prefix, found[0]);
}
} else if (found.length > 1) {
// must be a case sensitive $filesystem
const ix = found.indexOf(name);
if (ix >= 0) { // case sensitive
const prefix = realcaseSync(dir); // recurse
if (prefix) {
return join(prefix, found[ix]);
}
}
}
} catch (error) {
// silently ignore error
}

return null;
}

export async function realcase(path: string, token?: CancellationToken): Promise<string | null> {
if (isLinux) {
// This method is unsupported on OS that have case sensitive
Expand Down
26 changes: 1 addition & 25 deletions src/vs/base/test/node/extpath.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import * as fs from 'fs';
import assert from 'assert';
import { tmpdir } from 'os';
import { realcase, realcaseSync, realpath, realpathSync } from '../../node/extpath.js';
import { realcase, realpath, realpathSync } from '../../node/extpath.js';
import { Promises } from '../../node/pfs.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../common/utils.js';
import { flakySuite, getRandomTestPath } from './testUtils.js';
Expand All @@ -24,30 +24,6 @@ flakySuite('Extpath', () => {
return Promises.rm(testDir);
});

test('realcaseSync', async () => {

// assume case insensitive file system
if (process.platform === 'win32' || process.platform === 'darwin') {
const upper = testDir.toUpperCase();
const real = realcaseSync(upper);

if (real) { // can be null in case of permission errors
assert.notStrictEqual(real, upper);
assert.strictEqual(real.toUpperCase(), upper);
assert.strictEqual(real, testDir);
}
}

// linux, unix, etc. -> assume case sensitive file system
else {
let real = realcaseSync(testDir);
assert.strictEqual(real, testDir);

real = realcaseSync(testDir.toUpperCase());
assert.strictEqual(real, testDir.toUpperCase());
}
});

test('realcase', async () => {

// assume case insensitive file system
Expand Down
57 changes: 30 additions & 27 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 @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import parcelWatcher from '@parcel/watcher';
import { statSync, unlinkSync } from 'fs';
import { promises } from 'fs';
import { tmpdir, homedir } from 'os';
import { URI } from '../../../../../base/common/uri.js';
import { DeferredPromise, RunOnceScheduler, RunOnceWorker, ThrottledWorker } from '../../../../../base/common/async.js';
Expand All @@ -18,7 +18,7 @@ import { TernarySearchTree } from '../../../../../base/common/ternarySearchTree.
import { normalizeNFC } from '../../../../../base/common/normalization.js';
import { normalize, join } from '../../../../../base/common/path.js';
import { isLinux, isMacintosh, isWindows } from '../../../../../base/common/platform.js';
import { realcaseSync, realpathSync } from '../../../../../base/node/extpath.js';
import { realcase, realpath } from '../../../../../base/node/extpath.js';
import { FileChangeType, IFileChange } from '../../../common/files.js';
import { coalesceEvents, IRecursiveWatchRequest, parseWatcherPatterns, IRecursiveWatcherWithSubscribe, isFiltered, IWatcherErrorEvent } from '../../../common/watcher.js';
import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js';
Expand Down Expand Up @@ -201,7 +201,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
protected override async doWatch(requests: IRecursiveWatchRequest[]): Promise<void> {

// Figure out duplicates to remove from the requests
requests = this.removeDuplicateRequests(requests);
requests = await this.removeDuplicateRequests(requests);

// Figure out which watchers to start and which to stop
const requestsToStart: IRecursiveWatchRequest[] = [];
Expand Down Expand Up @@ -232,7 +232,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
// Start watching as instructed
for (const request of requestsToStart) {
if (request.pollingInterval) {
this.startPolling(request, request.pollingInterval);
await this.startPolling(request, request.pollingInterval);
} else {
await this.startWatching(request);
}
Expand All @@ -247,7 +247,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
return isLinux ? path : path.toLowerCase() /* ignore path casing */;
}

private startPolling(request: IRecursiveWatchRequest, pollingInterval: number, restarts = 0): void {
private async startPolling(request: IRecursiveWatchRequest, pollingInterval: number, restarts = 0): Promise<void> {
const cts = new CancellationTokenSource();

const instance = new DeferredPromise<void>();
Expand All @@ -268,13 +268,13 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
watcher.worker.dispose();

pollingWatcher.dispose();
unlinkSync(snapshotFile);
await promises.unlink(snapshotFile);
}
);
this._watchers.set(this.requestToWatcherKey(request), watcher);

// Path checks for symbolic links / wrong casing
const { realPath, realPathDiffers, realPathLength } = this.normalizePath(request);
const { realPath, realPathDiffers, realPathLength } = await this.normalizePath(request);

this.trace(`Started watching: '${realPath}' with polling interval '${pollingInterval}'`);

Expand Down Expand Up @@ -343,7 +343,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
this._watchers.set(this.requestToWatcherKey(request), watcher);

// Path checks for symbolic links / wrong casing
const { realPath, realPathDiffers, realPathLength } = this.normalizePath(request);
const { realPath, realPathDiffers, realPathLength } = await this.normalizePath(request);

try {
const parcelWatcherLib = parcelWatcher;
Expand Down Expand Up @@ -471,20 +471,20 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
}
}

private normalizePath(request: IRecursiveWatchRequest): { realPath: string; realPathDiffers: boolean; realPathLength: number } {
private async normalizePath(request: IRecursiveWatchRequest): Promise<{ realPath: string; realPathDiffers: boolean; realPathLength: number }> {
let realPath = request.path;
let realPathDiffers = false;
let realPathLength = request.path.length;

try {

// First check for symbolic link
realPath = realpathSync(request.path);
realPath = await realpath(request.path);

// Second check for casing difference
// Note: this will be a no-op on Linux platforms
if (request.path === realPath) {
realPath = realcaseSync(request.path) ?? request.path;
realPath = await realcase(request.path) ?? request.path;
}

// Correct watch path as needed
Expand Down Expand Up @@ -615,7 +615,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS

// Start watcher again counting the restarts
if (watcher.request.pollingInterval) {
this.startPolling(watcher.request, watcher.request.pollingInterval, watcher.restarts + 1);
await this.startPolling(watcher.request, watcher.request.pollingInterval, watcher.restarts + 1);
} else {
await this.startWatching(watcher.request, watcher.restarts + 1);
}
Expand All @@ -640,7 +640,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
}
}

protected removeDuplicateRequests(requests: IRecursiveWatchRequest[], validatePaths = true): IRecursiveWatchRequest[] {
protected async removeDuplicateRequests(requests: IRecursiveWatchRequest[], validatePaths = true): Promise<IRecursiveWatchRequest[]> {

// Sort requests by path length to have shortest first
// to have a way to prevent children to be watched if
Expand Down Expand Up @@ -686,26 +686,29 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS

for (const request of requestsForCorrelation.values()) {

// Check for overlapping requests
// Check for overlapping request paths (but preserve symbolic links)
if (requestTrie.findSubstr(request.path)) {
try {
const realpath = realpathSync(request.path);
if (realpath === request.path) {
this.trace(`ignoring a request for watching who's parent is already watched: ${this.requestToString(request)}`);
if (requestTrie.has(request.path)) {
this.trace(`ignoring a request for watching who's path is already watched: ${this.requestToString(request)}`);
} else {
try {
if (!(await promises.lstat(request.path)).isSymbolicLink()) {
this.trace(`ignoring a request for watching who's parent is already watched: ${this.requestToString(request)}`);

continue;
}
} catch (error) {
this.trace(`ignoring a request for watching who's realpath failed to resolve: ${this.requestToString(request)} (error: ${error})`);
continue;
}
} catch (error) {
this.trace(`ignoring a request for watching who's lstat failed to resolve: ${this.requestToString(request)} (error: ${error})`);

this._onDidWatchFail.fire(request);
this._onDidWatchFail.fire(request);

continue;
continue;
}
}
}

// Check for invalid paths
if (validatePaths && !this.isPathValid(request.path)) {
if (validatePaths && !(await this.isPathValid(request.path))) {
this._onDidWatchFail.fire(request);

continue;
Expand All @@ -720,9 +723,9 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
return normalizedRequests;
}

private isPathValid(path: string): boolean {
private async isPathValid(path: string): Promise<boolean> {
try {
const stat = statSync(path);
const stat = await promises.stat(path);
if (!stat.isDirectory()) {
this.trace(`ignoring a path for watching that is a file and not a folder: ${path}`);

Expand Down
6 changes: 5 additions & 1 deletion src/vs/platform/files/test/node/nodejsWatcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ suite.skip('File Watcher (node.js)', function () {
setup(async () => {
await createWatcher(undefined);

testDir = URI.file(getRandomTestPath(tmpdir(), 'vsctests', 'filewatcher')).fsPath;
// Rule out strange testing conditions by using the realpath
// here. for example, on macOS the tmp dir is potentially a
// symlink in some of the root folders, which is a rather
// unrealisic case for the file watcher.
testDir = URI.file(getRandomTestPath(fs.realpathSync(tmpdir()), 'vsctests', 'filewatcher')).fsPath;

const sourceDir = FileAccess.asFileUri('vs/platform/files/test/node/fixtures/service').fsPath;

Expand Down
Loading
Loading