From 65391abb3ca7a8dbf5aa8d4a3eb5a8c7e006cc18 Mon Sep 17 00:00:00 2001 From: JoostK Date: Tue, 18 Aug 2020 21:40:35 +0200 Subject: [PATCH] perf(compiler-cli): only emit directive/pipe references that are used For the compilation of a component, the compiler has to prepare some information about the directives and pipes that are used in the template. This information includes an expression for directives/pipes, for usage within the compilation output. For large NgModule compilation scopes this has shown to introduce a performance hotspot, as the generation of expressions is quite expensive. This commit reduces the performance overhead by only generating expressions for the directives/pipes that are actually used within the template, significantly cutting down on the compiler's resolve phase. --- .../src/ngtsc/annotations/src/component.ts | 62 +++++++++++-------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 5ea6aec53274e0..dcb8ee596a278b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {compileComponentFromMetadata, ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, Identifiers, InterpolationConfig, LexerRange, makeBindingParser, ParseError, ParseSourceFile, parseTemplate, ParseTemplateOptions, R3ComponentMetadata, R3FactoryTarget, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler'; +import {compileComponentFromMetadata, ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, Identifiers, InterpolationConfig, LexerRange, makeBindingParser, ParseError, ParseSourceFile, parseTemplate, R3ComponentMetadata, R3FactoryTarget, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {CycleAnalyzer} from '../../cycles'; @@ -30,7 +30,7 @@ import {createValueHasWrongTypeError, getDirectiveDiagnostics, getProviderDiagno import {extractDirectiveMetadata, parseFieldArrayValue} from './directive'; import {compileNgFactoryDefField} from './factory'; import {generateSetClassMetadataCall} from './metadata'; -import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, makeDuplicateDeclarationError, readBaseClass, resolveProvidersRequiringFactory, unwrapExpression, wrapFunctionExpressionsInParens} from './util'; +import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, readBaseClass, resolveProvidersRequiringFactory, unwrapExpression, wrapFunctionExpressionsInParens} from './util'; const EMPTY_MAP = new Map(); const EMPTY_ARRAY: any[] = []; @@ -471,36 +471,49 @@ export class ComponentDecoratorHandler implements // Set up the R3TargetBinder, as well as a 'directives' array and a 'pipes' map that are later // fed to the TemplateDefinitionBuilder. First, a SelectorMatcher is constructed to match // directives that are in scope. - const matcher = new SelectorMatcher(); - const directives: {selector: string, expression: Expression}[] = []; + type MatchedDirective = DirectiveMeta&{selector: string}; + const matcher = new SelectorMatcher(); for (const dir of scope.compilation.directives) { - const {ref, selector} = dir; - if (selector !== null) { - const expression = this.refEmitter.emit(ref, context); - directives.push({selector, expression}); - matcher.addSelectables(CssSelector.parse(selector), {...dir, expression}); + if (dir.selector !== null) { + matcher.addSelectables(CssSelector.parse(dir.selector), dir as MatchedDirective); } } - const pipes = new Map(); + const pipes = new Map>(); for (const pipe of scope.compilation.pipes) { - pipes.set(pipe.name, this.refEmitter.emit(pipe.ref, context)); + pipes.set(pipe.name, pipe.ref); } - // Next, the component template AST is bound using the R3TargetBinder. This produces an + // Next, the component template AST is bound using the R3TargetBinder. This produces a // BoundTarget, which is similar to a ts.TypeChecker. const binder = new R3TargetBinder(matcher); const bound = binder.bind({template: metadata.template.nodes}); // The BoundTarget knows which directives and pipes matched the template. - const usedDirectives = bound.getUsedDirectives(); - const usedPipes = bound.getUsedPipes().map(name => pipes.get(name)!); + const usedDirectives = bound.getUsedDirectives().map(directive => { + return { + selector: directive.selector, + expression: this.refEmitter.emit(directive.ref, context), + }; + }); + + const usedPipes: {pipeName: string, expression: Expression}[] = []; + for (const pipeName of bound.getUsedPipes()) { + if (!pipes.has(pipeName)) { + continue; + } + const pipe = pipes.get(pipeName)!; + usedPipes.push({ + pipeName, + expression: this.refEmitter.emit(pipe, context), + }); + } // Scan through the directives/pipes actually used in the template and check whether any // import which needs to be generated would create a cycle. const cycleDetected = usedDirectives.some(dir => this._isCyclicImport(dir.expression, context)) || - usedPipes.some(pipe => this._isCyclicImport(pipe, context)); + usedPipes.some(pipe => this._isCyclicImport(pipe.expression, context)); if (!cycleDetected) { // No cycle was detected. Record the imports that need to be created in the cycle detector @@ -508,8 +521,8 @@ export class ComponentDecoratorHandler implements for (const {expression} of usedDirectives) { this._recordSyntheticImport(expression, context); } - for (const pipe of usedPipes) { - this._recordSyntheticImport(pipe, context); + for (const {expression} of usedPipes) { + this._recordSyntheticImport(expression, context); } // Check whether the directive/pipe arrays in ɵcmp need to be wrapped in closures. @@ -518,16 +531,11 @@ export class ComponentDecoratorHandler implements const wrapDirectivesAndPipesInClosure = usedDirectives.some( dir => isExpressionForwardReference(dir.expression, node.name, context)) || - usedPipes.some(pipe => isExpressionForwardReference(pipe, node.name, context)); - - // Actual compilation still uses the full scope, not the narrowed scope determined by - // R3TargetBinder. This is a hedge against potential issues with the R3TargetBinder - right - // now the TemplateDefinitionBuilder is the "source of truth" for which directives/pipes are - // actually used (though the two should agree perfectly). - // - // TODO(alxhub): switch TemplateDefinitionBuilder over to using R3TargetBinder directly. - data.directives = directives; - data.pipes = pipes; + usedPipes.some( + pipe => isExpressionForwardReference(pipe.expression, node.name, context)); + + data.directives = usedDirectives; + data.pipes = new Map(usedPipes.map(pipe => [pipe.pipeName, pipe.expression])); data.wrapDirectivesAndPipesInClosure = wrapDirectivesAndPipesInClosure; } else { // Declaring the directiveDefs/pipeDefs arrays directly would require imports that would