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 - emit URI instead of string for faster fsPath compute (for #194341) #194774

Merged
merged 1 commit into from
Oct 4, 2023
+98 −96
Merged

watcher - emit URI instead of string for faster fsPath compute (for #194341) #194774

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
4 changes: 2 additions & 2 deletions src/vs/platform/files/common/io.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ export interface ICreateReadStreamOptions extends IFileReadStreamOptions {
/**
* The size of the buffer to use before sending to the stream.
*/
bufferSize: number;
readonly bufferSize: number;

/**
* Allows to massage any possibly error that happens during reading.
*/
errorTransformer?: IErrorTransformer;
readonly errorTransformer?: IErrorTransformer;
}

/**
Expand Down
38 changes: 19 additions & 19 deletions src/vs/platform/files/common/watcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,48 +8,48 @@ import { GLOBSTAR, IRelativePattern, parse, ParsedPattern } from 'vs/base/common
import { Disposable, DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
import { isAbsolute } from 'vs/base/common/path';
import { isLinux } from 'vs/base/common/platform';
import { URI as uri } from 'vs/base/common/uri';
import { URI } from 'vs/base/common/uri';
import { FileChangeType, IFileChange, isParent } from 'vs/platform/files/common/files';

interface IWatchRequest {

/**
* The path to watch.
*/
path: string;
readonly path: string;

/**
* Whether to watch recursively or not.
*/
recursive: boolean;
readonly recursive: boolean;

/**
* A set of glob patterns or paths to exclude from watching.
*/
excludes: string[];
readonly excludes: string[];

/**
* An optional set of glob patterns or paths to include for
* watching. If not provided, all paths are considered for
* events.
*/
includes?: Array<string | IRelativePattern>;
readonly includes?: Array<string | IRelativePattern>;
}

export interface INonRecursiveWatchRequest extends IWatchRequest {

/**
* The watcher will be non-recursive.
*/
recursive: false;
readonly recursive: false;
}

export interface IRecursiveWatchRequest extends IWatchRequest {

/**
* The watcher will be recursive.
*/
recursive: true;
readonly recursive: true;

/**
* @deprecated this only exists for WSL1 support and should never
Expand Down Expand Up @@ -116,7 +116,7 @@ export interface IRecursiveWatcherOptions {
* @deprecated this only exists for WSL1 support and should never
* be used in any other case.
*/
usePolling: boolean | string[];
readonly usePolling: boolean | string[];

/**
* If polling is enabled (via `usePolling`), defines the duration
Expand All @@ -125,7 +125,7 @@ export interface IRecursiveWatcherOptions {
* @deprecated this only exists for WSL1 support and should never
* be used in any other case.
*/
pollingInterval?: number;
readonly pollingInterval?: number;
}

export interface INonRecursiveWatcher extends IWatcher {
Expand Down Expand Up @@ -259,18 +259,18 @@ export abstract class AbstractUniversalWatcherClient extends AbstractWatcherClie

export interface IDiskFileChange {
type: FileChangeType;
path: string;
readonly resource: URI;
}

export interface ILogMessage {
type: 'trace' | 'warn' | 'error' | 'info' | 'debug';
message: string;
readonly type: 'trace' | 'warn' | 'error' | 'info' | 'debug';
readonly message: string;
}

export function toFileChanges(changes: IDiskFileChange[]): IFileChange[] {
return changes.map(change => ({
type: change.type,
resource: uri.file(change.path)
resource: URI.revive(change.resource)
}));
}

Expand Down Expand Up @@ -317,10 +317,10 @@ class EventCoalescer {

private toKey(event: IDiskFileChange): string {
if (isLinux) {
return event.path;
return event.resource.fsPath;
}

return event.path.toLowerCase(); // normalise to file system case sensitivity
return event.resource.fsPath.toLowerCase(); // normalise to file system case sensitivity
}

processEvent(event: IDiskFileChange): void {
Expand All @@ -335,7 +335,7 @@ class EventCoalescer {

// macOS/Windows: track renames to different case
// by keeping both CREATE and DELETE events
if (existingEvent.path !== event.path && (event.type === FileChangeType.DELETED || event.type === FileChangeType.ADDED)) {
if (existingEvent.resource.fsPath !== event.resource.fsPath && (event.type === FileChangeType.DELETED || event.type === FileChangeType.ADDED)) {
keepEvent = true;
}

Expand Down Expand Up @@ -390,14 +390,14 @@ class EventCoalescer {

return true; // keep DELETE
}).sort((e1, e2) => {
return e1.path.length - e2.path.length; // shortest path first
return e1.resource.fsPath.length - e2.resource.fsPath.length; // shortest path first
}).filter(e => {
if (deletedPaths.some(deletedPath => isParent(e.path, deletedPath, !isLinux /* ignorecase */))) {
if (deletedPaths.some(deletedPath => isParent(e.resource.fsPath, deletedPath, !isLinux /* ignorecase */))) {
return false; // DELETE is ignored if parent is deleted already
}

// otherwise mark as deleted
deletedPaths.push(e.path);
deletedPaths.push(e.resource.fsPath);

return true;
}).concat(addOrChangeEvents);
Expand Down
27 changes: 14 additions & 13 deletions src/vs/platform/files/node/watcher/nodejs/nodejsWatcherLib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/
import { normalizeNFC } from 'vs/base/common/normalization';
import { basename, dirname, join } from 'vs/base/common/path';
import { isLinux, isMacintosh } from 'vs/base/common/platform';
import { URI } from 'vs/base/common/uri';
import { realcase } from 'vs/base/node/extpath';
import { Promises } from 'vs/base/node/pfs';
import { FileChangeType } from 'vs/platform/files/common/files';
Expand Down Expand Up @@ -260,7 +261,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {
type = FileChangeType.DELETED;
}

this.onFileChange({ path: join(this.request.path, changedFileName), type });
this.onFileChange({ resource: URI.file(join(this.request.path, changedFileName)), type });
}, NodeJSFileWatcherLibrary.FILE_DELETE_HANDLER_DELAY);

mapPathToStatDisposable.set(changedFileName, toDisposable(() => clearTimeout(timeoutHandle)));
Expand All @@ -279,7 +280,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {
folderChildren.add(changedFileName);
}

this.onFileChange({ path: join(this.request.path, changedFileName), type });
this.onFileChange({ resource: URI.file(join(this.request.path, changedFileName)), type });
}
}

Expand Down Expand Up @@ -318,14 +319,14 @@ export class NodeJSFileWatcherLibrary extends Disposable {

// File still exists, so emit as change event and reapply the watcher
if (fileExists) {
this.onFileChange({ path: this.request.path, type: FileChangeType.UPDATED }, true /* skip excludes/includes (file is explicitly watched) */);
this.onFileChange({ resource: URI.file(this.request.path), type: FileChangeType.UPDATED }, true /* skip excludes/includes (file is explicitly watched) */);

disposables.add(await this.doWatch(path, false));
}

// File seems to be really gone, so emit a deleted event and dispose
else {
this.onFileChange({ path: this.request.path, type: FileChangeType.DELETED }, true /* skip excludes/includes (file is explicitly watched) */);
this.onFileChange({ resource: URI.file(this.request.path), type: FileChangeType.DELETED }, true /* skip excludes/includes (file is explicitly watched) */);

// Important to flush the event delivery
// before disposing the watcher, otherwise
Expand All @@ -344,7 +345,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {

// File changed
else {
this.onFileChange({ path: this.request.path, type: FileChangeType.UPDATED }, true /* skip excludes/includes (file is explicitly watched) */);
this.onFileChange({ resource: URI.file(this.request.path), type: FileChangeType.UPDATED }, true /* skip excludes/includes (file is explicitly watched) */);
}
}
});
Expand All @@ -367,17 +368,17 @@ export class NodeJSFileWatcherLibrary extends Disposable {

// Logging
if (this.verboseLogging) {
this.trace(`${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.path}`);
this.trace(`${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.resource.fsPath}`);
}

// Add to aggregator unless excluded or not included (not if explicitly disabled)
if (!skipIncludeExcludeChecks && this.excludes.some(exclude => exclude(event.path))) {
if (!skipIncludeExcludeChecks && this.excludes.some(exclude => exclude(event.resource.fsPath))) {
if (this.verboseLogging) {
this.trace(` >> ignored (excluded) ${event.path}`);
this.trace(` >> ignored (excluded) ${event.resource.fsPath}`);
}
} else if (!skipIncludeExcludeChecks && this.includes && this.includes.length > 0 && !this.includes.some(include => include(event.path))) {
} else if (!skipIncludeExcludeChecks && this.includes && this.includes.length > 0 && !this.includes.some(include => include(event.resource.fsPath))) {
if (this.verboseLogging) {
this.trace(` >> ignored (not included) ${event.path}`);
this.trace(` >> ignored (not included) ${event.resource.fsPath}`);
}
} else {
this.fileChangesAggregator.work(event);
Expand All @@ -393,7 +394,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {
// Logging
if (this.verboseLogging) {
for (const event of coalescedFileChanges) {
this.trace(`>> normalized ${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.path}`);
this.trace(`>> normalized ${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.resource.fsPath}`);
}
}

Expand All @@ -402,10 +403,10 @@ export class NodeJSFileWatcherLibrary extends Disposable {

// Logging
if (!worked) {
this.warn(`started ignoring events due to too many file change events at once (incoming: ${coalescedFileChanges.length}, most recent change: ${coalescedFileChanges[0].path}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
this.warn(`started ignoring events due to too many file change events at once (incoming: ${coalescedFileChanges.length}, most recent change: ${coalescedFileChanges[0].resource.fsPath}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
} else {
if (this.throttledFileChangesEmitter.pending > 0) {
this.trace(`started throttling events due to large amount of file change events at once (pending: ${this.throttledFileChangesEmitter.pending}, most recent change: ${coalescedFileChanges[0].path}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
this.trace(`started throttling events due to large amount of file change events at once (pending: ${this.throttledFileChangesEmitter.pending}, most recent change: ${coalescedFileChanges[0].resource.fsPath}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
}
}
}
Expand Down
15 changes: 8 additions & 7 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 @@ -6,6 +6,7 @@
import * as parcelWatcher from '@parcel/watcher';
import { existsSync, statSync, unlinkSync } from 'fs';
import { tmpdir } from 'os';
import { URI } from 'vs/base/common/uri';
import { DeferredPromise, RunOnceScheduler, RunOnceWorker, ThrottledWorker } from 'vs/base/common/async';
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
import { toErrorMessage } from 'vs/base/common/errorMessage';
Expand Down Expand Up @@ -337,7 +338,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
this.trace(` >> ignored (not included) ${path}`);
}
} else {
events.push({ type, path });
events.push({ type, resource: URI.file(path) });
}
}

Expand Down Expand Up @@ -369,7 +370,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
// Logging
if (this.verboseLogging) {
for (const event of events) {
this.trace(` >> normalized ${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.path}`);
this.trace(` >> normalized ${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.resource.fsPath}`);
}
}

Expand All @@ -378,10 +379,10 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {

// Logging
if (!worked) {
this.warn(`started ignoring events due to too many file change events at once (incoming: ${events.length}, most recent change: ${events[0].path}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
this.warn(`started ignoring events due to too many file change events at once (incoming: ${events.length}, most recent change: ${events[0].resource.fsPath}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
} else {
if (this.throttledFileChangesEmitter.pending > 0) {
this.trace(`started throttling events due to large amount of file change events at once (pending: ${this.throttledFileChangesEmitter.pending}, most recent change: ${events[0].path}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
this.trace(`started throttling events due to large amount of file change events at once (pending: ${this.throttledFileChangesEmitter.pending}, most recent change: ${events[0].resource.fsPath}). Use 'files.watcherExclude' setting to exclude folders with lots of changing files (e.g. compilation output).`);
}
}
}
Expand Down Expand Up @@ -444,7 +445,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
let rootDeleted = false;

for (const event of events) {
if (event.type === FileChangeType.DELETED && event.path === watcher.request.path) {
if (event.type === FileChangeType.DELETED && event.resource.fsPath === watcher.request.path) {

// Explicitly exclude changes to root if we have any
// to avoid VS Code closing all opened editors which
Expand Down Expand Up @@ -472,8 +473,8 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
}

// Watcher path came back! Restart watching...
for (const { path, type } of changes) {
if (path === watcher.request.path && (type === FileChangeType.ADDED || type === FileChangeType.UPDATED)) {
for (const { resource, type } of changes) {
if (resource.fsPath === watcher.request.path && (type === FileChangeType.ADDED || type === FileChangeType.UPDATED)) {
this.warn('Watcher restarts because watched path got created again', watcher);

// Stop watching that parent folder
Expand Down
Loading