Remove default 50ms Scroll Event Throttling in VirtualizedList #38648
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D47823772 |
Base commit: b0a8d45 |
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Jul 27, 2023
…ook#38648) Summary: Pull Request resolved: facebook#38648 This is specific to the scenario, and device, and time-based sampling as implemented on Android may inherently create stutters because we may not proess events at a consistent cadence (and we may never process the last events). facebook#38475 made this code no longer no-op on Android, which caused regressions documented in facebook#38470 due to VirtualizedList having more out-of-date information. We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay to new renders by default when high priority work is not happening (see `updateCellsBatchingPeriod`). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events. We can eventually remove this once VirtualizedList is able to use OffScreen universally. Changelog: [General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList Differential Revision: D47823772 fbshipit-source-id: c3cbd81bdd74ade79ec050b6ef02c4d0c43f4db2
NickGerleman
force-pushed
the
export-D47823772
branch
from
4a23fb4
to
c56ce67
Compare
|
This pull request was exported from Phabricator. Differential Revision: D47823772 |
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Jul 27, 2023
…ook#38648) Summary: Pull Request resolved: facebook#38648 This is specific to the scenario, and device, and time-based sampling as implemented on Android may inherently create stutters because we may not proess events at a consistent cadence (and we may never process the last events). facebook#38475 made this code no longer no-op on Android, which caused regressions documented in facebook#38470 (comment) due to VirtualizedList having more out-of-date information. We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay to new renders by default when high priority work is not happening (see `updateCellsBatchingPeriod`). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events. We can eventually remove this once VirtualizedList is able to use OffScreen universally. Changelog: [General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList Differential Revision: D47823772 fbshipit-source-id: 0abf312b9cf5c5f25457b58f4b8cbb393789b14d
NickGerleman
force-pushed
the
export-D47823772
branch
from
c56ce67
to
7e7628d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D47823772 |
NickGerleman
force-pushed
the
export-D47823772
branch
from
7e7628d
to
402e6c9
Compare
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Jul 31, 2023
…ook#38648) Summary: Pull Request resolved: facebook#38648 facebook#38475 made this code no longer no-op on Android, which caused regressions documented in facebook#38470 (comment) due to VirtualizedList having more out-of-date information. We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay to new renders by default when high priority work is not happening (see `updateCellsBatchingPeriod`). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events. We can eventually remove this once VirtualizedList is able to use OffScreen universally. Changelog: [General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList Reviewed By: ryancat Differential Revision: D47823772 fbshipit-source-id: bf0befb1b9146e2173069c484735dc287b956e37
|
This pull request was exported from Phabricator. Differential Revision: D47823772 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D47823772 |
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Jul 31, 2023
…ook#38648) Summary: Pull Request resolved: facebook#38648 facebook#38475 made this code no longer no-op on Android, which caused regressions documented in facebook#38470 (comment) due to VirtualizedList having more out-of-date information. We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay to new renders by default when high priority work is not happening (see `updateCellsBatchingPeriod`). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events. We can eventually remove this once VirtualizedList is able to use OffScreen universally. Changelog: [General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList Reviewed By: ryancat Differential Revision: D47823772 fbshipit-source-id: 5f538b7531a1907d4831fdb81ea0cda56af38bd2
NickGerleman
force-pushed
the
export-D47823772
branch
from
402e6c9
to
6636e55
Compare
…ook#38648) Summary: Pull Request resolved: facebook#38648 facebook#38475 made this code no longer no-op on Android, which caused regressions documented in facebook#38470 (comment) due to VirtualizedList having more out-of-date information. We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay to new renders by default when high priority work is not happening (see `updateCellsBatchingPeriod`). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events. We can eventually remove this once VirtualizedList is able to use OffScreen universally. Changelog: [General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList Reviewed By: ryancat Differential Revision: D47823772 fbshipit-source-id: dc6290ad02f9cbd40429270fc878d51cbb9b56b9
|
This pull request was exported from Phabricator. Differential Revision: D47823772 |
NickGerleman
force-pushed
the
export-D47823772
branch
from
6636e55
to
2508aa9
Compare
|
This pull request has been merged in 3eccc53. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This is specific to the scenario, and device, and time-based sampling as implemented on Android may inherently create stutters because we may not proess events at a consistent cadence (and we may never process the last events).
#38475 made this code no longer no-op on Android, which caused scroll regressions documented in #38470
We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay by default when high priority work is not happening (see
updateCellsBatchingPeriod). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events.We can eventually remove this once VirtualizedList is able to use OffScreen universally.
Changelog:
[General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList
Differential Revision: D47823772