perf(hooks): Optimize useSyncExternalStoreWithSelector selector calls #32040
Conversation
|
cc @markerikson |
Add intelligent selector execution tracking to useSyncExternalStoreWithSelector to avoid unnecessary re-renders and selector calls. The hook now tracks which properties are accessed during selection and only re-runs the selector when those specific properties change. - Add Proxy-based property access tracking - Track accessed properties to determine relevant state changes - Skip selector execution when only irrelevant state changes - Add tests verifying selector optimization behavior - Preserve existing memoization and isEqual behavior This improves performance by reducing unnecessary selector executions when unrelated parts of the state change.
Biki-das
force-pushed
the
useSync-optimise
branch
from
7b78610 to
aeab8bc
Compare
|
This would definitely have an effect, but I don't know if it would be positive or negative. I've done some prior research into something vaguely similar: and @dai-shi also had a PR a few years back that tried to implement proxy-based tracking updates: @dai-shi knows a lot more about overall Proxy performance and behavior in this kind of scenario. I'll definitely have to take a look at this and see what it actually does and how that affects various performance scenarios. That said, a general thought on the implementation approach: even if this does make an improvement on overall perf, I would say that changing the existing |
|
I'm looking at this, and I like the concept. My WIP "autotracking" PR from a couple years ago was trying to do deep tracking. That works better in terms of theoretical changes (the This one looks like it just tracks top-level accessed field names. That ought to be a lot faster, and also feels like it gives the most bang for the buck. It ties into what Slack told us about their homegrown "top level changed fields subscriptions" wrapper for their Redux store being a boost for perf, but that they didn't get as much benefit from trying to track deeper field changes. I'd love to see some perf benchmarks for various kinds of state updates to get a sense of how this may improve things. |
|
Thanks @markerikson for the detailed feedback!will try to test this out and see its effect. |
|
Also worth considering what happens if the wrapped state is a primitive instead of an object as well. |
|
Yeah. |
|
Use proxy-memoize. 😄 |
|
@dai-shi Not quite sure I follow. |
|
As I understand, proxy-memoize works the same as proposed here, with tracking deeply. That said, if what we want is the shallow tracking and the use case is limited to useSyncExternalStoreWithSelector, there can be a much simpler implementation. |
| const memoizedSelector = (nextSnapshot: Snapshot) => { | ||
| const getProxy = (): Snapshot => { | ||
| if ( |
There was a problem hiding this comment.
@markerikson you were curious for the case when a a primitive value is passed, this lines will handle the same.
For Primitive Values
- No Proxy wrapping occurs
- The primitive value is passed directly to the selector
- The property access tracking system (with accessedProps) is bypassed entirely
Summary
Add selector execution tracking to useSyncExternalStoreWithSelector to avoid unnecessary re-renders and selector calls. The hook now tracks which properties are accessed during selection and only re-runs the selector when those specific properties change.
This improves performance by reducing unnecessary selector executions when unrelated parts of the state change.
How did you test this change?
Have added related test to test selector calls on specific scenarios.