Optimizing incremental compilation status tracker #40728
Labels
area: compiler
Issues related to `ngc`, Angular's template compiler
area: language-service
Issues related to Angular's VS Code language service
area: performance
Issues related to performance
feature
Issue that requests a new feature
Milestone
Comments
JoostK
added
feature
|
Thank you for prioritizing this! |
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Feb 22, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes angular#34867 Fixes angular#40635 Closes angular#40728
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Feb 22, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes angular#34867 Fixes angular#40635 Closes angular#40728
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Feb 22, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes angular#34867 Fixes angular#40635 Closes angular#40728
alxhub
pushed a commit
to JoostK/angular
that referenced
this issue
Feb 23, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes angular#34867 Fixes angular#40635 Closes angular#40728
JoostK
added a commit
that referenced
this issue
Feb 26, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes #34867 Fixes #40635 Closes #40728
JoostK
added a commit
that referenced
this issue
Feb 28, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes #34867 Fixes #40635 Closes #40728
|
Thanks @JoostK |
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Mar 2, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes angular#34867 Fixes angular#40635 Closes angular#40728
14 tasks
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Mar 5, 2021
…cremental rebuild In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. These changes also fix a bug where template type-checking would fail to incorporate changes made to a transitive base class of a directive/component. This used to be a problem because transitive base classes were not recorded as a transitive dependency in the file dependency graph, such that prior type-check blocks would erroneously be reused. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes angular#34867 Fixes angular#40635 Closes angular#40728
AndrewKushnir
pushed a commit
that referenced
this issue
Mar 8, 2021
…cremental rebuild (#40947) In Angular programs, changing a file may require other files to be emitted as well due to implicit NgModule dependencies. For example, if the selector of a directive is changed then all components that have that directive in their compilation scope need to be recompiled, as the change of selector may affect the directive matching results. Until now, the compiler solved this problem using a single dependency graph. The implicit NgModule dependencies were represented in this graph, such that a changed file would correctly also cause other files to be re-emitted. This approach is limited in a few ways: 1. The file dependency graph is used to determine whether it is safe to reuse the analysis data of an Angular decorated class. This analysis data is invariant to unrelated changes to the NgModule scope, but because the single dependency graph also tracked the implicit NgModule dependencies the compiler had to consider analysis data as stale far more often than necessary. 2. It is typical for a change to e.g. a directive to not affect its public API—its selector, inputs, outputs, or exportAs clause—in which case there is no need to re-emit all declarations in scope, as their compilation output wouldn't have changed. This commit implements a mechanism by which the compiler is able to determine the impact of a change by comparing it to the prior compilation. To achieve this, a new graph is maintained that tracks all public API information of all Angular decorated symbols. During an incremental compilation this information is compared to the information that was captured in the most recently succeeded compilation. This determines the exact impact of the changes to the public API, which is then used to determine which files need to be re-emitted. Note that the file dependency graph remains, as it is still used to track the dependencies of analysis data. This graph does no longer track the implicit NgModule dependencies, which allows for better reuse of analysis data. These changes also fix a bug where template type-checking would fail to incorporate changes made to a transitive base class of a directive/component. This used to be a problem because transitive base classes were not recorded as a transitive dependency in the file dependency graph, such that prior type-check blocks would erroneously be reused. This commit also fixes an incorrectness where a change to a declaration in NgModule `A` would not cause the declarations in NgModules that import from NgModule `A` to be re-emitted. This was intentionally incorrect as otherwise the performance of incremental rebuilds would have been far worse. This is no longer a concern, as the compiler is now able to only re-emit when actually necessary. Fixes #34867 Fixes #40635 Closes #40728 PR Close #40947
This was referenced Mar 10, 2021
Merged
This was referenced Mar 15, 2021
Closed
Closed
|
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.
This issue is created as a status tracker to implement an optimization for incremental compilations to enable more effective reuse of work from the previous compilation. The general idea is that decorated classes will be compared to their equivalent in the prior compilation and skip emit if nothing relevant has changed. There has been a working prototype in #39791 which implements this idea, which has since been refined to a simplified yet more effective approach.
This work aims to address #34867 and #40635. One initial improvement in response to #40635 is pending in angular/angular-cli#19986 which avoids an inefficiency in Webpack's dependency mechanism resulting in a significant improvement of rebuild times. This work should take this one step further and avoid emit of files when their generated output could not possibly be affected by the change.
The text was updated successfully, but these errors were encountered: