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.

Sign up for GitHub

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

Jump to bottom

perf(core): optimize memory allocation when reconciling lists #52245

Closed
wants to merge 2 commits into from
Closed

perf(core): optimize memory allocation when reconciling lists #52245

wants to merge 2 commits into from

Conversation

pkozlowski-opensource
Copy link
Member

This change replaces the implementation of the multi-map used to store detached views while reconciling lists. The new implementation optimizes memory allocation for such map and avoid arrays allocation when there are no duplicated keys.

@pkozlowski-opensource pkozlowski-opensource added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: rc This PR is targeted for the next release-candidate labels Oct 17, 2023
@ngbot ngbot bot added this to the Backlog milestone Oct 17, 2023
@pkozlowski-opensource pkozlowski-opensource marked this pull request as ready for review October 17, 2023 16:31
@pkozlowski-opensource pkozlowski-opensource self-assigned this Oct 19, 2023
@pkozlowski-opensource
perf(core): optimize memory allocation when reconcilling lists
9653289 
This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.
@pkozlowski-opensource pkozlowski-opensource force-pushed the control_flow_multimap branch 2 times, most recently from 55e824e to 23f6cfa Compare December 6, 2023 14:11
@pkozlowski-opensource pkozlowski-opensource added the core: control flow Issues related to the built-in control flow (@if, @for, @switch) label Dec 6, 2023
@pkozlowski-opensource
Copy link
Member Author

@JoostK I believe I've addressed all the feedback (thnx for the review!) - could you please take another look?

JoostK
JoostK approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

packages/core/src/render3/list_reconciliation.ts
while (this.vMap.has(value)) {
value = this.vMap.get(value)!;
cb(value, key);
const vMap = this._vMap;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could possibly move this above the if and use it in the condition as well. Only relevant for minification now, as perf should be fine without this local variable either way.

@pkozlowski-opensource
Copy link
Member Author

pkozlowski-opensource commented Dec 6, 2023 •
edited

The test //packages/core/test/render3:render3 failure is legit so it suggests that there is some logic bug here leading to an infinite loop. Needs cleanup. Fixed.

@pkozlowski-opensource pkozlowski-opensource 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 Dec 6, 2023
@pkozlowski-opensource pkozlowski-opensource added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 6, 2023
@alxhub alxhub added target: patch This PR is targeted for the next patch release and removed target: rc This PR is targeted for the next release-candidate labels Dec 8, 2023
@alxhub
Copy link
Member

alxhub commented Dec 8, 2023

This PR was merged into the repository by commit e3a6bf9.

@alxhub alxhub closed this in e3a6bf9 Dec 8, 2023
alxhub pushed a commit that referenced this pull request Dec 8, 2023
@pkozlowski-opensource @alxhub
perf(core): optimize memory allocation when reconcilling lists (#52245)
044cb55 
This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close #52245
dylhunn pushed a commit to dylhunn/angular that referenced this pull request Dec 11, 2023
@pkozlowski-opensource @dylhunn
perf(core): optimize memory allocation when reconcilling lists (angul…
2e89421 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
dylhunn pushed a commit to dylhunn/angular that referenced this pull request Dec 11, 2023
@pkozlowski-opensource @dylhunn
perf(core): optimize memory allocation when reconcilling lists (angul…
c0b1196 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
josephperrott pushed a commit to josephperrott/angular that referenced this pull request Dec 11, 2023
@pkozlowski-opensource @josephperrott
perf(core): optimize memory allocation when reconcilling lists (angul…
ddd93ac 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 8, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@pkozlowski-opensource @ChellappanRajan
perf(core): optimize memory allocation when reconcilling lists (angul…
d0f7234 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
@pkozlowski-opensource @rlmestre
perf(core): optimize memory allocation when reconcilling lists (angul…
2d7bfaf 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
@pkozlowski-opensource @danieljancar
perf(core): optimize memory allocation when reconcilling lists (angul…
34f5a0b 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
@pkozlowski-opensource @amilamen
perf(core): optimize memory allocation when reconcilling lists (angul…
51262a0 
…ar#52245)

This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close angular#52245
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoostK JoostK JoostK approved these changes

Assignees

@pkozlowski-opensource pkozlowski-opensource

Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime core: control flow Issues related to the built-in control flow (@if, @for, @switch) target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pkozlowski-opensource @alxhub @JoostK