Skip to content

Commit

Permalink
Improve fast_path.resolve correctness and performance
Browse files Browse the repository at this point in the history
Summary:
## Correctness
Fixes a bug in `fastPath.resolve` where paths ending in `/..` would not collapse preceding segments, eg `resolve('/project/root', '../..')` would return `/project/..` rather than `/`.

This came up when implementing incremental resolution support - we end up resolving canonical paths like this during lookup of first missing directory nodes. The correctness fix is changelog-internal because Metro doesn't currently call `resolve` with paths like these, and this API isn't public.

## Performance
In implementing this, I've also squeezed out a bit more performance so that `fast_path.resolve` is now about 1.6x faster than before this diff and 20x faster than `path.resolve` for repeated calls with the same root. This is significant because it actually dominates `getRealPath`, which is called hundreds of thousands of times during module resolution on a typical graph. Zoomed out benchmarks to follow in the stack.

Changelog: [Performance] Improve resolution performance by optimising normal->absolute path conversion

## Benchmarks
Calling `resolve` 2m times on different, representative (FB internal) paths per sample.
```
┌─────────┬───────────┬─────────┬───────────────────┬──────────┬─────────┐
│ (index) │ Task Name │ ops/sec │ Average Time (ns) │ Margin   │ Samples │
├─────────┼───────────┼─────────┼───────────────────┼──────────┼─────────┤
│ 0       │ 'new'     │ '16'    │ 61555575.15459959 │ '±0.26%' │ 488     │
│ 1       │ 'old'     │ '10'    │ 99607323.81785941 │ '±0.32%' │ 302     │
│ 2       │ 'system'  │ '0'     │ 1472189198.380425 │ '±0.73%' │ 21      │
└─────────┴───────────┴─────────┴───────────────────┴──────────┴─────────┘
```

Reviewed By: motiz88

Differential Revision: D52464135
  • Loading branch information
robhogan authored and facebook-github-bot committed Feb 5, 2024
1 parent 3b467c3 commit 036bb3c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
2 changes: 2 additions & 0 deletions packages/metro-file-map/src/lib/__tests__/fast_path-test.js
Expand Up @@ -68,6 +68,8 @@ describe.each([['win32'], ['posix']])('fast_path on %s', platform => {
);

test.each([
p('..'),
p('../..'),
p('normal/path'),
p('../normal/path'),
p('../../normal/path'),
Expand Down
40 changes: 31 additions & 9 deletions packages/metro-file-map/src/lib/fast_path.js
Expand Up @@ -38,19 +38,41 @@ const UP_FRAGMENT = '..' + path.sep;
const UP_FRAGMENT_LENGTH = UP_FRAGMENT.length;
const CURRENT_FRAGMENT = '.' + path.sep;

// Optimise for the case where we're often repeatedly dealing with the same
// root by caching just the most recent.
let cachedDirName = null;
let dirnameCache = [];

// rootDir must be an absolute path and normalPath must be a normal relative
// path (e.g.: foo/bar or ../foo/bar, but never ./foo or foo/../bar)
// As of Node 18 this is several times faster than path.resolve, over
// thousands of real calls with 1-3 levels of indirection.
export function resolve(rootDir: string, normalPath: string): string {
if (normalPath.startsWith(UP_FRAGMENT)) {
const dirname = rootDir === '' ? '' : path.dirname(rootDir);
return resolve(dirname, normalPath.slice(UP_FRAGMENT_LENGTH));
} else {
return (
rootDir +
// If rootDir is the file system root, it will end in a path separator
(rootDir.endsWith(path.sep) ? normalPath : path.sep + normalPath)
);
let left = rootDir;
let i = 0;
let pos = 0;
while (
normalPath.startsWith(UP_FRAGMENT, pos) ||
(normalPath.endsWith('..') && normalPath.length === 2 + pos)
) {
if (i === 0 && cachedDirName !== rootDir) {
dirnameCache = [];
cachedDirName = rootDir;
}
if (dirnameCache.length === i) {
dirnameCache.push(path.dirname(left));
}
left = dirnameCache[i++];
pos += UP_FRAGMENT_LENGTH;
}
const right = pos === 0 ? normalPath : normalPath.slice(pos);
if (right.length === 0) {
return left;
}
// left may already end in a path separator only if it is a filesystem root,
// '/' or 'X:\'.
if (left.endsWith(path.sep)) {
return left + right;
}
return left + path.sep + right;
}

0 comments on commit 036bb3c

Please sign in to comment.