LFrame #33178

mhevery · 2019-10-15T16:34:51Z

This PR introduces a concept of an LFrame. LFrame is just a collection of global state which the instructions used. (This is not a new concept just giving an explicit name to something which we had before but was unnamed)
Change all of the places where we enter/leave LFrame to use proper methods such as enterView and leaveView rather than having hap hazard enter/leave/save state methods.

NOTE: This PR does not change how the system works, it only groups things into mental concepts and gives the mental concept a proper name.

NOTE: There seems to be still variation in measurements.

┌────────────────────────────────────┬─────────┬──────┬───────────┬───────────┬───────┐
│              (index)               │  time   │ unit │ base_time │ base_unit │   %   │
├────────────────────────────────────┼─────────┼──────┼───────────┼───────────┼───────┤
│       directive_instantiate        │  2.377  │ 'us' │   2.507   │   'us'    │ -5.19 │
│        element_text_create         │  1.27   │ 'us' │   1.319   │   'us'    │ -3.71 │
│           interpolation            │ 215.629 │ 'us' │  224.217  │   'us'    │ -3.83 │
│             listeners              │  1.915  │ 'us' │   2.021   │   'us'    │ -5.24 │
│ map_based_style_and_class_bindings │ 18.632  │ 'ms' │  18.523   │   'ms'    │ 0.59  │
│       noop_change_detection        │ 24.076  │ 'us' │  24.874   │   'us'    │ -3.21 │
│          property_binding          │ 219.683 │ 'us' │  216.736  │   'us'    │ 1.36  │
│      property_binding_update       │ 424.953 │ 'us' │  447.686  │   'us'    │ -5.08 │
│      style_and_class_bindings      │ 988.936 │ 'us' │   1.069   │   'ms'    │ -7.49 │
│           style_binding            │ 502.821 │ 'us' │  484.092  │   'us'    │ 3.87  │
└────────────────────────────────────┴─────────┴──────┴───────────┴───────────┴───────┘```

.notes.md

packages/core/src/render3/instructions/listener.ts

packages/core/src/render3/state.ts

packages/core/src/render3/instructions/shared.ts

integration/_payload-limits.json

mhevery · 2019-10-16T20:50:45Z

presubmit
ivy presubmit

AndrewKushnir · 2019-10-17T00:20:41Z

Restarting presubmits, since previous ones were not informative due to a set of pre-existing (non-related) failures that are now fixed.

AndrewKushnir · 2019-10-17T01:15:41Z

FYI, Ivy presubmit indicated a target that regressed in g3, related to these changes. I'm adding "blocked" label for now.

packages/core/src/render3/component.ts

pkozlowski-opensource · 2019-10-18T10:23:53Z

packages/core/src/render3/component_ref.ts

@@ -167,7 +164,11 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
        rootViewInjector);

    // rootView is the parent when bootstrapping
-    const oldLView = selectView(rootLView, null);
+    // TODO(misko): it looks like we are entering view here but we don't really need to as


+1, we should fix it, I was toying with this once

pkozlowski-opensource

LGTM. Thnx for resolving all my comments @mhevery

AndrewKushnir · 2019-10-23T18:26:52Z

AndrewKushnir · 2019-10-23T23:47:12Z

FYI, g3 presubmits are successful.

I've added "blocked" label for now to avoid merging it before @kara's review.

kara

Like the direction of this! Minor comments

packages/core/src/render3/state.ts

packages/core/src/render3/di.ts

googlebot · 2019-10-24T04:35:37Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

kara

LGTM with one more typo and moving the DI work to a new PR, as Pawel suggested.

packages/core/src/render3/state.ts

googlebot · 2019-10-24T20:07:52Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

`LFrame` stores information specifice to the current `LView` As the code enters and leaves `LView`s we use `enterView()` and `leaveView()` respectively to build a a stack of `LFrame`s. This allows us to easily restore the previous `LView` instruction state.

googlebot · 2019-10-24T21:22:34Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

PR Close #33178

…on (#33178) `LFrame` stores information specifice to the current `LView` As the code enters and leaves `LView`s we use `enterView()` and `leaveView()` respectively to build a a stack of `LFrame`s. This allows us to easily restore the previous `LView` instruction state. PR Close #33178

angular-automatic-lock-bot · 2019-11-24T16:11:00Z

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

googlebot added the cla: yes label Oct 15, 2019

mhevery force-pushed the LFrame branch 2 times, most recently from a918cb7 to 7050674 Compare October 15, 2019 20:42

mhevery marked this pull request as ready for review October 15, 2019 20:43

mhevery requested review from a team as code owners October 15, 2019 20:43

josephperrott reviewed Oct 15, 2019

View reviewed changes

.notes.md Outdated Show resolved Hide resolved

pkozlowski-opensource reviewed Oct 15, 2019

View reviewed changes

packages/core/src/render3/instructions/listener.ts Outdated Show resolved Hide resolved

mhevery added the area: core Issues related to the framework runtime label Oct 15, 2019

ngbot bot added this to the needsTriage milestone Oct 15, 2019

mhevery added the target: major This PR is targeted for the next major release label Oct 15, 2019

pkozlowski-opensource reviewed Oct 16, 2019

View reviewed changes

packages/core/src/render3/state.ts Outdated Show resolved Hide resolved

pkozlowski-opensource reviewed Oct 16, 2019

View reviewed changes

packages/core/src/render3/instructions/shared.ts Outdated Show resolved Hide resolved

mhevery force-pushed the LFrame branch 3 times, most recently from a5a97b4 to 747b754 Compare October 16, 2019 20:11

mhevery commented Oct 16, 2019

View reviewed changes

integration/_payload-limits.json Outdated Show resolved Hide resolved

AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Oct 17, 2019

AndrewKushnir added state: blocked and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 17, 2019

mhevery force-pushed the LFrame branch from 974300e to 787bfa8 Compare October 17, 2019 05:12

mhevery requested a review from a team as a code owner October 17, 2019 05:12

mhevery force-pushed the LFrame branch from 787bfa8 to 974300e Compare October 17, 2019 05:13

petebacondarwin removed the request for review from a team October 17, 2019 18:48

pkozlowski-opensource reviewed Oct 18, 2019

View reviewed changes

packages/core/src/render3/component.ts Outdated Show resolved Hide resolved

pkozlowski-opensource reviewed Oct 18, 2019

View reviewed changes

mhevery force-pushed the LFrame branch from e5e6110 to 8047ea7 Compare October 23, 2019 05:05

pkozlowski-opensource approved these changes Oct 23, 2019

View reviewed changes

AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Oct 23, 2019

mhevery mentioned this pull request Oct 23, 2019

Recursive change detection #33235

Closed

AndrewKushnir added state: blocked and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 23, 2019

kara requested changes Oct 24, 2019

View reviewed changes

googlebot added cla: no and removed cla: yes labels Oct 24, 2019

kara approved these changes Oct 24, 2019

View reviewed changes

packages/core/src/render3/state.ts Outdated Show resolved Hide resolved

mhevery force-pushed the LFrame branch from 26ff960 to 188c7b7 Compare October 24, 2019 20:06

mhevery added cla: yes and removed cla: no labels Oct 24, 2019

mhevery removed the state: blocked label Oct 24, 2019

mhevery added 2 commits October 24, 2019 14:17

style: ignore *.log files

73264fa

style: correct comments

ec84290

AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 24, 2019

mhevery force-pushed the LFrame branch from 188c7b7 to 20fe109 Compare October 24, 2019 21:22

mhevery removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 24, 2019

AndrewKushnir closed this in 4821330 Oct 24, 2019

AndrewKushnir pushed a commit that referenced this pull request Oct 24, 2019

style: correct comments (#33178)

9ea3430

PR Close #33178

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LFrame #33178

LFrame #33178

mhevery commented Oct 15, 2019 •

edited

mhevery commented Oct 16, 2019

AndrewKushnir commented Oct 17, 2019

AndrewKushnir commented Oct 17, 2019

pkozlowski-opensource Oct 18, 2019

pkozlowski-opensource left a comment

AndrewKushnir commented Oct 23, 2019

AndrewKushnir commented Oct 23, 2019

kara left a comment

googlebot commented Oct 24, 2019

kara left a comment •

edited

googlebot commented Oct 24, 2019

googlebot commented Oct 24, 2019

angular-automatic-lock-bot bot commented Nov 24, 2019

LFrame #33178

LFrame #33178

Conversation

mhevery commented Oct 15, 2019 • edited

mhevery commented Oct 16, 2019

AndrewKushnir commented Oct 17, 2019

AndrewKushnir commented Oct 17, 2019

pkozlowski-opensource Oct 18, 2019

Choose a reason for hiding this comment

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

AndrewKushnir commented Oct 23, 2019

AndrewKushnir commented Oct 23, 2019

kara left a comment

Choose a reason for hiding this comment

googlebot commented Oct 24, 2019

kara left a comment • edited

Choose a reason for hiding this comment

googlebot commented Oct 24, 2019

googlebot commented Oct 24, 2019

angular-automatic-lock-bot bot commented Nov 24, 2019

mhevery commented Oct 15, 2019 •

edited

kara left a comment •

edited