Skip to content

Commit 04896ea

Browse files
robhoganrobhogan
committed
Resolver: Collect file system observations per resolution under unstable_incrementalResolution
Stacked on #1948. ## Summary The first consumer of the `Observations` API from #1944. When `resolver.unstable_incrementalResolution` is enabled, `ModuleResolver.resolveDependency` creates an observations record at the start of each resolution, and everything that resolution looks up in the file map is recorded into it. The record is returned on the `BundlerResolution` as `unstable_observations`. Nothing reads it yet - invalidating from it is the next step - so there's no behaviour change, and with the flag off nothing is allocated. The resolution context given to `metro-resolver` doesn't change. `ModuleResolver` already builds a context per resolution, so when the flag is on it binds the file system capabilities to that resolution's record (`fileSystemLookup: p => fileSystemLookup(p, observations)`), and when it's off it passes the same functions as before. That means a custom `resolveRequest` records whatever it looks up through the context, without knowing about any of this. The observations parameter only appears on Metro's internal option types. What's recorded: - `fileSystemLookup`, and `resolveAsset` through it. - `getPackageForModule`, down to `hierarchicalLookup`. There's no memo in the way since #1948. - `doesFileExist`. `FileSystem.exists` can't record, so when recording this is answered by `lookup`, which agrees with it - true for exactly the paths found to be a file. Putting the record on the `BundlerResolution` means `DependencyGraph`'s whole-resolution cache replays it for free - every origin given a cached resolution gets its observations too. The one other memo is the empty module, which is resolved once and cached, so a resolution that lands on it returns a copy with the union of what led there and what resolving the empty module observed. Not included, to follow: - The contents of `package.json`. `getPackage` reads with `fs` outside the file map, and Metro has no way to turn an absolute path into a canonical one. The file is recorded in `existence` by `hierarchicalLookup`, so adding or removing one is covered, but editing one isn't. - Haste lookups, which have no bucket yet. - Failed resolutions. They throw, so there's nowhere to put what they observed. ## Benchmark Resolution time in isolation, replaying every `DependencyGraph.resolveDependency` call made by one iOS dev build of the benchmark app (described in the test plan), through the whole-resolution cache. Nothing is transformed or serialised, so these are percentages of resolution, not of a build. 7 interleaved rounds. Resolutions are identical with the flag on and off. | scenario | #1948 | this diff, flag off | this diff, flag on | |---|---|---|---| | first build (ms) | 95.9 | 95.8 (−0.1%) | 111.6 (+16.5%) | | rebuild, nothing changed (ms) | 7.61 | 7.64 (+0.4%) | 7.80 (+2.1%) | | after a source edit, re-resolving the edited module's dependencies (µs) | 153 | 159 (+4.1%) | 180 (+13.2%) | | after a `package.json` change, re-resolving everything (ms) | 79.8 | 80.1 (+0.4%) | 93.1 (+16.2%) | | heap retained after resolving (MB) | | 5.3 | 18.8 (3.5x) | Flag off is within noise of #1948. (The source edit row is 8 resolutions, so that's under a microsecond each, and the runs overlap.) Flag on costs about 16% of resolution time wherever everything is resolved, which is ~2µs per resolution, and 3.5x the retained heap. On the benchmark app that's 100k entries across 6.9k distinct resolutions - a median of 14 each, 90th percentile 27 - naming 28k distinct paths. Nearly all of it is `existence`, and most of that is extension and platform candidates that missed (`a.ios.js`, `a.native.js`, ...), which can't collapse because the directory they're in exists. That's the memory to go after before this is worth turning on, e.g. by recording the directory once instead. ## 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 typecheck-ts yarn verify-api-snapshots ``` New tests in `resolver-test.js` assert the exact sets recorded for a relative import's candidates, each level probed for a package, a symlink (recorded as `content`, with lookups recorded by real path), asset variants, and a resolution that lands on the empty module. They also check that there's no `unstable_observations` by default, and that origins sharing a cached resolution share its observations. **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. A real build of this app takes around 4s even with a warm transform cache, so resolution is about 2% of it. Changelog: Internal
1 parent 7d2a2d5 commit 04896ea

6 files changed

Lines changed: 324 additions & 26 deletions

File tree

‎packages/metro/src/DeltaBundler/__tests__/resolver-test.js‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2853,6 +2853,183 @@ function dep(name: string): TransformResultDependency {
28532853
});
28542854
});
28552855

2856+
describe('unstable_incrementalResolution', () => {
2857+
const incremental: InputConfigT = {
2858+
resolver: {unstable_incrementalResolution: true},
2859+
};
2860+
// Observed paths are canonical: relative to the root, with system
2861+
// separators.
2862+
const canonical = (...posixPaths: Array<string>) =>
2863+
new Set(posixPaths.map(posixPath => joinPath(...posixPath.split('/'))));
2864+
2865+
test('resolutions carry no observations by default', async () => {
2866+
setMockFileSystem({'index.js': '', 'a.js': ''});
2867+
resolver = await createResolver();
2868+
expect(
2869+
resolver.resolve(p('/root/index.js'), dep('./a')),
2870+
).not.toHaveProperty('unstable_observations');
2871+
});
2872+
2873+
test('records every candidate probed for a relative import', async () => {
2874+
setMockFileSystem({'index.js': '', 'a.js': ''});
2875+
resolver = await createResolver(incremental, 'ios');
2876+
expect(resolver.resolve(p('/root/index.js'), dep('./a'))).toEqual({
2877+
type: 'sourceFile',
2878+
filePath: p('/root/a.js'),
2879+
unstable_observations: {
2880+
// A more specific candidate appearing changes the result
2881+
existence: canonical(
2882+
'a',
2883+
'a.ios.js',
2884+
'a.native.js',
2885+
'a.js',
2886+
'package.json',
2887+
),
2888+
content: new Set(),
2889+
},
2890+
});
2891+
});
2892+
2893+
test('records each level probed for a package', async () => {
2894+
setMockFileSystem({
2895+
deep: {dir: {'index.js': ''}},
2896+
node_modules: {
2897+
pkg: {
2898+
'package.json': JSON.stringify({name: 'pkg', main: 'main.js'}),
2899+
'main.js': '',
2900+
},
2901+
},
2902+
});
2903+
resolver = await createResolver(incremental, 'ios');
2904+
expect(
2905+
resolver.resolve(p('/root/deep/dir/index.js'), dep('pkg')),
2906+
).toEqual({
2907+
type: 'sourceFile',
2908+
filePath: p('/root/node_modules/pkg/main.js'),
2909+
unstable_observations: {
2910+
existence: canonical(
2911+
// The closest package to the origin
2912+
'deep/dir/index.js',
2913+
'deep/dir/package.json',
2914+
'deep/package.json',
2915+
'package.json',
2916+
// Each node_modules on the way up. A missing one is one entry,
2917+
// however many packages are looked up beneath it.
2918+
'deep/dir/node_modules',
2919+
'deep/node_modules',
2920+
'node_modules',
2921+
// A file is preferred to a package of the same name
2922+
'node_modules/pkg.ios.js',
2923+
'node_modules/pkg.native.js',
2924+
'node_modules/pkg.js',
2925+
'node_modules/pkg.ios.json',
2926+
'node_modules/pkg.native.json',
2927+
'node_modules/pkg.json',
2928+
'node_modules/pkg',
2929+
'node_modules/pkg/package.json',
2930+
'node_modules/pkg/main.js',
2931+
),
2932+
content: new Set(),
2933+
},
2934+
});
2935+
});
2936+
2937+
test('records a traversed symlink as content, and lookups by real path', async () => {
2938+
setMockFileSystem({'index.js': '', real: {'x.js': ''}});
2939+
fs.symlinkSync(p('/root/real'), p('/root/link'));
2940+
resolver = await createResolver(incremental, 'ios');
2941+
expect(resolver.resolve(p('/root/index.js'), dep('./link/x'))).toEqual({
2942+
type: 'sourceFile',
2943+
filePath: p('/root/real/x.js'),
2944+
unstable_observations: {
2945+
existence: canonical(
2946+
'real/x',
2947+
'real/x.ios.js',
2948+
'real/x.native.js',
2949+
'real/x.js',
2950+
'real/package.json',
2951+
'package.json',
2952+
),
2953+
// Retargeting the link changes what every path through it means
2954+
content: canonical('link'),
2955+
},
2956+
});
2957+
});
2958+
2959+
test('records every asset variant probed', async () => {
2960+
setMockFileSystem({
2961+
'index.js': '',
2962+
'asset.png': '',
2963+
'asset@2x.png': '',
2964+
});
2965+
resolver = await createResolver(incremental, 'ios');
2966+
expect(
2967+
resolver.resolve(p('/root/index.js'), dep('./asset.png'))
2968+
.unstable_observations,
2969+
).toEqual({
2970+
existence: canonical(
2971+
'asset.png',
2972+
'asset@1x.png',
2973+
'asset@1.5x.png',
2974+
'asset@2x.png',
2975+
'asset@3x.png',
2976+
'asset@4x.png',
2977+
'package.json',
2978+
),
2979+
content: new Set(),
2980+
});
2981+
});
2982+
2983+
test('a resolution landing on the empty module includes what resolving the empty module observed', async () => {
2984+
setMockFileSystem({
2985+
'empty.js': '',
2986+
withBrowser: {
2987+
'package.json': JSON.stringify({
2988+
name: 'with-browser',
2989+
browser: {'./gone.js': false},
2990+
}),
2991+
'index.js': '',
2992+
'gone.js': '',
2993+
},
2994+
});
2995+
resolver = await createResolver(
2996+
{
2997+
resolver: {
2998+
emptyModulePath: p('/root/empty.js'),
2999+
unstable_incrementalResolution: true,
3000+
},
3001+
},
3002+
'ios',
3003+
);
3004+
expect(
3005+
resolver.resolve(p('/root/withBrowser/index.js'), dep('./gone')),
3006+
).toEqual({
3007+
type: 'sourceFile',
3008+
filePath: p('/root/empty.js'),
3009+
unstable_observations: {
3010+
existence: canonical(
3011+
// What led to the empty module
3012+
'withBrowser/gone',
3013+
'withBrowser/package.json',
3014+
// What resolving the empty module observed
3015+
'empty.js',
3016+
'package.json',
3017+
),
3018+
content: new Set(),
3019+
},
3020+
});
3021+
});
3022+
3023+
test('origins sharing a cached resolution share its observations', async () => {
3024+
setMockFileSystem({'index.js': '', 'other.js': '', 'a.js': ''});
3025+
resolver = await createResolver(incremental, 'ios');
3026+
const first = resolver.resolve(p('/root/index.js'), dep('./a'));
3027+
const second = resolver.resolve(p('/root/other.js'), dep('./a'));
3028+
expect(second).toBe(first);
3029+
expect(second.unstable_observations?.existence.size).toBeGreaterThan(0);
3030+
});
3031+
});
3032+
28563033
describe('schemeResolvers', () => {
28573034
test('config schemeResolvers are applied to scheme-prefixed specifiers', async () => {
28583035
setMockFileSystem({'index.js': '', 'a.js': ''});

‎packages/metro/src/DeltaBundler/types.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,21 @@ export type AllowOptionalDependenciesWithOptions = {
151151
export type AllowOptionalDependencies =
152152
boolean | AllowOptionalDependenciesWithOptions;
153153

154+
// What a resolution observed of the file system: canonical paths whose
155+
// addition or removal can change its result, and those whose modification
156+
// can too. Satisfies `metro-file-map`'s `Observations`, which populates it.
157+
export type ResolutionObservations = Readonly<{
158+
existence: Set<string>,
159+
content: Set<string>,
160+
}>;
161+
154162
export type BundlerResolution = Readonly<{
155163
type: 'sourceFile',
156164
filePath: string,
165+
// What this resolution observed of the file system, if
166+
// `resolver.unstable_incrementalResolution` is enabled. It is shared by
167+
// every caller given this resolution, so must not be mutated.
168+
unstable_observations?: ResolutionObservations,
157169
}>;
158170

159171
export type Options<T = MixedOutput> = Readonly<{

‎packages/metro/src/node-haste/DependencyGraph.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import type {
1313
BundlerResolution,
14+
ResolutionObservations,
1415
TransformResultDependency,
1516
} from '../DeltaBundler/types';
1617
import type {ResolverInputOptions} from '../shared/types';
@@ -130,8 +131,8 @@ export default class DependencyGraph extends EventEmitter {
130131
);
131132
this._resolutionCache = new Map();
132133
this.#packageCache = new PackageCache({
133-
getClosestPackage: absoluteModulePath =>
134-
this._getClosestPackage(absoluteModulePath),
134+
getClosestPackage: (absoluteModulePath, observations) =>
135+
this._getClosestPackage(absoluteModulePath, observations),
135136
});
136137
this._createModuleResolver();
137138
});
@@ -175,6 +176,7 @@ export default class DependencyGraph extends EventEmitter {
175176

176177
_getClosestPackage(
177178
absoluteModulePath: string,
179+
observations?: ?ResolutionObservations,
178180
): ?{packageJsonPath: string, packageRelativePath: string} {
179181
const result = this._fileSystem.hierarchicalLookup(
180182
absoluteModulePath,
@@ -183,6 +185,7 @@ export default class DependencyGraph extends EventEmitter {
183185
breakOnSegment: 'node_modules',
184186
subpathType: 'f',
185187
},
188+
observations,
186189
);
187190
return result
188191
? {

0 commit comments

Comments
 (0)