Skip to content

refactor(core): use quad linked signal nodes for performance#62284

Closed
milomg wants to merge 2 commits intoangular:mainfrom
milomg:linked
Closed

refactor(core): use quad linked signal nodes for performance#62284
milomg wants to merge 2 commits intoangular:mainfrom
milomg:linked

Conversation

@milomg
Copy link
Copy Markdown
Member

@milomg milomg commented Jun 26, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Switch to using https://preactjs.com/blog/signal-boosting/ for a performance boost in signal initialization

We introduce the idea of a ReactiveLink in addition to a ReactiveNode. Then, instead of storing an array of ReactiveNodes, we store a ReactiveLink'ed List.

By merging the sources and observers into one massive linked list, we avoid having to push and pop from many different arrays, and can instead use linked list operations that are faster.

This PR also avoids duplication of producers in two cases: when reading the same producer twice in a row, and when reading from a live producer

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Jun 26, 2025
@ngbot ngbot bot added this to the Backlog milestone Jun 26, 2025
@milomg milomg force-pushed the linked branch 2 times, most recently from 641b2fd to 50fe202 Compare June 26, 2025 08:19
@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: merge The PR is ready for merge by the caretaker labels Jun 26, 2025
@milomg milomg force-pushed the linked branch 7 times, most recently from dc613d8 to d19ace3 Compare June 28, 2025 06:58
@milomg milomg marked this pull request as ready for review June 28, 2025 07:03
@pullapprove pullapprove bot requested review from atscott, iteriani and kirjs June 28, 2025 07:03
@pullapprove pullapprove bot added the requires: TGP This PR requires a passing TGP before merging is allowed label Jun 28, 2025
@milomg
Copy link
Copy Markdown
Member Author

milomg commented Jun 30, 2025

Caretaker note: apply patch cl/777701693

TESTED=TGP

@milomg milomg force-pushed the linked branch 4 times, most recently from 3f9378c to ab612ff Compare July 3, 2025 00:12
@milomg milomg requested a review from alxhub July 11, 2025 21:22
inspired by the design of Preact's signals, use linked lists instead of arrays for faster signal creation
approve the new symbols that appear from bundling the signals changes
@milomg milomg added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 14, 2025
Copy link
Copy Markdown
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

LGTM - one comment overall

@pullapprove pullapprove bot requested a review from crisbeto July 21, 2025 18:56
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release and removed action: review The PR is still awaiting reviews from at least one requested reviewer action: merge The PR is ready for merge by the caretaker labels Jul 23, 2025
@thePunderWoman thePunderWoman added the action: global presubmit The PR is in need of a google3 global presubmit label Jul 23, 2025
@iteriani
Copy link
Copy Markdown
Contributor

reviewed-for: primitives

@JeanMeche JeanMeche added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jul 23, 2025
@milomg
Copy link
Copy Markdown
Member Author

milomg commented Jul 24, 2025

TESTED=TGP

Caretaker note: apply patch cl/777701693

@milomg milomg added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Jul 24, 2025
@crisbeto
Copy link
Copy Markdown
Member

This PR was merged into the repository by commit 92c2d2a.

The changes were merged into the following branches: main, 20.1.x

crisbeto pushed a commit that referenced this pull request Jul 28, 2025
inspired by the design of Preact's signals, use linked lists instead of arrays for faster signal creation

PR Close #62284
crisbeto pushed a commit that referenced this pull request Jul 28, 2025
approve the new symbols that appear from bundling the signals changes

PR Close #62284
@crisbeto crisbeto closed this in 3c008c9 Jul 28, 2025
crisbeto pushed a commit that referenced this pull request Jul 28, 2025
approve the new symbols that appear from bundling the signals changes

PR Close #62284
@milomg milomg deleted the linked branch July 28, 2025 18:11
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Aug 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note requires: TGP This PR requires a passing TGP before merging is allowed target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants