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

Long text in inverted Flatlist causes huge performance drop on Android #30034

Closed
divonelnc opened this issue Sep 25, 2020 · 81 comments
Closed

Long text in inverted Flatlist causes huge performance drop on Android #30034

divonelnc opened this issue Sep 25, 2020 · 81 comments
Labels
Component: FlatList Impact: Performance When the issue impacts the app running or build performance Platform: Android Android applications. Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@divonelnc
Copy link

Description

I recently upgraded to react 0.63.2 thanks to the latest Expo SDK 39. I immediately noticed that my most important FlatLists took a huge performance hit on Android (iOs seem unaffected).

After investigating, I found out that it happens when using inverted on a FlatList that contains items with a lot of text (see snack).

The same Flatlist, with the same data, is perfectly smooth when not inverted.

I have yet to test it in production, as the latest Expo SDK is causing trouble that stops me from building the app.

React Native version:

react-native: https://github.com/expo/react-native/archive/sdk-39.0.0.tar.gz => 0.63.2

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

  1. Create a Flatlist that renders items that contain a lot of text
  2. Set the Flatlist as inverted

Expected Results

The Flatlist should be as smooth when inverted as when normal.

Snack, code example, screenshot, or link to a repository:

https://snack.expo.io/@divone/4c8d68

@divonelnc
Copy link
Author

divonelnc commented Sep 30, 2020

Just checked it in production and the performance hit is still there, making the app feel laggy.

@divonelnc
Copy link
Author

This issue appears as soon as you invert a ScrollView actually, as you can see on this snack: https://snack.expo.io/@divone/c0c1f3

Again, only visible on a physical Android device.

@divonelnc
Copy link
Author

It seems that the issue only happens on some phones. For me, it happens on a Google Pixel 4 with Android 11.

@SuryaL
Copy link

SuryaL commented Oct 20, 2020

I see this issue on Pixel4 XL running Android 11, which was working fine on React Native 0.61.5 with Android 10. I notice that when the keyboard is open it does not seem to lag.

@safaiyeh
Copy link
Contributor

Hey @divonelnc thanks for the investigation! I noticed the same issue when running your example.

@liangjs19
Copy link

liangjs19 commented Jan 8, 2021

Facing this issue on a Samsung Galaxy Note10 running Android 11 as well with Expo SDK 39. Didn't happen on Android 10.

Discovered that changing the verticallyInverted style prop from transform: [{scaleY: -1}] to scaleY: -1 in VirtualizedList fixes it for Android, however it does not invert on iOS.

@chr-sk
Copy link

chr-sk commented Feb 9, 2021

Experienced this issue as well. I upgraded my Samsung Galaxy S10 from Android 10 to Android 11 and suddenly experienced a noticable lag on the inverted FlatList in my app. @cookiespam 's workaround fixed the problem, however.

@TwanLuttik
Copy link

TwanLuttik commented Feb 17, 2021

Im experience the same issue on android with a Samsung Galaxy s10 Android 11, and i noticed after updated the same issue. And i even have it if i don't have long text (The text element goes full width with short text)

@vinceplusplus
Copy link

@cookiespam's workaround works. for a less intrusive fix, we could "invert" twice ourselves, once in the container and once in every cell

return (
  <FlatList
    // ...
    style={{scaleY: -1}}
    renderItem={({item}) => {
      return (
        <View style={{scaleY: -1}}>
          {/* cell content */}
        </View
      );
    }}
  >
);

for ios, could just use the official inverted prop. however, that scaleY outside of transform seems undocumented. @cookiespam how did you come up with it? great thanks though

@liangjs19
Copy link

@vinceplusplus

I was just trying to invert the FlatList on my own without using the inverted prop and somehow accidentally used scaleY (was googling how to invert something using css) which had been deprecated.

@vinceplusplus
Copy link

@cookiespam then that was a good accident 👍

@pqkluan
Copy link

pqkluan commented Mar 25, 2021

I also encounter this issue with my chat screen. It's really hard to find to root cause.

  • Only happen on Android S10e device.
  • With inverted FlatList.
  • FlatList Item contains a Text with any given length.

It's only take 10 items in the list for the performance to be hit really bad.

I'm using @cookiespam bandaid + patch-package to resolve the issue for now now.

Here is the the content of the patch:

react-native+0.63.3.patch

diff --git a/node_modules/react-native/Libraries/Lists/VirtualizedList.js b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
index 9ec105f..a8f1d8c 100644
--- a/node_modules/react-native/Libraries/Lists/VirtualizedList.js
+++ b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
@@ -11,6 +11,7 @@
 'use strict';
 
 const Batchinator = require('../Interaction/Batchinator');
+const Platform = require('../Utilities/Platform');
 const FillRateHelper = require('./FillRateHelper');
 const PropTypes = require('prop-types');
 const React = require('react');
@@ -2185,9 +2186,14 @@ function describeNestedLists(childList: {
 }
 
 const styles = StyleSheet.create({
-  verticallyInverted: {
-    transform: [{scaleY: -1}],
-  },
+  verticallyInverted:
+    Platform.OS === 'android'
+      ? {
+          scaleY: -1,
+        }
+      : {
+          transform: [{scaleY: -1}]
+        },
   horizontallyInverted: {
     transform: [{scaleX: -1}],
   },

@OlivierFreyssinet-old
Copy link

Wow wow wow thank you guys this fix is a lifesaver !!! 🙏

I was actually trying to replace my FlatList by a ScrollView to see where the issue was coming from and noticed then that without inverting the FlatList the performance was fine.

Now I can keep my FlatList inverted, it works exactly same as before without changing any other props and I'm going from horrible 40-50fps (since updating to Android 11) to butter smooth 90fps 😍

the solution proposed by @vinceplusplus is what I will be using:

return (
  <FlatList
    // ...
    style={{scaleY: -1}}
    renderItem={({item}) => {
      return (
        <View style={{scaleY: -1}}>
          {/* cell content */}
        </View
      );
    }}
  >
);

Is someone considered making a pull request with @cookiespam and @pqkluan's patch? I could make one in a couple of days when I get some free time but I also don't want to steal your moment of glory haha

@Jovan1998
Copy link

Facing this issue on a Samsung Galaxy Note10 running Android 11 as well with Expo SDK 39. Didn't happen on Android 10.

Discovered that changing the verticallyInverted style prop from transform: [{scaleY: -1}] to scaleY: -1 in VirtualizedList fixes it for Android, however it does not invert on iOS.

@cookiespam You are legend, worked for me too.

@enahum
Copy link
Contributor

enahum commented May 18, 2021

using the scaleY: -1 instead of transform: [{scaleY: -1}] indeed seems to fix the performance issue, BUT (as usual there is a but) the refresh control becomes an issue.

It renders on the top of the screen as you attempt to scroll down and in order to actually scroll you'll need to scroll up then down to "free" the pan responder somehow.

@JamesMahy
Copy link

@vinceplusplus

I was just trying to invert the FlatList on my own without using the inverted prop and somehow accidentally used scaleY (was googling how to invert something using css) which had been deprecated.

I think I love you, thank you!

@KochMario
Copy link

KochMario commented Aug 9, 2021

I'd just wanted to drop in and say thank you as well 🙏🏼💙

I was severely confused when I upgraded to Android 11 (on a Redmi Note 8 Pro) and the performance on the chat screen in my app was suddenly horrible. Scrolling for a couple of seconds resulted in 1000+ dropped frames. It was so laggy and jerky that one got almost dizzy by watching it.

Based on your suggestions I've implemented a simple check (with react-native-device-info) so the fix is only applied for android & version >= 11.

const useAndroidInvertedFix = useMemo(
  () => Platform.OS === 'android' && Number(getSystemVersion()) >= 11,
  [],
);

I'm extremely grateful for this workaround, however the underlying issue should be officially addressed by react-native itself, right?
Does anyone know the procedure for doing that or is it enough that this issue gets enough attention? 🤔

@safaiyeh
Copy link
Contributor

safaiyeh commented Aug 9, 2021

@KochMario the best way to get this fix in is by submitting a PR. Unfortunately the team can’t address every issue as most are not a priority. But there is a massive effort right now to evaluate PrS submitted by contributors.

@nero2009
Copy link

I have been struggling to understand why my chat was lagging when tested on a Samsung S20 ultra with 12gb of RAM while it works prefectly on my xiaomi 9 with just 3GB of RAM. And the comments in this thread helped me fix the issue.
Thanks everyone🎉

@eggybot
Copy link

eggybot commented Oct 27, 2021

resolved my issue on react-native-gifted-chat with the solution from @pqkluan

As @nero2009 mention it has issue with Android devices for lag scrolling (not smooth like in iOS). If you are using react-native-gifted-chat and experience this issue with the RN version you have. Apply the solution mention on this thread.

@Andrija00
Copy link

I see but I am not using Expo or Virtualized List. I am using basic flatList from React Native and rotate will fix the issue. Thanks for help!

@hannojg
Copy link
Contributor

hannojg commented Aug 10, 2023

VirtualizedList is the base component that FlatList uses to implement the list.

@anatooly
Copy link

anatooly commented Aug 11, 2023

@Andrija00 are you check using shopify/FlashList for you chat (10k char messages)?

@fmorau
Copy link

fmorau commented Aug 11, 2023

@hannojg do you have an idea when it will go out?

scaleY does not work anymore on 0.72 with new arch (just updated) for "top level view" containers, which I used over all types of messages in my chat - so now I need to refactor to scaleY each and every deeply nested View/Text, which has content directly in it to make it work back again 😬

@Andrija00
Copy link

Andrija00 commented Aug 11, 2023

@anatooly We had issues using FlashList in the past for our ChatScreen component so we stick with FlatList

@Nasseratic
Copy link

@hannojg great work!! 💪 😍

@hannojg
Copy link
Contributor

hannojg commented Aug 14, 2023

The fix is part of RN 0.72.4, you can upgrade and test 😊

@retrixe
Copy link

retrixe commented Oct 14, 2023

Perhaps this issue should be closed?

@kelset
Copy link
Contributor

kelset commented Oct 16, 2023

if anyone can confirm that it's fixed I'd be happy to close

@retrixe
Copy link

retrixe commented Oct 16, 2023

The issue seems fixed for me after updating my app @kelset I don't experience any performance issues anymore on either the new or old architecture, the UI thread's fps is solid now

@daidonpah
Copy link

daidonpah commented Oct 21, 2023

Unfortunately for me updating did not resolve the issue. On Samsung S23 Ultra Android 13, when removing the inverted prop, FlatList behaves perfectly fine as it should, running at smooth 120fps. Adding the inverted prop sporadically brings the fps down to 0.5 and a lot of frame drops. After updating to RN 0.72.4 it seemed liked the issue "improved" at first, but after 20 seconds of scrolling, same performance drop again.

EDIT: I had tried to play around with FlashList instead and forgotten to change it back to FlatList. By using FlatList the issue does indeed seem to be fixed, although now the scroll indicator is on the left. The remedy in this PR (38073) did not work for me. I tried deleting caches and such, but to no avail. I am using expo under windows and I followed this cache invalidation list. RN 0.72.6 and this is my code:

<FlatList
    data={list}
    renderItem={({ item }) => <ListItem item={item} />}
    inverted
/>

So nothing exotic going on. Vertical scroll bar still on the left.

@hannojg
Copy link
Contributor

hannojg commented Oct 23, 2023

@daidonpah Are you sure your flatlist doesn't have any custom styles? Because that may cause the wrong position of the scrollbar.

If you don't, are you able to reproduce in a clean new app? If so, I think I'd be good to open a new issue 😊

@mirkos93
Copy link

mirkos93 commented Nov 3, 2023

@daidonpah Have you solved the issue using FlashList?

Edit: I tried using FlashList and the problem seems to be solved.

@foloinfo
Copy link

foloinfo commented Nov 8, 2023

Looks like the performance issue is fixed with v0.72.4 with this line using {transform: [{scale: -1}]} not scaleY.
Thank you for the fix @hannojg

I'm using Expo 48 and it uses "react-native": "0.71.14" so I had to patch myself.
This will hide the scrollbar for Android, but better than huge performance drop while I wait to update RN.

const styles = StyleSheet.create({
  inverted: {
    ...Platform.select({
      android: {
        transform: [{ scale: -1 }],
      },
    })
  }
})

...

return (
  <FlatList
    // inverted={true} on Android 33+ will cause performance issue
    inverted={Platform.OS === 'ios'}
    // apply the inverted style on android
    style={styles.inverted}
    // hide the scroll indicator on android because it shows on the left
    showsVerticalScrollIndicator={Platform.OS === 'ios'}
    listFooterComponent={
      <View style={styles.inverted}>
        <Header/>
      </View>
    }
    renderItem={({ item }) => {
      return (
        <View style={styles.inverted}>
          {renderItem(item)}
        </View>
      )
    }}
  />
)

@retrixe
Copy link

retrixe commented Nov 8, 2023

@foloinfo on earlier versions this workaround works without compromising the scrollbar:
#30034 (comment)
#30034 (comment)

Ashoat added a commit to CommE2E/comm that referenced this issue Nov 28, 2023
Summary:
Three changes here:

1. We don't need the Android `FlatList` hack anymore. It's [solved in 0.72.4](facebook/react-native#30034 (comment)). This patchfile now only needs to cover our custom iOS image pasting logic.
2. An upstream React Native change caused the order of the lines in `- (void)paste:(id)sender` to flip. I kept the new order.
3. I had to move the changes in `TextInput.js` to `TextInput.flow.js`. I could have left them in `TextInput.js`, but Flow doesn't seem to pay attention to that file, and I figured a simpler patchfile would be better.

Test Plan: Flow

Reviewers: atul
Ashoat added a commit to CommE2E/comm that referenced this issue Jan 3, 2024
Summary:
Three changes here:

1. We don't need the Android `FlatList` hack anymore. It's [solved in 0.72.4](facebook/react-native#30034 (comment)). This patchfile now only needs to cover our custom iOS image pasting logic.
2. An upstream React Native change caused the order of the lines in `- (void)paste:(id)sender` to flip. I kept the new order.
3. I had to move the changes in `TextInput.js` to `TextInput.flow.js`. I could have left them in `TextInput.js`, but Flow doesn't seem to pay attention to that file, and I figured a simpler patchfile would be better.

Test Plan: Flow

Reviewers: atul
Ashoat added a commit to CommE2E/comm that referenced this issue Jan 4, 2024
Summary:
Three changes here:

1. We don't need the Android `FlatList` hack anymore. It's [solved in 0.72.4](facebook/react-native#30034 (comment)). This patchfile now only needs to cover our custom iOS image pasting logic.
2. An upstream React Native change caused the order of the lines in `- (void)paste:(id)sender` to flip. I kept the new order.
3. I had to move the changes in `TextInput.js` to `TextInput.flow.js`. I could have left them in `TextInput.js`, but Flow doesn't seem to pay attention to that file, and I figured a simpler patchfile would be better.

Test Plan: Flow

Reviewers: atul
Ashoat added a commit to CommE2E/comm that referenced this issue Jan 4, 2024
Summary:
Three changes here:

1. We don't need the Android `FlatList` hack anymore. It's [solved in 0.72.4](facebook/react-native#30034 (comment)). This patchfile now only needs to cover our custom iOS image pasting logic.
2. An upstream React Native change caused the order of the lines in `- (void)paste:(id)sender` to flip. I kept the new order.
3. I had to move the changes in `TextInput.js` to `TextInput.flow.js`. I could have left them in `TextInput.js`, but Flow doesn't seem to pay attention to that file, and I figured a simpler patchfile would be better.

Test Plan: Flow

Reviewers: atul
Ashoat added a commit to CommE2E/comm that referenced this issue Jan 5, 2024
Summary:
Three changes here:

1. We don't need the Android `FlatList` hack anymore. It's [solved in 0.72.4](facebook/react-native#30034 (comment)). This patchfile now only needs to cover our custom iOS image pasting logic.
2. An upstream React Native change caused the order of the lines in `- (void)paste:(id)sender` to flip. I kept the new order.
3. I had to move the changes in `TextInput.js` to `TextInput.flow.js`. I could have left them in `TextInput.js`, but Flow doesn't seem to pay attention to that file, and I figured a simpler patchfile would be better.

Test Plan: Flow

Reviewers: atul
Copy link

github-actions bot commented May 6, 2024

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label May 6, 2024
Copy link

This issue was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 14, 2024
@JJSLIoT
Copy link

JJSLIoT commented Sep 19, 2024

Not stale!

@elias96
Copy link

elias96 commented Sep 19, 2024

Keep this open amigo

@retrixe
Copy link

retrixe commented Sep 26, 2024

This issue has been fixed in recent versions of React Native unless you are still experiencing it, in which case it may be more appropriate to open a new issue

The fix is part of RN 0.72.4, you can upgrade and test 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: FlatList Impact: Performance When the issue impacts the app running or build performance Platform: Android Android applications. Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests