Skip to content

Commit

Permalink
Optimise fast_path.resolve, remove TreeFS._normalToAbsolutePath
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@robhogan @facebook-github-bot
robhogan authored and facebook-github-bot committed Jun 24, 2023
1 parent d376529 commit dc3cddf
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 50 deletions.
14 changes: 4 additions & 10 deletions packages/metro-file-map/src/lib/TreeFS.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,9 @@ export default class TreeFS implements MutableFileSystem {
}

getAllFiles(): Array<Path> {
const rootDir = this.#rootDir;
return Array.from(this._regularFileIterator(), normalPath =>
this._normalToAbsolutePath(normalPath),
fastPath.resolve(rootDir, normalPath),
);
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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<Path> {
for (const [normalPath, metadata] of this.#files) {
if (metadata[H.SYMLINK] !== 0) {
Expand Down
70 changes: 37 additions & 33 deletions packages/metro-file-map/src/lib/__tests__/fast_path-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
});
});
});
25 changes: 18 additions & 7 deletions packages/metro-file-map/src/lib/fast_path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,30 @@

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)
: path.relative(rootDir, filename);
}

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)
);
}
}

0 comments on commit dc3cddf

Please sign in to comment.