perf(compiler-cli): template type-checking performance improvements #38418
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoostK
added
state: WIP
refactoring
alxhub
approved these changes
Aug 12, 2020
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Outdated
Show resolved
Hide resolved
…elements
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.
The template type-checker would always generate a directive declaration even if its type was never used. For example, directives without any input nor output bindings nor exportAs references don't need the directive to be declared, as its type would never be used. This commit makes the `TcbOp`s that are responsible for declaring a directive as optional, such that they are only executed when requested from another operation.
For a template that contains for example `<span *ngIf="first"></span>` there's no need to render the `NgIf` guard expression, as the child scope does not have any type-checking statements, so any narrowing effect of the guard is not applicable. This seems like a minor improvement, however it reduces the number of flow-node antecedents that TypeScript needs to keep into account for such cases, resulting in an overall reduction of type-checking time.
JoostK
added
target: major
JoostK
added
action: merge
15 tasks
atscott
pushed a commit
that referenced
this pull request
Aug 13, 2020
…38418) The template type-checker would always generate a directive declaration even if its type was never used. For example, directives without any input nor output bindings nor exportAs references don't need the directive to be declared, as its type would never be used. This commit makes the `TcbOp`s that are responsible for declaring a directive as optional, such that they are only executed when requested from another operation. PR Close #38418
atscott
pushed a commit
that referenced
this pull request
Aug 13, 2020
…pty (#38418) For a template that contains for example `<span *ngIf="first"></span>` there's no need to render the `NgIf` guard expression, as the child scope does not have any type-checking statements, so any narrowing effect of the guard is not applicable. This seems like a minor improvement, however it reduces the number of flow-node antecedents that TypeScript needs to keep into account for such cases, resulting in an overall reduction of type-checking time. PR Close #38418
profanis
pushed a commit
to profanis/angular
that referenced
this pull request
Sep 5, 2020
…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
profanis
pushed a commit
to profanis/angular
that referenced
this pull request
Sep 5, 2020
…ngular#38418) The template type-checker would always generate a directive declaration even if its type was never used. For example, directives without any input nor output bindings nor exportAs references don't need the directive to be declared, as its type would never be used. This commit makes the `TcbOp`s that are responsible for declaring a directive as optional, such that they are only executed when requested from another operation. PR Close angular#38418
profanis
pushed a commit
to profanis/angular
that referenced
this pull request
Sep 5, 2020
…pty (angular#38418) For a template that contains for example `<span *ngIf="first"></span>` there's no need to render the `NgIf` guard expression, as the child scope does not have any type-checking statements, so any narrowing effect of the guard is not applicable. This seems like a minor improvement, however it reduces the number of flow-node antecedents that TypeScript needs to keep into account for such cases, resulting in an overall reduction of type-checking time. PR Close angular#38418
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits.
I tested these changes in the ng-speed-rebuild repo, using Angular 10.1-next.5 and the changes in this PR, using roughly the following:
Prior to this PR, using 10.1-next.5,
ngctook 75 seconds to execute. With the changes from this PR, it is reduced to 45 seconds; a 30 second reduction in time. The 30 second improvement is mostly from reduced type-checking time that TypeScript performs; from 35s to 7s. This is beneficial for incremental builds as well, given that type-checking is not done incrementally and has to be repeated from scratch on each incremental compilation. Side-note: the CLI performs incremental type-checking in a background process, so the effect of this may not be immediately visible (although your CPU will enjoy the extra free time!).Thanks to @elvisbegovic for the awesome ng-speed-rebuild repo ❤️ Having these repo's available is super valuable in chasing down performance bottlenecks and experimenting with ways to address them.