perf(ngcc): do not rescan program source files when referenced from multiple root files (#39254)

When ngcc is configured to run with the `--use-program-dependencies`
flag, as is the case in the CLI's asynchronous processing, it will scan
all source files in the program, starting from the program's root files
as configured in the tsconfig. Each individual root file could
potentially rescan files that had already been scanned for an earlier
root file, causing a severe performance penalty if the number of root
files is large. This would be the case if glob patterns are used in the
"include" specification of a tsconfig file.

This commit avoids the performance penalty by keeping track of the files
that have been scanned across all root files, such that no source file
is scanned multiple times.

Fixes #39240

PR Close #39254
This commit is contained in:
JoostK 2020-10-13 23:06:07 +02:00 committed by atscott
parent f6d5cdfbd7
commit 898be92f70
3 changed files with 103 additions and 11 deletions

View File

@ -53,6 +53,22 @@ export abstract class DependencyHostBase implements DependencyHost {
} }
} }
/**
* Find all the dependencies for the provided paths.
*
* @param files The list of absolute paths of JavaScript files to scan for dependencies.
* @param dependencyInfo An object containing information about the dependencies of the
* entry-point, including those that were missing or deep imports into other entry-points. The
* sets in this object will be updated with new information about the entry-point's dependencies.
*/
collectDependenciesInFiles(
files: AbsoluteFsPath[], {dependencies, missing, deepImports}: DependencyInfo): void {
const alreadySeen = new Set<AbsoluteFsPath>();
for (const file of files) {
this.processFile(file, dependencies, missing, deepImports, alreadySeen);
}
}
/** /**
* Compute the dependencies of the given file. * Compute the dependencies of the given file.
* *
@ -102,12 +118,7 @@ export abstract class DependencyHostBase implements DependencyHost {
return false; return false;
} }
if (resolvedModule instanceof ResolvedRelativeModule) { if (resolvedModule instanceof ResolvedRelativeModule) {
const internalDependency = resolvedModule.modulePath; this.processFile(resolvedModule.modulePath, dependencies, missing, deepImports, alreadySeen);
if (!alreadySeen.has(internalDependency)) {
alreadySeen.add(internalDependency);
this.recursivelyCollectDependencies(
internalDependency, dependencies, missing, deepImports, alreadySeen);
}
} else if (resolvedModule instanceof ResolvedDeepImport) { } else if (resolvedModule instanceof ResolvedDeepImport) {
deepImports.add(resolvedModule.importPath); deepImports.add(resolvedModule.importPath);
} else { } else {
@ -115,4 +126,18 @@ export abstract class DependencyHostBase implements DependencyHost {
} }
return true; return true;
} }
/**
* Processes the file if it has not already been seen. This will also recursively process
* all files that are imported from the file, while taking the set of already seen files
* into account.
*/
protected processFile(
file: AbsoluteFsPath, dependencies: Set<AbsoluteFsPath>, missing: Set<string>,
deepImports: Set<string>, alreadySeen: Set<AbsoluteFsPath>): void {
if (!alreadySeen.has(file)) {
alreadySeen.add(file);
this.recursivelyCollectDependencies(file, dependencies, missing, deepImports, alreadySeen);
}
}
} }

View File

@ -48,14 +48,12 @@ export class ProgramBasedEntryPointFinder extends TracingEntryPointFinder {
const moduleResolver = new ModuleResolver(this.fs, this.pathMappings, ['', '.ts', '/index.ts']); const moduleResolver = new ModuleResolver(this.fs, this.pathMappings, ['', '.ts', '/index.ts']);
const host = new EsmDependencyHost(this.fs, moduleResolver); const host = new EsmDependencyHost(this.fs, moduleResolver);
const dependencies = createDependencyInfo(); const dependencies = createDependencyInfo();
const rootFiles = this.tsConfig.rootNames.map(rootName => this.fs.resolve(rootName));
this.logger.debug( this.logger.debug(
`Using the program from ${this.tsConfig.project} to seed the entry-point finding.`); `Using the program from ${this.tsConfig.project} to seed the entry-point finding.`);
this.logger.debug( this.logger.debug(
`Collecting dependencies from the following files:` + `Collecting dependencies from the following files:` + rootFiles.map(file => `\n- ${file}`));
this.tsConfig.rootNames.map(file => `\n- ${file}`)); host.collectDependenciesInFiles(rootFiles, dependencies);
this.tsConfig.rootNames.forEach(rootName => {
host.collectDependencies(this.fs.resolve(rootName), dependencies);
});
return Array.from(dependencies.dependencies); return Array.from(dependencies.dependencies);
} }

View File

@ -59,6 +59,75 @@ runInEachFileSystem(() => {
]); ]);
}); });
it('should scan source files only once, even if they are referenced from multiple root files',
() => {
// https://github.com/angular/angular/issues/39240
// When scanning the program for imports to determine which entry-points to process, the
// root files as configured in the tsconfig file are used to start scanning from. This
// test asserts that a file is not scanned multiple times even if it is referenced from
// multiple root files.
loadTestFiles([
{
name: _Abs(`${projectPath}/package.json`),
contents: '',
},
{
name: _Abs(`${projectPath}/tsconfig.json`),
contents: `{
"files": [
"root-one.ts",
"root-two.ts",
],
"compilerOptions": {
"baseUrl": ".",
"paths": {
"lib/*": ["lib/*"]
}
}
}`,
},
{
name: _Abs(`${projectPath}/root-one.ts`),
contents: `
import './root-two';
import './not-root';
`,
},
{
name: _Abs(`${projectPath}/root-two.ts`),
contents: `
import './not-root';
`,
},
{
name: _Abs(`${projectPath}/not-root.ts`),
contents: `
import {Component} from '@angular/core';
`,
},
...createPackage(angularNamespacePath, 'core'),
]);
const extractImportsSpy =
spyOn(EsmDependencyHost.prototype, 'extractImports' as any).and.callThrough();
const finder = createFinder();
const {entryPoints} = finder.findEntryPoints();
expect(dumpEntryPointPaths(basePath, entryPoints)).toEqual([
['@angular/core', '@angular/core'],
]);
// Three `extractImports` calls should have been made corresponding with the three
// distinct source files in the project.
const extractImportFiles =
extractImportsSpy.calls.all().map((call: jasmine.CallInfo<any>) => call.args[0]);
expect(extractImportFiles).toEqual([
_Abs(`${projectPath}/root-one.ts`),
_Abs(`${projectPath}/root-two.ts`),
_Abs(`${projectPath}/not-root.ts`),
]);
});
function createFinder(): ProgramBasedEntryPointFinder { function createFinder(): ProgramBasedEntryPointFinder {
const tsConfig = readConfiguration(`${projectPath}/tsconfig.json`); const tsConfig = readConfiguration(`${projectPath}/tsconfig.json`);
const baseUrl = fs.resolve(projectPath, tsConfig.options.basePath!); const baseUrl = fs.resolve(projectPath, tsConfig.options.basePath!);