perf(ngcc): speed up the `getBasePaths()` computation (#36881)
This function needs to deduplicate the paths that are found from the paths mappings. Previously this deduplication was not linear and also called the expensive `relative()` function many times. This commit, suggested by @JoostK, reduces the complexity of the deduplication by using a tree structure built from the segments of each path. PR Close #36881
This commit is contained in:
parent
ec6b9cc17d
commit
e037840b88
|
@ -5,7 +5,7 @@
|
||||||
* Use of this source code is governed by an MIT-style license that can be
|
* Use of this source code is governed by an MIT-style license that can be
|
||||||
* found in the LICENSE file at https://angular.io/license
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
import {AbsoluteFsPath, getFileSystem, relative, resolve} from '../../../src/ngtsc/file_system';
|
import {AbsoluteFsPath, getFileSystem, isRoot, resolve} from '../../../src/ngtsc/file_system';
|
||||||
import {Logger} from '../logging/logger';
|
import {Logger} from '../logging/logger';
|
||||||
import {PathMappings} from '../path_mappings';
|
import {PathMappings} from '../path_mappings';
|
||||||
|
|
||||||
|
@ -57,8 +57,8 @@ export function getBasePaths(
|
||||||
}
|
}
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
basePaths.sort().reverse(); // Get the paths in order with the longer ones first.
|
|
||||||
const dedupedBasePaths = basePaths.filter(removeContainedPaths);
|
const dedupedBasePaths = dedupePaths(basePaths);
|
||||||
|
|
||||||
// We want to ensure that the `sourceDirectory` is included when it is a node_modules folder.
|
// We want to ensure that the `sourceDirectory` is included when it is a node_modules folder.
|
||||||
// Otherwise our entry-point finding algorithm would fail to walk that folder.
|
// Otherwise our entry-point finding algorithm would fail to walk that folder.
|
||||||
|
@ -79,30 +79,6 @@ function extractPathPrefix(path: string) {
|
||||||
return path.split('*', 1)[0];
|
return path.split('*', 1)[0];
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* A filter function that removes paths that are contained by other paths.
|
|
||||||
*
|
|
||||||
* For example:
|
|
||||||
* Given `['a/b/c', 'a/b/x', 'a/b', 'd/e', 'd/f']` we will end up with `['a/b', 'd/e', 'd/f]`.
|
|
||||||
* (Note that we do not get `d` even though `d/e` and `d/f` share a base directory, since `d` is not
|
|
||||||
* one of the base paths.)
|
|
||||||
*
|
|
||||||
* @param value The current path.
|
|
||||||
* @param index The index of the current path.
|
|
||||||
* @param array The array of paths (sorted in reverse alphabetical order).
|
|
||||||
* @returns true if this path is not contained by another path.
|
|
||||||
*/
|
|
||||||
function removeContainedPaths(value: AbsoluteFsPath, index: number, array: AbsoluteFsPath[]) {
|
|
||||||
// We only need to check the following paths since the `array` is sorted in reverse alphabetic
|
|
||||||
// order.
|
|
||||||
for (let i = index + 1; i < array.length; i++) {
|
|
||||||
// We need to use `relative().startsWith()` rather than a simple `startsWith()` to ensure we
|
|
||||||
// don't assume that `a/b` contains `a/b-2`.
|
|
||||||
if (!relative(array[i], value).startsWith('..')) return false;
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Run a task and track how long it takes.
|
* Run a task and track how long it takes.
|
||||||
*
|
*
|
||||||
|
@ -118,3 +94,76 @@ export function trackDuration<T = void>(task: () => T extends Promise<unknown>?
|
||||||
log(duration);
|
log(duration);
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Remove paths that are contained by other paths.
|
||||||
|
*
|
||||||
|
* For example:
|
||||||
|
* Given `['a/b/c', 'a/b/x', 'a/b', 'd/e', 'd/f']` we will end up with `['a/b', 'd/e', 'd/f]`.
|
||||||
|
* (Note that we do not get `d` even though `d/e` and `d/f` share a base directory, since `d` is not
|
||||||
|
* one of the base paths.)
|
||||||
|
*/
|
||||||
|
export function dedupePaths(paths: AbsoluteFsPath[]): AbsoluteFsPath[] {
|
||||||
|
const root: Node = {children: new Map()};
|
||||||
|
for (const path of paths) {
|
||||||
|
addPath(root, path);
|
||||||
|
}
|
||||||
|
return flattenTree(root);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Add a path (defined by the `segments`) to the current `node` in the tree.
|
||||||
|
*/
|
||||||
|
function addPath(root: Node, path: AbsoluteFsPath): void {
|
||||||
|
let node = root;
|
||||||
|
if (!isRoot(path)) {
|
||||||
|
const segments = path.split('/');
|
||||||
|
for (let index = 0; index < segments.length; index++) {
|
||||||
|
if (isLeaf(node)) {
|
||||||
|
// We hit a leaf so don't bother processing any more of the path
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// This is not the end of the path continue to process the rest of this path.
|
||||||
|
const next = segments[index];
|
||||||
|
if (!node.children.has(next)) {
|
||||||
|
node.children.set(next, {children: new Map()});
|
||||||
|
}
|
||||||
|
node = node.children.get(next)!;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// This path has finished so convert this node to a leaf
|
||||||
|
convertToLeaf(node, path);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Flatten the tree of nodes back into an array of absolute paths
|
||||||
|
*/
|
||||||
|
function flattenTree(root: Node): AbsoluteFsPath[] {
|
||||||
|
const paths: AbsoluteFsPath[] = [];
|
||||||
|
const nodes: Node[] = [root];
|
||||||
|
for (let index = 0; index < nodes.length; index++) {
|
||||||
|
const node = nodes[index];
|
||||||
|
if (isLeaf(node)) {
|
||||||
|
// We found a leaf so store the currentPath
|
||||||
|
paths.push(node.path);
|
||||||
|
} else {
|
||||||
|
node.children.forEach(value => nodes.push(value));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return paths;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isLeaf(node: Node): node is Leaf {
|
||||||
|
return node.path !== undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
function convertToLeaf(node: Node, path: AbsoluteFsPath) {
|
||||||
|
node.path = path;
|
||||||
|
}
|
||||||
|
|
||||||
|
interface Node {
|
||||||
|
children: Map<string, Node>;
|
||||||
|
path?: AbsoluteFsPath;
|
||||||
|
}
|
||||||
|
|
||||||
|
type Leaf = Required<Node>;
|
||||||
|
|
|
@ -27,7 +27,7 @@ runInEachFileSystem(() => {
|
||||||
expect(basePaths).toEqual([sourceDirectory]);
|
expect(basePaths).toEqual([sourceDirectory]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should use each path mapping prefix and sort in descending order', () => {
|
it('should use each path mapping prefix', () => {
|
||||||
const projectDirectory = _('/path/to/project');
|
const projectDirectory = _('/path/to/project');
|
||||||
const fs = getFileSystem();
|
const fs = getFileSystem();
|
||||||
fs.ensureDir(fs.resolve(projectDirectory, 'dist-1'));
|
fs.ensureDir(fs.resolve(projectDirectory, 'dist-1'));
|
||||||
|
@ -41,10 +41,27 @@ runInEachFileSystem(() => {
|
||||||
};
|
};
|
||||||
const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
|
const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
|
||||||
expect(basePaths).toEqual([
|
expect(basePaths).toEqual([
|
||||||
fs.resolve(projectDirectory, 'sub-folder/dist-2'),
|
|
||||||
sourceDirectory,
|
sourceDirectory,
|
||||||
fs.resolve(projectDirectory, 'libs'),
|
|
||||||
fs.resolve(projectDirectory, 'dist-1'),
|
fs.resolve(projectDirectory, 'dist-1'),
|
||||||
|
fs.resolve(projectDirectory, 'libs'),
|
||||||
|
fs.resolve(projectDirectory, 'sub-folder/dist-2'),
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not be confused by folders that have the same starting string', () => {
|
||||||
|
const projectDirectory = _('/path/to/project');
|
||||||
|
const fs = getFileSystem();
|
||||||
|
fs.ensureDir(fs.resolve(projectDirectory, 'a/b'));
|
||||||
|
fs.ensureDir(fs.resolve(projectDirectory, 'a/b-2'));
|
||||||
|
fs.ensureDir(fs.resolve(projectDirectory, 'a/b/c'));
|
||||||
|
|
||||||
|
const sourceDirectory = _('/path/to/project/node_modules');
|
||||||
|
const pathMappings = {baseUrl: projectDirectory, paths: {'@dist': ['a/b', 'a/b-2', 'a/b/c']}};
|
||||||
|
const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
|
||||||
|
expect(basePaths).toEqual([
|
||||||
|
sourceDirectory,
|
||||||
|
fs.resolve(projectDirectory, 'a/b'),
|
||||||
|
fs.resolve(projectDirectory, 'a/b-2'),
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -105,8 +122,8 @@ runInEachFileSystem(() => {
|
||||||
const pathMappings = {baseUrl: _('/'), paths: {'@dist': ['dist']}};
|
const pathMappings = {baseUrl: _('/'), paths: {'@dist': ['dist']}};
|
||||||
const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
|
const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
|
||||||
expect(basePaths).toEqual([
|
expect(basePaths).toEqual([
|
||||||
sourceDirectory,
|
|
||||||
fs.resolve('/dist'),
|
fs.resolve('/dist'),
|
||||||
|
sourceDirectory,
|
||||||
]);
|
]);
|
||||||
expect(logger.logs.warn).toEqual([
|
expect(logger.logs.warn).toEqual([
|
||||||
[`The provided pathMappings baseUrl is the root path ${_('/')}.\n` +
|
[`The provided pathMappings baseUrl is the root path ${_('/')}.\n` +
|
||||||
|
|
Loading…
Reference in New Issue