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

[WIP] Possible work loop bugfix #22007

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Aug 2, 2021

Got a non-reproducible bug report internally. Based on the info in the bug report, I think it might be related to this code here.

The original code is definitely wrong but I don't know if it's the same bug that's being reported.

Pushing this so we can try it and see. Will try to write a test and land the fix regardless.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 2, 2021
@sizebot
Copy link

sizebot commented Aug 2, 2021

Comparing: 4225133...7a67816

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 127.58 kB 127.48 kB = 40.71 kB 40.65 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 130.40 kB 130.30 kB = 41.64 kB 41.57 kB
facebook-www/ReactDOM-prod.classic.js = 406.43 kB 405.96 kB = 75.24 kB 75.11 kB
facebook-www/ReactDOM-prod.modern.js = 394.79 kB 394.32 kB = 73.51 kB 73.37 kB
facebook-www/ReactDOMForked-prod.classic.js = 406.43 kB 405.96 kB = 75.25 kB 75.11 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-art/umd/react-art.development.js = 765.83 kB 764.15 kB = 161.67 kB 161.23 kB
oss-stable-semver/react-art/umd/react-art.development.js = 749.11 kB 747.43 kB = 158.22 kB 157.78 kB
oss-stable/react-art/umd/react-art.development.js = 749.11 kB 747.43 kB = 158.22 kB 157.78 kB
react-native/implementations/ReactNativeRenderer-profiling.fb.js = 298.09 kB 297.42 kB = 53.46 kB 53.33 kB
react-native/implementations/ReactNativeRenderer-profiling.js = 295.49 kB 294.83 kB = 52.84 kB 52.71 kB
react-native/implementations/ReactNativeRenderer-dev.fb.js = 731.65 kB 729.99 kB = 158.31 kB 157.87 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js = 716.34 kB 714.71 kB = 152.20 kB 151.76 kB
react-native/implementations/ReactFabric-profiling.fb.js = 291.96 kB 291.29 kB = 52.37 kB 52.24 kB
react-native/implementations/ReactNativeRenderer-dev.js = 719.60 kB 717.94 kB = 156.22 kB 155.78 kB
react-native/implementations/ReactFabric-profiling.js = 289.35 kB 288.68 kB = 51.73 kB 51.59 kB
react-native/implementations/ReactFabric-dev.fb.js = 718.76 kB 717.10 kB = 155.53 kB 155.05 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.development.js = 700.26 kB 698.64 kB = 148.75 kB 148.31 kB
oss-stable/react-reconciler/cjs/react-reconciler.development.js = 700.26 kB 698.64 kB = 148.75 kB 148.31 kB
facebook-www/ReactART-dev.classic.js = 710.30 kB 708.64 kB = 151.41 kB 150.98 kB
react-native/implementations/ReactFabric-dev.js = 706.69 kB 705.03 kB = 153.39 kB 152.95 kB
facebook-www/ReactART-dev.modern.js = 700.02 kB 698.36 kB = 149.32 kB 148.88 kB
react-native/implementations/ReactNativeRenderer-prod.fb.js = 280.13 kB 279.46 kB = 50.58 kB 50.44 kB
react-native/implementations/ReactNativeRenderer-prod.js = 277.53 kB 276.86 kB = 49.97 kB 49.84 kB
react-native/implementations/ReactFabric-prod.fb.js = 273.97 kB 273.30 kB = 49.36 kB 49.24 kB
react-native/implementations/ReactFabric-prod.js = 271.36 kB 270.69 kB = 48.74 kB 48.62 kB
oss-experimental/react-art/cjs/react-art.development.js = 662.12 kB 660.49 kB = 143.33 kB 142.88 kB
oss-stable-semver/react-art/cjs/react-art.development.js = 646.24 kB 644.61 kB = 139.91 kB 139.47 kB
oss-stable/react-art/cjs/react-art.development.js = 646.24 kB 644.61 kB = 139.91 kB 139.47 kB
oss-experimental/react-test-renderer/umd/react-test-renderer.development.js = 647.23 kB 645.56 kB = 136.24 kB 135.80 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.development.js = 617.04 kB 615.41 kB = 134.76 kB 134.34 kB
oss-stable-semver/react-test-renderer/umd/react-test-renderer.development.js = 630.54 kB 628.87 kB = 132.81 kB 132.37 kB
oss-stable/react-test-renderer/umd/react-test-renderer.development.js = 630.54 kB 628.87 kB = 132.81 kB 132.37 kB
facebook-www/ReactTestRenderer-dev.modern.js = 623.18 kB 621.52 kB = 133.61 kB 133.16 kB
facebook-www/ReactTestRenderer-dev.classic.js = 623.16 kB 621.51 kB = 133.60 kB 133.15 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-profiling.js = 247.28 kB 246.62 kB = 44.95 kB 44.84 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.development.js = 601.19 kB 599.56 kB = 131.38 kB 130.97 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.development.js = 601.19 kB 599.56 kB = 131.38 kB 130.97 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-dev.js = 611.71 kB 610.05 kB = 132.38 kB 131.95 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-prod.js = 232.66 kB 231.99 kB = 42.69 kB 42.56 kB

Generated by 🚫 dangerJS against 7a67816

@@ -752,6 +752,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
return null;
} else {
// Current task was not canceled. Continue.
originalCallbackNode = root.callbackNode;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What am I missing?

This seems semantically the same since we only set it if root.callbackNode === originalCallbackNode

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh you're not missing anything, I pushed the wrong commit 🤦‍♂️

@acdlite
Copy link
Collaborator Author

acdlite commented Aug 2, 2021

I don't think this is the culprit but there's definitely something fishy going on with continuations so I'm going to use this branch to debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants