perf(compiler-cli): use bound symbol in import graph in favor of module resolution (#40948)

The import graph scans source files for its import and export statements
to extract the source files that it imports/exports. Such statements
contain a module specifier string and this module specifier used to be
resolved to the actual source file using an explicit module resolution
step. This is especially expensive in incremental rebuilds, as the
module resolution cache has not been primed during program creation
(assuming that the incremental program was able to reuse the module
resolution results from a prior compilation). This meant that all module
resolution requests would have to hit the filesystem, which is
relatively slow.

This commit is able to replace the module resolution with TypeScript's
bound symbol of the module specifier. This symbol corresponds with the
`ts.SourceFile` that is being imported/exported, which is exactly what
the import graph was interested in. As a result, no filesystem accesses
are done anymore.

PR Close #40948
This commit is contained in:
JoostK 2021-02-15 21:21:37 +01:00 committed by Andrew Kushnir
parent 153e3a8960
commit 9cec94a008
8 changed files with 26 additions and 31 deletions

View File

@ -89,7 +89,7 @@ export class DecorationAnalyzer {
fullRegistry = new CompoundMetadataRegistry([this.metaRegistry, this.scopeRegistry]);
evaluator =
new PartialEvaluator(this.reflectionHost, this.typeChecker, /* dependencyTracker */ null);
importGraph = new ImportGraph(this.moduleResolver);
importGraph = new ImportGraph(this.typeChecker);
cycleAnalyzer = new CycleAnalyzer(this.importGraph);
injectableRegistry = new InjectableClassRegistry(this.reflectionHost);
typeCheckScopeRegistry = new TypeCheckScopeRegistry(this.scopeRegistry, this.fullMetaReader);

View File

@ -41,7 +41,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil
const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null);
const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const importGraph = new ImportGraph(moduleResolver);
const importGraph = new ImportGraph(checker);
const cycleAnalyzer = new CycleAnalyzer(importGraph);
const metaRegistry = new LocalMetadataRegistry();
const dtsReader = new DtsMetadataReader(checker, reflectionHost);

View File

@ -322,7 +322,7 @@ export class NgCompiler {
this.moduleResolver =
new ModuleResolver(tsProgram, this.options, this.adapter, moduleResolutionCache);
this.resourceManager = new AdapterResourceLoader(adapter, this.options);
this.cycleAnalyzer = new CycleAnalyzer(new ImportGraph(this.moduleResolver));
this.cycleAnalyzer = new CycleAnalyzer(new ImportGraph(tsProgram.getTypeChecker()));
this.incrementalStrategy.setIncrementalDriver(this.incrementalDriver, tsProgram);
this.ignoreForDiagnostics =

View File

@ -9,7 +9,6 @@ ts_library(
]),
module_name = "@angular/compiler-cli/src/ngtsc/cycles",
deps = [
"//packages/compiler-cli/src/ngtsc/imports",
"@npm//typescript",
],
)

View File

@ -8,8 +8,6 @@
import * as ts from 'typescript';
import {ModuleResolver} from '../../imports';
/**
* A cached graph of imports in the `ts.Program`.
*
@ -19,7 +17,7 @@ import {ModuleResolver} from '../../imports';
export class ImportGraph {
private map = new Map<ts.SourceFile, Set<ts.SourceFile>>();
constructor(private resolver: ModuleResolver) {}
constructor(private checker: ts.TypeChecker) {}
/**
* List the direct (not transitive) imports of a given `ts.SourceFile`.
@ -102,25 +100,30 @@ export class ImportGraph {
private scanImports(sf: ts.SourceFile): Set<ts.SourceFile> {
const imports = new Set<ts.SourceFile>();
// Look through the source file for import statements.
sf.statements.forEach(stmt => {
if ((ts.isImportDeclaration(stmt) || ts.isExportDeclaration(stmt)) &&
stmt.moduleSpecifier !== undefined && ts.isStringLiteral(stmt.moduleSpecifier)) {
// Resolve the module to a file, and check whether that file is in the ts.Program.
const moduleName = stmt.moduleSpecifier.text;
const moduleFile = this.resolver.resolveModule(moduleName, sf.fileName);
if (moduleFile !== null && isLocalFile(moduleFile)) {
// Look through the source file for import and export statements.
for (const stmt of sf.statements) {
if ((!ts.isImportDeclaration(stmt) && !ts.isExportDeclaration(stmt)) ||
stmt.moduleSpecifier === undefined) {
continue;
}
const symbol = this.checker.getSymbolAtLocation(stmt.moduleSpecifier);
if (symbol === undefined || symbol.valueDeclaration === undefined) {
// No symbol could be found to skip over this import/export.
continue;
}
const moduleFile = symbol.valueDeclaration;
if (ts.isSourceFile(moduleFile) && isLocalFile(moduleFile)) {
// Record this local import.
imports.add(moduleFile);
}
}
});
return imports;
}
}
function isLocalFile(sf: ts.SourceFile): boolean {
return !sf.fileName.endsWith('.d.ts');
return !sf.isDeclarationFile;
}
/**

View File

@ -13,7 +13,6 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/cycles",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/testing",
"@npm//typescript",
],

View File

@ -8,7 +8,6 @@
import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {ModuleResolver} from '../../imports';
import {Cycle, CycleAnalyzer} from '../src/analyzer';
import {ImportGraph} from '../src/imports';
import {importPath, makeProgramFromGraph} from './util';
@ -73,12 +72,10 @@ runInEachFileSystem(() => {
});
function makeAnalyzer(graph: string): {program: ts.Program, analyzer: CycleAnalyzer} {
const {program, options, host} = makeProgramFromGraph(getFileSystem(), graph);
const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const {program} = makeProgramFromGraph(getFileSystem(), graph);
return {
program,
analyzer: new CycleAnalyzer(new ImportGraph(moduleResolver)),
analyzer: new CycleAnalyzer(new ImportGraph(program.getTypeChecker())),
};
}
});

View File

@ -8,7 +8,6 @@
import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {ModuleResolver} from '../../imports';
import {ImportGraph} from '../src/imports';
import {importPath, makeProgramFromGraph} from './util';
@ -84,12 +83,10 @@ runInEachFileSystem(() => {
});
function makeImportGraph(graph: string): {program: ts.Program, graph: ImportGraph} {
const {program, options, host} = makeProgramFromGraph(getFileSystem(), graph);
const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const {program} = makeProgramFromGraph(getFileSystem(), graph);
return {
program,
graph: new ImportGraph(moduleResolver),
graph: new ImportGraph(program.getTypeChecker()),
};
}