-
-
Notifications
You must be signed in to change notification settings - Fork 8k
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
feat(reactivity): bitwise dep markers to optimize re-tracking #4017
Conversation
Note: this PR has been tested and verified in our local project which uses advanced reactivity. |
49ac57d
to
70c4abf
Compare
2046d36
to
ba1d97c
Compare
I rebase 3.2 branch on top of latest |
I'll try to review and merge this one after we resolve #2195 so hopefully this should be the last time 😅 |
70c4abf
to
2839bd4
Compare
Rebase to 3.2 I suppose? Done! |
2839bd4
to
9b9dc7f
Compare
Great work, thank you so much @basvanmeurs ! Note I refactored this PR in ab8bc71 mainly to cut down the size increase (+0.29kb -> 0.21kb) since the new additions are non-tree-shakeable. |
Currently for every effect run, all existing dependencies are first cleaned up and then retracked. This involves Set additions and removals. In many cases, dependencies rarely change so we already discussed that there was room for optmization here (#2345 (comment)).
This PR optimizes re-tracking of effect dependencies.
It uses markers which identify whether a dependency is new, stable or old. Because effect execution/tracking may be recursive, the markers are actually bitwise and support up to 30 levels deep (the amount of bits we can safely use for a SMI small integer). When the depth is more than 30 levels (notice that this is unlikely to occur), the 'old' full cleanup method is used instead.
Component render/update recursion
Notice that this optimization, unlike the old cleanup/retrack method, postpones deleting the old dependencies until the end of the effect execution. Ideally, this should not make a difference, but I found one situation in which it did: the component update.
The
onBeforeUpdate
test in apiLifecycle.spec.ts failed. It turned out that the component update 'effect' has theallowRecursive
flag turned on, because child components must be able to re-trigger the parent. However, when one of the (beforeMount/beforeUpdate) life cycle hooks is invoked, it is possible that this triggers any of the dependencies of the render method. This causes the render/update effect to be queued again unnecessarily.Currently, the 'cleanup' cleared all dependencies just in time, making sure that this recursive trigger doesn't occur (I don't think this is intentional, it rather feels like a lucky side effect to me). To make this PR work, I had to explicitly disable allowRecursive while executing these pre-render hooks.
Performance effect
Ideally, this algorithm only needs to iterate the Dep array twice without having to touch any of the sets. It is a robust performance enhancement, and performs stable even when the order or tracked dependencies changes!
This has a huge boost on all effects, and especially those with a lot of dependencies. The mix benchmark, which represents a real-world scenario, was boosted by almost 40%!