Skip to content

Commit 49fe974

Browse files
alxhubthePunderWoman
alxhub
authored and
thePunderWoman
committed
perf(compiler-cli): optimize NgModule emit for standalone components (#49837)
NgModules which import standalone components currently list those components in their injector definitions, because we assume that any standalone component may export providers from its own imports. This commit adds an optimization for that emit, which attempts to statically analyze the NgModule imports and determine which standalone components, if any are present, do not export providers and thus can be omitted. This analysis is imperfect, because some imported components may be declared outside of the current compilation, or transitively import types which are declared outside the compilation. These types are therefore _assumed_ to carry providers and so the optimization isn't applied to them. PR Close #49837
1 parent 6a09cc1 commit 49fe974

File tree

16 files changed

+392
-21
lines changed

16 files changed

+392
-21
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,7 @@ export class ComponentDecoratorHandler implements
503503
animationTriggerNames: analysis.animationTriggerNames,
504504
schemas: analysis.schemas,
505505
decorator: analysis.decorator,
506+
assumedToExportProviders: false,
506507
});
507508

508509
this.resourceRegistry.registerResources(analysis.resources, node);

‎packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ export class DirectiveDecoratorHandler implements
158158
imports: null,
159159
schemas: null,
160160
decorator: analysis.decorator,
161+
// Directives analyzed within our own compilation are not _assumed_ to export providers.
162+
// Instead, we statically analyze their imports to make a direct determination.
163+
assumedToExportProviders: false,
161164
});
162165

163166
this.injectableRegistry.registerInjectable(node, {

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

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import ts from 'typescript';
1111

1212
import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics';
1313
import {assertSuccessfulReferenceEmit, Reference, ReferenceEmitter} from '../../../imports';
14-
import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticReference, SemanticSymbol,} from '../../../incremental/semantic_graph';
15-
import {MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata';
14+
import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticDepGraphUpdater, SemanticReference, SemanticSymbol,} from '../../../incremental/semantic_graph';
15+
import {ExportedProviderStatusResolver, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata';
1616
import {PartialEvaluator, ResolvedValue, SyntheticValue} from '../../../partial_evaluator';
1717
import {PerfEvent, PerfRecorder} from '../../../perf';
1818
import {ClassDeclaration, DeclarationNode, Decorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral,} from '../../../reflection';
@@ -59,12 +59,32 @@ export class NgModuleSymbol extends SemanticSymbol {
5959
usedPipes: SemanticReference[]
6060
}[] = [];
6161

62+
/**
63+
* `SemanticSymbol`s of the transitive imports of this NgModule which came from imported
64+
* standalone components.
65+
*
66+
* Standalone components are excluded/included in the `InjectorDef` emit output of the NgModule
67+
* based on whether the compiler can prove that their transitive imports may contain exported
68+
* providers, so a change in this set of symbols may affect the compilation output of this
69+
* NgModule.
70+
*/
71+
private transitiveImportsFromStandaloneComponents = new Set<SemanticSymbol>();
72+
73+
constructor(decl: ClassDeclaration, public readonly hasProviders: boolean) {
74+
super(decl);
75+
}
76+
6277
override isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
6378
if (!(previousSymbol instanceof NgModuleSymbol)) {
6479
return true;
6580
}
6681

67-
// NgModules don't have a public API that could affect emit of Angular decorated classes.
82+
// Changes in the provider status of this NgModule affect downstream dependencies, which may
83+
// consider provider status in their own emits.
84+
if (previousSymbol.hasProviders !== this.hasProviders) {
85+
return true;
86+
}
87+
6888
return false;
6989
}
7090

@@ -102,6 +122,25 @@ export class NgModuleSymbol extends SemanticSymbol {
102122
return true;
103123
}
104124
}
125+
126+
if (previousSymbol.transitiveImportsFromStandaloneComponents.size !==
127+
this.transitiveImportsFromStandaloneComponents.size) {
128+
return true;
129+
}
130+
131+
const previousImports = Array.from(previousSymbol.transitiveImportsFromStandaloneComponents);
132+
for (const transitiveImport of this.transitiveImportsFromStandaloneComponents) {
133+
const prevEntry =
134+
previousImports.find(prevEntry => isSymbolEqual(prevEntry, transitiveImport));
135+
if (prevEntry === undefined) {
136+
return true;
137+
}
138+
139+
if (transitiveImport.isPublicApiAffected(prevEntry)) {
140+
return true;
141+
}
142+
}
143+
105144
return false;
106145
}
107146

@@ -118,6 +157,10 @@ export class NgModuleSymbol extends SemanticSymbol {
118157
usedPipes: SemanticReference[]): void {
119158
this.remotelyScopedComponents.push({component, usedDirectives, usedPipes});
120159
}
160+
161+
addTransitiveImportFromStandaloneComponent(importedSymbol: SemanticSymbol): void {
162+
this.transitiveImportsFromStandaloneComponents.add(importedSymbol);
163+
}
121164
}
122165

123166
/**
@@ -129,7 +172,9 @@ export class NgModuleDecoratorHandler implements
129172
private reflector: ReflectionHost, private evaluator: PartialEvaluator,
130173
private metaReader: MetadataReader, private metaRegistry: MetadataRegistry,
131174
private scopeRegistry: LocalModuleScopeRegistry,
132-
private referencesRegistry: ReferencesRegistry, private isCore: boolean,
175+
private referencesRegistry: ReferencesRegistry,
176+
private exportedProviderStatusResolver: ExportedProviderStatusResolver,
177+
private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, private isCore: boolean,
133178
private refEmitter: ReferenceEmitter, private annotateForClosureCompiler: boolean,
134179
private onlyPublishPublicTypings: boolean,
135180
private injectableRegistry: InjectableClassRegistry, private perf: PerfRecorder) {}
@@ -433,8 +478,8 @@ export class NgModuleDecoratorHandler implements
433478
};
434479
}
435480

436-
symbol(node: ClassDeclaration): NgModuleSymbol {
437-
return new NgModuleSymbol(node);
481+
symbol(node: ClassDeclaration, analysis: NgModuleAnalysis): NgModuleSymbol {
482+
return new NgModuleSymbol(node, analysis.providers !== null);
438483
}
439484

440485
register(node: ClassDeclaration, analysis: NgModuleAnalysis): void {
@@ -452,6 +497,7 @@ export class NgModuleDecoratorHandler implements
452497
rawImports: analysis.rawImports,
453498
rawExports: analysis.rawExports,
454499
decorator: analysis.decorator,
500+
mayDeclareProviders: analysis.providers !== null,
455501
});
456502

457503
this.injectableRegistry.registerInjectable(node, {
@@ -489,11 +535,38 @@ export class NgModuleDecoratorHandler implements
489535
}
490536

491537
const refsToEmit: Reference<ClassDeclaration>[] = [];
538+
let symbol: NgModuleSymbol|null = null;
539+
if (this.semanticDepGraphUpdater !== null) {
540+
const sym = this.semanticDepGraphUpdater.getSymbol(node) as NgModuleSymbol;
541+
if (sym instanceof NgModuleSymbol) {
542+
symbol = sym;
543+
}
544+
}
545+
492546
for (const ref of topLevelImport.resolvedReferences) {
493547
const dirMeta = this.metaReader.getDirectiveMetadata(ref);
494-
if (dirMeta !== null && !dirMeta.isComponent) {
495-
// Skip emit of directives in imports - directives can't carry providers.
496-
continue;
548+
if (dirMeta !== null) {
549+
if (!dirMeta.isComponent) {
550+
// Skip emit of directives in imports - directives can't carry providers.
551+
continue;
552+
}
553+
554+
// Check whether this component has providers.
555+
const mayExportProviders =
556+
this.exportedProviderStatusResolver.mayExportProviders(dirMeta.ref, (importRef) => {
557+
// We need to keep track of which transitive imports were used to decide
558+
// `mayExportProviders`, since if those change in a future compilation this
559+
// NgModule will need to be re-emitted.
560+
if (symbol !== null && this.semanticDepGraphUpdater !== null) {
561+
const importSymbol = this.semanticDepGraphUpdater.getSymbol(importRef.node);
562+
symbol.addTransitiveImportFromStandaloneComponent(importSymbol);
563+
}
564+
});
565+
566+
if (!mayExportProviders) {
567+
// Skip emit of components that don't carry providers.
568+
continue;
569+
}
497570
}
498571

499572
const pipeMeta = dirMeta === null ? this.metaReader.getPipeMetadata(ref) : null;

‎packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import ts from 'typescript';
1212
import {absoluteFrom} from '../../../file_system';
1313
import {runInEachFileSystem} from '../../../file_system/testing';
1414
import {LocalIdentifierStrategy, ReferenceEmitter} from '../../../imports';
15-
import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../../metadata';
15+
import {CompoundMetadataReader, DtsMetadataReader, ExportedProviderStatusResolver, LocalMetadataRegistry} from '../../../metadata';
1616
import {PartialEvaluator} from '../../../partial_evaluator';
1717
import {NOOP_PERF_RECORDER} from '../../../perf';
1818
import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection';
@@ -68,11 +68,12 @@ runInEachFileSystem(() => {
6868
new ReferenceEmitter([]), null);
6969
const refEmitter = new ReferenceEmitter([new LocalIdentifierStrategy()]);
7070
const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false);
71+
const exportedProviderStatusResolver = new ExportedProviderStatusResolver(metaReader);
7172

7273
const handler = new NgModuleDecoratorHandler(
7374
reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry,
74-
/* isCore */ false, refEmitter,
75-
/* annotateForClosureCompiler */ false, /* onlyPublishPublicTypings */ false,
75+
exportedProviderStatusResolver, /* semanticDepGraphUpdater */ null, /* isCore */ false,
76+
refEmitter, /* annotateForClosureCompiler */ false, /* onlyPublishPublicTypings */ false,
7677
injectableRegistry, NOOP_PERF_RECORDER);
7778
const TestModule =
7879
getDeclaration(program, _('/entry.ts'), 'TestModule', isNamedClassDeclaration);

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracke
1818
import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from '../../incremental';
1919
import {SemanticSymbol} from '../../incremental/semantic_graph';
2020
import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer';
21-
import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, HostDirectivesResolver, LocalMetadataRegistry, MetadataReader, MetadataReaderWithIndex, PipeMeta, ResourceRegistry} from '../../metadata';
21+
import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, ExportedProviderStatusResolver, HostDirectivesResolver, LocalMetadataRegistry, MetadataReader, MetadataReaderWithIndex, PipeMeta, ResourceRegistry} from '../../metadata';
2222
import {NgModuleIndexImpl} from '../../metadata/src/ng_module_index';
2323
import {PartialEvaluator} from '../../partial_evaluator';
2424
import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf';
@@ -991,6 +991,7 @@ export class NgCompiler {
991991
const metaRegistry = new CompoundMetadataRegistry([localMetaRegistry, ngModuleScopeRegistry]);
992992
const injectableRegistry = new InjectableClassRegistry(reflector, isCore);
993993
const hostDirectivesResolver = new HostDirectivesResolver(metaReader);
994+
const exportedProviderStatusResolver = new ExportedProviderStatusResolver(metaReader);
994995

995996
const typeCheckScopeRegistry =
996997
new TypeCheckScopeRegistry(scopeReader, metaReader, hostDirectivesResolver);
@@ -1061,9 +1062,9 @@ export class NgCompiler {
10611062
this.delegatingPerfRecorder),
10621063
new NgModuleDecoratorHandler(
10631064
reflector, evaluator, metaReader, metaRegistry, ngModuleScopeRegistry, referencesRegistry,
1064-
isCore, refEmitter, this.closureCompilerEnabled,
1065-
this.options.onlyPublishPublicTypingsForNgModules ?? false, injectableRegistry,
1066-
this.delegatingPerfRecorder),
1065+
exportedProviderStatusResolver, semanticDepGraphUpdater, isCore, refEmitter,
1066+
this.closureCompilerEnabled, this.options.onlyPublishPublicTypingsForNgModules ?? false,
1067+
injectableRegistry, this.delegatingPerfRecorder),
10671068
];
10681069

10691070
const traitCompiler = new TraitCompiler(

‎packages/compiler-cli/src/ngtsc/metadata/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ export {CompoundMetadataRegistry, LocalMetadataRegistry} from './src/registry';
1313
export {ResourceRegistry, Resource, ComponentResources, isExternalResource, ExternalResource} from './src/resource_registry';
1414
export {extractDirectiveTypeCheckMeta, hasInjectableFields, CompoundMetadataReader} from './src/util';
1515
export {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, InputOrOutput} from './src/property_mapping';
16+
export {ExportedProviderStatusResolver} from './src/providers';
1617
export {HostDirectivesResolver} from './src/host_directives_resolver';

‎packages/compiler-cli/src/ngtsc/metadata/src/api.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ export interface NgModuleMeta {
5555
* If this is `null`, no decorator exists, meaning it's probably from a .d.ts file.
5656
*/
5757
decorator: ts.Decorator|null;
58+
59+
/**
60+
* Whether this NgModule may declare providers.
61+
*
62+
* If the compiler does not know if the NgModule may declare providers, this will be `true` (for
63+
* example, NgModules declared outside the current compilation are assumed to declare providers).
64+
*/
65+
mayDeclareProviders: boolean;
5866
}
5967

6068
/**
@@ -195,6 +203,11 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta {
195203

196204
/** Additional directives applied to the directive host. */
197205
hostDirectives: HostDirectiveMeta[]|null;
206+
207+
/**
208+
* Whether the directive should be assumed to export providers if imported as a standalone type.
209+
*/
210+
assumedToExportProviders: boolean;
198211
}
199212

200213
/** Metadata collected about an additional directive that is being applied to a directive host. */

‎packages/compiler-cli/src/ngtsc/metadata/src/dts.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ export class DtsMetadataReader implements MetadataReader {
6060
rawImports: null,
6161
rawExports: null,
6262
decorator: null,
63+
// NgModules declared outside the current compilation are assumed to contain providers, as it
64+
// would be a non-breaking change for a library to introduce providers at any point.
65+
mayDeclareProviders: true,
6366
};
6467
}
6568

@@ -128,6 +131,8 @@ export class DtsMetadataReader implements MetadataReader {
128131
// The same goes for schemas.
129132
schemas: null,
130133
decorator: null,
134+
// Assume that standalone components from .d.ts files may export providers.
135+
assumedToExportProviders: isComponent && isStandalone,
131136
};
132137
}
133138

‎packages/compiler-cli/src/ngtsc/metadata/src/providers.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Reference} from '../../imports';
10+
import {ClassDeclaration} from '../../reflection';
11+
12+
import {MetadataReader} from './api';
13+
14+
/**
15+
* Determines whether types may or may not export providers to NgModules, by transitively walking
16+
* the NgModule & standalone import graph.
17+
*/
18+
export class ExportedProviderStatusResolver {
19+
/**
20+
* `ClassDeclaration`s that we are in the process of determining the provider status for.
21+
*
22+
* This is used to detect cycles in the import graph and avoid getting stuck in them.
23+
*/
24+
private calculating = new Set<ClassDeclaration>();
25+
26+
constructor(private metaReader: MetadataReader) {}
27+
28+
/**
29+
* Determines whether `ref` may or may not export providers to NgModules which import it.
30+
*
31+
* NgModules export providers if any are declared, and standalone components export providers from
32+
* their `imports` array (if any).
33+
*
34+
* If `true`, then `ref` should be assumed to export providers. In practice, this could mean
35+
* either that `ref` is a local type that we _know_ exports providers, or it's imported from a
36+
* .d.ts library and is declared in a way where the compiler cannot prove that it doesn't.
37+
*
38+
* If `false`, then `ref` is guaranteed not to export providers.
39+
*
40+
* @param `ref` the class for which the provider status should be determined
41+
* @param `dependencyCallback` a callback that, if provided, will be called for every type
42+
* which is used in the determination of provider status for `ref`
43+
* @returns `true` if `ref` should be assumed to export providers, or `false` if the compiler can
44+
* prove that it does not
45+
*/
46+
mayExportProviders(
47+
ref: Reference<ClassDeclaration>,
48+
dependencyCallback?: (importRef: Reference<ClassDeclaration>) => void): boolean {
49+
if (this.calculating.has(ref.node)) {
50+
// For cycles, we treat the cyclic edge as not having providers.
51+
return false;
52+
}
53+
this.calculating.add(ref.node);
54+
55+
if (dependencyCallback !== undefined) {
56+
dependencyCallback(ref);
57+
}
58+
59+
try {
60+
const dirMeta = this.metaReader.getDirectiveMetadata(ref);
61+
if (dirMeta !== null) {
62+
if (!dirMeta.isComponent || !dirMeta.isStandalone) {
63+
return false;
64+
}
65+
66+
if (dirMeta.assumedToExportProviders) {
67+
return true;
68+
}
69+
70+
// If one of the imports contains providers, then so does this component.
71+
return (dirMeta.imports ?? [])
72+
.some(importRef => this.mayExportProviders(importRef, dependencyCallback));
73+
}
74+
75+
const pipeMeta = this.metaReader.getPipeMetadata(ref);
76+
if (pipeMeta !== null) {
77+
return false;
78+
}
79+
80+
const ngModuleMeta = this.metaReader.getNgModuleMetadata(ref);
81+
if (ngModuleMeta !== null) {
82+
if (ngModuleMeta.mayDeclareProviders) {
83+
return true;
84+
}
85+
86+
// If one of the NgModule's imports may contain providers, then so does this NgModule.
87+
return ngModuleMeta.imports.some(
88+
importRef => this.mayExportProviders(importRef, dependencyCallback));
89+
}
90+
91+
return false;
92+
} finally {
93+
this.calculating.delete(ref.node);
94+
}
95+
}
96+
}

0 commit comments

Comments
 (0)