From 221da109a2363642d11618fec86746339aa2bf13 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 5 Feb 2024 13:15:33 -0800 Subject: [PATCH] Improve fast_path.resolve correctness and performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: 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. It's changelog-internal because Metro doesn't currently call `resolve` with paths like these, and this API isn't public. 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: Internal ## Performance 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 --- .../src/lib/__tests__/fast_path-test.js | 2 + packages/metro-file-map/src/lib/fast_path.js | 40 ++++++++++++++----- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/metro-file-map/src/lib/__tests__/fast_path-test.js b/packages/metro-file-map/src/lib/__tests__/fast_path-test.js index a06af55444..d575fbcf6e 100644 --- a/packages/metro-file-map/src/lib/__tests__/fast_path-test.js +++ b/packages/metro-file-map/src/lib/__tests__/fast_path-test.js @@ -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'), diff --git a/packages/metro-file-map/src/lib/fast_path.js b/packages/metro-file-map/src/lib/fast_path.js index 2f554a8193..d2572f8fb2 100644 --- a/packages/metro-file-map/src/lib/fast_path.js +++ b/packages/metro-file-map/src/lib/fast_path.js @@ -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; }