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

Android performance issues #16714

Closed
pavloburykh opened this issue Jul 18, 2023 · 11 comments
Closed

Android performance issues #16714

pavloburykh opened this issue Jul 18, 2023 · 11 comments

Comments

@pavloburykh
Copy link
Contributor

Status app has significant performance issues on Android.
That is not really noticeable on Google Pixel, but you can see how it looks on Samsung Galaxy A52 (Android 12)
specs: https://www.gsmarena.com/samsung_galaxy_a52-10641.php - it is quite good device.

FILE_2023-07-18_121623.mp4

Steps:

  1. Create different types of chats/comunities
  2. Generate some chat history with different types of messages
  3. Scroll through chat history, interact with chat elements (composer, context menus, bottom sheets etc)
  4. Pay attention if performance is ok

Actual result: poor performance can be observed. Animation is laggy and twitchy.

Additional Information

  • Status version: nightly
  • Operating System: Android
@Parveshdhull
Copy link
Member

Hi @pavloburykh, Thank you very much for reporting this issue.
In #16433 for toast animations we started to use layout animations. And in the current version of reanimated layout animations are not that stable and causing weird problems like #16693.

Please can you confirm if performance issues happen just after the start of the app, or after a few toasts are shown (like contact request accepted, etc.). If issue only happens once toasts are shown, please can you try build from #16729 and check if issue also present there.

@pavloburykh
Copy link
Contributor Author

pavloburykh commented Aug 3, 2023

Hi @pavloburykh, Thank you very much for reporting this issue. In #16433 for toast animations we started to use layout animations. And in the current version of reanimated layout animations are not that stable and causing weird problems like #16693.

Please can you confirm if performance issues happen just after the start of the app, or after a few toasts are shown (like contact request accepted, etc.). If issue only happens once toasts are shown, please can you try build from #16729 and check if issue also present there.

Hi @Parveshdhull! Thank you for looking at this issue.

I have checked build from #16729 and can confirm that performance issues still exist. It doesn't seem to me that issue is related to toast appearing as interface starts lagging even without toasts being shown.

@Parveshdhull sorry that I didn't help much. I will try to investigate in future to understand what exactly causing the issues. Cause for some time after app is just being opened performance seems to be ok but after scrolling chat history which contains images, link previews, long messages - interface starts lagging.

Here is a video recorded on build from #16729

telegram-cloud-document-2-5359650965253272248.mp4

@Parveshdhull
Copy link
Member

@Parveshdhull
Copy link
Member

Parveshdhull commented Oct 24, 2023

Update

Flashlist

As mentioned in the above notion document, the app works well after opening, but after some time performance starts to degrade. So it might be related to high memory usage and leaking.

The article (https://www.bam.tech/article/measuring-react-native-performance-with-flashlight) compares performances of flash-list and flat-list and shows a significant decrease in memory usage. So I tried this library in our app and found out some limitations of this library.

1. Blank spaces

How does flash-list improve the app performance?

Unlike FlatList, FlashList uses recycling. Basically items do not get unmounted-remounted anymore, instead items disappearing from the screen are reused for new items appearing by changing their prop.

For this, We need to provide a prop estimatedItemSize to flash-list

estimatedItemSize is a single numeric value that hints FlashList about the approximate size of the items before they're rendered. FlashList can then use this information to decide how many items it needs to draw on the screen before initial load and while scrolling.

The value for this number is suggested(if not provided) and printed in metro logs by the library as a warning message.

But this technique for improving performance is causing blank spaces when scrolling fast in our chat screen as shown in the below video. (even with small estimatedItemSize)

Video
output-2023-10-24_10.42.52.mp4

Shopify/flash-list#868 (comment) mentions that flash-list has these problems when we have different sizes of items in list (as in our case, messages, image, links etc.)
Also Shopify/flash-list#618 mentions the problem with fast scrolling and for that recommended solution is to optimize the items/components. More details on optimzation.

2. Problem with columnWrapperStyle

Our current implementation is not working well for jump-to screen when migrating to flash-list as columnWrapperStyle don't work for it.

Screenshot

There is probably a work around for this issue, by adding required margins etc in cards itself. Currently we are just using justify-content :space-between, to create same effect we need to use screen width and card size etc. Issue is fixable but further complicates implementation.

From initial implementation it didn't looked like there was much drop in memory usage by using flash-list. And we also need to optimize components to get rid of blank spaces and might need to decrease scroll speed etc. Not sure if all these will fix the issues thougs.

Memory consumption

Currently memory consumption is very high, and with every close and open of chat screen it keeps adding up, as can be be seen as ladder like pattern in below screenshot.

Screenshot

While trying to find out which component is causing this problem, I tried to remove 1 by 1 big components and measure increase in memory consumption using flash light, these were results. (while opening and closing of 1-1 chat 10 times)

  • Without any change = 175 MB Increase
  • Removed chat list = 156 MB Increase
  • + Removed navigation top bar = 111 MB Increase
  • + Removed composer = 20 MB Increase

Extra, Only removing composer = 90 MB Increase

These results are not sufficient enough to tell why memory is not freeing after closing the screen. This pattern was also visible with native navigation, surprisingly same 175 MB Increase.

While using flashlight we need to consider its not reliable way to measure as it can give varying results based on device environment and resources. I also tried to use leak-canary using flipper, but for some reason my app is not being detected by it.

Improvement Scope

Navigation

How we differ from native navigation?

Some of details about navigation can be found in this notion document.

First what's common.
Similar to our navigation system, native navigation also keeps previous screens rendered and active re-frame subscriptions active in the background.

And What's different

  1. React Native Tree Size - Native navigation pushes native screen while navigation and renders new React Native Root inside that. Smaller tree size helps updating components etc. This is the major drawback of our custom navigation we are not able to fix.
  2. Animations - As Native navigation don't allow that much customization, there animations are somewhat simple.

Example, if we open a community chat from switcher card
Native navigation - Just pushes a chat screen with simple slide animation from right or bottom
Shell navigation - Opens chat screen with a zoom animation from a particular point in screen (switcher card position), with animating position, width, height, border radius etc. And in the background we open community screen, open community home, update switcher card position.

So, as we can see there are so much going on in case of shell navigation which we have to keep if we want to keep all the perks of jump-to navigation and cool animations.

Ok, that was limitations, now let's discuss what is better(or can be).

As shell navigation is custom navigation system it gives complete freedom to us for improving performance. Currently we are destroying and recreating screen while navigating. But if we optimize and simplify screens enough, we can keep them rendered.
This is the main thing which makes flash list and our bottom tabs implementation have very good performance. If we are not re-mounting big and heavy components like composer, navigation bar, loading skeleton etc. every time it can significantly improve chat opening time, which is not possible with native navigation system and causes stumbled opening of the screen.

Things we should do to improve performance

  • As we have seen tree size is problem, we should avoid using subscriptions and reagent atoms in roots components. Because it will lead to rerender whole screen
  • Use smaller subscriptions and in child views instead of parent. This will also help in keeping screens rendered, as whole screen is not re-rendering when we are navigating to different chat or community etc.
  • Chat screen is main performance screen. It is very slow and also consumes lots of memory. To improve this screen we have to optimize and simplify it. There are several calculations we probably don't need. For example in case of Improve the floating shell button and fix its position in the screens #16981, we just needed to embed floating shell button in composer instead of calculating its position based on composer state etc. We also need to simplify effects, (they probably not needed). As in [WIP] fix muted colors of composer elements after reopening the chat #17717 empty-effect gets called even before data is loaded and after that it becomes useless. And with dependencies in effects, they are working more like listeners, which is probably not right way to do. Instead of listening and acting on subscriptions etc, these changes should be made while we are performing action which is causing that subscription to change.
  • Better compatibility with chat screen navigation. Currently we are just showing loading skeleton while chat screen is opening, than allowing subscriptions etc. This makes whole chat screen re-render. We need to simplify that.
  • Improve memory consumption.

@pavloburykh
Copy link
Contributor Author

@Parveshdhull many thanks for this detailed research.

@pavloburykh
Copy link
Contributor Author

pavloburykh commented Jan 25, 2024

UPDATE (25.01.24):

Just a small update related to performance on my Android 12, Samsung Galaxy A52.

When recovering accounts that have a lot of data (communities with history, many contacts etc) I am still facing serious performance issues. On the video below you can see how laggy is the UI when scrolling the history, interacting with app elements etc. Such level of performance in fact makes the app hardly usable for future android users.

I understand that this problem is very hard to debug and resolve but I would like point out that this is a potential critical issue in terms of public release. So it is necessary to improve it.

telegram-cloud-document-2-5303391618139700413.mp4

cc @cammellos

@churik
Copy link
Member

churik commented Nov 7, 2024

@pavloburykh

can we create separate issues out of it in order to split the work in pieces?

how do you feel about that @Parveshdhull ?

@pavloburykh
Copy link
Contributor Author

@pavloburykh

can we create separate issues out of it in order to split the work in pieces?

@churik sure. I will re-investigate which performance issues are still valid and log them separately.

@Parveshdhull
Copy link
Member

@pavloburykh what's the state of performance now as we simplified the composer, and hide(will remove) jump-to behind the toggle? Do you think this still requires some work? If this is the case we can create separete issues as per requirments, otherwise we can just close it

@pavloburykh
Copy link
Contributor Author

pavloburykh commented Nov 7, 2024

@pavloburykh what's the state of performance now as we simplified the composer, and hide(will remove) jump-to behind the toggle? Do you think this still requires some work? If this is the case we can create separete issues as per requirments, otherwise we can just close it

@Parveshdhull performance definitely got better now. I will re-check and close this issue in case everything is ok. Otherwise will open separate issues.

@pavloburykh
Copy link
Contributor Author

@Parveshdhull @churik
Based on my investigation all performance issues mentioned in this issue are gone. So I am closing this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants