Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ivy): ngtsc incremental compilation improvements #30238

Conversation

petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented May 2, 2019

ngtsc incremental compilation improvements (FW-1241)

Tried out on AIO and seems to be faster. By logging I can see that it is definitely not analysing large numbers of files now.

@petebacondarwin petebacondarwin added area: performance state: WIP area: core Issues related to the framework runtime target: major This PR is targeted for the next major release comp: ivy labels May 2, 2019
@ngbot ngbot bot added this to the needsTriage milestone May 2, 2019
@petebacondarwin petebacondarwin changed the title feat(ivy): track file dependencies due to partial evaluation feat(ivy): ngtsc incremental compilation improvements May 2, 2019
@petebacondarwin petebacondarwin force-pushed the ngtsc-partial-eval-tracking branch 4 times, most recently from 74fcb5b to abf1b20 Compare May 3, 2019 16:05
@petebacondarwin petebacondarwin marked this pull request as ready for review May 3, 2019 20:38
@petebacondarwin petebacondarwin requested review from a team as code owners May 3, 2019 20:38
@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels May 3, 2019
@petebacondarwin

This comment has been minimized.

@petebacondarwin petebacondarwin force-pushed the ngtsc-partial-eval-tracking branch 5 times, most recently from ff3073b to eab50ba Compare May 8, 2019 14:25
@petebacondarwin petebacondarwin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 10, 2019
No behavioural changes.
As part of incremental compilation performance improvements, we need
to track the dependencies of files due to expressions being evaluated by
the `PartialEvaluator`.

The `PartialEvaluator` now accepts a `DependencyTracker` object, which is
used to track which files are visited when evaluating an expression.
The interpreter computes this `originatingFile` and stores it in the evaluation
`Context` so it can pass this to the `DependencyTracker.

The `IncrementalState` object implements this interface, which allows it to be
passed to the `PartialEvaluator` and so capture the file dependencies.
To support skipping analysis of a file containing a component
we need to know that none of the declarations that might affect
its ngtsc compilation have not changed. The files that we need to
check are those that contain classes from the `CompilationScope`
of the component. These classes are already tracked in the
`LocalModuleScopeRegistry`.

This commit modifies the `IvyCompilation` class to record the
files that are in each declared class's `CompilationScope` via
a new method, `recordNgModuleScopeDependencies()`, that is called
after all the handlers have been "resolved".

Further, if analysis is skipped for a declared class, then we need
to recover the analysis from the previous compilation run. To
support this, the `IncrementalState` class has been updated to
expose the `MetadataReader` and `MetadataRegistry` interfaces.
This is included in the `metaRegistry` object to capture these analyses,
and also in the `localMetaReader` as a fallback to use if the
current compilation analysis was skipped.
Now that the dependent files and compilation scopes are being tracked in
the incremental state, we can skip analysing and emitting source files if
none of their dependent files have changed since the last compile.

The computation of what files (and their dependencies) are unchanged is
computed during reconciliation.

This commit also removes the previous emission skipping logic, since this
approach covers those cases already.
@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 10, 2019
@alxhub alxhub closed this in 5887ddf May 10, 2019
alxhub pushed a commit that referenced this pull request May 10, 2019
As part of incremental compilation performance improvements, we need
to track the dependencies of files due to expressions being evaluated by
the `PartialEvaluator`.

The `PartialEvaluator` now accepts a `DependencyTracker` object, which is
used to track which files are visited when evaluating an expression.
The interpreter computes this `originatingFile` and stores it in the evaluation
`Context` so it can pass this to the `DependencyTracker.

The `IncrementalState` object implements this interface, which allows it to be
passed to the `PartialEvaluator` and so capture the file dependencies.

PR Close #30238
alxhub pushed a commit that referenced this pull request May 10, 2019
To support skipping analysis of a file containing a component
we need to know that none of the declarations that might affect
its ngtsc compilation have not changed. The files that we need to
check are those that contain classes from the `CompilationScope`
of the component. These classes are already tracked in the
`LocalModuleScopeRegistry`.

This commit modifies the `IvyCompilation` class to record the
files that are in each declared class's `CompilationScope` via
a new method, `recordNgModuleScopeDependencies()`, that is called
after all the handlers have been "resolved".

Further, if analysis is skipped for a declared class, then we need
to recover the analysis from the previous compilation run. To
support this, the `IncrementalState` class has been updated to
expose the `MetadataReader` and `MetadataRegistry` interfaces.
This is included in the `metaRegistry` object to capture these analyses,
and also in the `localMetaReader` as a fallback to use if the
current compilation analysis was skipped.

PR Close #30238
alxhub pushed a commit that referenced this pull request May 10, 2019
Now that the dependent files and compilation scopes are being tracked in
the incremental state, we can skip analysing and emitting source files if
none of their dependent files have changed since the last compile.

The computation of what files (and their dependencies) are unchanged is
computed during reconciliation.

This commit also removes the previous emission skipping logic, since this
approach covers those cases already.

PR Close #30238
@petebacondarwin petebacondarwin deleted the ngtsc-partial-eval-tracking branch May 10, 2019 19:25
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
…#30238)

As part of incremental compilation performance improvements, we need
to track the dependencies of files due to expressions being evaluated by
the `PartialEvaluator`.

The `PartialEvaluator` now accepts a `DependencyTracker` object, which is
used to track which files are visited when evaluating an expression.
The interpreter computes this `originatingFile` and stores it in the evaluation
`Context` so it can pass this to the `DependencyTracker.

The `IncrementalState` object implements this interface, which allows it to be
passed to the `PartialEvaluator` and so capture the file dependencies.

PR Close angular#30238
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
…ar#30238)

To support skipping analysis of a file containing a component
we need to know that none of the declarations that might affect
its ngtsc compilation have not changed. The files that we need to
check are those that contain classes from the `CompilationScope`
of the component. These classes are already tracked in the
`LocalModuleScopeRegistry`.

This commit modifies the `IvyCompilation` class to record the
files that are in each declared class's `CompilationScope` via
a new method, `recordNgModuleScopeDependencies()`, that is called
after all the handlers have been "resolved".

Further, if analysis is skipped for a declared class, then we need
to recover the analysis from the previous compilation run. To
support this, the `IncrementalState` class has been updated to
expose the `MetadataReader` and `MetadataRegistry` interfaces.
This is included in the `metaRegistry` object to capture these analyses,
and also in the `localMetaReader` as a fallback to use if the
current compilation analysis was skipped.

PR Close angular#30238
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
Now that the dependent files and compilation scopes are being tracked in
the incremental state, we can skip analysing and emitting source files if
none of their dependent files have changed since the last compile.

The computation of what files (and their dependencies) are unchanged is
computed during reconciliation.

This commit also removes the previous emission skipping logic, since this
approach covers those cases already.

PR Close angular#30238
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request May 28, 2019
Testing on AIO with Angular master as of 28/05/2019 I got these results:
JIT ~414ms (369, 378, 408, 323, 593)
AOT using VE ~1383ms (1365, 1185, 1767, 1135, 1467)
AOT using Ivy ~517ms (600, 391, 444, 756, 394)

This is largely due to angular/angular#29380 and angular/angular#30238.

The second PR above was not merged to master, and thus will not be in 8.0.0. This PR should be merged to match it.
filipesilva added a commit to filipesilva/angular that referenced this pull request May 28, 2019
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request May 28, 2019
Testing on AIO with Angular master as of 28/05/2019 I got these results:
JIT ~414ms (369, 378, 408, 323, 593)
AOT using VE ~1383ms (1365, 1185, 1767, 1135, 1467)
AOT using Ivy ~517ms (600, 391, 444, 756, 394)

This is largely due to angular/angular#29380 and angular/angular#30238.

The second PR above was not merged to master, and thus will not be in 8.0.0. This PR should be merged to match it.
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request May 28, 2019
Testing on AIO with Angular master as of 28/05/2019 I got these results:
JIT ~414ms (369, 378, 408, 323, 593)
AOT using VE ~1383ms (1365, 1185, 1767, 1135, 1467)
AOT using Ivy ~517ms (600, 391, 444, 756, 394)

This is largely due to angular/angular#29380 and angular/angular#30238.

The second PR above was not merged to master, and thus will not be in 8.0.0. This PR should be merged to match it.
alexeagle pushed a commit to angular/angular-cli that referenced this pull request Jun 6, 2019
Testing on AIO with Angular master as of 28/05/2019 I got these results:
JIT ~414ms (369, 378, 408, 323, 593)
AOT using VE ~1383ms (1365, 1185, 1767, 1135, 1467)
AOT using Ivy ~517ms (600, 391, 444, 756, 394)

This is largely due to angular/angular#29380 and angular/angular#30238.

The second PR above was not merged to master, and thus will not be in 8.0.0. This PR should be merged to match it.
alexeagle pushed a commit to angular/angular-cli that referenced this pull request Jun 6, 2019
Testing on AIO with Angular master as of 28/05/2019 I got these results:
JIT ~414ms (369, 378, 408, 323, 593)
AOT using VE ~1383ms (1365, 1185, 1767, 1135, 1467)
AOT using Ivy ~517ms (600, 391, 444, 756, 394)

This is largely due to angular/angular#29380 and angular/angular#30238.

The second PR above was not merged to master, and thus will not be in 8.0.0. This PR should be merged to match it.
@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 Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime area: performance cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants