From dc3cddf21803eb3addb084627e4d536318e3da0a Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 24 Jun 2023 13:40:37 -0700 Subject: [PATCH] Optimise fast_path.resolve, remove `TreeFS._normalToAbsolutePath` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Consolidate some path resolution logic in `metro-file-map`, and optimise `fast_path.resolve` for normalised relative paths beginning `..`. Benchmarks across >100k real paths at FB: ``` Loaded data for 342252 files ┌─────────┬───────────────────────────────┬─────────┬────────────────────┬──────────┬─────────┐ │ (index) │ Task Name │ ops/sec │ Average Time (ns) │ Margin │ Samples │ ├─────────┼───────────────────────────────┼─────────┼────────────────────┼──────────┼─────────┤ │ 0 │ '_normalToAbsolute' │ '2' │ 381483262.86121655 │ '±6.19%' │ 27 │ │ 1 │ 'path.resolve (native)' │ '1' │ 804749585.7477188 │ '±8.28%' │ 13 │ │ 2 │ 'fastPath.resolve (original)' │ '2' │ 388311313.0285189 │ '±8.00%' │ 26 │ │ 3 │ 'fastPath.resolve (improved)' │ '11' │ 87437206.5772181 │ '±5.45%' │ 115 │ └─────────┴───────────────────────────────┴─────────┴────────────────────┴──────────┴─────────┘ ``` Changelog: ``` - **[Performance]**: Improve resolution performance for files outside the project root. ``` Reviewed By: huntie Differential Revision: D45446644 fbshipit-source-id: 42c4c49c7560cd5a0d6446aba19ed7cc24f3a10b --- packages/metro-file-map/src/lib/TreeFS.js | 14 ++-- .../src/lib/__tests__/fast_path-test.js | 70 ++++++++++--------- packages/metro-file-map/src/lib/fast_path.js | 25 +++++-- 3 files changed, 59 insertions(+), 50 deletions(-) diff --git a/packages/metro-file-map/src/lib/TreeFS.js b/packages/metro-file-map/src/lib/TreeFS.js index 354bbacf9b..62f5738bcc 100644 --- a/packages/metro-file-map/src/lib/TreeFS.js +++ b/packages/metro-file-map/src/lib/TreeFS.js @@ -127,8 +127,9 @@ export default class TreeFS implements MutableFileSystem { } getAllFiles(): Array { + const rootDir = this.#rootDir; return Array.from(this._regularFileIterator(), normalPath => - this._normalToAbsolutePath(normalPath), + fastPath.resolve(rootDir, normalPath), ); } @@ -149,8 +150,9 @@ export default class TreeFS implements MutableFileSystem { const regexpPattern = pattern instanceof RegExp ? pattern : new RegExp(pattern); const files = []; + const rootDir = this.#rootDir; for (const filePath of this._pathIterator()) { - const absolutePath = this._normalToAbsolutePath(filePath); + const absolutePath = fastPath.resolve(rootDir, filePath); if (regexpPattern.test(absolutePath)) { files.push(absolutePath); } @@ -423,14 +425,6 @@ export default class TreeFS implements MutableFileSystem { : path.normalize(relativeOrAbsolutePath); } - _normalToAbsolutePath(normalPath: Path): Path { - if (normalPath[0] === '.') { - return path.normalize(this.#rootDir + path.sep + normalPath); - } else { - return this.#rootDir + path.sep + normalPath; - } - } - *_regularFileIterator(): Iterator { for (const [normalPath, metadata] of this.#files) { if (metadata[H.SYMLINK] !== 0) { 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 f3d36c6fb9..a7eb96097f 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 @@ -5,47 +5,51 @@ * LICENSE file in the root directory of this source tree. * * @format + * @flow strict * @oncall react_native */ -import {relative, resolve} from '../fast_path'; -import path from 'path'; +import typeof * as FastPath from '../fast_path'; -describe('fastPath.relative', () => { - it('should get relative paths inside the root', () => { - const root = path.join(__dirname, 'foo', 'bar'); - const filename = path.join(__dirname, 'foo', 'bar', 'baz', 'foobar'); - const relativeFilename = path.join('baz', 'foobar'); - expect(relative(root, filename)).toBe(relativeFilename); - }); +let mockPathModule; +jest.mock('path', () => mockPathModule); - it('should get relative paths outside the root', () => { - const root = path.join(__dirname, 'foo', 'bar'); - const filename = path.join(__dirname, 'foo', 'baz', 'foobar'); - const relativeFilename = path.join('..', 'baz', 'foobar'); - expect(relative(root, filename)).toBe(relativeFilename); - }); +describe.each([['win32'], ['posix']])('fast_path on %s', platform => { + // Convenience function to write paths with posix separators but convert them + // to system separators + const p: string => string = filePath => + platform === 'win32' + ? filePath.replace(/\//g, '\\').replace(/^\\/, 'C:\\') + : filePath; - it('should get relative paths outside the root when start with same word', () => { - const root = path.join(__dirname, 'foo', 'bar'); - const filename = path.join(__dirname, 'foo', 'barbaz', 'foobar'); - const relativeFilename = path.join('..', 'barbaz', 'foobar'); - expect(relative(root, filename)).toBe(relativeFilename); - }); -}); + let fastPath: FastPath; -describe('fastPath.resolve', () => { - it('should get the absolute path for paths inside the root', () => { - const root = path.join(__dirname, 'foo', 'bar'); - const relativeFilename = path.join('baz', 'foobar'); - const filename = path.join(__dirname, 'foo', 'bar', 'baz', 'foobar'); - expect(resolve(root, relativeFilename)).toBe(filename); + beforeEach(() => { + jest.resetModules(); + mockPathModule = jest.requireActual<{}>('path')[platform]; + fastPath = require('../fast_path'); }); - it('should get the absolute path for paths outside the root', () => { - const root = path.join(__dirname, 'foo', 'bar'); - const relativeFilename = path.join('..', 'baz', 'foobar'); - const filename = path.join(__dirname, 'foo', 'baz', 'foobar'); - expect(resolve(root, relativeFilename)).toBe(filename); + describe.each([p('/project/root'), p('/')])('root: %s', rootDir => { + test.each([ + p('/project/root/baz/foobar'), + p('/project/baz/foobar'), + p('/project/rootfoo/baz'), + ])(`relative('${rootDir}', '%s') matches path.relative`, normalPath => { + expect(fastPath.relative(rootDir, normalPath)).toEqual( + mockPathModule.relative(rootDir, normalPath), + ); + }); + + test.each([ + p('normal/path'), + p('../normal/path'), + p('../../normal/path'), + p('../../../normal/path'), + ])(`resolve('${rootDir}', '%s') matches path.resolve`, normalPath => { + expect(fastPath.resolve(rootDir, normalPath)).toEqual( + mockPathModule.resolve(rootDir, normalPath), + ); + }); }); }); diff --git a/packages/metro-file-map/src/lib/fast_path.js b/packages/metro-file-map/src/lib/fast_path.js index f72f291e65..427f1d8186 100644 --- a/packages/metro-file-map/src/lib/fast_path.js +++ b/packages/metro-file-map/src/lib/fast_path.js @@ -10,7 +10,8 @@ import * as path from 'path'; -// rootDir and filename must be absolute paths (resolved) +// rootDir must be normalized and absolute, filename may be any absolute path. +// (but will optimally start with rootDir) export function relative(rootDir: string, filename: string): string { return filename.indexOf(rootDir + path.sep) === 0 ? filename.substr(rootDir.length + 1) @@ -18,11 +19,21 @@ export function relative(rootDir: string, filename: string): string { } const INDIRECTION_FRAGMENT = '..' + path.sep; +const INDIRECTION_FRAGMENT_LENGTH = INDIRECTION_FRAGMENT.length; -// rootDir must be an absolute path and relativeFilename must be simple -// (e.g.: foo/bar or ../foo/bar, but never ./foo or foo/../bar) -export function resolve(rootDir: string, relativeFilename: string): string { - return relativeFilename.indexOf(INDIRECTION_FRAGMENT) === 0 - ? path.resolve(rootDir, relativeFilename) - : rootDir + path.sep + relativeFilename; +// 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(INDIRECTION_FRAGMENT)) { + const dirname = rootDir === '' ? '' : path.dirname(rootDir); + return resolve(dirname, normalPath.slice(INDIRECTION_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) + ); + } }