perf(compiler-cli): avoid module resolution in cycle analysis (#40948)

The compiler performs cycle analysis for the used directives and pipes
of a component's template to avoid introducing a cyclic import into the
generated output. The used directives and pipes are represented by their
output expression which would typically be an `ExternalExpr`; those are
responsible for the generation of an `import` statement. Cycle analysis
needs to determine the `ts.SourceFile` that would end up being imported
by these `ExternalExpr`s, as the `ts.SourceFile` is then checked against
the program's `ImportGraph` to determine if the import is allowed, i.e.
does not introduce a cycle. To accomplish this, the `ExternalExpr` was
dissected and ran through module resolution to obtain the imported
`ts.SourceFile`.

This module resolution step is relatively expensive, as it typically
needs to hit the filesystem. Even in the presence of a module resolution
cache would these module resolution requests generally see cache misses,
as the generated import originates from a file for which the cache has
not previously seen the imported module specifier.

This commit removes the need for the module resolution by wrapping the
generated `Expression` in an `EmittedReference` struct. This allows the
reference emitter mechanism that is responsible for generating the
`Expression` to also communicate from which `ts.SourceFile` the
generated `Expression` would be imported, precluding the need for module
resolution down the road.

PR Close #40948

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -12,7 +12,7 @@ import * as ts from 'typescript';
 import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
 import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics';
 import {absoluteFrom, relative} from '../../file_system';
-import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
+import {DefaultImportRecorder, ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
 import {DependencyTracker} from '../../incremental/api';
 import {extractSemanticTypeParameters, isArrayEqual, isReferenceEqual, SemanticDepGraphUpdater, SemanticReference, SemanticSymbol} from '../../incremental/semantic_graph';
 import {IndexingContext} from '../../indexer';
@@ -628,29 +628,40 @@ export class ComponentDecoratorHandler implements
       const bound = binder.bind({template: metadata.template.nodes});
 
       // The BoundTarget knows which directives and pipes matched the template.
-      type UsedDirective = R3UsedDirectiveMetadata&{ref: Reference<ClassDeclaration>};
+      type UsedDirective =
+          R3UsedDirectiveMetadata&{ref: Reference<ClassDeclaration>, importedFile: ImportedFile};
       const usedDirectives: UsedDirective[] = bound.getUsedDirectives().map(directive => {
+        const type = this.refEmitter.emit(directive.ref, context);
         return {
           ref: directive.ref,
-          type: this.refEmitter.emit(directive.ref, context),
+          type: type.expression,
+          importedFile: type.importedFile,
           selector: directive.selector,
           inputs: directive.inputs.propertyNames,
           outputs: directive.outputs.propertyNames,
           exportAs: directive.exportAs,
           isComponent: directive.isComponent,
         };
       });
-      type UsedPipe = {ref: Reference<ClassDeclaration>, pipeName: string, expression: Expression};
+
+      type UsedPipe = {
+        ref: Reference<ClassDeclaration>,
+        pipeName: string,
+        expression: Expression,
+        importedFile: ImportedFile,
+      };
       const usedPipes: UsedPipe[] = [];
       for (const pipeName of bound.getUsedPipes()) {
         if (!pipes.has(pipeName)) {
           continue;
         }
         const pipe = pipes.get(pipeName)!;
+        const type = this.refEmitter.emit(pipe, context);
         usedPipes.push({
           ref: pipe,
           pipeName,
-          expression: this.refEmitter.emit(pipe, context),
+          expression: type.expression,
+          importedFile: type.importedFile,
         });
       }
       if (this.semanticDepGraphUpdater !== null) {
@@ -665,14 +676,16 @@ export class ComponentDecoratorHandler implements
       // import which needs to be generated would create a cycle.
       const cyclesFromDirectives = new Map<UsedDirective, Cycle>();
       for (const usedDirective of usedDirectives) {
-        const cycle = this._checkForCyclicImport(usedDirective.ref, usedDirective.type, context);
+        const cycle =
+            this._checkForCyclicImport(usedDirective.importedFile, usedDirective.type, context);
         if (cycle !== null) {
           cyclesFromDirectives.set(usedDirective, cycle);
         }
       }
       const cyclesFromPipes = new Map<UsedPipe, Cycle>();
       for (const usedPipe of usedPipes) {
-        const cycle = this._checkForCyclicImport(usedPipe.ref, usedPipe.expression, context);
+        const cycle =
+            this._checkForCyclicImport(usedPipe.importedFile, usedPipe.expression, context);
         if (cycle !== null) {
           cyclesFromPipes.set(usedPipe, cycle);
         }
@@ -682,11 +695,11 @@ export class ComponentDecoratorHandler implements
       if (!cycleDetected) {
         // No cycle was detected. Record the imports that need to be created in the cycle detector
         // so that future cyclic import checks consider their production.
-        for (const {type} of usedDirectives) {
-          this._recordSyntheticImport(type, context);
+        for (const {type, importedFile} of usedDirectives) {
+          this._recordSyntheticImport(importedFile, type, context);
         }
-        for (const {expression} of usedPipes) {
-          this._recordSyntheticImport(expression, context);
+        for (const {expression, importedFile} of usedPipes) {
+          this._recordSyntheticImport(importedFile, expression, context);
         }
 
         // Check whether the directive/pipe arrays in ɵcmp need to be wrapped in closures.
@@ -1189,7 +1202,17 @@ export class ComponentDecoratorHandler implements
     }
   }
 
-  private _expressionToImportedFile(expr: Expression, origin: ts.SourceFile): ts.SourceFile|null {
+  private _resolveImportedFile(importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile):
+      ts.SourceFile|null {
+    // If `importedFile` is not 'unknown' then it accurately reflects the source file that is
+    // being imported.
+    if (importedFile !== 'unknown') {
+      return importedFile;
+    }
+
+    // Otherwise `expr` has to be inspected to determine the file that is being imported. If `expr`
+    // is not an `ExternalExpr` then it does not correspond with an import, so return null in that
+    // case.
     if (!(expr instanceof ExternalExpr)) {
       return null;
     }
@@ -1204,18 +1227,19 @@ export class ComponentDecoratorHandler implements
    *
    * @returns a `Cycle` object if a cycle would be created, otherwise `null`.
    */
-  private _checkForCyclicImport(ref: Reference, expr: Expression, origin: ts.SourceFile): Cycle
-      |null {
-    const importedFile = this._expressionToImportedFile(expr, origin);
-    if (importedFile === null) {
+  private _checkForCyclicImport(
+      importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile): Cycle|null {
+    const imported = this._resolveImportedFile(importedFile, expr, origin);
+    if (imported === null) {
       return null;
     }
     // Check whether the import is legal.
-    return this.cycleAnalyzer.wouldCreateCycle(origin, importedFile);
+    return this.cycleAnalyzer.wouldCreateCycle(origin, imported);
   }
 
-  private _recordSyntheticImport(expr: Expression, origin: ts.SourceFile): void {
-    const imported = this._expressionToImportedFile(expr, origin);
+  private _recordSyntheticImport(
+      importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile): void {
+    const imported = this._resolveImportedFile(importedFile, expr, origin);
     if (imported === null) {
       return;
     }

packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts

@@ -418,7 +418,7 @@ export class NgModuleDecoratorHandler implements
       const context = getSourceFile(node);
       for (const exportRef of analysis.exports) {
         if (isNgModule(exportRef.node, scope.compilation)) {
-          data.injectorImports.push(this.refEmitter.emit(exportRef, context));
+          data.injectorImports.push(this.refEmitter.emit(exportRef, context).expression);
         }
       }
 
@@ -466,12 +466,12 @@ export class NgModuleDecoratorHandler implements
     for (const decl of analysis.declarations) {
       const remoteScope = this.scopeRegistry.getRemoteScope(decl.node);
       if (remoteScope !== null) {
-        const directives =
-            remoteScope.directives.map(directive => this.refEmitter.emit(directive, context));
-        const pipes = remoteScope.pipes.map(pipe => this.refEmitter.emit(pipe, context));
+        const directives = remoteScope.directives.map(
+            directive => this.refEmitter.emit(directive, context).expression);
+        const pipes = remoteScope.pipes.map(pipe => this.refEmitter.emit(pipe, context).expression);
         const directiveArray = new LiteralArrayExpr(directives);
         const pipesArray = new LiteralArrayExpr(pipes);
-        const declExpr = this.refEmitter.emit(decl, context)!;
+        const declExpr = this.refEmitter.emit(decl, context).expression;
         const setComponentScope = new ExternalExpr(R3Identifiers.setComponentScope);
         const callExpr =
             new InvokeFunctionExpr(setComponentScope, [declExpr, directiveArray, pipesArray]);

packages/compiler-cli/src/ngtsc/annotations/src/util.ts

@@ -267,13 +267,12 @@ function createUnsuitableInjectionTokenError(
 export function toR3Reference(
     valueRef: Reference, typeRef: Reference, valueContext: ts.SourceFile,
     typeContext: ts.SourceFile, refEmitter: ReferenceEmitter): R3Reference {
-  const value = refEmitter.emit(valueRef, valueContext);
-  const type = refEmitter.emit(
-      typeRef, typeContext, ImportFlags.ForceNewImport | ImportFlags.AllowTypeImports);
-  if (value === null || type === null) {
-    throw new Error(`Could not refer to ${ts.SyntaxKind[valueRef.node.kind]}`);
-  }
-  return {value, type};
+  return {
+    value: refEmitter.emit(valueRef, valueContext).expression,
+    type: refEmitter
+              .emit(typeRef, typeContext, ImportFlags.ForceNewImport | ImportFlags.AllowTypeImports)
+              .expression,
+  };
 }
 
 export function isAngularCore(decorator: Decorator): decorator is Decorator&{import: Import} {

packages/compiler-cli/src/ngtsc/imports/index.ts

@@ -9,7 +9,7 @@
 export {AliasingHost, AliasStrategy, PrivateExportAliasingHost, UnifiedModulesAliasingHost} from './src/alias';
 export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core';
 export {DefaultImportRecorder, DefaultImportTracker, NOOP_DEFAULT_IMPORT_RECORDER} from './src/default';
-export {AbsoluteModuleStrategy, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter';
+export {AbsoluteModuleStrategy, EmittedReference, ImportedFile, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter';
 export {Reexport} from './src/reexport';
 export {OwningModule, Reference} from './src/references';
 export {ModuleResolver} from './src/resolver';

packages/compiler-cli/src/ngtsc/imports/src/alias.ts

@@ -10,13 +10,12 @@ import {Expression, ExternalExpr} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {UnifiedModulesHost} from '../../core/api';
-import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../reflection';
+import {ClassDeclaration, ReflectionHost} from '../../reflection';
 
-import {ImportFlags, ReferenceEmitStrategy} from './emitter';
+import {EmittedReference, ImportFlags, ReferenceEmitStrategy} from './emitter';
 import {Reference} from './references';
 
 
-
 // Escape anything that isn't alphanumeric, '/' or '_'.
 const CHARS_TO_ESCAPE = /[^a-zA-Z0-9/_]/g;
 
@@ -213,11 +212,11 @@ export class PrivateExportAliasingHost implements AliasingHost {
  * directive or pipe, if it exists.
  */
 export class AliasStrategy implements ReferenceEmitStrategy {
-  emit(ref: Reference<ts.Node>, context: ts.SourceFile, importMode: ImportFlags): Expression|null {
-    if (importMode & ImportFlags.NoAliasing) {
+  emit(ref: Reference, context: ts.SourceFile, importMode: ImportFlags): EmittedReference|null {
+    if (importMode & ImportFlags.NoAliasing || ref.alias === null) {
       return null;
     }
 
-    return ref.alias;
+    return {expression: ref.alias, importedFile: 'unknown'};
   }
 }