Repository navigation
Commit 1c450cd
committed
Remove PackageCache's per-module closest package memo
Stacked on #1946.
## Summary
`PackageCache` remembers the closest `package.json` for every module path it's asked about, with a reverse index from each `package.json` to the module paths under it and a set of paths known to have no package, both of which exist only to invalidate the first. `DependencyGraph` calls `invalidate` for every changed file to keep it honest.
With the previous two PRs that memo no longer pays for itself. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #1946 made each one a walk of single map lookups, 2.1x faster (870ns -> 406ns). This removes the memo and both indices, and looks up the closest package on every call. The parsed contents of each `package.json` are still cached per path, and `invalidate` now just drops those.
The other reason to do this is incremental resolution (#1944). Any cache that sits between the resolver and the file system has to store the observations that produced each entry and replay them on a hit, or the second resolution to hit the cache can never be invalidated. That's a fair amount of machinery to add to a cache that's worth 3% of first-build resolution time, which is itself a small part of a build.
This only makes sense on top of the stack - at #1944, removing the memo costs 14% on a first pass, and nearly doubles a pass that re-resolves everything.
\* Described in the test plan.
## Bugs fixed
This also fixes two invalidation bugs, both present on main, where the watcher picks up a change on disk but resolution keeps its old answer until Metro restarts:
- A nearer `package.json` is added, e.g. creating a nested package in a monorepo. The memo is invalidated through a reverse index from each `package.json` to the modules that resolved to it, and a new `package.json` has no entries, so the modules beneath it keep pointing at the farther one. (It's masked when there was previously no enclosing package at all, which is special-cased.)
- A directory symlink is retargeted into another package. Entries are keyed by the path as it was asked for, through the link, and only the link's own path is invalidated.
Both changes reset the graph, so everything is re-resolved - against the stale memo, which is probably why they've gone unnoticed. Reproduced with a real `DependencyGraph` and watcher on main and on #1946, using a `browser` redirect in the affected `package.json` to show which one was consulted. With no memo there's nothing to go stale - every lookup walks the live file map.
## Benchmark
Resolution time in isolation, replaying every `DependencyGraph.resolveDependency` call made by one iOS dev build of the benchmark app. Nothing is transformed or serialised, so these are percentages of resolution, not of a build. This goes through `DependencyGraph`'s whole-resolution cache, which is cleared only by real file change events, so each row is something that actually happens in a session. 7 interleaved rounds, medians of 15 iterations. Resolver output is byte-identical.
| scenario | #1946 | this diff |
|---|---|---|
| first build (ms) | 92.9 | 95.6 (+2.9%) |
| rebuild, nothing changed (ms) | 7.58 | 7.61 |
| after a source edit, re-resolving the edited module's 8 dependencies (µs) | 155 | 155 |
| after a `package.json` change, re-resolving everything (ms) | 73.6 | 80.4 (+9.3%) |
| heap retained after resolving (MB) | 3.0 | **1.9 (−37%)** |
The tradeoffs are:
- ~3% of resolution time on a first build (3ms here). A real build does far more than resolve - the same build takes around 4s end to end with a warm transform cache - so that's under 0.1% of it.
- ~9% (7ms here) when a `package.json` or symlink change resets the whole graph, which is the only time everything is re-resolved against a warm `PackageCache`. Incremental resolution aims to remove those full resets.
- Nothing on an ordinary edit. Metro re-resolves only the edited module's dependencies, and the difference there is within noise.
## Test plan
```
yarn jest packages/metro-file-map packages/metro-resolver packages/metro/src/node-haste packages/metro/src/DeltaBundler packages/metro/src/integration_tests
yarn flow check
yarn verify-api-snapshots
```
`PackageCache-test.js` loses its tests of memo invalidation, along with the structures they tested - with no memo, invalidation is a non-issue by construction. What's left covers the one remaining cache: each `package.json` is parsed once however many modules share it, and is re-read after it's invalidated.
**Benchmark app** - a synthetic mid-size RN 0.87 app: 23 common dependencies (Reanimated, React Navigation, TanStack Query, lodash, etc.) and ~1,000 generated first-party modules, 60% of them in a workspace package outside `projectRoot` with its own `node_modules`. The iOS dev bundle is 2.8k modules, from a file map of 34k files. Timings replay the `resolveDependency` calls a real build of it makes against a fresh `DependencyGraph` per process, in interleaved rounds, with source and `package.json` edits made on disk and picked up by the watcher. These timings isolate resolution - no transformation, serialisation or file reads are included. A real build of this app takes around 4s even with a warm transform cache, so resolution is about 2% of it.
Changelog:
```
- **[Fix]**: Resolver: Fix stale resolutions after adding a nearer `package.json` or retargeting a symlinked directory, which previously needed a restart
```1 parent 9aa3724 commit 1c450cd
2 files changed
Lines changed: 61 additions & 427 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
| 15 | + | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | 43 | | |
50 | 44 | | |
51 | 45 | | |
| |||
55 | 49 | | |
56 | 50 | | |
57 | 51 | | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | 52 | | |
62 | 53 | | |
63 | 54 | | |
| |||
75 | 66 | | |
76 | 67 | | |
77 | 68 | | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
78 | 73 | | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | 74 | | |
89 | 75 | | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | 76 | | |
108 | 77 | | |
109 | | - | |
110 | | - | |
111 | | - | |
| 78 | + | |
| 79 | + | |
112 | 80 | | |
113 | 81 | | |
114 | 82 | | |
115 | 83 | | |
116 | | - | |
117 | | - | |
118 | 84 | | |
119 | 85 | | |
120 | 86 | | |
121 | 87 | | |
122 | | - | |
123 | | - | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | 88 | | |
160 | 89 | | |
0 commit comments