From 1fab9895a7739241bba19a67a113a4c2f2c950e0 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:20:00 +0000 Subject: [PATCH] metro-file-map: Skip the per-file worker pass when no file needs a worker With Metro's defaults - lazy SHA-1s, no `hasteImplModulePath` and `enableGlobalPackages: false` - no file needs a worker at startup. Even so, every crawled file goes through `FileProcessor` to find that out: a check for `node_modules` in its path, a call to the Haste plugin's filter, which always says no, and an array for the plugins to run. This makes `HastePlugin.getWorker()` return `null` when the plugin has nothing to compute, so it gets no worker and no per-file data slot (as `MockPlugin` already does). `processBatch` then returns straight away when there's no SHA-1 to compute and no plugin runs on batches. Any plugin after Haste moves down a slot in each file's metadata, so this bumps `CACHE_BREAKER`. ## Benchmark This takes 13-14% off applying the crawl on a cold start on macOS and Linux (Linux: 84ms -> 73ms), or 4-5% off a cold start on Linux and Windows (Linux: 315ms -> 298ms). macOS's cold starts are too noisy to show it. On Linux and Windows, a cache write takes one of two times (on Linux, about 14ms or 20ms), and this PR's writes take the shorter one more often (Linux: 12 of 20 runs against 5, Windows: 20 against 8). That's presumably because there's less garbage to collect, but it makes the cache write medians below jumpy. On macOS, writes are 0.1ms slower. | | `main` | #2219 | This PR | vs `main` | vs #2219 | |---|---|---|---|---|---| | macOS: Applying the crawl in a cold start | 18.7ms | 17.6ms | 15.1ms | -18.9% (-20.7% to -17.3%) | -14.2% (-16.2% to -12.5%) | | Linux: Applying the crawl in a cold start | 85ms | 84ms | 73ms | -14.1% (-16.4% to -10.2%) | -13.2% (-15.5% to -9.7%) | | Windows: Applying the crawl in a cold start | 47ms | 42ms | 40ms | -15.5% (-18.1% to -11.8%) | -6.2% (-13.1% to +0.1%) | | macOS: Cache write in a cold start | 7.8ms | 4.4ms | 4.5ms | -41.9% (-42.7% to -40.3%) | +3.5% (+2.6% to +4.6%) | | Linux: Cache write in a cold start | 33ms | 20ms | 14.0ms | -57.8% (-59.0% to -42.0%) | -30.5% (-32.3% to -5.1%) | | Windows: Cache write in a cold start | 20ms | 13.8ms | 9.4ms | -54.2% (-55.9% to -52.6%) | -32.1% (-34.8% to +2.1%) | | macOS: Cold start | 129ms | 95ms | 91ms | -29.8% (-38.6% to -24.2%) | -4.7% (-16.7% to +4.8%) | | Linux: Cold start | 646ms | 315ms | 298ms | -53.9% (-54.6% to -53.5%) | -5.4% (-7.2% to -4.6%) | | Windows: Cold start | 362ms | 172ms | 164ms | -54.7% (-55.7% to -53.7%) | -4.5% (-5.7% to -2.2%) | ## Changelog ``` - **[Performance]**: metro-file-map: Skip the per-file worker pass when no file needs a worker ``` ## Test plan New unit tests. Benchmark as in #2104, with 22 rounds on macOS. --- packages/metro-file-map/API.md | 2 +- .../src/__tests__/worker-test.js | 11 ++-- packages/metro-file-map/src/index.js | 2 +- .../metro-file-map/src/lib/FileProcessor.js | 22 ++++++-- .../src/lib/__tests__/FileProcessor-test.js | 55 ++++++++++++++++--- .../metro-file-map/src/plugins/HastePlugin.js | 30 +++++++--- .../haste/__tests__/HastePlugin-test.js | 28 +++++++++- 7 files changed, 121 insertions(+), 29 deletions(-) diff --git a/packages/metro-file-map/API.md b/packages/metro-file-map/API.md index 87af82ff8c..fa29769a4b 100644 --- a/packages/metro-file-map/API.md +++ b/packages/metro-file-map/API.md @@ -233,7 +233,7 @@ export class HastePlugin implements HasteMap, FileMapPlugin getModuleNameByPath(mixedPath: Path): null | undefined | string; getPackage(name: string, platform: null | undefined | string, _supportsNativePlatform?: null | undefined | boolean): null | undefined | Path; getSerializableSnapshot(): null; - getWorker(): FileMapPluginWorker; + getWorker(): null | undefined | FileMapPluginWorker; initialize(opts: FileMapPluginInitOptions): Promise; readonly name: 'haste'; onChanged(delta: ReadonlyFileSystemChanges): void; diff --git a/packages/metro-file-map/src/__tests__/worker-test.js b/packages/metro-file-map/src/__tests__/worker-test.js index 8e57ac90da..1418aadb0a 100644 --- a/packages/metro-file-map/src/__tests__/worker-test.js +++ b/packages/metro-file-map/src/__tests__/worker-test.js @@ -17,6 +17,7 @@ import {HastePlugin} from '..'; import {Worker} from '../worker'; import * as path from 'node:path'; import * as vm from 'node:vm'; +import nullthrows from 'nullthrows'; jest.mock('node:fs', () => { const path = require('node:path'); @@ -98,10 +99,12 @@ function workerWithHaste( ) { return new Worker({ plugins: [ - new HastePlugin({ - ...defaultHasteConfig, - ...hasteOverrides, - }).getWorker().worker, + nullthrows( + new HastePlugin({ + ...defaultHasteConfig, + ...hasteOverrides, + }).getWorker(), + ).worker, ], }).processFile({ ...message, diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index a62ae0e7e4..926bd5b3b0 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -172,7 +172,7 @@ export type { // This should be bumped whenever a code change to `metro-file-map` itself // would cause a change to the cache data structure and/or content (for a given // filesystem state and build parameters). -const CACHE_BREAKER = '15'; +const CACHE_BREAKER = '16'; const CHANGE_INTERVAL = 30; diff --git a/packages/metro-file-map/src/lib/FileProcessor.js b/packages/metro-file-map/src/lib/FileProcessor.js index 72bc41346f..5cafd2a021 100644 --- a/packages/metro-file-map/src/lib/FileProcessor.js +++ b/packages/metro-file-map/src/lib/FileProcessor.js @@ -53,6 +53,8 @@ const NODE_MODULES_SEP = 'node_modules' + sep; const MAX_FILES_PER_WORKER = 100; export class FileProcessor { + // Whether any plugin's worker runs on a batch, rather than on demand. + #hasEagerPluginWorker: boolean; #maxFilesPerWorker: number; #maxWorkers: number; #perfLogger: ?PerfLogger; @@ -72,6 +74,9 @@ export class FileProcessor { this.#maxFilesPerWorker = opts.maxFilesPerWorker ?? MAX_FILES_PER_WORKER; this.#maxWorkers = opts.maxWorkers; this.#pluginWorkers = opts.pluginWorkers ?? []; + this.#hasEagerPluginWorker = this.#pluginWorkers.some( + plugin => plugin.lazy !== true, + ); this.#inBandWorker = new Worker({ plugins: this.#pluginWorkers.map(plugin => plugin.worker), }); @@ -90,6 +95,17 @@ export class FileProcessor { }> { const errors = []; + if (req.maybeReturnContent) { + throw new Error( + 'Batch processing does not support returning file contents', + ); + } + + if (!req.computeSha1 && !this.#hasEagerPluginWorker) { + // No file needs a worker. + return {errors}; + } + const workerJobs = files .map(([normalFilePath, fileMetadata]) => { const maybeWorkerInput = this.#getWorkerInput( @@ -110,12 +126,6 @@ export class FileProcessor { ); const batchWorker = this.#getBatchWorker(numWorkers); - if (req.maybeReturnContent) { - throw new Error( - 'Batch processing does not support returning file contents', - ); - } - await Promise.all( workerJobs.map(([workerInput, fileMetadata]) => { return batchWorker diff --git a/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js b/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js index 2dfbd426d3..000933e266 100644 --- a/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js +++ b/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js @@ -20,6 +20,7 @@ import H from '../../constants'; import DependencyPlugin from '../../plugins/DependencyPlugin'; import HastePlugin from '../../plugins/HastePlugin'; import path from 'node:path'; +import nullthrows from 'nullthrows'; const MockJestWorker = jest.fn().mockImplementation(() => ({ processFile: async () => ({}), @@ -276,6 +277,43 @@ describe('processBatch', () => { expect(mockWorkerFn).not.toHaveBeenCalled(); }); + test.each([ + ['no plugins', [] as ReadonlyArray], + [ + 'only lazy plugins', + [ + { + worker: {modulePath: 'mock-lazy-plugin', setupArgs: {}}, + filter: () => true, + lazy: true, + }, + ], + ], + ])( + 'reads no file without a SHA-1 to compute and with %s', + async (_, pluginWorkers) => { + const readKeys: Array = []; + const fileMetadata = new Proxy( + [123, 234, 0, null, 0, null], + { + get(target, key, receiver) { + readKeys.push(key); + return Reflect.get(target, key, receiver); + }, + }, + ); + const processor = new FileProcessor({...defaultOptions, pluginWorkers}); + + await processor.processBatch([[p('src/index.js'), fileMetadata]], { + computeSha1: false, + maybeReturnContent: false, + }); + + expect(readKeys).toEqual([]); + expect(mockWorkerFn).not.toHaveBeenCalled(); + }, + ); + describe('node_modules files', () => { const nodeModulesFiles = (): Array<[string, FileMetadata]> => [ [p('node_modules/pkg/index.js'), [123, 234, 0, null, 0, null]], @@ -335,13 +373,16 @@ describe('processBatch', () => { test('built-in plugins only run on them when a SHA-1 is needed', async () => { const pluginWorkers = [ - new HastePlugin({ - enableHastePackages: true, - failValidationOnConflicts: false, - hasteImplModulePath: require.resolve('../../__tests__/haste_impl.js'), - platforms: new Set(), - rootDir: defaultOptions.rootDir, - }).getWorker(), + nullthrows( + new HastePlugin({ + enableHastePackages: true, + failValidationOnConflicts: false, + hasteImplModulePath: + require.resolve('../../__tests__/haste_impl.js'), + platforms: new Set(), + rootDir: defaultOptions.rootDir, + }).getWorker(), + ), new DependencyPlugin({ computeDependencies: true, dependencyExtractor: null, diff --git a/packages/metro-file-map/src/plugins/HastePlugin.js b/packages/metro-file-map/src/plugins/HastePlugin.js index 64a7d93fdd..207587fc02 100644 --- a/packages/metro-file-map/src/plugins/HastePlugin.js +++ b/packages/metro-file-map/src/plugins/HastePlugin.js @@ -108,14 +108,16 @@ export default class HastePlugin await new Promise(setImmediate); } } - this.#getModuleNameByPath = mixedPath => { - const result = opts.getOrComputeFileData(mixedPath); - return result.exists && - result.type === 'f' && - typeof result.data === 'string' - ? result.data - : null; - }; + this.#getModuleNameByPath = this.#hasHasteNames() + ? mixedPath => { + const result = opts.getOrComputeFileData(mixedPath); + return result.exists && + result.type === 'f' && + typeof result.data === 'string' + ? result.data + : null; + } + : () => null; this.#perfLogger?.point('constructHasteMap_end'); this.#perfLogger?.annotate({int: {hasteFiles}}); } @@ -491,7 +493,17 @@ export default class HastePlugin ]); } - getWorker(): FileMapPluginWorker { + // Files have Haste names only through `hasteImplModulePath` or, for + // `package.json`, Haste packages. + #hasHasteNames(): boolean { + return this.#hasteImplModulePath != null || this.#enableHastePackages; + } + + getWorker(): ?FileMapPluginWorker { + if (!this.#hasHasteNames()) { + // Nothing to compute, so no file needs reading for this plugin. + return null; + } return { worker: { modulePath: require.resolve('./haste/worker.js'), diff --git a/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js b/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js index 00421797d7..ba68563ee5 100644 --- a/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js +++ b/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js @@ -50,6 +50,9 @@ describe.each([['win32'], ['posix']])('HastePlugin on %s', platform => { let HasteMap: Class; let DuplicateHasteCandidatesError; + const hasteImplModulePath = + require.resolve('../../../__tests__/haste_impl.js'); + const opts = { enableHastePackages: false, hasteImplModulePath: null, @@ -95,6 +98,29 @@ describe.each([['win32'], ['posix']])('HastePlugin on %s', platform => { expect(hasteMap.getModule('NameForFoo')).toEqual(p('/root/project/Foo.js')); }); + test.each([ + ['a hasteImplModulePath', {hasteImplModulePath}], + ['Haste packages', {enableHastePackages: true}], + ])('has a worker with %s', (_, overrides) => { + expect(new HasteMap({...opts, ...overrides}).getWorker()).not.toBeNull(); + }); + + test('has no worker or module names without a hasteImplModulePath or Haste packages', async () => { + const hasteMap = new HasteMap(opts); + expect(hasteMap.getWorker()).toBeNull(); + const getOrComputeFileData = jest.fn(); + await hasteMap.initialize({ + files: { + fileIterator: jest.fn().mockReturnValue([]), + lookup: jest.fn(), + }, + getOrComputeFileData, + pluginState: null, + }); + expect(hasteMap.getModuleNameByPath(p('/root/Foo.js'))).toBeNull(); + expect(getOrComputeFileData).not.toHaveBeenCalled(); + }); + describe('remove file', () => { let hasteMap: HasteMapType; let removeFile: (path: CanonicalPath, name: ?string) => void; @@ -184,7 +210,7 @@ describe.each([['win32'], ['posix']])('HastePlugin on %s', platform => { let getOrComputeFileData; beforeEach(async () => { - hasteMap = new HasteMap(opts); + hasteMap = new HasteMap({...opts, hasteImplModulePath}); getOrComputeFileData = jest.fn().mockReturnValue(null); await hasteMap.initialize({