-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
perf(ivy): avoid storing raw selectors in projectionDef #29578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should make ngProjectAs
attributes special, in the sense that we create a new AttributeMarker.ngProjectAs
flag. This would mean that our processing of "normal" attributes is not affected and fast as possible, which is the most common case.
Introducing a flag would definitely make it more consistent, but I'm not sure whether it would help with processing since the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am concerned about the changes we have made to the generateInitialInputs
Could we discuss?
c1a6635
to
e974d32
Compare
I've reworked it to use a marker for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to see some tests that exercise the code with projectAs
using classes and not
too. Currently there only appears to be attribute based selectors. But I might have missed them.
e974d32
to
54213d7
Compare
Rebased and addressed the feedback @kara. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
54213d7
to
5fa8f04
Compare
merge assistance: global approval |
@crisbeto this PR now has a big conflict. can you please resolve it? thank you! |
Currently in Ivy we pass both the raw and parsed selectors to the projectionDef instruction, because the parsed selectors are used to match most nodes, whereas the raw ones are used to match against nodes with the ngProjectAs attribute. The raw selectors add a fair bit of code that won't be used in most cases, because ngProjectAs is somewhat rare. These changes rework the compiler not to output the raw selectors in the projectionDef, but to parse the selector in ngProjectAs and to store it on the TAttributes. The logic for matching has also been changed so that it matches the pre-parsed ngProjectAs selector against the list of projection selectors.
43a90d0
to
c59c34e
Compare
c59c34e
to
0400a3d
Compare
Conflicts have been resolved. |
@crisbeto thank you! ❤️ |
Currently in Ivy we pass both the raw and parsed selectors to the projectionDef instruction, because the parsed selectors are used to match most nodes, whereas the raw ones are used to match against nodes with the ngProjectAs attribute. The raw selectors add a fair bit of code that won't be used in most cases, because ngProjectAs is somewhat rare. These changes rework the compiler not to output the raw selectors in the projectionDef, but to parse the selector in ngProjectAs and to store it on the TAttributes. The logic for matching has also been changed so that it matches the pre-parsed ngProjectAs selector against the list of projection selectors. PR Close angular#29578
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. |
Currently in Ivy we pass both the raw and parsed selectors to the
projectionDef
instruction, because the parsed selectors are used to match most nodes, whereas the raw ones are used to match against nodes with thengProjectAs
attribute. The raw selectors add a fair bit of code that won't be used in most cases, becausengProjectAs
is somewhat rare.These changes rework the compiler not to output the raw selectors in the
projectionDef
, but to parse the selector inngProjectAs
and to store it on theTAttributes
. The logic for matching has also been changed so that it matches the pre-parsedngProjectAs
selector against the list of projection selectors.