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

When a root expires, flush all expired work in a single batch #13503

Merged
merged 1 commit into from Sep 6, 2018

Conversation

@acdlite
Copy link
Member

@acdlite acdlite commented Aug 28, 2018

Still need to add some Suspense tests, and some comments. Pushing so Dan can test if it fixes the issue in the fixture.

@pull-bot
Copy link

@pull-bot pull-bot commented Aug 28, 2018

ReactDOM: size: 🔺+0.2%, gzip: 🔺+0.1%

Details of bundled changes.

Comparing: bb62722...077addd

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.2% +0.2% 644.92 KB 645.9 KB 151.12 KB 151.36 KB UMD_DEV
react-dom.production.min.js 🔺+0.2% 🔺+0.1% 92.16 KB 92.31 KB 30.08 KB 30.1 KB UMD_PROD
react-dom.development.js +0.2% +0.2% 640.27 KB 641.25 KB 149.71 KB 149.96 KB NODE_DEV
react-dom.production.min.js 🔺+0.2% 🔺+0.1% 92.1 KB 92.25 KB 29.67 KB 29.71 KB NODE_PROD
ReactDOM-dev.js +0.1% +0.2% 662.6 KB 663.58 KB 152.06 KB 152.31 KB FB_WWW_DEV
ReactDOM-prod.js 🔺+0.1% 🔺+0.1% 286.89 KB 287.29 KB 53.15 KB 53.2 KB FB_WWW_PROD
react-dom.profiling.min.js +0.2% +0.2% 95.05 KB 95.2 KB 30.31 KB 30.36 KB NODE_PROFILING
ReactDOM-profiling.js +0.1% +0.1% 293.27 KB 293.67 KB 54.48 KB 54.53 KB FB_WWW_PROFILING

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.2% +0.3% 438.59 KB 439.57 KB 98.24 KB 98.5 KB UMD_DEV
react-art.production.min.js 🔺+0.2% 🔺+0.1% 84.57 KB 84.72 KB 25.98 KB 26.01 KB UMD_PROD
react-art.development.js +0.3% +0.3% 370.34 KB 371.32 KB 81.14 KB 81.4 KB NODE_DEV
react-art.production.min.js 🔺+0.3% 🔺+0.3% 49.54 KB 49.68 KB 15.26 KB 15.31 KB NODE_PROD
ReactART-dev.js +0.3% +0.3% 375.29 KB 376.27 KB 79.93 KB 80.17 KB FB_WWW_DEV
ReactART-prod.js 🔺+0.3% 🔺+0.2% 159.84 KB 160.24 KB 27.11 KB 27.18 KB FB_WWW_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.3% +0.3% 382.37 KB 383.34 KB 83.72 KB 83.97 KB UMD_DEV
react-test-renderer.production.min.js 🔺+0.3% 🔺+0.3% 50.75 KB 50.89 KB 15.51 KB 15.56 KB UMD_PROD
react-test-renderer.development.js +0.3% +0.3% 377.95 KB 378.93 KB 82.6 KB 82.85 KB NODE_DEV
react-test-renderer.production.min.js 🔺+0.3% 🔺+0.4% 50.46 KB 50.6 KB 15.35 KB 15.4 KB NODE_PROD
ReactTestRenderer-dev.js +0.3% +0.3% 383.07 KB 384.05 KB 81.64 KB 81.89 KB FB_WWW_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.3% +0.3% 366.23 KB 367.21 KB 79.27 KB 79.51 KB NODE_DEV
react-reconciler.production.min.js 🔺+0.3% 🔺+0.3% 49.1 KB 49.25 KB 14.76 KB 14.79 KB NODE_PROD
react-reconciler-persistent.development.js +0.3% +0.3% 364.8 KB 365.78 KB 78.69 KB 78.94 KB NODE_DEV
react-reconciler-persistent.production.min.js 🔺+0.3% 🔺+0.3% 49.11 KB 49.26 KB 14.76 KB 14.8 KB NODE_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.2% +0.2% 497.65 KB 498.63 KB 110.11 KB 110.37 KB RN_FB_DEV
ReactNativeRenderer-prod.js 🔺+0.2% 🔺+0.1% 214.75 KB 215.15 KB 37.45 KB 37.5 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.2% +0.2% 497.38 KB 498.36 KB 110.04 KB 110.3 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 🔺+0.2% 🔺+0.2% 204.57 KB 204.97 KB 35.8 KB 35.86 KB RN_OSS_PROD
ReactFabric-dev.js +0.2% +0.2% 487.83 KB 488.81 KB 107.69 KB 107.96 KB RN_FB_DEV
ReactFabric-prod.js 🔺+0.2% 🔺+0.2% 197.07 KB 197.47 KB 34.31 KB 34.36 KB RN_FB_PROD
ReactFabric-dev.js +0.2% +0.2% 487.87 KB 488.85 KB 107.71 KB 107.97 KB RN_OSS_DEV
ReactFabric-prod.js 🔺+0.2% 🔺+0.2% 197.1 KB 197.5 KB 34.32 KB 34.37 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +0.2% +0.1% 211.68 KB 212.08 KB 37.14 KB 37.2 KB RN_OSS_PROFILING
ReactFabric-profiling.js +0.2% +0.2% 203.01 KB 203.41 KB 35.59 KB 35.65 KB RN_OSS_PROFILING
ReactNativeRenderer-profiling.js +0.2% +0.1% 220.16 KB 220.56 KB 38.63 KB 38.68 KB RN_FB_PROFILING
ReactFabric-profiling.js +0.2% +0.2% 202.97 KB 203.37 KB 35.57 KB 35.63 KB RN_FB_PROFILING

schedule

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
schedule.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
schedule.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD

Generated by 🚫 dangerJS

Loading

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 28, 2018

It looks like it's helping. Still gets stuck sometimes but I suppose it's just two expirations happening one after the other (because the commit itself took a long time). But nothing like it was getting stuck before.

Loading

@acdlite acdlite force-pushed the batchexpiredupdates branch 2 times, most recently from f28c369 to 801a2d8 Sep 5, 2018
@acdlite
Copy link
Member Author

@acdlite acdlite commented Sep 5, 2018

@gaearon Ready for review

Loading

@acdlite acdlite force-pushed the batchexpiredupdates branch 3 times, most recently from 7f19945 to 0d9802f Sep 5, 2018
Instead of flushing each level one at a time.
@acdlite acdlite force-pushed the batchexpiredupdates branch from 0d9802f to 077addd Sep 5, 2018
@gaearon gaearon mentioned this pull request Sep 5, 2018
gaearon
gaearon approved these changes Sep 5, 2018
Copy link
Member

@gaearon gaearon left a comment

This makes sense to me by itself but doesn't actually help the issue reproducible in the fixture. If you try to type a lot and then constantly add and remove characters it will eventually expire with isExpired = false. However the scheduler thinks there's still some time before the timeout, so didTimeout is false. I'm not sure why but we need to fix this too?

Loading

@gaearon gaearon merged commit f765f02 into facebook:master Sep 6, 2018
1 check passed
Loading
@gaearon
Copy link
Member

@gaearon gaearon commented Sep 6, 2018

Let's get this fix in, but we'll need a follow-up for scheduler itself.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants