From fcf8d86b2a2a47289dde77999e021083e01e5c23 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Wed, 11 Nov 2026 19:15:35 +0000 Subject: [PATCH] metro-file-map: Write the cache without copying the file map first `TreeFS.getSerializableSnapshot()` deep-copies the whole tree - a `Map` per directory and an array per file - before every cache write: https://github.com/react/metro/blob/1461d81e97fa8fb83a1017925c2870e260d833cc/packages/metro-file-map/src/lib/TreeFS.js#L1476-L1486 `DiskCacheManager` serialises the snapshot as soon as it asks for it, so nothing can change it in between and the copy is thrown away: https://github.com/react/metro/blob/1461d81e97fa8fb83a1017925c2870e260d833cc/packages/metro-file-map/src/cache/DiskCacheManager.js#L119-L121 This returns the tree itself. That saves a copy of the file map on every cold start, and on every save of the cache while watching. A custom cache manager (`unstable_fileMapCacheManagerFactory`) that holds on to the snapshot to serialise later would now see later changes, so `CacheManager.write`'s docs now say to serialise what `getSnapshot` returns straight away, or copy it. Expo's fork (`@expo/metro-file-map`) already skips the deep copy, for the same reason. ## Benchmark This takes 28-51% off writing the cache (Windows: 19.4ms -> 9.5ms), or 2-5% off a cold start on Linux and Windows. macOS's cold starts are too noisy to show it. The same copy was made on every save of the cache while watching, which this benchmark doesn't exercise. | | `main` | #2108 | This PR | vs `main` | vs #2108 | |---|---|---|---|---|---| | macOS: Cache write in a cold start | 7.8ms | 6.6ms | 4.3ms | -44.4% (-45.1% to -43.6%) | -34.2% (-35.4% to -33.7%) | | Linux: Cache write in a cold start | 30ms | 28ms | 20ms | -31.9% (-39.7% to -22.0%) | -27.9% (-32.0% to -25.1%) | | Windows: Cache write in a cold start | 20ms | 19.4ms | 9.5ms | -53.5% (-54.9% to -36.4%) | -51.0% (-53.2% to -32.1%) | | macOS: Cold start | 129ms | 96ms | 90ms | -30.2% (-36.0% to -22.6%) | -5.7% (-14.6% to +8.9%) | | Linux: Cold start | 648ms | 324ms | 319ms | -50.8% (-51.7% to -50.1%) | -1.8% (-5.0% to -0.5%) | | Windows: Cold start | 363ms | 179ms | 170ms | -53.2% (-53.8% to -52.5%) | -5.1% (-6.8% to -2.6%) | | macOS: Watch-mode start, cold cache | 134ms | 98ms | 96ms | -28.6% (-29.5% to -27.0%) | -2.7% (-4.5% to +0.2%) | | Linux: Watch-mode start, cold cache | 1,892ms | 470ms | 451ms | -76.1% (-76.3% to -75.8%) | -3.9% (-4.9% to -0.9%) | | Windows: Watch-mode start, cold cache | 2,556ms | 496ms | 490ms | -80.8% (-80.9% to -80.8%) | -1.1% (-1.6% to -0.7%) | ## Changelog ``` - **[Performance]**: metro-file-map: Write the cache without copying the file map first ``` ## Test plan New unit test that `DiskCacheManager` serialises the snapshot before anything else can run. Benchmark as in #2104, with 24 rounds on macOS. --- .../cache/__tests__/DiskCacheManager-test.js | 28 +++++++++++++++++++ packages/metro-file-map/src/flow-types.js | 5 +++- packages/metro-file-map/src/lib/TreeFS.js | 14 +--------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js b/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js index e00d96feb7..def4f67cbc 100644 --- a/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js +++ b/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js @@ -215,6 +215,34 @@ describe('cacheManager', () => { ); }); + test('serialises the snapshot before anything can change it', async () => { + const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig); + // The file map's own state, which it changes once control returns to the + // event loop. + const fileSystemData = new Map(); + const snapshot: CacheData = { + clocks: new Map(), + fileSystemData, + plugins: new Map(), + }; + const written = serialize(snapshot); + const getSnapshot = () => { + queueMicrotask(() => { + fileSystemData.set('later.js', [0, 0, 0, null, 0]); + }); + return snapshot; + }; + await cacheManager.write(getSnapshot, { + changedSinceCacheRead: true, + eventSource: {onChange: () => () => {}}, + onWriteError: () => {}, + }); + expect(mockWriteFile).toHaveBeenCalledWith( + cacheManager.getCacheFilePath(), + written, + ); + }); + test('creates the cache directory if it does not exist', async () => { const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig); const getSnapshot = jest.fn( diff --git a/packages/metro-file-map/src/flow-types.js b/packages/metro-file-map/src/flow-types.js index 988326af0d..3ff7057e1a 100644 --- a/packages/metro-file-map/src/flow-types.js +++ b/packages/metro-file-map/src/flow-types.js @@ -51,7 +51,9 @@ export interface CacheManager { * crawler - i.e. internal state reflects the current file system state. * * getSnapshot may be retained and called at any time before end(), such as - * in response to eventSource 'change' events. + * in response to eventSource 'change' events. What it returns is the file + * map's own state, not a copy, and changes once control returns to the + * event loop. Serialize it before then, or copy it to keep it. */ write( getSnapshot: () => CacheData, @@ -392,6 +394,7 @@ export interface FileSystem { removedFiles: Set, filesToStat: Map, }; + // The file system's own data, not a copy (see `CacheManager.write`). getSerializableSnapshot(): CacheData['fileSystemData']; getSha1(file: Path): ?string; getOrComputeSha1(file: Path): Promise; diff --git a/packages/metro-file-map/src/lib/TreeFS.js b/packages/metro-file-map/src/lib/TreeFS.js index a2b1ea7ec2..29327240a3 100644 --- a/packages/metro-file-map/src/lib/TreeFS.js +++ b/packages/metro-file-map/src/lib/TreeFS.js @@ -207,7 +207,7 @@ export default class TreeFS implements MutableFileSystem { } getSerializableSnapshot(): CacheData['fileSystemData'] { - return this.#cloneTree(this.#rootNode); + return this.#rootNode; } static fromDeserializedSnapshot(args: DeserializedSnapshotInput): TreeFS { @@ -1490,16 +1490,4 @@ export default class TreeFS implements MutableFileSystem { } return result.node; } - - #cloneTree(root: DirectoryNode): DirectoryNode { - const clone: DirectoryNode = new Map(); - for (const [name, node] of root) { - if (isDirectory(node)) { - clone.set(name, this.#cloneTree(node)); - } else { - clone.set(name, [...node]); - } - } - return clone; - } }