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

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

Merged
merged 4 commits into from
Jan 17, 2019

Conversation

acdlite
Copy link
Collaborator

@acdlite 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
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 hook-state-bailout branch 3 times, most recently from f3804aa to 6a1c6f8 Compare January 11, 2019 23:52
@acdlite acdlite force-pushed the hook-state-bailout branch 2 times, most recently from d076142 to 695f36f Compare January 14, 2019 23:55
@gaearon
Copy link
Collaborator

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
Copy link
Collaborator Author

acdlite commented Jan 16, 2019

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

@acdlite
Copy link
Collaborator 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
Copy link
Collaborator

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
Copy link
Collaborator

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.

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.
Should not bail out during subsequent update. (This isn't directly
related to this PR because we should have had this test, anyway.)
This was referenced Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants