From 9cec94a008dfeaa5a81b21071be55af4588b4fe2 Mon Sep 17 00:00:00 2001 From: JoostK Date: Mon, 15 Feb 2021 21:21:37 +0100 Subject: [PATCH] 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 --- .../ngcc/src/analysis/decoration_analyzer.ts | 2 +- .../ngtsc/annotations/test/component_spec.ts | 2 +- .../src/ngtsc/core/src/compiler.ts | 2 +- .../compiler-cli/src/ngtsc/cycles/BUILD.bazel | 1 - .../src/ngtsc/cycles/src/imports.ts | 35 ++++++++++--------- .../src/ngtsc/cycles/test/BUILD.bazel | 1 - .../src/ngtsc/cycles/test/analyzer_spec.ts | 7 ++-- .../src/ngtsc/cycles/test/imports_spec.ts | 7 ++-- 8 files changed, 26 insertions(+), 31 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index dd35ddc332..b2fb42c113 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -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); diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts index 77cc48cc4a..da3f93532d 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts @@ -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); diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 311d768f71..7a5b8ed90d 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -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 = diff --git a/packages/compiler-cli/src/ngtsc/cycles/BUILD.bazel b/packages/compiler-cli/src/ngtsc/cycles/BUILD.bazel index ef45798579..78a4cbf391 100644 --- a/packages/compiler-cli/src/ngtsc/cycles/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/cycles/BUILD.bazel @@ -9,7 +9,6 @@ ts_library( ]), module_name = "@angular/compiler-cli/src/ngtsc/cycles", deps = [ - "//packages/compiler-cli/src/ngtsc/imports", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts b/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts index 4f9915a283..1696cab7a2 100644 --- a/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts +++ b/packages/compiler-cli/src/ngtsc/cycles/src/imports.ts @@ -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>(); - 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 { const imports = new Set(); - // 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)) { - // Record this local import. - imports.add(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; } /** diff --git a/packages/compiler-cli/src/ngtsc/cycles/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/cycles/test/BUILD.bazel index 81efd86e91..8ba04d7446 100644 --- a/packages/compiler-cli/src/ngtsc/cycles/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/cycles/test/BUILD.bazel @@ -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", ], diff --git a/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts b/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts index 0d697e3005..388a1d9b6b 100644 --- a/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts +++ b/packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts @@ -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())), }; } }); diff --git a/packages/compiler-cli/src/ngtsc/cycles/test/imports_spec.ts b/packages/compiler-cli/src/ngtsc/cycles/test/imports_spec.ts index 6f1fe6ad4e..e4713f44f6 100644 --- a/packages/compiler-cli/src/ngtsc/cycles/test/imports_spec.ts +++ b/packages/compiler-cli/src/ngtsc/cycles/test/imports_spec.ts @@ -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()), }; }