Chain elementStart/elementEnd instructions #44994
Closed
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
crisbeto
force-pushed
the
chain-element-instructions
branch
from
1b1cd37
to
2d2137c
Compare
|
OOC don't compression algorithms already take care of this? Can it be opt-in at least? I think for debugging the ability to step on each call is easier with the current way |
|
A while ago when we first started chaining instructions, we had a testing setup that showed it compressing slightly better with the chained instructions. Unfortunately, I haven't been able to dig it up, but a quick test against the |
crisbeto
added
action: review
atscott
approved these changes
Feb 7, 2022
|
Caretaker note: I'll run a TGP for this tomorrow. It moves around a bunch of code in the template builder and I want to double-check that there are no regressions. |
atscott
added
action: presubmit
…flexibility Previously the logic for generating chained instructions was somewhat rigid, because we had to collect all of the calls ahead of time and then call one of the chained instruction helpers. This doesn't work for something like `elementStart`, because we have to descend into other elements that could add to the chain. These changes refactor the code so that we collect the list of instructions in a flat array and we do the chaining only once at the end when we have the entire instruction set for the code block. The new approach has the advantage of being (almost) entirely configuration-based via the `CHAINABLE_INSTRUCTIONS` array and being more flexible in allowing us to chain instructions that span across elements.
crisbeto
force-pushed
the
chain-element-instructions
branch
from
2d2137c
to
947c0a5
Compare
In templates with several levels of nested nodes, it's common for several `elementStart`/`elementEnd` instructions to show up in a row which can be optimized away. These changes add chaining support for `elementStart`, `elementEnd`, `elementContainerStart` and `elementContainerEnd` to shave off some bytes when possible.
crisbeto
force-pushed
the
chain-element-instructions
branch
from
947c0a5
to
2b09d76
Compare
crisbeto
added
action: merge
|
Passes TGP, marking as ready to merge. |
dylhunn
pushed a commit
that referenced
this pull request
Feb 8, 2022
In templates with several levels of nested nodes, it's common for several `elementStart`/`elementEnd` instructions to show up in a row which can be optimized away. These changes add chaining support for `elementStart`, `elementEnd`, `elementContainerStart` and `elementContainerEnd` to shave off some bytes when possible. PR Close #44994
|
This PR was merged into the repository by commit 1b91e10. |
dylhunn
pushed a commit
that referenced
this pull request
Feb 8, 2022
…flexibility (#44994) Previously the logic for generating chained instructions was somewhat rigid, because we had to collect all of the calls ahead of time and then call one of the chained instruction helpers. This doesn't work for something like `elementStart`, because we have to descend into other elements that could add to the chain. These changes refactor the code so that we collect the list of instructions in a flat array and we do the chaining only once at the end when we have the entire instruction set for the code block. The new approach has the advantage of being (almost) entirely configuration-based via the `CHAINABLE_INSTRUCTIONS` array and being more flexible in allowing us to chain instructions that span across elements. PR Close #44994
dylhunn
pushed a commit
that referenced
this pull request
Feb 8, 2022
In templates with several levels of nested nodes, it's common for several `elementStart`/`elementEnd` instructions to show up in a row which can be optimized away. These changes add chaining support for `elementStart`, `elementEnd`, `elementContainerStart` and `elementContainerEnd` to shave off some bytes when possible. PR Close #44994
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Feb 19, 2022
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.2.2/13.2.3) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.2.2/13.2.3) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.2.2/13.2.3) | | [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.2.2/13.2.3) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.2.2/13.2.3) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.2.2/13.2.3) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.2.2/13.2.3) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.2.2/13.2.3) | | [@angular/router](https://github.com/angular/angular) | dependencies | patch | [`13.2.2` -> `13.2.3`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.2.2/13.2.3) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v13.2.3`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#​1323-2022-02-16) [Compare Source](angular/angular@13.2.2...13.2.3) ##### animations | Commit | Type | Description | | -- | -- | -- | | [0050b01b62](angular/angular@0050b01) | perf | made errors in the animations package tree shakeable ([#​45079](angular/angular#45079)) | ##### compiler | Commit | Type | Description | | -- | -- | -- | | [09f0254bdd](angular/angular@09f0254) | perf | chain element start/end instructions ([#​44994](angular/angular#44994)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [92cf9fbebe](angular/angular@92cf9fb) | fix | `ViewContainerRef.createComponent` should consult module injector when custom one is provided ([#​44966](angular/angular#44966)) | #### Special Thanks AlirezaEbrahimkhani, Amer Yousuf, Andrew Kushnir, Aristeidis Bampakos, Dario Piotrowicz, Esteban Gehring, Jessica Janiuk, JiaLiPassion, Kristiyan Kostadinov, Mina Hosseini Moghadam, Patrick Cameron, Srdjan Milic, Yousaf Nawaz, dario-piotrowicz, markostanimirovic, mgechev and zuckjet <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1178 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
|
this caused a lot of pain TypeError: t.TgZ(...) is not a function
the compiler produced something like this, but the ɵɵelementStart was returning null:
it was fixed after this change:
this seems like a breaking change |
@mihaipanait Angular framework packages must be exactly in sync with each other, any other configuration is not supported. |
This was referenced Feb 25, 2022
|
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.
Split up into two commits to make it easier to review:
Previously the logic for generating chained instructions was somewhat rigid, because we had to collect all of the calls ahead of time and then call one of the chained instruction helpers. This doesn't work for something like
elementStart, because we have to descend into other elements that could add to the chain.These changes refactor the code so that we collect the list of instructions in a flat array and we do the chaining only once at the end when we have the entire instruction set for the code block.
The new approach has the advantage of being (almost) entirely configuration-based via the
CHAINABLE_INSTRUCTIONSarray and being more flexible in allowing us to chain instructions that span across elements.In templates with several levels of nested nodes, it's common for several
elementStart/elementEndinstructions to show up in a row which can be optimized away.These changes add chaining support for
elementStart,elementEnd,elementContainerStartandelementContainerEndto shave off some bytes when possible.