From 9ded43038458a1fd1f7c5f96e09e7b7928a16a7e Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:20:19 +0000 Subject: [PATCH] metro-file-map: Start watching while the cache is read Without Watchman, a watch-mode start has to be watching before it crawls, so that a change made during the crawl isn't missed. `NativeWatcher` (macOS) isn't live as soon as it starts, so `Watcher` also confirms each root's watch with a health check before the crawl begins: https://github.com/react/metro/blob/ef50b94fba3bac242d06b20f03c7149987b3c2d6/packages/metro-file-map/src/Watcher.js#L375-L382 That takes about 13ms on macOS, and `build()` only starts it once the cache has been read, although the watch doesn't need anything from the cache. This starts the watch, and the Watchman capability check (which spawns `watchman`), while the cache is read. The crawl still waits for the confirmed watch, so nothing can be missed that wasn't before. To allow this, `Watcher` takes the cached state in `crawl()` rather than at construction. ## Benchmark This takes 13% off a watch-mode start on macOS (102ms -> 88ms). Linux and Windows watch with `FallbackWatcher`, which is live as soon as it starts, so they're unchanged, as are starts outside watch mode. With a cold cache there's no read to overlap with. | | `main` | #2220 | This PR | vs `main` | vs #2220 | |---|---|---|---|---|---| | macOS: Watch-mode start | 130ms | 102ms | 88ms | -32.1% (-32.8% to -31.2%) | -13.0% (-14.0% to -11.3%) | | Linux: Watch-mode start | 1,889ms | 493ms | 491ms | -74.0% (-74.2% to -73.6%) | -0.5% (-1.4% to +0.5%) | | Windows: Watch-mode start | 2,569ms | 524ms | 524ms | -79.6% (-79.7% to -79.5%) | +0.1% (-0.5% to +0.6%) | | macOS: Warm start | 118ms | 88ms | 88ms | -25.1% (-26.4% to -23.6%) | +0.1% (-1.8% to +1.9%) | | Linux: Warm start | 636ms | 353ms | 353ms | -44.5% (-45.3% to -44.1%) | -0.1% (-1.8% to +1.1%) | | Windows: Warm start | 355ms | 201ms | 201ms | -43.4% (-43.7% to -42.7%) | +0.0% (-0.9% to +0.7%) | ## Changelog ``` - **[Performance]**: metro-file-map: Start watching while the cache is read ``` ## Test plan New unit tests. Benchmark as in #2104, with 22 rounds on macOS. --- packages/metro-file-map/src/Watcher.js | 77 ++++++++++----- .../src/__tests__/Watcher-test.js | 98 +++++++++++++------ .../src/__tests__/index-test.js | 59 +++++++++++ packages/metro-file-map/src/index.js | 35 ++++--- 4 files changed, 198 insertions(+), 71 deletions(-) diff --git a/packages/metro-file-map/src/Watcher.js b/packages/metro-file-map/src/Watcher.js index 797f79944c..3192bbd2ae 100644 --- a/packages/metro-file-map/src/Watcher.js +++ b/packages/metro-file-map/src/Watcher.js @@ -76,7 +76,6 @@ type WatcherOptions = { healthCheckFilePrefix: string, ignoreForCrawl: (filePath: string) => boolean, ignorePatternForWatch: RegExp, - previousState: CrawlerOptions['previousState'], perfLogger: ?PerfLogger, roots: ReadonlyArray, rootDir: string, @@ -107,6 +106,9 @@ export class Watcher extends EventEmitter { #nextHealthCheckId: number = 0; #onChange: ?(change: WatcherBackendChangeEvent) => void; readonly #options: WatcherOptions; + // What prepareCrawl() started, resolving to the node crawl's watchers, if + // any. + #prepared: ?Promise>; readonly #pendingHealthChecks: Map< /* basename */ string, /* resolve */ () => void, @@ -122,36 +124,59 @@ export class Watcher extends EventEmitter { this.#useWatchmanWatcher = options.useWatchman; } - async crawl(): Promise { - this.#options.perfLogger?.point('crawl_start'); - const options = this.#options; + /** + * Start anything that must be running before the crawl begins, which in + * watch mode without Watchman is the backends, with their watches + * confirmed. crawl() waits for this, and calling it earlier lets other work, + * such as reading a cache, overlap with it. + */ + async prepareCrawl(): Promise { + await this.#prepareCrawl(); + } - // Non-Watchman backends have no clock to subscribe `since`, so they must - // already be watching before the crawl begins, and their events buffered, - // for changes made while crawling not to be missed. Watchman backends are - // started by watch(), which subscribes `since` the crawl's clock to cover - // the same window. If the Watchman crawl falls back to the node crawler, - // there is no clock, so #crawl() starts non-Watchman backends before the - // node crawl instead. - // - // The node crawler lists every directory under the roots, so a - // FallbackWatcher started for it can watch each just before it is listed, - // instead of walking the roots again. - let crawlWatchers: ?ReadonlyArray = null; - if (!options.useWatchman) { - const crawlFed = - options.crawl == null && this.#getBackendClass() === FallbackWatcher; - await this.#startBackendsBeforeCrawl({crawlFed}); - if (crawlFed) { - crawlWatchers = this.#backends.map(backend => - backend instanceof FallbackWatcher ? backend.getCrawlWatcher() : null, - ); + #prepareCrawl(): Promise> { + this.#prepared ??= (async () => { + const options = this.#options; + options.perfLogger?.point('prepareCrawl_start'); + // Non-Watchman backends have no clock to subscribe `since`, so they must + // already be watching before the crawl begins, and their events + // buffered, for changes made while crawling not to be missed. Watchman + // backends are started by watch(), which subscribes `since` the crawl's + // clock to cover the same window. If the Watchman crawl falls back to + // the node crawler, there is no clock, so #crawl() starts non-Watchman + // backends before the node crawl instead. + let crawlWatchers: ?ReadonlyArray = null; + if (!options.useWatchman) { + // The node crawler lists every directory under the roots, so a + // FallbackWatcher started for it can watch each just before it is + // listed, instead of walking the roots again. + const crawlFed = + options.crawl == null && this.#getBackendClass() === FallbackWatcher; + await this.#startBackendsBeforeCrawl({crawlFed}); + if (crawlFed) { + crawlWatchers = this.#backends.map(backend => + backend instanceof FallbackWatcher + ? backend.getCrawlWatcher() + : null, + ); + } } - } + options.perfLogger?.point('prepareCrawl_end'); + return crawlWatchers; + })(); + return this.#prepared; + } + + async crawl( + previousState: CrawlerOptions['previousState'], + ): Promise { + this.#options.perfLogger?.point('crawl_start'); + const options = this.#options; + const crawlWatchers = await this.#prepareCrawl(); const result = await this.#crawl({ crawlWatchers, - previousState: options.previousState, + previousState, roots: options.roots, useWatchman: options.useWatchman, }); diff --git a/packages/metro-file-map/src/__tests__/Watcher-test.js b/packages/metro-file-map/src/__tests__/Watcher-test.js index 54dc67145b..d2ab098d95 100644 --- a/packages/metro-file-map/src/__tests__/Watcher-test.js +++ b/packages/metro-file-map/src/__tests__/Watcher-test.js @@ -156,15 +156,6 @@ function createWatcher( ignoreForCrawl: () => false, ignorePatternForWatch: /^$/, perfLogger: null, - previousState: { - clocks: new Map(), - fileSystem: new TreeFS({ - processFile: () => { - throw new Error('not implemented'); - }, - rootDir, - }), - }, rootDir, roots: [rootDir], useWatchman, @@ -173,6 +164,18 @@ function createWatcher( }); } +function emptyState(): CrawlerOptions['previousState'] { + return { + clocks: new Map(), + fileSystem: new TreeFS({ + processFile: () => { + throw new Error('not implemented'); + }, + rootDir, + }), + }; +} + beforeEach(() => { mockStartedBackends.length = 0; mockNativeLiveOnStart = true; @@ -191,7 +194,7 @@ describe('Watcher crawl fallback', () => { const canonicalPath = path.join('src', 'index.js'); mockNodeCrawl.mockImplementation(async () => crawlResult([canonicalPath])); - const result = await createWatcher(false).crawl(); + const result = await createWatcher(false).crawl(emptyState()); expect([...result.changedFiles.keys()]).toEqual([canonicalPath]); expect(mockNodeCrawl).toHaveBeenCalledTimes(1); @@ -201,7 +204,7 @@ describe('Watcher crawl fallback', () => { test('a failing node crawler propagates, with no retry', async () => { mockNodeCrawl.mockImplementation(rejectWith('node crawl failed')); - await expect(createWatcher(false).crawl()).rejects.toThrow( + await expect(createWatcher(false).crawl(emptyState())).rejects.toThrow( 'node crawl failed', ); // The retry exists for Watchman only - retrying node with node is pointless. @@ -214,7 +217,7 @@ describe('Watcher crawl fallback', () => { crawlResult([canonicalPath]), ); - const result = await createWatcher(true).crawl(); + const result = await createWatcher(true).crawl(emptyState()); expect([...result.changedFiles.keys()]).toEqual([canonicalPath]); expect(mockWatchmanCrawl).toHaveBeenCalledTimes(1); @@ -226,7 +229,7 @@ describe('Watcher crawl fallback', () => { mockWatchmanCrawl.mockImplementation(rejectWith('watchman unavailable')); mockNodeCrawl.mockImplementation(async () => crawlResult([canonicalPath])); - const result = await createWatcher(true).crawl(); + const result = await createWatcher(true).crawl(emptyState()); expect([...result.changedFiles.keys()]).toEqual([canonicalPath]); expect(mockNodeCrawl).toHaveBeenCalledTimes(1); @@ -238,7 +241,7 @@ describe('Watcher crawl fallback', () => { mockNodeCrawl.mockImplementation(rejectWith('node crawl failed')); const thrown = await createWatcher(true) - .crawl() + .crawl(emptyState()) .then( () => null, (error: Error) => error, @@ -253,7 +256,7 @@ describe('Watcher crawl fallback', () => { mockWatchmanCrawl.mockImplementation(rejectWith('watchman unavailable')); mockNodeCrawl.mockImplementation(async () => crawlResult([])); - await createWatcher(true).crawl(); + await createWatcher(true).crawl(emptyState()); expect(mockNodeCrawl.mock.calls[0][0]).toBe( mockWatchmanCrawl.mock.calls[0][0], @@ -273,7 +276,7 @@ describe('Watcher backend choice in watch mode', () => { mockWatchmanCrawl.mockImplementation(async () => crawlResult([])); const watcher = createWatcher(true, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); expect(mockStartedBackends).toEqual([]); await watcher.watch(() => {}); @@ -284,7 +287,7 @@ describe('Watcher backend choice in watch mode', () => { mockWatchmanCrawl.mockImplementation(async () => crawlResult([])); const watcher = createWatcher(true, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); await watcher.watch(() => {}, new Map([['', 'c:crawl-clock:1']])); const addedRoot = path.join(path.sep, 'other'); await watcher.watchRoot(addedRoot); @@ -307,7 +310,7 @@ describe('Watcher backend choice in watch mode', () => { }); const watcher = createWatcher(true, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); const onChange = jest.fn<[WatcherBackendChangeEvent], void>(); await watcher.watch(onChange); @@ -366,7 +369,7 @@ describe('Watcher with a backend that is not live on start', () => { const watcher = createWatcher(false, null, true); const warn = jest.spyOn(console, 'warn'); - await watcher.crawl(); + await watcher.crawl(emptyState()); // The first health check went unreported, the second was reported. expect(writesBeforeCrawl).toBe(2); @@ -386,7 +389,7 @@ describe('Watcher with a backend that is not live on start', () => { const watcher = createWatcher(false, null, true); const warn = jest.spyOn(console, 'warn'); - const crawled = watcher.crawl(); + const crawled = watcher.crawl(emptyState()); // $FlowFixMe[prop-missing] - in Jest 29.5+, missing from flow-typed's defs await jest.advanceTimersByTimeAsync(11000); await crawled; @@ -402,7 +405,7 @@ describe('Watcher with a backend that is not live on start', () => { test('confirms the watch of a root added later', async () => { mockNodeCrawl.mockImplementation(async () => crawlResult([])); const watcher = createWatcher(false, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); await watcher.watch(() => {}); writtenFiles = []; @@ -412,12 +415,30 @@ describe('Watcher with a backend that is not live on start', () => { expect(writtenFiles.map(f => path.dirname(f))).toEqual([addedRoot]); }); + test('prepareCrawl() starts and confirms the watch, which the crawl then uses', async () => { + mockNodeCrawl.mockImplementation(async () => crawlResult([])); + const watcher = createWatcher(false, null, true); + + await watcher.prepareCrawl(); + + expect(mockStartedBackends.map(b => b.root)).toEqual([rootDir]); + expect(writtenFiles.map(f => path.dirname(f))).toEqual([rootDir]); + expect(mockNodeCrawl).not.toHaveBeenCalled(); + + await watcher.crawl(emptyState()); + + // Neither started nor confirmed again + expect(mockStartedBackends).toHaveLength(1); + expect(writtenFiles).toHaveLength(1); + expect(mockNodeCrawl).toHaveBeenCalledTimes(1); + }); + test('a backend that is live on start is not checked', async () => { mockNativeLiveOnStart = true; mockNodeCrawl.mockImplementation(async () => crawlResult([])); const watcher = createWatcher(false, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); expect(writtenFiles).toEqual([]); }); @@ -441,15 +462,32 @@ describe('Watcher with FallbackWatcher and the node crawler', () => { }); const watcher = createWatcher(false, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); + + expect(mockStartedBackends[0].crawledDirectories).toEqual([src]); + }); + + test('after prepareCrawl(), the crawl reports to the watcher it started', async () => { + const src = path.join(rootDir, 'src'); + mockNodeCrawl.mockImplementation(async (options, crawlWatchers) => { + crawlWatchers?.[0]?.onDirectory(src); + return crawlResult([]); + }); + const watcher = createWatcher(false, null, true); + + await watcher.prepareCrawl(); + await watcher.crawl(emptyState()); + expect(mockStartedBackends.map(b => [b.kind, b.crawlFed])).toEqual([ + ['fallback', true], + ]); expect(mockStartedBackends[0].crawledDirectories).toEqual([src]); }); test('a scoped recrawl is not reported to the watcher', async () => { mockNodeCrawl.mockImplementation(async () => crawlResult([])); const watcher = createWatcher(false, null, true); - await watcher.crawl(); + await watcher.crawl(emptyState()); mockNodeCrawl.mockClear(); await watcher.recrawl( @@ -468,7 +506,7 @@ describe('Watcher with FallbackWatcher and the node crawler', () => { test('with a custom crawler, the watcher walks the roots itself', async () => { const watcher = createWatcher(false, async () => crawlResult([]), true); - await watcher.crawl(); + await watcher.crawl(emptyState()); expect(mockStartedBackends.map(b => [b.kind, b.crawlFed])).toEqual([ ['fallback', false], @@ -488,7 +526,7 @@ describe('Watcher crawl option', () => { const result = await createWatcher( true, stubCrawler([canonicalPath]), - ).crawl(); + ).crawl(emptyState()); expect([...result.changedFiles.keys()]).toEqual([canonicalPath]); expect(mockWatchmanCrawl).not.toHaveBeenCalled(); @@ -500,7 +538,7 @@ describe('Watcher crawl option', () => { stubCrawler([]), ); - await createWatcher(true, crawl).crawl(); + await createWatcher(true, crawl).crawl(emptyState()); const options = crawl.mock.calls[0][0]; expect(options).toMatchObject({ @@ -543,9 +581,9 @@ describe('Watcher crawl option', () => { // what a supplied crawler exists to avoid. const crawl: Crawler = rejectWith('crawler exploded'); - await expect(createWatcher(true, crawl).crawl()).rejects.toThrow( - 'crawler exploded', - ); + await expect( + createWatcher(true, crawl).crawl(emptyState()), + ).rejects.toThrow('crawler exploded'); expect(mockNodeCrawl).not.toHaveBeenCalled(); }); }); diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index 76f0434b02..d5473abe37 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/packages/metro-file-map/src/__tests__/index-test.js @@ -1859,6 +1859,65 @@ describe('FileMap', () => { }, ); + test.each([ + [false, [p('/project/fruits'), p('/project/vegetables')]], + [true, []], + ])( + 'starts watching while the cache is read (useWatchman: %s)', + async (useWatchman, watchedDuringRead) => { + let watchedRoots: ?Array = null; + if (!useWatchman) { + mockNodeCrawler.mockImplementationOnce(async () => ({ + changedFiles: new Map(), + removedFiles: new Set(), + })); + } + mockCacheManager.read.mockImplementationOnce(async () => { + await new Promise(resolve => setTimeout(resolve, 10)); + watchedRoots = Object.keys(mockEmitters); + return null; + }); + + const {fileMap} = await buildNewFileMap({useWatchman, watch: true}); + await fileMap.end(); + + // Watchman backends start after the crawl, from its clocks. + expect(watchedRoots).toEqual(watchedDuringRead); + }, + ); + + test('end() while the cache is read stops the watch started meanwhile', async () => { + let finishRead: () => void = () => {}; + mockCacheManager.read.mockImplementationOnce( + () => + new Promise(resolve => { + finishRead = () => resolve(null); + }), + ); + const fileMap = new FileMap({ + ...defaultConfig, + plugins: [], + useWatchman: false, + watch: true, + }); + + const built = fileMap.build(); + await new Promise(resolve => setTimeout(resolve, 10)); + const backends = Object.values(mockEmitters); + expect(backends).toHaveLength(2); + const stops = backends.map(backend => + jest.spyOn(backend, 'stopWatching'), + ); + await fileMap.end(); + finishRead(); + + await expect(built).rejects.toMatchObject({name: 'AbortError'}); + for (const stop of stops) { + expect(stop).toHaveBeenCalled(); + } + expect(mockNodeCrawler).not.toHaveBeenCalled(); + }); + test('does not miss a change made while the file map is being built', async () => { mockNodeCrawler.mockImplementationOnce(async () => { // The watcher is already running at this point, so this reports a diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index 926bd5b3b0..9fecafdfed 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -409,10 +409,13 @@ export default class FileMap extends EventEmitter { this.#startupPerfLogger?.point('build_start'); if (!this.#buildPromise) { this.#buildPromise = (async () => { - let initialData: ?CacheData; - if (this.#options.resetCache !== true) { - initialData = await this.read(); - } + // Neither the Watchman capability check nor what must be running + // before the crawl (in watch mode without Watchman, a confirmed watch) + // depends on the cache, so both start while the cache is read. + const [initialData] = await Promise.all([ + this.#options.resetCache !== true ? this.read() : null, + this.#createWatcher().then(watcher => watcher.prepareCrawl()), + ]); if (!initialData) { debug('Not using a cache'); } else { @@ -726,14 +729,7 @@ export default class FileMap extends EventEmitter { return data; } - /** - * 2. crawl the file system. - */ - async #buildFileDelta( - previousState: CrawlerOptions['previousState'], - ): Promise { - this.#startupPerfLogger?.point('buildFileDelta_start'); - + async #createWatcher(): Promise { const { computeSha1, enableSymlinks, @@ -763,7 +759,6 @@ export default class FileMap extends EventEmitter { }, ignorePatternForWatch: ignorePattern, perfLogger: this.#startupPerfLogger, - previousState, rootDir, roots, // A supplied crawler makes the Watchman capability probe - which spawns @@ -773,10 +768,20 @@ export default class FileMap extends EventEmitter { watchmanDeferStates, }); const watcher = this.#watcher; - watcher.on('status', status => this.emit('status', status)); + return watcher; + } - const result = await watcher.crawl(); + /** + * 2. crawl the file system. + */ + async #buildFileDelta( + previousState: CrawlerOptions['previousState'], + ): Promise { + this.#startupPerfLogger?.point('buildFileDelta_start'); + const watcher = this.#watcher; + invariant(watcher != null, 'Expected #createWatcher() to have been called'); + const result = await watcher.crawl(previousState); this.#startupPerfLogger?.point('buildFileDelta_end'); return result; }