perf(core): make DI decorators tree-shakable when used in useFactory deps config
#40145
Conversation
This file contains 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
AndrewKushnir
added
area: performance
state: WIP
refactoring
|
Related PR: #40122. |
AndrewKushnir
changed the title
useFactory deps configuseFactory deps config
3 tasks
AndrewKushnir
force-pushed
the
di_decorators_tree_shaking
branch
2 times, most recently
from
0be3e77
to
ae5904c
Compare
AndrewKushnir
added
action: review
AndrewKushnir
force-pushed
the
di_decorators_tree_shaking
branch
from
ae5904c
to
78ce6d5
Compare
AndrewKushnir
commented
Jan 11, 2021
pullapprove
bot
requested review from
alxhub,
atscott,
jessicajaniuk and
petebacondarwin
jessicajaniuk
approved these changes
Jan 11, 2021
AndrewKushnir
force-pushed
the
di_decorators_tree_shaking
branch
from
55f567c
to
5e2e5bd
Compare
…y` deps config This commit updates the logic that calculates `useFactory` function arguments to avoid relying on `instanceof` checks (thus always retaining symbols) and relies on flags that DI decorators contain (as a monkey-patched property). Another perf benefit is having less megamorphic reads while calculating args for the `useFactory` call: we used to check whether a token has `ngMetadataName` property 4 times (in worst case), now we have just 1 megamorphic read in all cases. Closes angular#40143.
AndrewKushnir
force-pushed
the
di_decorators_tree_shaking
branch
from
5e2e5bd
to
b4de94a
Compare
AndrewKushnir
commented
Jan 13, 2021
| @@ -838,11 +838,11 @@ export declare interface RendererType2 { | |||
| } | |||
|
|
|||
| export declare class ResolvedReflectiveFactory { | |||
| dependencies: ɵangular_packages_core_core_d[]; | |||
| dependencies: ɵangular_packages_core_core_e[]; | |||
There was a problem hiding this comment.
Note for public-api group reviewers: this change is a noop from public API perspective, this change is likely a result of some symbol reordering in generated bundle (since new logic was added as a part of this PR).
atscott
approved these changes
Jan 13, 2021
mhevery
approved these changes
Jan 13, 2021
jessicajaniuk
approved these changes
Jan 13, 2021
AndrewKushnir
removed
the
action: review
petebacondarwin
approved these changes
Jan 13, 2021
AndrewKushnir
added
action: merge
|
Merge assistance: please change the target label to "target: rc" if this PR is merged after v11.1.0-rc.0 is released (so that this change makes it into v11.1.0). Thank you. |
atscott
added
target: rc
atscott
pushed a commit
that referenced
this pull request
Jan 13, 2021
…y` deps config (#40145) This commit updates the logic that calculates `useFactory` function arguments to avoid relying on `instanceof` checks (thus always retaining symbols) and relies on flags that DI decorators contain (as a monkey-patched property). Another perf benefit is having less megamorphic reads while calculating args for the `useFactory` call: we used to check whether a token has `ngMetadataName` property 4 times (in worst case), now we have just 1 megamorphic read in all cases. Closes #40143. PR Close #40145
|
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.
This commit updates the logic that calculates
useFactoryfunction arguments to avoid relying oninstanceofchecks (thus always retaining symbols) and relies on flags that DI decorators contain (as a monkey-patched property).
Another perf benefit is having less megamorphic reads while calculating args for the
useFactorycall: we used tocheck whether a token has
ngMetadataNameproperty 4 times (in worst case), now we have just 1 megamorphic read inall cases.
Closes #40143.
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?