Skip to content

Fork performWork instead of using boolean flag #15169

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 21, 2019
Merged

Fork performWork instead of using boolean flag #15169

merged 1 commit into from
Mar 21, 2019

Conversation

sebmarkbage
Copy link
Collaborator

I inline it into performAsyncWork instead.

Code that was only relevant to the async callback had leaked into the
performWork call which is an indication that this was a bad abstraction
and therefore the wrong place to DRY.

By inlining I also discovered that minExpirationTime is actually irrelevant
in the yieldy case so we can clean that up.

@sebmarkbage sebmarkbage requested a review from acdlite March 20, 2019 20:16
@sizebot
Copy link

sizebot commented Mar 20, 2019 •
edited
Loading

ReactDOM: size: -0.0%, gzip: -0.0%

Details of bundled changes.

Comparing: b1a56ab...b399b9e

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.1% 0.0% 800.33 KB 800.82 KB 181.39 KB 181.4 KB UMD_DEV
react-dom.production.min.js -0.0% -0.0% 105.09 KB 105.09 KB 33.98 KB 33.96 KB UMD_PROD
react-dom.profiling.min.js -0.0% 0.0% 108.07 KB 108.06 KB 34.61 KB 34.62 KB UMD_PROFILING
react-dom.development.js +0.1% 0.0% 794.72 KB 795.21 KB 179.79 KB 179.79 KB NODE_DEV
react-dom.production.min.js -0.0% 0.0% 105.07 KB 105.06 KB 33.46 KB 33.47 KB NODE_PROD
react-dom.profiling.min.js -0.0% -0.0% 108.17 KB 108.17 KB 34.07 KB 34.07 KB NODE_PROFILING
ReactDOM-dev.js +0.1% 0.0% 818.78 KB 819.27 KB 181.25 KB 181.26 KB FB_WWW_DEV
ReactDOM-prod.js 0.0% 0.0% 322.52 KB 322.65 KB 59.3 KB 59.3 KB FB_WWW_PROD
ReactDOM-profiling.js 0.0% -0.0% 327.39 KB 327.52 KB 60.54 KB 60.52 KB FB_WWW_PROFILING
react-dom-unstable-fire.development.js +0.1% 0.0% 800.68 KB 801.17 KB 181.53 KB 181.54 KB UMD_DEV
react-dom-unstable-fire.production.min.js -0.0% -0.0% 105.11 KB 105.1 KB 33.99 KB 33.97 KB UMD_PROD
react-dom-unstable-fire.profiling.min.js -0.0% 0.0% 108.08 KB 108.08 KB 34.62 KB 34.63 KB UMD_PROFILING
react-dom-unstable-fire.development.js +0.1% 0.0% 795.07 KB 795.56 KB 179.93 KB 179.94 KB NODE_DEV
react-dom-unstable-fire.production.min.js -0.0% 0.0% 105.08 KB 105.08 KB 33.47 KB 33.48 KB NODE_PROD
react-dom-unstable-fire.profiling.min.js -0.0% -0.0% 108.18 KB 108.18 KB 34.08 KB 34.07 KB NODE_PROFILING
ReactFire-dev.js +0.1% 0.0% 817.99 KB 818.48 KB 181.13 KB 181.14 KB FB_WWW_DEV
ReactFire-prod.js 0.0% 0.0% 311.11 KB 311.21 KB 56.97 KB 56.97 KB FB_WWW_PROD
ReactFire-profiling.js 0.0% -0.0% 316.03 KB 316.13 KB 58.2 KB 58.2 KB FB_WWW_PROFILING
react-dom-test-utils.development.js 0.0% 0.0% 47.49 KB 47.49 KB 12.98 KB 12.98 KB UMD_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 9.95 KB 9.95 KB 3.66 KB 3.66 KB UMD_PROD
react-dom-test-utils.development.js 0.0% 0.0% 47.21 KB 47.21 KB 12.91 KB 12.91 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 9.73 KB 9.73 KB 3.58 KB 3.59 KB NODE_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.53 KB 60.53 KB 15.8 KB 15.8 KB UMD_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 0.0% 10.69 KB 10.69 KB 3.67 KB 3.67 KB UMD_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.2 KB 60.2 KB 15.67 KB 15.67 KB NODE_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 0.0% 10.42 KB 10.42 KB 3.56 KB 3.57 KB NODE_PROD
react-dom-server.browser.development.js 0.0% 0.0% 133.68 KB 133.68 KB 35.37 KB 35.37 KB UMD_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 18.93 KB 18.93 KB 7.14 KB 7.14 KB UMD_PROD
react-dom-server.browser.development.js 0.0% 0.0% 129.81 KB 129.81 KB 34.43 KB 34.43 KB NODE_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 18.86 KB 18.86 KB 7.14 KB 7.14 KB NODE_PROD
ReactDOMServer-dev.js 0.0% 0.0% 132.15 KB 132.15 KB 34.19 KB 34.19 KB FB_WWW_DEV
ReactDOMServer-prod.js 0.0% -0.0% 45.67 KB 45.67 KB 10.57 KB 10.56 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% 0.0% 131.75 KB 131.75 KB 34.97 KB 34.97 KB NODE_DEV
react-dom-server.node.production.min.js 0.0% 0.0% 19.72 KB 19.72 KB 7.44 KB 7.44 KB NODE_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.66 KB 3.66 KB 1.45 KB 1.45 KB UMD_DEV
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.49 KB 3.49 KB 1.41 KB 1.41 KB NODE_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.2% 1.05 KB 1.05 KB 636 B 637 B NODE_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.1% 0.0% 565.39 KB 565.89 KB 122.15 KB 122.15 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 96.76 KB 96.78 KB 29.67 KB 29.68 KB UMD_PROD
react-art.development.js +0.1% 0.0% 496.3 KB 496.8 KB 104.76 KB 104.76 KB NODE_DEV
react-art.production.min.js 0.0% 🔺+0.1% 61.77 KB 61.78 KB 18.89 KB 18.91 KB NODE_PROD
ReactART-dev.js +0.1% 0.0% 506.22 KB 506.73 KB 104.07 KB 104.08 KB FB_WWW_DEV
ReactART-prod.js 🔺+0.1% 0.0% 196.33 KB 196.46 KB 33.27 KB 33.27 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.1% 0.0% 626.3 KB 626.79 KB 133.69 KB 133.7 KB RN_FB_DEV
ReactNativeRenderer-prod.js -0.0% -0.0% 245.6 KB 245.55 KB 42.96 KB 42.95 KB RN_FB_PROD
ReactNativeRenderer-profiling.js -0.0% -0.0% 251.85 KB 251.79 KB 44.3 KB 44.29 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.1% 0.0% 626.22 KB 626.71 KB 133.65 KB 133.66 KB RN_OSS_DEV
ReactNativeRenderer-prod.js -0.0% -0.0% 245.62 KB 245.56 KB 42.95 KB 42.95 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js -0.0% -0.0% 251.87 KB 251.81 KB 44.3 KB 44.29 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.1% 0.0% 616.73 KB 617.22 KB 131.33 KB 131.34 KB RN_FB_DEV
ReactFabric-prod.js -0.0% -0.0% 239.68 KB 239.62 KB 41.79 KB 41.78 KB RN_FB_PROD
ReactFabric-profiling.js -0.0% -0.0% 245.02 KB 244.96 KB 43.11 KB 43.1 KB RN_FB_PROFILING
ReactFabric-dev.js +0.1% 0.0% 616.64 KB 617.13 KB 131.29 KB 131.3 KB RN_OSS_DEV
ReactFabric-prod.js -0.0% -0.0% 239.68 KB 239.63 KB 41.78 KB 41.77 KB RN_OSS_PROD
ReactFabric-profiling.js -0.0% -0.0% 245.03 KB 244.97 KB 43.11 KB 43.1 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.1% 0.0% 507.68 KB 508.18 KB 106.91 KB 106.92 KB UMD_DEV
react-test-renderer.production.min.js 0.0% 🔺+0.1% 62.76 KB 62.77 KB 19.12 KB 19.14 KB UMD_PROD
react-test-renderer.development.js +0.1% -0.0% 503.2 KB 503.7 KB 105.77 KB 105.76 KB NODE_DEV
react-test-renderer.production.min.js 0.0% 🔺+0.1% 62.45 KB 62.46 KB 18.95 KB 18.96 KB NODE_PROD
ReactTestRenderer-dev.js +0.1% 0.0% 514.26 KB 514.77 KB 105.5 KB 105.51 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% 0.0% 38.98 KB 38.98 KB 9.81 KB 9.81 KB UMD_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.42 KB 11.42 KB 3.5 KB 3.51 KB UMD_PROD
react-test-renderer-shallow.development.js 0.0% 0.0% 33.2 KB 33.2 KB 8.41 KB 8.42 KB NODE_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.61 KB 11.61 KB 3.63 KB 3.63 KB NODE_PROD

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% -0.0% 496.41 KB 496.9 KB 103.57 KB 103.57 KB NODE_DEV
react-reconciler.production.min.js -0.0% 0.0% 62.97 KB 62.97 KB 18.73 KB 18.73 KB NODE_PROD
react-reconciler-persistent.development.js +0.1% -0.0% 494.3 KB 494.79 KB 102.71 KB 102.7 KB NODE_DEV
react-reconciler-persistent.production.min.js -0.0% 0.0% 62.98 KB 62.98 KB 18.74 KB 18.74 KB NODE_PROD
react-reconciler-reflection.development.js 0.0% 0.0% 15.79 KB 15.79 KB 4.91 KB 4.91 KB NODE_DEV
react-reconciler-reflection.production.min.js 0.0% 🔺+0.1% 2.37 KB 2.37 KB 1.07 KB 1.07 KB NODE_PROD

Generated by 🚫 dangerJS

@sebmarkbage @acdlite
Fork performWork instead of using boolean flag
b399b9e 
I inline it into performAsyncWork instead.

Code that was only relevant to the async callback had leaked into the
performWork call which is an indication that this was a bad abstraction
and therefore the wrong place to DRY.

By inlining I also discovered that minExpirationTime is actually irrelevant
in the yieldy case so we can clean that up.
@acdlite acdlite force-pushed the dontstopthescheduler branch from 5a96961 to b399b9e Compare March 21, 2019 00:01
@acdlite
Copy link
Collaborator

acdlite commented Mar 21, 2019

Test?

@sebmarkbage sebmarkbage merged commit 4b8e164 into facebook:master Mar 21, 2019
@sebmarkbage
Copy link
Collaborator Author

I don't have a reduced test case atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acdlite acdlite Awaiting requested review from acdlite

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sebmarkbage @sizebot @acdlite @facebook-github-bot