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.

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

Switch back to Subscription in useSelector to fix unsubscribe perf #1870

Merged
merged 1 commit into from
Feb 5, 2022

Commits on Feb 5, 2022

  1. Switch back to Subscription in useSelector to fix unsubscribe perf

    The Subscription impl originally used an array of listeners just
    like the actual Redux store, but in #1523 we changed it to be a
    linked list to fix perf issues. The use of `indexOf+splice` caused
    a quadratic cost to unmounting many components.
    
    In v8, I originally dropped use of `Subscription` to potentially
    save on bundle size. However, `Provider` still uses `Subscription`,
    so that will always be in the bundle.
    
    For this issue, a user pointed out that directly subscribing to the
    store brought back the original quadratic unsubscription behavior,
    which I've confirmed locally.
    
    Swapping `store.subscribe` for `subscription.addNestedSub` fixes
    that issue.
    
    I've added a unit test that mounts and unmounts a massive tree and
    measures the elapsed time. There's a distinct difference between
    the "correct" and quadratic behavior. Weirdly, there's also a big
    diff in "correct" time between React 18, and 17 + the "compat" entry
    point, and I have no idea why.  It's not as bad as the quadratic
    time, but it's still pretty expensive.
    
    I've written the test to set an acceptable max unmount time based on
    which React version we're running against, with some buffer added.
    markerikson committed Feb 5, 2022
    Configuration menu
    Copy the full SHA
    43d1cc6 View commit details
    Browse the repository at this point in the history