Skip to content

metro-file-map: Start FallbackWatcher without stat-ing every file #2105

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

Open
robhogan wants to merge 1 commit into from
Open

metro-file-map: Start FallbackWatcher without stat-ing every file #2105

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
141 changes: 69 additions & 72 deletions packages/metro-file-map/src/watchers/FallbackWatcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type {
ChangeEventMetadata,
WatcherBackendChangeEvent,
} from '../flow-types';
import type {FSWatcher, Stats} from 'node:fs';
import type {Dirent, FSWatcher} from 'node:fs';

import {AbstractWatcher} from './AbstractWatcher';
import * as common from './common';
Expand All @@ -41,9 +41,9 @@ const RECRAWL_EVENT = common.RECRAWL_EVENT;
const DEBOUNCE_MS = 100;

/**
* The number of entries `recReaddir` visits at a time. Enough to keep the libuv
* thread pool saturated, few enough that a large tree does not queue up an
* unbounded number of fs requests.
* The number of directories `recReaddir` lists at a time. Enough to keep the
* libuv thread pool saturated, few enough that a large tree does not queue up
* an unbounded number of listings.
*/
const CRAWL_CONCURRENCY = 32;

Expand Down Expand Up @@ -352,45 +352,10 @@ export default class FallbackWatcher extends AbstractWatcher {
}
await recReaddir(
path.resolve(this.root, relativePath),
(dir, stats) => {
if (this.#watchdir(dir)) {
this.#emitEvent({
event: TOUCH_EVENT,
relativePath: path.relative(this.root, dir),
metadata: {
modifiedTime: stats.mtime.getTime(),
size: stats.size,
type: 'd',
},
});
}
},
(file, stats) => {
if (this.#register(file, 'f')) {
this.#emitEvent({
event: TOUCH_EVENT,
relativePath: path.relative(this.root, file),
metadata: {
modifiedTime: stats.mtime.getTime(),
size: stats.size,
type: 'f',
},
});
}
},
(symlink, stats) => {
if (this.#register(symlink, 'l')) {
this.emitFileEvent({
event: TOUCH_EVENT,
relativePath: path.relative(this.root, symlink),
metadata: {
modifiedTime: stats.mtime.getTime(),
size: stats.size,
type: 'l',
},
});
}
},
dir => (this.#watchdir(dir) ? this.#touch(dir, 'd') : null),
file => (this.#register(file, 'f') ? this.#touch(file, 'f') : null),
symlink =>
this.#register(symlink, 'l') ? this.#touch(symlink, 'l') : null,
this.#checkedEmitError,
this.ignored,
);
Expand Down Expand Up @@ -447,6 +412,32 @@ export default class FallbackWatcher extends AbstractWatcher {
}
}

/**
* Report an entry of a newly found directory, with its stats.
*/
async #touch(
fullPath: string,
type: ChangeEventMetadata['type'],
): Promise<void> {
let stats;
try {
stats = await fsPromises.lstat(fullPath);
} catch (error) {
this.#checkedEmitError(error);
return;
}
const change: Omit<WatcherBackendChangeEvent, 'root'> = {
event: TOUCH_EVENT,
relativePath: path.relative(this.root, fullPath),
metadata: {modifiedTime: stats.mtime.getTime(), size: stats.size, type},
};
if (type === 'l') {
this.emitFileEvent(change);
} else {
this.#emitEvent(change);
}
}

/**
* Emits the given event after debouncing, to emit only the latest
* information when we receive several events in quick succession. E.g.,
Expand Down Expand Up @@ -480,6 +471,8 @@ export default class FallbackWatcher extends AbstractWatcher {
function isIgnorableFileError(error: Error | {code: string}) {
return (
error.code === 'ENOENT' ||
// A directory replaced by a file, which its parent's watch reports.
error.code === 'ENOTDIR' ||
// Workaround Windows EPERM on watched folder deletion, and when
// reading locked files (pending further writes or pending deletion).
// In such cases, we'll receive a subsequent event when the file is
Expand All @@ -491,49 +484,53 @@ function isIgnorableFileError(error: Error | {code: string}) {
}

/**
* Traverse a directory recursively, reporting every entry with its stats.
* Traverse a directory recursively, reporting each entry by the type `readdir`
* gives it, without a stat. (Node stats an entry itself where the file system
* doesn't give its type.) The traversal waits for any promise a callback
* returns.
*
* Entries are visited depth-first by a bounded pool, so neither the queue of
* pending entries nor the number of in-flight fs requests grows with the size
* of the tree.
* Directories are visited depth-first by a bounded pool, so neither the queue
* of pending directories nor the number of listings in flight grows with the
* size of the tree.
*/
async function recReaddir(
root: string,
dirCallback: (dirPath: string, stats: Stats) => void,
fileCallback: (filePath: string, stats: Stats) => void,
symlinkCallback: (symlinkPath: string, stats: Stats) => void,
dirCallback: (dirPath: string) => ?Promise<void>,
fileCallback: (filePath: string) => ?Promise<void>,
symlinkCallback: (symlinkPath: string) => ?Promise<void>,
errorCallback: (error: Error) => void,
ignored: ?RegExp,
): Promise<void> {
const isIgnored = (dir: string) =>
ignored != null && common.posixPathMatchesPattern(ignored, dir);
if (isIgnored(root)) {
return;
}
const pending = [root];

const visit = async (entry: string) => {
let stats;
let names;
const visit = async (dir: string) => {
// Report the directory before listing it. A consumer that starts watching
// here would otherwise miss anything written between the two.
const reported = [dirCallback(dir)];
let entries: ReadonlyArray<Dirent> = [];
try {
stats = await fsPromises.lstat(entry);
if (!stats.isDirectory()) {
if (stats.isSymbolicLink()) {
symlinkCallback(entry, stats);
} else if (stats.isFile()) {
fileCallback(entry, stats);
}
return;
}
if (ignored != null && common.posixPathMatchesPattern(ignored, entry)) {
return;
}
// Report the directory before listing it. A consumer that starts watching
// here would otherwise miss anything written between the two.
dirCallback(entry, stats);
names = await fsPromises.readdir(entry);
entries = await fsPromises.readdir(dir, {withFileTypes: true});
} catch (error) {
errorCallback(error);
return;
}
for (const name of names) {
pending.push(path.join(entry, name));
for (const entry of entries) {
const entryPath = path.join(dir, entry.name.toString());
if (entry.isDirectory()) {
if (!isIgnored(entryPath)) {
pending.push(entryPath);
}
} else if (entry.isSymbolicLink()) {
reported.push(symlinkCallback(entryPath));
} else if (entry.isFile()) {
reported.push(fileCallback(entryPath));
}
}
await Promise.all(reported);
};

await new Promise((resolve, reject) => {
Expand Down
41 changes: 40 additions & 1 deletion packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ jest.useRealTimers();
jest.setTimeout(10 * 1000);

const {mkdir, rm, writeFile} = fs.promises;
// Captured before any test replaces `fs.watch`.
// Captured before any test replaces `fs.watch` or `fs.promises.readdir`.
const {watch: unmockedWatch} = fs;
const {readdir: unmockedReaddir} = fs.promises;

// An `FSWatcher` after it has reported an error: Node closes the handle before
// emitting 'error', so a subsequent `close()` returns early and emits nothing.
Expand Down Expand Up @@ -129,6 +130,44 @@ describe('FallbackWatcher', () => {
});
});

test('starts without stat-ing what it finds', async () => {
await mkdir(join(watchRoot, 'a', 'b'), {recursive: true});
await writeFile(join(watchRoot, 'a', 'file.js'), '');
await fs.promises.symlink('file.js', join(watchRoot, 'a', 'link.js'));
// Windows reports a directory as changed when it is listed. Withhold those
// events, whose handling stats the directory.
for (const dir of ['', 'a', join('a', 'b')]) {
mutedDirs.add(join(watchRoot, dir));
}
const lstat = jest.spyOn(fs.promises, 'lstat');

await watcher?.startWatching();

// Types come from the listings
expect(lstat).not.toHaveBeenCalled();
expectWatchedBeforeListed(join(watchRoot, 'a', 'b'));
});

// A directory's type comes from its parent's listing, so it may be replaced
// before it is visited, eg by a package manager. Its parent's watch reports
// the replacement.
test('starts past a directory replaced by a file before it is listed', async () => {
await mkdir(join(watchRoot, 'a'));
jest
.spyOn(fs.promises, 'readdir')
.mockImplementation(async (dir, ...args) => {
const entries = await unmockedReaddir(dir, ...args);
if (String(dir) === watchRoot) {
await rm(join(watchRoot, 'a'), {recursive: true});
await writeFile(join(watchRoot, 'a'), '');
}
return entries;
});

// Without an error listener, as Metro runs it
await expect(watcher?.startWatching()).resolves.toBeUndefined();
});

// A watch we cannot establish - one directory over the inotify limit, say -
// must not cost us the files under it, which would silently truncate the
// file map.
Expand Down
Loading