status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.91k stars 987 forks source link

Manage assets view lags on scrolls #10957

Closed Serhy closed 4 years ago

Serhy commented 4 years ago

Bug Report

Problem

Manage assets view is heavy: when scrolling through assets there are heavy lags (depends of device but on middle level device it's already visible). Or when check/uncheck token, - it may take 5+ seconds for check mark to appear/disappear.

If Manage Assets view opened then all the reset views on different from wallet tabs laggy too

Expected behavior

Smooth experience when scrolling through Manage asseets view

Actual behavior

From Galaxy J7 Android 8.1 video:

https://drive.google.com/file/d/1CiAY_j5FvRo73jnOzfUnD8d5AUAqFWAE/view?usp=sharing

Reproduction

Additional Information

hesterbruikman commented 4 years ago

it may take 5+ seconds for check mark to appear/disappear.

This I recognize and IMO seems relatively more urgent to fix. Tried on Samsung Galaxy s10e (Adroid 10) and Fairphone 2 (Android 7)

Looks like there's a transition going on when checking a box

flexsurfer commented 4 years ago

@Ferossgp i know you were looking at this, any updates? thanks

cammellos commented 4 years ago

@Ferossgp is this still an issue?

Ferossgp commented 4 years ago

@cammellos yes, with my latest solution @Serhy mentioned that it was still slow on low-end devices

rasom commented 4 years ago

@Ferossgp @cammellos @flexsurfer Slowness is caused by checkbox accessory (specifically by its animation). I tested it on a relatively slow android device by measuring time between log messages which were printed in render-token component, specifically between the first and the last items in the list. There are a bit more than 100 elements in the list.

;; no :accessory at all     6.689s
;; without animation        7.81s
;; with animation           15.579s

Those results show that it is about two times slower with animation, but in fact it is even worse: if you try to use the app when entire list is rendered it doesn't respond because UI thread is blocked (as far as I understand by animation or related code). So, for instance, if I open "manage assets" screen, then go back to the wallet, the app does not respond on any interaction during the next 5-6 seconds (transition back to the wallet works because screen is already rendered and transition itself happens on a native thread).

So we either need to optimise that animation or to make it optional for "manage assets" list.

flexsurfer commented 4 years ago

@rasom why all elements are rendered and not only visible ?

rasom commented 4 years ago

@flexsurfer for testing I scrolled the list to the end, so that's why all elements were rendered. But SectionList would render first 60-80 items in the background anyway, even with initialNumToRender set to a smaller number. That property only sets a size of the initial batch.

Ferossgp commented 4 years ago

@rasom yes, the animation slows the list (seems like there is a lot of bridge usage in this case). But the problem with performance was always there even before animations, with the old list-item and checkbox.

rasom commented 4 years ago

@Ferossgp yep there is a separate issue when you check/uncheck the item, but animation made it way slower when scrolling, thus more noticeable. At the moment I'm looking what can be done with slow check/uncheck.

Ferossgp commented 4 years ago

I've tried using simply pressable here https://github.com/status-im/status-react/pull/11078/files#diff-0a905251f106f1d0bb8a4716380a5a1fe40bb60a0868373cccf711529fe64915R50 and @Serhy mentioned that there is almost no difference in performance on a low-end device.

rasom commented 4 years ago

@Ferossgp it is slow because the change on "manage assets" screen requires entire wallet screen to be re-rendered. So if we comment out wallet screen and interact with checkboxes in assets list it will become noticeably faster.

rasom commented 4 years ago

I will prepare fixes for both slow scrolling and slow checkboxes and file a PR soon, then let's test it and decide if it is enough.