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

Allow useReducer to bail out of rendering by returning previous state #14569

Merged
merged 4 commits into from Jan 17, 2019

Conversation

Projects
None yet
8 participants
@acdlite
Copy link
Member

acdlite commented Jan 11, 2019

This is conceptually similar to shouldComponentUpdate, except because there could be multiple useReducer (or useState) Hooks in a single component, we can only bail out if none of the Hooks produce a new value. We also can't bail out if any the other types of inputs — props and context — have changed.

These optimizations rely on the constraint that components are pure functions of props, state, and context.

In some cases, we can bail out without entering the render phase by eagerly computing the next state and comparing it to the current one. This only works if we are absolutely certain that the queue is empty at the time of the update. In concurrent mode, this is difficult to determine, because there could be multiple copies of the queue and we don't know which one is current without doing lots of extra work, which would defeat the purpose of the optimization. However, in our implementation, there are at most only two copies of the queue, and if both are empty then we know that the current queue must be.

Summary of changes:

  • We use the PerformedWork effect tag bit to track whether a component's props, state, or context has changed. This means we need to mark it dirty every time we detect that one of those inputs has a new value. Props and state are fairly straightforward, but context is tricky. I needed to store something on the fiber that represents whether it contains a pending context update. I didn't want to add an additional fiber field, because most fibers do not read from context. So I changed the existing firstContextDependency field to contextDependencies. Instead of pointing to the first item in the list, it points to a list object contains both first and expirationTime.
  • If a component bails out, we can remove the update priority flag from the current fiber. Normally we only mutate the current tree in the commit phase, never in the render phase. In this case it's ok to do because the update priority field only affects whether React needs to visit that fiber. If we receive new props, state, or context, then the fiber will be visited again anyway.
@sizebot

This comment has been minimized.

Copy link

sizebot commented Jan 11, 2019

ReactDOM: size: 🔺+0.4%, gzip: 🔺+0.4%

Details of bundled changes.

Comparing: 8a12009...6c9ae30

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.4% +0.4% 726.34 KB 729.36 KB 167.79 KB 168.5 KB UMD_DEV
react-dom.production.min.js 🔺+0.4% 🔺+0.4% 98.47 KB 98.88 KB 32.09 KB 32.2 KB UMD_PROD
react-dom.profiling.min.js +0.4% +0.3% 101.46 KB 101.86 KB 32.76 KB 32.84 KB UMD_PROFILING
react-dom.development.js +0.4% +0.4% 721.4 KB 724.42 KB 166.37 KB 167.07 KB NODE_DEV
react-dom.production.min.js 🔺+0.4% 🔺+0.3% 98.47 KB 98.88 KB 31.58 KB 31.67 KB NODE_PROD
react-dom.profiling.min.js +0.4% +0.4% 101.58 KB 101.98 KB 32.17 KB 32.29 KB NODE_PROFILING
ReactDOM-dev.js +0.4% +0.5% 742.69 KB 745.9 KB 167.38 KB 168.16 KB FB_WWW_DEV
ReactDOM-prod.js 🔺+0.6% 🔺+0.6% 309.02 KB 310.91 KB 57.09 KB 57.44 KB FB_WWW_PROD
ReactDOM-profiling.js +0.6% +0.6% 316.17 KB 318.07 KB 58.44 KB 58.78 KB FB_WWW_PROFILING
react-dom-unstable-fire.development.js +0.4% +0.4% 726.62 KB 729.65 KB 167.9 KB 168.61 KB UMD_DEV
react-dom-unstable-fire.production.min.js 🔺+0.4% 🔺+0.3% 98.49 KB 98.89 KB 32.1 KB 32.21 KB UMD_PROD
react-dom-unstable-fire.profiling.min.js +0.4% +0.3% 101.47 KB 101.88 KB 32.76 KB 32.85 KB UMD_PROFILING
react-dom-unstable-fire.development.js +0.4% +0.4% 721.68 KB 724.71 KB 166.48 KB 167.18 KB NODE_DEV
react-dom-unstable-fire.production.min.js 🔺+0.4% 🔺+0.3% 98.49 KB 98.89 KB 31.59 KB 31.68 KB NODE_PROD
react-dom-unstable-fire.profiling.min.js +0.4% +0.4% 101.59 KB 102 KB 32.18 KB 32.3 KB NODE_PROFILING
ReactFire-dev.js +0.4% +0.5% 741.84 KB 745.05 KB 167.3 KB 168.08 KB FB_WWW_DEV
ReactFire-prod.js 🔺+0.6% 🔺+0.8% 297.61 KB 299.5 KB 54.74 KB 55.17 KB FB_WWW_PROD
ReactFire-profiling.js +0.6% +0.7% 304.69 KB 306.59 KB 56.07 KB 56.46 KB FB_WWW_PROFILING
react-dom-test-utils.development.js 0.0% -0.0% 44.87 KB 44.87 KB 12.3 KB 12.3 KB UMD_DEV
react-dom-test-utils.development.js 0.0% -0.0% 44.59 KB 44.59 KB 12.24 KB 12.24 KB NODE_DEV
react-dom-unstable-native-dependencies.development.js 0.0% -0.0% 60.29 KB 60.29 KB 15.79 KB 15.79 KB NODE_DEV
react-dom-server.browser.development.js 0.0% -0.0% 123.97 KB 123.97 KB 33.11 KB 33.1 KB UMD_DEV
react-dom-server.browser.development.js 0.0% -0.0% 120.1 KB 120.1 KB 32.18 KB 32.18 KB NODE_DEV
react-dom-server.browser.production.min.js 0.0% -0.0% 16.87 KB 16.87 KB 6.51 KB 6.51 KB NODE_PROD
ReactDOMServer-prod.js 0.0% 0.0% 44.49 KB 44.49 KB 10.3 KB 10.3 KB FB_WWW_PROD
react-dom-server.node.production.min.js 0.0% -0.0% 17.74 KB 17.74 KB 6.83 KB 6.83 KB NODE_PROD
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.1% 1.21 KB 1.21 KB 706 B 705 B UMD_PROD
react-dom-unstable-fizz.browser.development.js 0.0% -0.1% 3.45 KB 3.45 KB 1.39 KB 1.39 KB NODE_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.3% 1.05 KB 1.05 KB 638 B 636 B NODE_PROD
react-dom-unstable-fizz.node.development.js 0.0% -0.1% 3.7 KB 3.7 KB 1.42 KB 1.42 KB NODE_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% -0.1% 1.1 KB 1.1 KB 668 B 667 B NODE_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.6% +0.7% 506.74 KB 509.75 KB 111.74 KB 112.47 KB UMD_DEV
react-art.production.min.js 🔺+0.4% 🔺+0.5% 90.59 KB 90.97 KB 27.83 KB 27.97 KB UMD_PROD
react-art.development.js +0.7% +0.8% 438.24 KB 441.27 KB 94.62 KB 95.35 KB NODE_DEV
react-art.production.min.js 🔺+0.7% 🔺+0.7% 55.57 KB 55.95 KB 17.12 KB 17.24 KB NODE_PROD
ReactART-dev.js +0.7% +0.8% 446.37 KB 449.57 KB 93.59 KB 94.37 KB FB_WWW_DEV
ReactART-prod.js 🔺+1.0% 🔺+1.3% 185.1 KB 186.95 KB 31.66 KB 32.09 KB FB_WWW_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.6% +0.6% 570.86 KB 574.06 KB 124.11 KB 124.9 KB RN_FB_DEV
ReactNativeRenderer-prod.js 🔺+0.8% 🔺+0.9% 240.65 KB 242.49 KB 42.3 KB 42.69 KB RN_FB_PROD
ReactNativeRenderer-profiling.js +0.8% +0.8% 246.78 KB 248.63 KB 43.71 KB 44.04 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.6% +0.6% 570.77 KB 573.98 KB 124.07 KB 124.86 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 🔺+0.5% 🔺+0.4% 225.9 KB 227.08 KB 39.33 KB 39.49 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +0.5% +0.4% 231.9 KB 233.1 KB 40.7 KB 40.87 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.6% +0.7% 561.71 KB 564.92 KB 121.81 KB 122.61 KB RN_FB_DEV
ReactFabric-prod.js 🔺+0.8% 🔺+0.9% 233.85 KB 235.69 KB 40.86 KB 41.21 KB RN_FB_PROD
ReactFabric-profiling.js +0.8% +0.8% 239.13 KB 240.98 KB 42.22 KB 42.57 KB RN_FB_PROFILING
ReactFabric-dev.js +0.6% +0.7% 561.62 KB 564.82 KB 121.76 KB 122.57 KB RN_OSS_DEV
ReactFabric-prod.js 🔺+0.5% 🔺+0.5% 219.13 KB 220.31 KB 37.84 KB 38.03 KB RN_OSS_PROD
ReactFabric-profiling.js +0.5% +0.4% 224.2 KB 225.4 KB 39.24 KB 39.41 KB RN_OSS_PROFILING

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.7% +0.8% 451.46 KB 454.48 KB 97.38 KB 98.12 KB UMD_DEV
react-test-renderer.production.min.js 🔺+0.7% 🔺+0.6% 56.97 KB 57.35 KB 17.51 KB 17.62 KB UMD_PROD
react-test-renderer.development.js +0.7% +0.8% 446.41 KB 449.44 KB 96.16 KB 96.9 KB NODE_DEV
react-test-renderer.production.min.js 🔺+0.7% 🔺+0.6% 56.64 KB 57.02 KB 17.35 KB 17.46 KB NODE_PROD
ReactTestRenderer-dev.js +0.7% +0.8% 454.75 KB 457.96 KB 95.49 KB 96.27 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% -0.0% 25.67 KB 25.67 KB 6.95 KB 6.95 KB UMD_DEV
react-test-renderer-shallow.development.js 0.0% -0.0% 19.97 KB 19.97 KB 5.51 KB 5.51 KB NODE_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.7% +0.8% 436.07 KB 439.1 KB 93.13 KB 93.87 KB NODE_DEV
react-reconciler.production.min.js 🔺+0.7% 🔺+0.6% 56.7 KB 57.11 KB 16.98 KB 17.07 KB NODE_PROD
react-reconciler-persistent.development.js +0.7% +0.8% 434.45 KB 437.47 KB 92.49 KB 93.22 KB NODE_DEV
react-reconciler-persistent.production.min.js 🔺+0.7% 🔺+0.6% 56.71 KB 57.12 KB 16.98 KB 17.08 KB NODE_PROD
react-reconciler-reflection.development.js 0.0% -0.0% 15.4 KB 15.4 KB 4.84 KB 4.83 KB NODE_DEV
react-reconciler-reflection.production.min.js 0.0% -0.1% 2.56 KB 2.56 KB 1.13 KB 1.13 KB NODE_PROD

Generated by 🚫 dangerJS

@acdlite acdlite force-pushed the acdlite:hook-state-bailout branch 3 times, most recently from f3804aa to 6a1c6f8 Jan 11, 2019

@acdlite acdlite force-pushed the acdlite:hook-state-bailout branch 2 times, most recently from d076142 to 695f36f Jan 14, 2019

@acdlite acdlite force-pushed the acdlite:hook-state-bailout branch from 695f36f to 4c65d54 Jan 16, 2019

@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Jan 16, 2019

We should also change the SSR, no?

For example

function Foo(props) {
  const [name, setName] = useState('Dan');
  if (props.name === 'Andrew') {
    setName(props.name);
  }
  return name; 
}

renderToString(<Foo name="Andrew" />

would previously go into an infinite loop but I think should now stabilize.

@acdlite

This comment has been minimized.

Copy link
Member Author

acdlite commented Jan 16, 2019

@gaearon The eager bailout doesn't apply to render phase updates, but that's an interesting idea.

@acdlite

This comment has been minimized.

Copy link
Member Author

acdlite commented Jan 16, 2019

I think we should only eagerly compute render phase updates and bail out if we're sure the bailout can be relied on completely and predictably. Not sure about that. Seems safer to always require the guard around the update.

@sebmarkbage

This comment has been minimized.

Copy link
Member

sebmarkbage commented Jan 16, 2019

The guard provides a natural place to other code that doesn't need to be evaluated if nothing has changed so it seems good to encourage that.

@sebmarkbage

This comment has been minimized.

Copy link
Member

sebmarkbage commented Jan 16, 2019

Although, counter-argument: This optimization isn’t really meant for useState. It’s mostly for useReducer where it is difficult to know if a reducer will yield the same value or not.

acdlite added some commits Jan 10, 2019

Allow useReducer to bail out of rendering by returning previous state
This is conceptually similar to `shouldComponentUpdate`, except because
there could be multiple useReducer (or useState) Hooks in a single
component, we can only bail out if none of the Hooks produce a new
value. We also can't bail out if any the other types of inputs — state
and context — have changed.

These optimizations rely on the constraint that components are pure
functions of props, state, and context.

In some cases, we can bail out without entering the render phase by
eagerly computing the next state and comparing it to the current one.
This only works if we are absolutely certain that the queue is empty at
the time of the update. In concurrent mode, this is difficult to
determine, because there could be multiple copies of the queue and we
don't know which one is current without doing lots of extra work, which
would defeat the purpose of the optimization. However, in our
implementation, there are at most only two copies of the queue, and if
*both* are empty then we know that the current queue must be.
Add test for context consumers inside hidden subtree
Should not bail out during subsequent update. (This isn't directly
related to this PR because we should have had this test, anyway.)

@acdlite acdlite force-pushed the acdlite:hook-state-bailout branch from 15ca89d to e7a99f9 Jan 17, 2019

@acdlite acdlite force-pushed the acdlite:hook-state-bailout branch from f178758 to 6c9ae30 Jan 17, 2019

@acdlite acdlite merged commit 790c8ef into facebook:master Jan 17, 2019

1 check was pending

ci/circleci CircleCI is running your tests
Details
@thysultan

This comment has been minimized.

Copy link

thysultan commented Jan 18, 2019

Does this still allow for mutable data-strucutures where something akin to the following is used for performance reasons:

const [state, setState] = useState(() => [1, 2, 3, 4, 5])

setState(state => {
    state[4] = 'swap'
    return state
})
@brunolemos

This comment has been minimized.

Copy link

brunolemos commented Jan 18, 2019

@thysultan maybe you should use useRef for that.

If you need to force a rerender you can do something like setState(c => c+ 1), there may be other ways.

@thysultan

This comment has been minimized.

Copy link

thysultan commented Jan 18, 2019

@brunolemos Yes you could also return NaN, though that is assuming the equality function is not using the Object.is semantics, in which case it is would prove to be even harder.

That aside using refs for what is intuitively state seems counter intuitive to the advise regarding using refs in effects.

You would also need a lot more coordination in that regard as opposed to passing a function callback to setState.

Mutable data-structures are still the best choice when dealing with high throughput, so it would go a long way to avoid a design that paves a path with these obstacles .

@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Jan 18, 2019

@thysultan Functional setState callback (first argument) never supported mutation. We intentionally invoke it twice in Strict Mode to make the issue more visible. React supports mutation as escape hatch, but if you mutate things, do it in event handlers and not in the functional updater.

jetoneza added a commit to jetoneza/react that referenced this pull request Jan 23, 2019

Allow useReducer to bail out of rendering by returning previous state (
…facebook#14569)

* Allow useReducer to bail out of rendering by returning previous state

This is conceptually similar to `shouldComponentUpdate`, except because
there could be multiple useReducer (or useState) Hooks in a single
component, we can only bail out if none of the Hooks produce a new
value. We also can't bail out if any the other types of inputs — state
and context — have changed.

These optimizations rely on the constraint that components are pure
functions of props, state, and context.

In some cases, we can bail out without entering the render phase by
eagerly computing the next state and comparing it to the current one.
This only works if we are absolutely certain that the queue is empty at
the time of the update. In concurrent mode, this is difficult to
determine, because there could be multiple copies of the queue and we
don't know which one is current without doing lots of extra work, which
would defeat the purpose of the optimization. However, in our
implementation, there are at most only two copies of the queue, and if
*both* are empty then we know that the current queue must be.

* Add test for context consumers inside hidden subtree

Should not bail out during subsequent update. (This isn't directly
related to this PR because we should have had this test, anyway.)

* Refactor to use module-level variable instead of effect bit

* Add test combining state bailout and props bailout (memo)

MQuy added a commit to MQuy/qreact that referenced this pull request Jan 29, 2019

MQuy added a commit to MQuy/qreact that referenced this pull request Jan 29, 2019

MQuy added a commit to MQuy/qreact that referenced this pull request Jan 29, 2019

@thomschke

This comment has been minimized.

Copy link

thomschke commented Jan 30, 2019

When using Immutable-js you have to use a special equality check.

The useMemoizeArgs hook (see #14476) for useEffect is not applicable in useState/useReducer.

Any idea to support immutable data structure libraries?


[update]

If using React.memo with a custom comparison function, then you can allow children to bail out of rendering but not allow yourself, isn't it?

n8schloss added a commit to n8schloss/react that referenced this pull request Jan 31, 2019

Allow useReducer to bail out of rendering by returning previous state (
…facebook#14569)

* Allow useReducer to bail out of rendering by returning previous state

This is conceptually similar to `shouldComponentUpdate`, except because
there could be multiple useReducer (or useState) Hooks in a single
component, we can only bail out if none of the Hooks produce a new
value. We also can't bail out if any the other types of inputs — state
and context — have changed.

These optimizations rely on the constraint that components are pure
functions of props, state, and context.

In some cases, we can bail out without entering the render phase by
eagerly computing the next state and comparing it to the current one.
This only works if we are absolutely certain that the queue is empty at
the time of the update. In concurrent mode, this is difficult to
determine, because there could be multiple copies of the queue and we
don't know which one is current without doing lots of extra work, which
would defeat the purpose of the optimization. However, in our
implementation, there are at most only two copies of the queue, and if
*both* are empty then we know that the current queue must be.

* Add test for context consumers inside hidden subtree

Should not bail out during subsequent update. (This isn't directly
related to this PR because we should have had this test, anyway.)

* Refactor to use module-level variable instead of effect bit

* Add test combining state bailout and props bailout (memo)

Kiku-git added a commit to Kiku-git/react that referenced this pull request Feb 10, 2019

Allow useReducer to bail out of rendering by returning previous state (
…facebook#14569)

* Allow useReducer to bail out of rendering by returning previous state

This is conceptually similar to `shouldComponentUpdate`, except because
there could be multiple useReducer (or useState) Hooks in a single
component, we can only bail out if none of the Hooks produce a new
value. We also can't bail out if any the other types of inputs — state
and context — have changed.

These optimizations rely on the constraint that components are pure
functions of props, state, and context.

In some cases, we can bail out without entering the render phase by
eagerly computing the next state and comparing it to the current one.
This only works if we are absolutely certain that the queue is empty at
the time of the update. In concurrent mode, this is difficult to
determine, because there could be multiple copies of the queue and we
don't know which one is current without doing lots of extra work, which
would defeat the purpose of the optimization. However, in our
implementation, there are at most only two copies of the queue, and if
*both* are empty then we know that the current queue must be.

* Add test for context consumers inside hidden subtree

Should not bail out during subsequent update. (This isn't directly
related to this PR because we should have had this test, anyway.)

* Refactor to use module-level variable instead of effect bit

* Add test combining state bailout and props bailout (memo)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment