Skip to content

perf(compiler-cli): detect semantic changes and their effect on an in…#39791

Closed
JoostK wants to merge 2 commits intoangular:masterfrom
JoostK:ngtsc/semantic-dependencies
Closed

perf(compiler-cli): detect semantic changes and their effect on an in…#39791
JoostK wants to merge 2 commits intoangular:masterfrom
JoostK:ngtsc/semantic-dependencies

Conversation

@JoostK
Copy link
Member

@JoostK JoostK commented Nov 20, 2020

…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.

@JoostK JoostK added state: WIP area: compiler Issues related to `ngc`, Angular's template compiler labels Nov 20, 2020
@google-cla google-cla bot added the cla: yes label Nov 20, 2020
@ngbot ngbot bot added this to the needsTriage milestone Nov 20, 2020
@JoostK JoostK force-pushed the ngtsc/semantic-dependencies branch 2 times, most recently from 0e87841 to e9274ae Compare November 21, 2020 21:25
…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.
@JoostK JoostK force-pushed the ngtsc/semantic-dependencies branch from e9274ae to b55b80f Compare November 29, 2020 21:20
/**
* The various ways in which a change may affect the compilation.
*/
export enum Affects {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit pick: the correct spelling is Effects :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope you haven't looked into these changes too much, as this was the initial prototype which did NgModule impact analysis. I'll close this PR to avoid confusion. This really was meant as the verb Affects here, though; this enum models the various ways in which a certain change affects the NgModule scope.

@JoostK JoostK closed this Feb 24, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: compiler Issues related to `ngc`, Angular's template compiler cla: yes state: WIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants