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

Revert expiration for retry lanes #21300

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Apr 16, 2021

Retries should be allowed to expire if they are CPU bound for too long, but when I made this change it caused a spike in browser crashes. There must be some other underlying bug; not super urgent but ideally should figure out why and fix it. Unfortunately we don't have a repro for the crashes, only detected via production metrics.

Retries should be allowed to expire if they are CPU bound for too long,
but when I made this change it caused a spike in browser crashes. There
must be some other underlying bug; not super urgent but ideally should
figure out why and fix it. Unfortunately we don't have a repro for the
crashes, only detected via production metrics.
@acdlite acdlite requested a review from rickhanlonii April 16, 2021 15:13
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 16, 2021
@sizebot
Copy link

sizebot commented Apr 16, 2021

Comparing: b38ac13...ed60c64

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 = 122.58 kB 122.59 kB = 39.32 kB 39.32 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.14 kB 129.15 kB = 41.41 kB 41.41 kB
facebook-www/ReactDOM-prod.classic.js = 411.73 kB 411.75 kB = 76.17 kB 76.18 kB
facebook-www/ReactDOM-prod.modern.js = 399.79 kB 399.80 kB = 74.29 kB 74.29 kB
facebook-www/ReactDOMForked-prod.classic.js = 411.73 kB 411.75 kB = 76.18 kB 76.18 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against ed60c64

@acdlite
Copy link
Collaborator Author

acdlite commented Apr 16, 2021

Confirmed this fixed the spike in crashes

@acdlite acdlite merged commit af1a4cb into facebook:master Apr 16, 2021
acdlite added a commit to acdlite/react that referenced this pull request Apr 19, 2021
Retries should be allowed to expire if they are CPU bound for too long,
but when I made this change it caused a spike in browser crashes. There
must be some other underlying bug; not super urgent but ideally should
figure out why and fix it. Unfortunately we don't have a repro for the
crashes, only detected via production metrics.
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Apr 21, 2021
Summary:
This sync includes the following changes:
- **[bd7f4a013](facebook/react@bd7f4a013 )**: Fix sloppy factoring in `performSyncWorkOnRoot` ([#21246](facebook/react#21246)) //<Andrew Clark>//
- **[78120032d](facebook/react@78120032d )**: Remove `flushDiscreteUpdates` from end of event ([#21223](facebook/react#21223)) //<Andrew Clark>//
- **[a3a7adb83](facebook/react@a3a7adb83 )**: Turn off enableSyncDefaultUpdates in test renderer ([#21319](facebook/react#21319)) //<Ricky>//
- **[cdb6b4c55](facebook/react@cdb6b4c55 )**: Only hide outermost host nodes when Offscreen is hidden ([#21250](facebook/react#21250)) //<Brian Vaughn>//
- **[b9c6a2b30](facebook/react@b9c6a2b30 )**: Remove LayoutStatic check from commit phase ([#21249](facebook/react#21249)) //<Brian Vaughn>//
- **[af1a4cbf7](facebook/react@af1a4cbf7 )**: Revert expiration for retry lanes ([#21300](facebook/react#21300)) //<Andrew Clark>//
- **[cc4b431da](facebook/react@cc4b431da )**: Mark boundary as client rendered even if aborting fallback ([#21294](facebook/react#21294)) //<Sebastian Markbåge>//

Changelog:
[General][Changed] - React Native sync for revisions f7cdc89...bd7f4a0

jest_e2e[run_all_tests]

Reviewed By: JoshuaGross

Differential Revision: D27909257

fbshipit-source-id: 36ec4cf1de9df109f1fe1542031df10a693baae0
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
Retries should be allowed to expire if they are CPU bound for too long,
but when I made this change it caused a spike in browser crashes. There
must be some other underlying bug; not super urgent but ideally should
figure out why and fix it. Unfortunately we don't have a repro for the
crashes, only detected via production metrics.
kassens added a commit that referenced this pull request Nov 14, 2023
An attempt to see if we can bring back expiration of retry lanes to
avoid cases resolving Suspense can be starved by frequent updates.

In the past, this caused increase browser crashes, but a lot of time has
passed since then. Just trying if we can re-enable this.

Old PR that reverted adding the timeout:
#21300
github-actions bot pushed a commit that referenced this pull request Nov 14, 2023
An attempt to see if we can bring back expiration of retry lanes to
avoid cases resolving Suspense can be starved by frequent updates.

In the past, this caused increase browser crashes, but a lot of time has
passed since then. Just trying if we can re-enable this.

Old PR that reverted adding the timeout:
#21300

DiffTrain build for [593ecee](593ecee)
jerrydev0927 added a commit to jerrydev0927/react that referenced this pull request Jan 5, 2024
An attempt to see if we can bring back expiration of retry lanes to
avoid cases resolving Suspense can be starved by frequent updates.

In the past, this caused increase browser crashes, but a lot of time has
passed since then. Just trying if we can re-enable this.

Old PR that reverted adding the timeout:
facebook/react#21300

DiffTrain build for [593ecee66a609d4a4c2b36b39b1e5e23b2456dd1](facebook/react@593ecee)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
An attempt to see if we can bring back expiration of retry lanes to
avoid cases resolving Suspense can be starved by frequent updates.

In the past, this caused increase browser crashes, but a lot of time has
passed since then. Just trying if we can re-enable this.

Old PR that reverted adding the timeout:
facebook#21300
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