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

Throttle retries even if everything has loaded #26611

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Apr 12, 2023

If a Suspense fallback is shown, and the data finishes loading really quickly after that, we throttle the content from appearing for 500ms to reduce thrash.

This already works for successive fallback states (like if one fallback is nested inside another) but it wasn't being applied to the final step in the sequence: if there were no more unresolved Suspense boundaries in the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders that are the result of suspended data being loaded. (Our internal jargon term for this is a "retry".)

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 12, 2023
@acdlite acdlite force-pushed the throttle-all-retries branch from 8fdca16 to c1a19de Compare April 12, 2023 15:35
@acdlite acdlite requested a review from sebmarkbage April 12, 2023 15:37
@react-sizebot
Copy link

react-sizebot commented Apr 12, 2023

Comparing: 72c890e...045a9a8

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 = 164.40 kB 164.35 kB = 51.65 kB 51.65 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 166.84 kB 166.79 kB +0.01% 52.30 kB 52.31 kB
facebook-www/ReactDOM-prod.classic.js = 565.32 kB 564.62 kB = 99.49 kB 99.43 kB
facebook-www/ReactDOM-prod.modern.js = 549.11 kB 548.41 kB = 96.79 kB 96.75 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
react-native/implementations/ReactFabric-prod.fb.js = 328.92 kB 328.26 kB = 57.75 kB 57.73 kB
react-native/implementations/ReactNativeRenderer-prod.js = 326.88 kB 326.21 kB = 57.32 kB 57.30 kB
react-native/implementations/ReactFabric-prod.js = 317.98 kB 317.32 kB = 55.63 kB 55.61 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-profiling.js = 312.60 kB 311.94 kB = 54.64 kB 54.61 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-prod.js = 296.72 kB 296.05 kB = 52.19 kB 52.16 kB

Generated by 🚫 dangerJS against 045a9a8

@acdlite acdlite force-pushed the throttle-all-retries branch 2 times, most recently from 701ba2b to b88a660 Compare April 12, 2023 17:37
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)
@acdlite acdlite force-pushed the throttle-all-retries branch from b88a660 to 045a9a8 Compare April 13, 2023 00:19
@acdlite acdlite merged commit 8256781 into facebook:main Apr 13, 2023
github-actions bot pushed a commit that referenced this pull request Apr 13, 2023
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)

DiffTrain build for [8256781](8256781)
kassens added a commit to kassens/react that referenced this pull request Apr 17, 2023
acdlite added a commit to acdlite/react that referenced this pull request Apr 20, 2023
This puts the change introduced by facebook#26611 behind a flag until Meta is
able to roll it out. Disabling the flag reverts back to the old
behavior, where retries are throttled if there's still data remaining
in the tree, but not if all the data has finished loading.

The new behavior is still enabled in the public builds.
acdlite added a commit that referenced this pull request Apr 20, 2023
This puts the change introduced by #26611 behind a flag until Meta is
able to roll it out. Disabling the flag reverts back to the old
behavior, where retries are throttled if there's still data remaining in
the tree, but not if all the data has finished loading.

The new behavior is still enabled in the public builds.
kassens pushed a commit that referenced this pull request Apr 21, 2023
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)
kassens pushed a commit that referenced this pull request Apr 21, 2023
This puts the change introduced by #26611 behind a flag until Meta is
able to roll it out. Disabling the flag reverts back to the old
behavior, where retries are throttled if there's still data remaining in
the tree, but not if all the data has finished loading.

The new behavior is still enabled in the public builds.
acdlite added a commit to acdlite/react that referenced this pull request May 11, 2023
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by facebook#26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still
get jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.
acdlite added a commit to acdlite/react that referenced this pull request May 16, 2023
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by facebook#26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still
get jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.
acdlite added a commit that referenced this pull request May 16, 2023
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by #26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still get
jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.
github-actions bot pushed a commit that referenced this pull request May 16, 2023
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by #26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still get
jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.

DiffTrain build for [4bfcd02](4bfcd02)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
This puts the change introduced by facebook#26611 behind a flag until Meta is
able to roll it out. Disabling the flag reverts back to the old
behavior, where retries are throttled if there's still data remaining in
the tree, but not if all the data has finished loading.

The new behavior is still enabled in the public builds.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by facebook#26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still get
jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)

DiffTrain build for commit 8256781.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
This puts the change introduced by #26611 behind a flag until Meta is
able to roll it out. Disabling the flag reverts back to the old
behavior, where retries are throttled if there's still data remaining in
the tree, but not if all the data has finished loading.

The new behavior is still enabled in the public builds.

DiffTrain build for commit d73d7d5.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by #26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still get
jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.

DiffTrain build for commit 4bfcd02.
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