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

ɵɵtemplate instruction refactor #33856

Conversation

@pkozlowski-opensource
Copy link
Member

pkozlowski-opensource commented Nov 15, 2019

This refactoring clearly separates the first and subsequent creation execution
of the template instruction. This approach has the following benefits:

  • it is clear what happens during the first vs. subsequent executions;
  • we can avoid several memory reads and checks after the first creation pass
    (there is measurable performance improvement on various benchmarks);
  • the template instructions becomes smaller and should become a candidate
    for optimisations / inlining faster;
@pkozlowski-opensource pkozlowski-opensource requested a review from angular/fw-core as a code owner Nov 15, 2019
@googlebot googlebot added the cla: yes label Nov 15, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 15, 2019
@pkozlowski-opensource pkozlowski-opensource force-pushed the pkozlowski-opensource:template_refactor branch from 994561e to ee10286 Nov 15, 2019
Copy link
Member

mhevery left a comment

I ❤️ this change. Yes we should do more of this. My only nit is to make the text of the comment more clear as it is not clear too me what you are trying to say. I find that examples tend to make things more clear.

embeddedTView.queries = tView.queries.embeddedTView(tContainerNode);
}
}
const tNode = tView.firstCreatePass ?

This comment has been minimized.

Copy link
@mhevery

mhevery Nov 15, 2019

Member

I ❤️ this, it is so clear!

packages/core/src/render3/instructions/container.ts Outdated Show resolved Hide resolved
ngDevMode && assertFirstCreatePass(tView);
ngDevMode && ngDevMode.firstCreatePass++;
const tViewConsts = tView.consts;
// TODO(pk): refactor getOrCreateTNode to have the "create" only version

This comment has been minimized.

Copy link
@mhevery
This refactorings clearly separates the first and subsequent creation execution
of the `template` instruction. This approach has the following benefits:
- it is clear what happens during the first vs. subsequent executions;
- we can avoid several memory reads and checks after the first creation pass
(there is measurable performance improvement on various benchmarks);
- the template instructions becomes smaller and should become a candidate
for optimisations / inlining faster;
@pkozlowski-opensource pkozlowski-opensource force-pushed the pkozlowski-opensource:template_refactor branch from ee10286 to ca19763 Nov 18, 2019
@kara kara self-requested a review Nov 18, 2019
@mhevery

This comment has been minimized.

Copy link
Member

mhevery commented Nov 18, 2019

@kara
kara approved these changes Nov 18, 2019
Copy link
Contributor

kara left a comment

LGTM

@pkozlowski-opensource

This comment has been minimized.

Copy link
Member Author

pkozlowski-opensource commented Nov 19, 2019

merge-assistance due to "Pending — Status "google3" is pending"

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor

AndrewKushnir commented Nov 19, 2019

@pkozlowski-opensource VE and Ivy presubmits are successful, I updated the status of the g3 check. Thank you.

@alxhub alxhub closed this in ad4300f Nov 19, 2019
alxhub added a commit that referenced this pull request Nov 19, 2019
#33856)

This refactorings clearly separates the first and subsequent creation execution
of the `template` instruction. This approach has the following benefits:
- it is clear what happens during the first vs. subsequent executions;
- we can avoid several memory reads and checks after the first creation pass
(there is measurable performance improvement on various benchmarks);
- the template instructions becomes smaller and should become a candidate
for optimisations / inlining faster;

PR Close #33856
alxhub added a commit that referenced this pull request Nov 19, 2019
alxhub added a commit that referenced this pull request Nov 19, 2019
#33856)

This refactorings clearly separates the first and subsequent creation execution
of the `template` instruction. This approach has the following benefits:
- it is clear what happens during the first vs. subsequent executions;
- we can avoid several memory reads and checks after the first creation pass
(there is measurable performance improvement on various benchmarks);
- the template instructions becomes smaller and should become a candidate
for optimisations / inlining faster;

PR Close #33856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.