Reenable scrollEventThrottle prop for ScrollView and HorizontalScrollView #38475
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: D47492259 |
Base commit: ab3c00d |
…View (facebook#38475) Summary: Pull Request resolved: facebook#38475 We added `scrollEventThrottle` for android in D35735978, but the experiment was never executed and the flag got removed in D39449184. Since the same feature is on iOS and the implementation here is the same to iOS (https://fburl.com/code/htcuhq4w), it should be safe to support. Changelog: [Android][Add] - Add scrollEventThrottle prop support for android Reviewed By: cortinico Differential Revision: D47492259 fbshipit-source-id: bf273fff4549d112e2278f6af00b01eef5b00a59
|
This pull request was exported from Phabricator. Differential Revision: D47492259 |
ryancat
force-pushed
the
export-D47492259
branch
from
0bfef39 to
5865f3d
Compare
|
This pull request has been merged in 777934e. |
juniorklawa
pushed a commit
to juniorklawa/react-native
that referenced
this pull request
Jul 20, 2023
…View (facebook#38475) Summary: Pull Request resolved: facebook#38475 We added `scrollEventThrottle` for android in D35735978, but the experiment was never executed and the flag got removed in D39449184. Since the same feature is on iOS and the implementation here is the same to iOS (https://fburl.com/code/htcuhq4w), it should be safe to support. Changelog: [Android][Add] - Add scrollEventThrottle prop support for android Reviewed By: cortinico Differential Revision: D47492259 fbshipit-source-id: 09a2bead652bfef4a2c70b996cf66f6983604db2
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Jul 27, 2023
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). facebook#38475 made this code no longer no-op on Android, which caused scroll regressions documented in facebook#38470 We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. Though after this change, it is possible some heavy scenarios will see more time dedicated to FlatList work after this. Changelog: [General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList Differential Revision: D47823772 fbshipit-source-id: 8e78f2349c1a8134908a2e190368b8f84ff84e06
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
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
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
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
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: dc6290ad02f9cbd40429270fc878d51cbb9b56b9
facebook-github-bot
pushed a commit
that referenced
this pull request
Jul 31, 2023
Summary: Pull Request resolved: #38648 #38475 made this code no longer no-op on Android, which caused regressions documented in #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: 55d22a1074235ccc1b2cf167f6b1758640c79edb
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:
We added
scrollEventThrottlefor android in D35735978, but the experiment was never executed and the flag got removed in D39449184.Since the same feature is on iOS and the implementation here is the same to iOS (https://fburl.com/code/htcuhq4w), it should be safe to support.
Reviewed By: cortinico
Differential Revision: D47492259