metamask-mobile icon indicating copy to clipboard operation
metamask-mobile copied to clipboard

[Bug]: iOS Account List Latency

Open cortisiko opened this issue 6 months ago • 9 comments

Describe the bug

While testing the latest RC build (1970) on iOS i am noticing latency when opening the account list. I also experience latency while trying to dismiss the account bottom sheet as well.

Things to know:

  • I am using a SRP that has 55 evm accounts and 2 Solana accounts
  • I am using OS version is 18.5

Expected behavior

There should nto be latency while opening the account picker

Screenshots/Recordings

https://github.com/user-attachments/assets/2242216e-630a-4880-b47c-dec3a0425648

Steps to reproduce

Fresh install of 7.47.0 (1970) Import wallet with multiple evm and solana accounts. (in my case my wallet has 55 evm and 2 solana accounts) While on the wallet view, try to open the account list Notice latency

Error messages or log output


Detection stage

During release testing

Version

7.47.0

Build type

None

Device

iPhone 16 Pro

Operating system

iOS

Additional context

No response

Severity

No response

cortisiko avatar Jun 12 '25 20:06 cortisiko

Importing SRP - no need to watch the entire recording, it's just a loader for ~40-50 sec.

7.47.0 (1970), iPhone 15, 50 EVM + 3 Solana accounts:

https://github.com/user-attachments/assets/4a52e0f7-2391-4bde-906a-42e9a962bab8

sleepytanya avatar Jun 12 '25 21:06 sleepytanya

v7.46.2 (Current prod version at the time of this message) is more performant than 7.47.0

https://github.com/user-attachments/assets/98e1fe25-2643-4843-9b26-9fec0627356c

cortisiko avatar Jun 12 '25 21:06 cortisiko

Unable to reproduce latency on SRP with 24 EVM accounts And 4 Solana accounts.

Reproduced on SRP with 45 EVM accounts, 5 Solana accounts and 1 imported:

https://github.com/user-attachments/assets/f358e77b-5b59-4af1-b96f-64e3ec4971eb

Unik0rnMaggie avatar Jun 13 '25 07:06 Unik0rnMaggie

I also tested this with Android, and there is a very significant performance downgrade when I imported 3 different SRP with 17 accounts in total

javiergarciavera avatar Jun 13 '25 09:06 javiergarciavera

Reproduced latency more significant on iOS.

Also, adding a Solana Account sometimes fails for iOS. unable to reproduce consistently.

https://github.com/user-attachments/assets/61371a05-4dbd-44c6-9958-65212a92eaab

https://github.com/user-attachments/assets/342a24e1-9957-4443-99c3-9e546ece3dde

Unik0rnMaggie avatar Jun 13 '25 14:06 Unik0rnMaggie

is this something Wallet UX has bandwidth to handle @hesterbruikman ? If not, Assets can help

alfeng6 avatar Jun 17 '25 04:06 alfeng6

For visibility, the assets team hasn't introduced any changes to the accounts list page. This may be related to the snap performance which is hindering the account list performance.

Prithpal-Sooriya avatar Jun 17 '25 18:06 Prithpal-Sooriya

It is worth noting that the Virtualised Flatlist will only start virtualising after 999 accounts https://github.com/MetaMask/metamask-mobile/blob/fb2011f1ddb034a196822a546c48a59976488870/app/components/UI/EvmAccountSelectorList/EvmAccountSelectorList.tsx#L392

This prop defeats the purpose of the flatlist and is trying to render all accounts in the list. Will see if we can remove this and retain the list virtualisation.

Prithpal-Sooriya avatar Jun 17 '25 18:06 Prithpal-Sooriya

This proposed PR cannot be cherry-picked, it is too large and diverges again main.

I think we should hand this to the accounts team and can use the linked PR as guidelines on how to improve performance on this list.

Prithpal-Sooriya avatar Jun 17 '25 22:06 Prithpal-Sooriya

Reproduced on 7.50.0 (2000), iOS and Android (accounts list latency and switching accounts):

On Android, there is also a new issue where one finger scroll is not possible for the accounts list. Two finger scroll is needed. (possibly related https://github.com/MetaMask/metamask-mobile/issues/15900#issuecomment-2991063124)

https://github.com/user-attachments/assets/73f37f75-0b79-4051-b105-4251e630eb36

https://github.com/user-attachments/assets/f14a9282-0795-4109-9351-9b2f51b869c6

Unik0rnMaggie avatar Jun 23 '25 10:06 Unik0rnMaggie

Reproduced on v7.50.0 (2018) with 4 EVM accounts, 1 Solana Account and 1 imported account:

https://www.loom.com/share/dde1a29496504892a966127544b383e4?sid=6e283fa2-274c-4fa0-8156-a0f7d61df514

Adding and EVM account on wallet with 2 SRPs is also slow:

https://www.loom.com/share/0ac8c9621a974d268bbb8a4d6f10164f?sid=4159d3de-9a3d-4887-a6b2-601b70c34794

Unik0rnMaggie avatar Jun 25 '25 08:06 Unik0rnMaggie

It is worth noting that the Virtualised Flatlist will only start virtualising after 999 accounts

metamask-mobile/app/components/UI/EvmAccountSelectorList/EvmAccountSelectorList.tsx

Line 392 in fb2011f initialNumToRender={999}

This prop defeats the purpose of the flatlist and is trying to render all accounts in the list. Will see if we can remove this and retain the list virtualisation.

As Prithpal-Sooriya mentioned initialNumToRender prop is causing the list to load slower.

Removing this prop alleviates the problem, but it still not as fast as previous builds (7.45.0, 7.46.2)

https://github.com/user-attachments/assets/67573ecb-537d-4d40-b277-f1618f28379d

I've also replaced it with a FlashList, but the gains are not substantial

https://github.com/user-attachments/assets/44c1a210-8025-4ca2-88a6-1f8d7af61cbd

This changes however, do not improve the latency we're seeing when we pick an account from the list. The list dismisses and we've left with an overlay for around two seconds.

We can break this issue into two pieces

  • Opening the account list is slow
  • Picking an account from the list is slow.

Opening the account list is slow

Changing from a SectionList to a FlatList to a FlashList does not have the performance improvement we're seeking. Could it be that the Bottom Sheet animations are not aligned with the current library versions?

 WARN  [Reanimated] Writing to `value` during component render. Please ensure that you don't access the `value` property nor use `set` method of a shared value while React is rendering a component.

If you don't want to see this message, you can disable the `strict` mode. Refer to:
https://docs.swmansion.com/react-native-reanimated/docs/debugging/logger-configuration for more details.
 WARN  [Reanimated] Reading from `value` during component render. Please ensure that you don't access the `value` property nor use `get` method of a shared value while React is rendering a component.

If you don't want to see this message, you can disable the `strict` mode. Refer to:
https://docs.swmansion.com/react-native-reanimated/docs/debugging/logger-configuration for more details.

The dev console is spammed with these logs whenever the AccountSelector is opened and when a new account is picked.

  • TODO: verify if disabling animations improve the performance of Account Selector

Picking an account from the list is slow.

With trial and error I've managed to pinpoint the function call that hangs the UI for around 2 seconds

AccountsController.setSelectedAccount(account.id); from Engine instance.setSelectedAccount

We can also see these dev console warning every time this function runs

WARN  [Reanimated] Writing to `value` during component render. Please ensure that you don't access the `value` property nor use `set` method of a shared value while React is rendering a component.

If you don't want to see this message, you can disable the `strict` mode. Refer to:
https://docs.swmansion.com/react-native-reanimated/docs/debugging/logger-configuration for more details.
 INFO  TRACK event saved {"event": "Switched Account", "properties": {"number_of_accounts": 28, "source": "Wallet Tab"}, "type": "track"}
 WARN  Animated.event now requires a second argument for options [Component Stack]
 WARN  Animated.event now requires a second argument for options [Component Stack]
  • TODO: Check AccountController stateChange event side effects that might block the UI

Notes

There's also some bitrise builds with different code changes, as local dev builds are optimized and cannot be compared to production builds

  • android ci build with flatlist: https://app.bitrise.io/build/eb4fdbf9-74be-4552-a790-23a132f23c95?tab=artifacts no noticeable improvement.
  • android ci build with flatlist and no initialNumToRender https://app.bitrise.io/build/d2972998-e633-4b35-9ad7-c5d19929973d?tab=artifacts
  • android ci build with sectionlist with no initialNumToRender https://app.bitrise.io/build/45910c12-04b6-47ff-b841-4ad10419c7f8
  • android ci build with sectionlist, no initialNumToRender and no bottomsheetoverlay https://app.bitrise.io/build/169ca9ec-a1f0-4798-87b6-800bea546eb1

joaoloureirop avatar Jun 25 '25 12:06 joaoloureirop

TODO: verify if disabling animations improve the performance of Account Selector

Disabling animations made the app appear more performant by making transitions abrupt, while retaining the same delay. Animations are out of the equation.

Looking further, with the help of @andrepimenta the useAccounts hook was identified as being expensive to execute, slowing down the app.

After many iterations of isolating each function call of this hook, and evaluating its impact, we managed to disable the balance fetching and retain all the remaining functionalities, which rely on the hook useMultichainBalancesForAllAccounts.

Partially disabling this hook useMultichainBalancesForAllAccounts by removing some of the function calls and replace the return values with mocked data proved to be difficult and time consuming due to their tight dependency.

Here's a test build with useMultichainBalancesForAllAccounts disabled inside useAccounts hook, which means no account balances, and initialNumToRender prop removed from the FlatList inside EvmAccountSelectorList component.

https://app.bitrise.io/build/b37684e2-daff-44b9-bfee-302b07679eed?tab=artifacts

Build from branch fix-750-account-perf

https://github.com/user-attachments/assets/aa202593-f806-4da4-9217-4835c64c2d0b

The second piece remains slow, as tracking AccountController setSelectedAccount method side effects is not trivial.

joaoloureirop avatar Jun 25 '25 21:06 joaoloureirop

@sleepytanya your issues seems to be more related to importing an SRP than the account list. Secondly, is this a regression compared to the previous release or has it always been slow? I agree we should improve it, I just want to determine if we should hold off the RC.

owencraston avatar Jul 02 '25 10:07 owencraston

This bug was never just iOS only. it applied to both platforms and was even worse on Android.

Testing results after https://github.com/MetaMask/metamask-mobile/pull/16860 and https://github.com/MetaMask/metamask-mobile/pull/16739 were cherry picked into the release....

Result from prod (7.46.2)

https://github.com/user-attachments/assets/93ed38d2-eba1-4260-9607-6198929a0235

Result from latest pre release of 7.50.0 (link to build) on my Pixel 6.

https://github.com/user-attachments/assets/840340a1-794a-4660-ab7f-5cd779edbc5b

The performance of this list seems to have slightly improved compared to 7.46.2 and has similar to 7.47.0 (pre release build). The account list seems to open faster now but for super long lists, there is a small amount of stuttering on the individual cells. This is a fair trade off for faster time to interaction.

https://github.com/user-attachments/assets/cfc5952b-1e2d-45a6-a644-6dcf5f075915

owencraston avatar Jul 02 '25 14:07 owencraston

Hi @gantunesr,

This issue has been closed. Please complete this RCA form: https://docs.google.com/forms/d/e/1FAIpQLSdnPbJISzFlR_aQD2uRpnMKSoGAopuTd_yeZK7J4Q5GzgbsOA/viewform?entry.340898780=16345&entry.1228565618=regression-RC-7.47.0%2Cregression-RC-7.50.0&entry.1698032149=team-accounts%2Cteam-assets

github-actions[bot] avatar Jul 02 '25 14:07 github-actions[bot]