FlatList performance slow on large changes to list

[davidliu]

Is this a bug report?

Yes. A reopening of #16186, as it is reproducible on 0.53.3

Have you read the Contributing Guidelines?

Yes.

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 9.4.0
Yarn: 1.3.2
npm: 4.6.1
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 2.3 AI-162.4069837

Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.53.3 => 0.53.3

Target Platform: iOS and Android (various OS versions, up to and including latest on each).

Steps to Reproduce

(Copied from the previous issue, as it still applies).

I have a FlatList that contains many items (1000+). It represents a lengthy checklist. I have a "Select All" button which causes every item in the list to be selected. The performance of rendering this is noticeably bad.

1. Open the Snack https://snack.expo.io/S1z6PMZnZ (preferably on a physical device since iOS simulator will likely have better performance than a physical device)
2. Tap on an individual row to see a slight lag in performance
3. Tap on "Select All" or "Deselect All" to change every item in the list and see a big delay in rendering the new state

Note that in the Snack, I am only recreating a single object of id->boolean key/value pairs when a change is made. This is my effort to isolate FlatList performance.

Expected Behavior

Since a FlatList doesn't render offscreen components, it should not synchronously update any offscreen elements, but instead asynchronously update those elements as it does for its initial load. This will result in a quick and responsive update of onscreen elements without blocking the ui for too long.

Actual Behavior

If you tap "Select All" or "Deselect All" (whichever changes all the items in a list), there is a large amount of rendering lag.

If you profile it on a device with Chrome Debugging tools, you'll notice that it spends a lot of time updating every single row, even though most of the rows are offscreen.

Reproducible Demo

https://snack.expo.io/S1z6PMZnZ

[davidliu]

The profile.json for the above chrome debugging profile.

Profile-20180228T164420.json.txt

[react-native-bot]

Thanks for posting this! It looks like your issue may refer to an older version of React Native. Can you reproduce the issue on the latest stable release?

Thank you for your contributions.

_{How to Contribute • What to Expect from Maintainers}

[davidliu]

Confirmed bug still exists in 0.54.0

[rawrmaan]

Have you tried adjusting the maxToRenderPerBatch or windowSize props on FlatList?

[davidliu]

@rawrmaan seems like the issue is related to the default windowSize for these small rows. With a default windowSize of 21, 10 screens of rows in each direction are rendered. When the select all happens, all of those "rendered" views will each get the update call propagated to them synchronously, regardless of if they're onscreen or not. For these small rows, that's about 300ish views total.

windowSize can be tweaked to band-aid around this issue, but I think it should be the responsibility of VirtualizedList to only immediately re-render synchronously onscreen items, and not offscreen items.

[davidliu]

@react-native-bot still exists on latest v0.55.

[davidliu]

@kelset These two bugs seem unrelated? #14968 is an issue of cells not being initially rendered and not added to the view fast enough due to the asynchronous nature of FlatList/VirtualizedList batch rendering.

This issue is different, in that views that have been already rendered (due to windowSize) will get rerendered synchronously after a setState call. Due to the large number of views, a synchronous update of all children views will cause a freeze.

[kelset]

I see your point - I'll reopen this. (usually issues related to lists tend to be more, like, collections of different issues so I thought I saw in the comments there something related to yours and was trying to reduce the redundancy)

Anyway, I'll reopen and tag properly.

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

[hcyildirim]

I can confirm that bug is still exists.

[fogil]

FlatList renders every item in a list, regardless if they are within the view space or not. Put a console.log('hi i am being rendered') in the renderItem function. For a list of a thousand items, you'll get a thousand console log messages.

Use RecyclerListView instead, it does what it advertises but unfortunately does not support Headers/Footers.

[cmacdonnacha]

Any update on this? Same issue occurs for a SectionList. Any workaround?

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

[stale]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

[bvaughn]

Doesn't seem like this should have been closed. Re-opening.

cc @sahrens in case you're interested.

[sahrens]

@davidliu: I don't think adjusting the parameters to better fit for a specific workload is that big of a problem - does it not address your use-case? You can also try https://github.com/Flipkart/recyclerlistview - it might be a better fit for your usecase.

I don't think it's feasible for VirtualizedList to implement even more complex scheduling of react component rendering and updates - the ReactJS core is enabling that with async/concurrent mode with priorities, and VirtualizedList (or maybe even ScrollView itself) will be able to integrate with those APIs, but it is quite a ways off (maybe post-Fabric which will unlock a lot of awesome potential there, including optional synchronous rendering to prevent checkerboarding). However, if you have ideas for how to fix things now that doesn't complicate the code too much or compromise other features, I'm happy to consider a PR.

@bvaughn: how does react-window handle the "select all" case for thousands of elements? Any tricks or does it just rely on faster desktops and smaller window sizes by default?

[bvaughn]

@bvaughn: how does react-window handle the "select all" case for thousands of elements? Any tricks or does it just rely on faster desktops and smaller window sizes by default?

@sahrens I think I'm missing some context. "select all" sounds like a data/state update. Why would it render anything that isn't visible on the screen?

[sahrens]

The list could have thousands of entries, many of which aren't rendered thanks to windowing. But VirtualizedList and react-window both have a concept of buffer/overscan to render content outside of the immediately visible viewport. It sounds like react-window just has a small enough overscanCount (2 rows by default?) that it's not an issue if every row that's rendered needs to be updated at the same time.

@davidliu: please try a smaller windowSize - 1.0, 0.5, or maybe even 0.25. Also, make sure your row components are optimized to avoid deep renders of children that don't actually need to be updated when the selected state changes.

I'm going to close this out because I don't think there is any further action we are going to take until Fabric rolls out.

[bvaughn]

@sahrens That's not what I understood was happening. Based on this comment it sounded like the list was rendering every item (not just an overscanned batch).

For a list of a thousand items, you'll get a thousand console log messages.

[sahrens]

For a list of a thousand items, you'll get a thousand console log messages.

We have recently confirmed this is not the case - windowing works correctly. If @fogil is seeing otherwise, they should submit another issue with a repro case.

[bvaughn]

Gotcha. Thanks for clarifying.