Skip to content

Commit 97d1a02

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 9d5006a commit 97d1a02

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
@@ -2852,6 +2852,183 @@ function dep(name: string): TransformResultDependency {
28522852
});
28532853
});
28542854

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