Skip to content

Commit f42e6ce

Browse files
JoostKatscott
authored andcommitted
perf(compiler-cli): only generate type-check code for referenced DOM elements (angular#38418)
The template type-checker would generate a statement with a call expression for all DOM elements in a template of the form: ``` const _t1 = document.createElement("div"); ``` Profiling has shown that this is a particularly expensive call to perform type inference on, as TypeScript needs to perform signature selection of `Document.createElement` and resolve the exact type from the `HTMLElementTagNameMap`. However, it can be observed that the statement by itself does not contribute anything to the type-checking result if `_t1` is not actually used anywhere, which is only rarely the case---it requires that the element is referenced by its name from somewhere else in the template. Consequently, the type-checker can skip generating this statement altogether for most DOM elements. The effect of this optimization is significant in several phases: 1. Less type-check code to generate 2. Less type-check code to emit and parse again 3. No expensive type inference to perform for the call expression The effect on phase 3 is the most significant here, as type-checking is not currently incremental in the sense that only phases 1 and 2 can be reused from a prior compilation. The actual type-checking of all templates in phase 3 needs to be repeated on each incremental compilation, so any performance gains we achieve here are very beneficial. PR Close angular#38418
1 parent 175c79d commit f42e6ce

File tree

4 files changed

+166
-86
lines changed

4 files changed

+166
-86
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,13 @@ export function generateTypeCheckBlock(
9999
* `ts.Expression` which can be used to reference the operation's result.
100100
*/
101101
abstract class TcbOp {
102+
/**
103+
* Set to true if this operation can be considered optional. Optional operations are only executed
104+
* when depended upon by other operations, otherwise they are disregarded. This allows for less
105+
* code to generate, parse and type-check, overall positively contributing to performance.
106+
*/
107+
abstract readonly optional: boolean;
108+
102109
abstract execute(): ts.Expression|null;
103110

104111
/**
@@ -125,6 +132,13 @@ class TcbElementOp extends TcbOp {
125132
super();
126133
}
127134

135+
get optional() {
136+
// The statement generated by this operation is only used for type-inference of the DOM
137+
// element's type and won't report diagnostics by itself, so the operation is marked as optional
138+
// to avoid generating statements for DOM elements that are never referenced.
139+
return true;
140+
}
141+
128142
execute(): ts.Identifier {
129143
const id = this.tcb.allocateId();
130144
// Add the declaration of the element using document.createElement.
@@ -148,6 +162,10 @@ class TcbVariableOp extends TcbOp {
148162
super();
149163
}
150164

165+
get optional() {
166+
return false;
167+
}
168+
151169
execute(): ts.Identifier {
152170
// Look for a context variable for the template.
153171
const ctx = this.scope.resolve(this.template);
@@ -176,6 +194,10 @@ class TcbTemplateContextOp extends TcbOp {
176194
super();
177195
}
178196

197+
get optional() {
198+
return false;
199+
}
200+
179201
execute(): ts.Identifier {
180202
// Allocate a template ctx variable and declare it with an 'any' type. The type of this variable
181203
// may be narrowed as a result of template guard conditions.
@@ -198,6 +220,10 @@ class TcbTemplateBodyOp extends TcbOp {
198220
super();
199221
}
200222

223+
get optional() {
224+
return false;
225+
}
226+
201227
execute(): null {
202228
// An `if` will be constructed, within which the template's children will be type checked. The
203229
// `if` is used for two reasons: it creates a new syntactic scope, isolating variables declared
@@ -301,6 +327,10 @@ class TcbTextInterpolationOp extends TcbOp {
301327
super();
302328
}
303329

330+
get optional() {
331+
return false;
332+
}
333+
304334
execute(): null {
305335
const expr = tcbExpression(this.binding.value, this.tcb, this.scope);
306336
this.scope.addStatement(ts.createExpressionStatement(expr));
@@ -324,6 +354,10 @@ class TcbDirectiveTypeOp extends TcbOp {
324354
super();
325355
}
326356

357+
get optional() {
358+
return false;
359+
}
360+
327361
execute(): ts.Identifier {
328362
const id = this.tcb.allocateId();
329363

@@ -352,6 +386,10 @@ class TcbDirectiveCtorOp extends TcbOp {
352386
super();
353387
}
354388

389+
get optional() {
390+
return false;
391+
}
392+
355393
execute(): ts.Identifier {
356394
const id = this.tcb.allocateId();
357395

@@ -409,6 +447,10 @@ class TcbDirectiveInputsOp extends TcbOp {
409447
super();
410448
}
411449

450+
get optional() {
451+
return false;
452+
}
453+
412454
execute(): null {
413455
const dirId = this.scope.resolve(this.node, this.dir);
414456

@@ -514,6 +556,10 @@ class TcbDirectiveCtorCircularFallbackOp extends TcbOp {
514556
super();
515557
}
516558

559+
get optional() {
560+
return false;
561+
}
562+
517563
execute(): ts.Identifier {
518564
const id = this.tcb.allocateId();
519565
const typeCtor = this.tcb.env.typeCtorFor(this.dir);
@@ -541,6 +587,10 @@ class TcbDomSchemaCheckerOp extends TcbOp {
541587
super();
542588
}
543589

590+
get optional() {
591+
return false;
592+
}
593+
544594
execute(): ts.Expression|null {
545595
if (this.checkElement) {
546596
this.tcb.domSchemaChecker.checkElement(this.tcb.id, this.element, this.tcb.schemas);
@@ -597,10 +647,14 @@ class TcbUnclaimedInputsOp extends TcbOp {
597647
super();
598648
}
599649

650+
get optional() {
651+
return false;
652+
}
653+
600654
execute(): null {
601655
// `this.inputs` contains only those bindings not matched by any directive. These bindings go to
602656
// the element itself.
603-
const elId = this.scope.resolve(this.element);
657+
let elId: ts.Expression|null = null;
604658

605659
// TODO(alxhub): this could be more efficient.
606660
for (const binding of this.element.inputs) {
@@ -622,6 +676,9 @@ class TcbUnclaimedInputsOp extends TcbOp {
622676

623677
if (this.tcb.env.config.checkTypeOfDomBindings && binding.type === BindingType.Property) {
624678
if (binding.name !== 'style' && binding.name !== 'class') {
679+
if (elId === null) {
680+
elId = this.scope.resolve(this.element);
681+
}
625682
// A direct binding to a property.
626683
const propertyName = ATTR_TO_PROP[binding.name] || binding.name;
627684
const prop = ts.createElementAccess(elId, ts.createStringLiteral(propertyName));
@@ -656,6 +713,10 @@ class TcbDirectiveOutputsOp extends TcbOp {
656713
super();
657714
}
658715

716+
get optional() {
717+
return false;
718+
}
719+
659720
execute(): null {
660721
const dirId = this.scope.resolve(this.node, this.dir);
661722

@@ -723,8 +784,12 @@ class TcbUnclaimedOutputsOp extends TcbOp {
723784
super();
724785
}
725786

787+
get optional() {
788+
return false;
789+
}
790+
726791
execute(): null {
727-
const elId = this.scope.resolve(this.element);
792+
let elId: ts.Expression|null = null;
728793

729794
// TODO(alxhub): this could be more efficient.
730795
for (const output of this.element.outputs) {
@@ -749,6 +814,9 @@ class TcbUnclaimedOutputsOp extends TcbOp {
749814
// base `Event` type.
750815
const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer);
751816

817+
if (elId === null) {
818+
elId = this.scope.resolve(this.element);
819+
}
752820
const call = ts.createCall(
753821
/* expression */ ts.createPropertyAccess(elId, 'addEventListener'),
754822
/* typeArguments */ undefined,
@@ -965,7 +1033,7 @@ class Scope {
9651033
*/
9661034
render(): ts.Statement[] {
9671035
for (let i = 0; i < this.opQueue.length; i++) {
968-
this.executeOp(i);
1036+
this.executeOp(i, /* skipOptional */ true);
9691037
}
9701038
return this.statements;
9711039
}
@@ -1031,7 +1099,7 @@ class Scope {
10311099
* Like `executeOp`, but assert that the operation actually returned `ts.Expression`.
10321100
*/
10331101
private resolveOp(opIndex: number): ts.Expression {
1034-
const res = this.executeOp(opIndex);
1102+
const res = this.executeOp(opIndex, /* skipOptional */ false);
10351103
if (res === null) {
10361104
throw new Error(`Error resolving operation, got null`);
10371105
}
@@ -1045,12 +1113,16 @@ class Scope {
10451113
* and also protects against a circular dependency from the operation to itself by temporarily
10461114
* setting the operation's result to a special expression.
10471115
*/
1048-
private executeOp(opIndex: number): ts.Expression|null {
1116+
private executeOp(opIndex: number, skipOptional: boolean): ts.Expression|null {
10491117
const op = this.opQueue[opIndex];
10501118
if (!(op instanceof TcbOp)) {
10511119
return op;
10521120
}
10531121

1122+
if (skipOptional && op.optional) {
1123+
return null;
1124+
}
1125+
10541126
// Set the result of the operation in the queue to its circular fallback. If executing this
10551127
// operation results in a circular dependency, this will prevent an infinite loop and allow for
10561128
// the resolution of such cycles.

packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ describe('type check blocks diagnostics', () => {
158158
}];
159159
const TEMPLATE = `<my-cmp #a></my-cmp>{{ a || a }}`;
160160
expect(tcbWithSpans(TEMPLATE, DIRECTIVES))
161-
.toContain('((_t2 /*23,24*/) || (_t2 /*28,29*/) /*23,29*/);');
161+
.toContain('((_t1 /*23,24*/) || (_t1 /*28,29*/) /*23,29*/);');
162162
});
163163
});
164164
});

0 commit comments

Comments
 (0)