From f279239c71d063d37e64f777d0e3560e65bc88f7 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:37:20 +0000 Subject: [PATCH] metro-file-map: Start FallbackWatcher without stat-ing every file Without Watchman, Metro watches with `FallbackWatcher` on Linux and Windows. Its startup walk `lstat`s every entry, files included, just to learn its type: https://github.com/react/metro/blob/56efab302200568c919ac1b3c28dacf8d84fc692/packages/metro-file-map/src/watchers/FallbackWatcher.js#L505-L515 This walks by the types `readdir` reports instead. A directory that appears while watching still needs stats for its events, so that walk now stats only the entries it reports. The walk also now descends into a watch root that's a symlink, as the crawler does. ## Benchmark This takes **40% off watch-mode startup on Linux (1,614ms -> 975ms) and 20% off on Windows**. macOS uses `NativeWatcher`, so it's unaffected. | | `main` | #2104 | This PR | vs `main` | vs #2104 | |---|---|---|---|---|---| | Linux: Watch-mode start, warm cache | 1,887ms | 1,614ms | 975ms | -48.3% (-49.0% to -47.9%) | -39.6% (-40.3% to -39.1%) | | Windows: Watch-mode start, warm cache | 2,577ms | 2,417ms | 1,941ms | -24.7% (-25.1% to -24.3%) | -19.7% (-20.0% to -19.4%) | | Linux: Watch-mode start, cold cache | 1,885ms | 1,579ms | 921ms | -51.1% (-51.6% to -50.5%) | -41.7% (-42.3% to -41.0%) | | Windows: Watch-mode start, cold cache | 2,562ms | 2,386ms | 1,918ms | -25.2% (-25.3% to -24.7%) | -19.6% (-19.8% to -19.3%) | ## Changelog ``` - **[Performance]**: metro-file-map: Start the file watcher without stat-ing every file (Linux and Windows, without Watchman) ``` ## Test plan New unit test that startup doesn't `lstat` anything it finds. Benchmark as in #2104. --- .../src/watchers/FallbackWatcher.js | 141 +++++++++--------- .../__tests__/FallbackWatcher-test.js | 41 ++++- 2 files changed, 109 insertions(+), 73 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FallbackWatcher.js b/packages/metro-file-map/src/watchers/FallbackWatcher.js index eac161e2ea..a805afab5d 100644 --- a/packages/metro-file-map/src/watchers/FallbackWatcher.js +++ b/packages/metro-file-map/src/watchers/FallbackWatcher.js @@ -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'; @@ -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; @@ -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, ); @@ -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 { + let stats; + try { + stats = await fsPromises.lstat(fullPath); + } catch (error) { + this.#checkedEmitError(error); + return; + } + const change: Omit = { + 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., @@ -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 @@ -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, + fileCallback: (filePath: string) => ?Promise, + symlinkCallback: (symlinkPath: string) => ?Promise, errorCallback: (error: Error) => void, ignored: ?RegExp, ): Promise { + 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 = []; 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) => { diff --git a/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js index 73c5efadbb..861d54c7bf 100644 --- a/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js @@ -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. @@ -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.