Skip to content

Commit 581a322

Browse files
committed
metro-file-map: Check hierarchicalLookup candidates from their own nodes
Stacked on #1945. `hierarchicalLookup` walks up from a start path asking whether each ancestor contains a given subpath - it's how `DependencyGraph` finds the closest `package.json`. For every candidate ancestor it joined the candidate's path to the subpath and traversed the result from the root again, even though the traversal that collected the ancestors already holds each candidate's node. The loop even built a `start` argument for `#lookupByNormalPath` to resume from, but it was never read. Each candidate is now checked from its own node: - When the subpath is a single path segment, which is the only way Metro calls it, with one `Map.get`. Whether that applies is decided once per call, by checking the subpath for a path separator. - Otherwise, by traversing only the subpath from the candidate. Expo CLI's `createTypescriptResolver` calls it this way, with deep subpaths such as `node_modules/${targetPath}/tsconfig.json`. Some single segments still take the traversal from the candidate: - A symlinked child, because we need to follow it to learn its type and real path. - A segment on an ancestor of the project root. Its node doesn't hold the segment that leads back down towards the root (with a root of `/A/B/C`, the node for `/A/B` has no `C` entry), so a map lookup there could wrongly report a directory as missing. A subpath that re-enters the project root from an ancestor of it, e.g. `B/C/a/package.json` from `/A`, is still traversed from the root. The joined path collapses to `a/package.json`, of which the candidate's path is no longer a prefix. An empty subpath, `.` and `..` also take the traversal, since a map lookup would answer those differently. The API is unchanged, and observations are recorded exactly as before - the existing test table, which asserts the recorded sets, passes unchanged. This also removes the unused `startPathIdx` and `startNode` options of `#lookupByNormalPath`. I also tried the more explicit alternative of having the caller pass either a `basename` or a `subpath`, which avoids the separator check. It measured 387ns per walk against 394ns for the separator check, within noise, and it changes the call shape for every caller, so it isn't worth it. ## Benchmark Each closest-package walk is 2.1x faster and each deep-subpath walk 1.4x faster, which takes 13% off first-pass resolution time against `main` on our benchmark app (described in the test plan). Resolver output is byte-identical. ### Walks Replaying the closest-package walks of one resolver pass directly against `hierarchicalLookup` (4,483 walks on the benchmark app), once for `package.json` and once for a deep subpath (`node_modules/react/package.json`) from the same start paths. Nanoseconds per walk, medians of 9 interleaved rounds. The middle rows are each half of this diff on its own, to show which half does what: | variant | `package.json` | deep subpath | |---|---|---| | `main` | 911 | 1,150 | | traversal from the candidate only | 598 (−34%) | 792 (−31%) | | `Map.get` for a single segment only | 426 (−53%) | 1,161 (+1%) | | this diff | **426 (−53%)** | **813 (−29%)** | The two halves are complementary: the map lookup is the bigger win for a single segment, and only the traversal from the candidate helps a deep subpath. Together they cost nothing on either path - the ~20ns behind "traversal only" on a deep subpath is the separator check. ### Resolver, end to end Resolution in isolation, 7 interleaved rounds. Only the first pass can move - the later passes are served by `PackageCache` and make no `hierarchicalLookup` calls, and are within ±2% of `main`. | metric | `main` | this diff | |---|---|---| | first pass after startup (ms) | 125.3 | **109.2 (−12.9%)** | ## Changelog Internal ## Test plan ``` yarn jest packages/metro-file-map packages/metro-resolver packages/metro/src/node-haste packages/metro/src/DeltaBundler/__tests__/resolver-test.js yarn flow check yarn typecheck-ts yarn verify-api-snapshots ``` New tests cover a directory-type lookup with a single-segment subpath, a deep subpath, and a subpath that re-enters the project root from an ancestor of it - that last one fails if traversal always starts from the candidate. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. **Benchmarks** - measured with this diff applied directly to `main` (at a95869a) rather than on top of #1945, so that the baseline is `main`. The ports substitute `main`'s `invalidatedBy` for #1944's observations, and give identical results for every walk, including the recorded paths. **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 resolutions a real build of it makes against a fresh `DependencyGraph` per process, in interleaved rounds. 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.
1 parent 727ba55 commit 581a322

2 files changed

Lines changed: 149 additions & 26 deletions

File tree

‎packages/metro-file-map/src/lib/TreeFS.js‎

Lines changed: 86 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -578,9 +578,9 @@ export default class TreeFS implements MutableFileSystem {
578578
// traversal, useful when adding files. Will throw if an expected
579579
// directory is already present as a file.
580580
makeDirectories?: boolean,
581-
startPathIdx?: number,
582-
startNode?: DirectoryNode,
583-
start?: {
581+
// Resume traversal from a known directory node, whose normal path is
582+
// the prefix of the requested path ending at pathIdx - 1.
583+
start?: ?{
584584
ancestorOfRootIdx: ?number,
585585
node: DirectoryNode,
586586
pathIdx: number,
@@ -845,6 +845,10 @@ export default class TreeFS implements MutableFileSystem {
845845
* X = dirname(X)
846846
* while X !== dirname(X)
847847
*
848+
* Each candidate is checked from its own node: a subpath that is a single
849+
* path segment, such as `package.json`, with one map lookup, and a deeper
850+
* subpath by traversing only the subpath.
851+
*
848852
* If `observations` is given, records the canonical paths this result
849853
* depends upon.
850854
*
@@ -864,6 +868,12 @@ export default class TreeFS implements MutableFileSystem {
864868
absolutePath: string,
865869
containerRelativePath: string,
866870
} {
871+
// Only a single path segment can be looked up on a candidate's own node.
872+
const isBasename =
873+
subpath !== '' &&
874+
subpath !== '.' &&
875+
subpath !== '..' &&
876+
!subpath.includes(path.sep);
867877
const ancestorsOfInput: Array<{
868878
ancestorOfRootIdx: ?number,
869879
node: DirectoryNode,
@@ -882,7 +892,11 @@ export default class TreeFS implements MutableFileSystem {
882892
subpath,
883893
opts.subpathType,
884894
observations,
885-
null,
895+
{
896+
ancestorOfRootIdx: closestLookup.ancestorOfRootIdx,
897+
node: closestLookup.node,
898+
},
899+
isBasename,
886900
);
887901
if (maybeAbsolutePathMatch != null) {
888902
return {
@@ -955,14 +969,8 @@ export default class TreeFS implements MutableFileSystem {
955969
subpath,
956970
opts.subpathType,
957971
observations,
958-
{
959-
ancestorOfRootIdx: candidate.ancestorOfRootIdx,
960-
node: candidate.node,
961-
pathIdx:
962-
candidate.normalPath.length > 0
963-
? candidate.normalPath.length + 1
964-
: 0,
965-
},
972+
candidate,
973+
isBasename,
966974
);
967975
if (maybeAbsolutePathMatch != null) {
968976
// Determine the input path relative to the current candidate. Note
@@ -1002,7 +1010,11 @@ export default class TreeFS implements MutableFileSystem {
10021010
subpath,
10031011
opts.subpathType,
10041012
observations,
1005-
null,
1013+
{
1014+
ancestorOfRootIdx: commonRootDepth + depthBelowCommonRoot,
1015+
node: nextNode,
1016+
},
1017+
isBasename,
10061018
);
10071019
if (maybeAbsolutePathMatch != null) {
10081020
const rootDirParts = this.#pathUtils.getParts();
@@ -1031,24 +1043,74 @@ export default class TreeFS implements MutableFileSystem {
10311043
return null;
10321044
}
10331045

1046+
/**
1047+
* Whether the real directory at `normalCandidatePath` has `subpath` of the
1048+
* given type, returning the absolute real path of the match.
1049+
*
1050+
* `candidate` is the directory node at `normalCandidatePath`. When
1051+
* `isBasename` and the candidate is the root or a descendant of it, the
1052+
* child is checked with one map lookup. An ancestor of the root is
1053+
* traversed instead, because its node does not hold the segment leading
1054+
* back towards the root. Otherwise, traversal starts from the candidate.
1055+
*/
10341056
#checkCandidateHasSubpath(
10351057
normalCandidatePath: string,
10361058
subpath: string,
10371059
subpathType: 'f' | 'd',
10381060
observations: ?Observations,
1039-
start: ?{
1040-
ancestorOfRootIdx: ?number,
1041-
node: DirectoryNode,
1042-
pathIdx: number,
1061+
candidate: {
1062+
readonly ancestorOfRootIdx: ?number,
1063+
readonly node: DirectoryNode,
1064+
...
10431065
},
1066+
isBasename: boolean,
10441067
): ?string {
1045-
const lookupResult = this.#lookupByNormalPath(
1046-
this.#pathUtils.joinNormalToRelative(normalCandidatePath, subpath)
1047-
.normalPath,
1048-
{
1049-
observations,
1050-
},
1051-
);
1068+
if (
1069+
isBasename &&
1070+
(candidate.ancestorOfRootIdx == null || candidate.ancestorOfRootIdx === 0)
1071+
) {
1072+
const childNode = candidate.node.get(subpath);
1073+
// A symlink must be traversed to learn its type and real path, so only
1074+
// a missing child, a directory or a regular file is answered here.
1075+
if (
1076+
childNode == null ||
1077+
isDirectory(childNode) ||
1078+
isRegularFile(childNode)
1079+
) {
1080+
const isMatch =
1081+
childNode != null && isDirectory(childNode) === (subpathType === 'd');
1082+
if (!isMatch && observations == null) {
1083+
return null;
1084+
}
1085+
const childNormalPath =
1086+
normalCandidatePath === ''
1087+
? subpath
1088+
: normalCandidatePath + path.sep + subpath;
1089+
if (observations) {
1090+
observations.existence.add(childNormalPath);
1091+
}
1092+
return isMatch
1093+
? this.#pathUtils.normalToAbsolute(childNormalPath)
1094+
: null;
1095+
}
1096+
}
1097+
const {normalPath: normalSubpath, collapsedSegments} =
1098+
this.#pathUtils.joinNormalToRelative(normalCandidatePath, subpath);
1099+
const lookupResult = this.#lookupByNormalPath(normalSubpath, {
1100+
observations,
1101+
// Traverse only the subpath, starting from the candidate node. If the
1102+
// join collapsed segments (e.g. '..' + 'root/package.json'), the
1103+
// candidate path is no longer a prefix, so start from the root.
1104+
start:
1105+
collapsedSegments === 0
1106+
? {
1107+
ancestorOfRootIdx: candidate.ancestorOfRootIdx,
1108+
node: candidate.node,
1109+
pathIdx:
1110+
normalCandidatePath === '' ? 0 : normalCandidatePath.length + 1,
1111+
}
1112+
: null,
1113+
});
10521114
if (
10531115
lookupResult.exists &&
10541116
// Should be a Map iff subpathType is directory

‎packages/metro-file-map/src/lib/__tests__/TreeFS-test.js‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,67 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
471471
});
472472
});
473473

474+
test('finds the closest directory with a single-segment subpath', () => {
475+
const observations = emptyObservations();
476+
expect(
477+
tfs.hierarchicalLookup(
478+
p('/A/B/C/a/n_m/pkg/subpath/deep'),
479+
'n_m',
480+
{breakOnSegment: null, subpathType: 'd'},
481+
observations,
482+
),
483+
).toEqual({
484+
absolutePath: p('/A/B/C/a/n_m/pkg/n_m'),
485+
containerRelativePath: p('subpath/deep'),
486+
});
487+
expect(observations).toEqual({
488+
content: new Set(),
489+
existence: new Set([
490+
p('a/n_m/pkg/subpath/deep/n_m'),
491+
p('a/n_m/pkg/subpath/n_m'),
492+
p('a/n_m/pkg/n_m'),
493+
]),
494+
});
495+
});
496+
497+
test('finds the closest match for a deep subpath', () => {
498+
const observations = emptyObservations();
499+
expect(
500+
tfs.hierarchicalLookup(
501+
p('/A/B/C/a/b/c'),
502+
p('n_m/pkg'),
503+
{breakOnSegment: null, subpathType: 'd'},
504+
observations,
505+
),
506+
).toEqual({
507+
absolutePath: p('/A/B/C/a/n_m/pkg'),
508+
containerRelativePath: p('b/c'),
509+
});
510+
expect(observations).toEqual({
511+
content: new Set(),
512+
existence: new Set([p('a/b/c/n_m'), p('a/b/n_m'), p('a/n_m/pkg')]),
513+
});
514+
});
515+
516+
test('finds a subpath re-entering the root from an ancestor of the root', () => {
517+
const observations = emptyObservations();
518+
expect(
519+
tfs.hierarchicalLookup(
520+
p('/A/foo'),
521+
p('B/C/a/package.json'),
522+
{breakOnSegment: null, subpathType: 'f'},
523+
observations,
524+
),
525+
).toEqual({
526+
absolutePath: p('/A/B/C/a/package.json'),
527+
containerRelativePath: 'foo',
528+
});
529+
expect(observations).toEqual({
530+
content: new Set(),
531+
existence: new Set([p('../../foo'), p('a/package.json')]),
532+
});
533+
});
534+
474535
// The fourth column is paths, besides the match itself, whose addition or
475536
// removal changes the result, the fifth those whose modification does too -
476537
// the symlinks traversed.
@@ -605,6 +666,8 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
605666
) => {
606667
const pathMap = (normalPosixPath: string) =>
607668
mockPathModule.resolve(p('/A/B/C'), p(normalPosixPath));
669+
const toCanonical = (posixPath: string) =>
670+
canonicalTo(p('/A/B/C'))(pathMap(posixPath));
608671
const observations = emptyObservations();
609672
expect(
610673
tfs.hierarchicalLookup(
@@ -624,8 +687,6 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
624687
containerRelativePath: p(expectedRelativeSubpath),
625688
},
626689
);
627-
const toCanonical = (posixPath: string) =>
628-
canonicalTo(p('/A/B/C'))(pathMap(posixPath));
629690
expect(observations).toEqual({
630691
content: new Set(expectedContent.map(toCanonical)),
631692
existence: new Set(

0 commit comments

Comments
 (0)