ivy-wallet icon indicating copy to clipboard operation
ivy-wallet copied to clipboard

[Performance] Fix Unstable composables

Open ILIYANGERMANOV opened this issue 1 year ago • 9 comments

Please confirm the following

What would you like to improve?

  • Ivy Wallet is full of unstable composables that make the app slow.
  • We should fix that by making the unstable parameters - stable.

Because...

  • The app slow and lags when scrolling (losing frames) or animating.

Description

  1. Remove the unstable Composables baseline from https://github.com/Ivy-Apps/ivy-wallet/blob/main/ci-actions/compose-stability/ivy-compose-stability-baseline.txt
  2. Submit a draft PR so the "Compose stability" CI can execute and fail
  3. See the failure in the Compose Stability workflow - it'll show you which composable is unstable and what parameters cause that.

Fixing unstable composable

  1. Identify unstable parameters (the Compose Stability action will tell you)
  2. Fixing an unstbale parameter: (best to worse)
  • Replace it with a primitive (e.g. String, Int, Double, ...)
  • Replace List / Set with ImmutableList / ImmutableSet from kotlinx Immutable collections
  • Explicitly mark as @Immutable or @Stable for classes

Read this https://developer.android.com/jetpack/compose/performance/stability.

Fix stability issues guide: https://developer.android.com/jetpack/compose/performance/stability/fix

Success Criteria

  • Compose Stability CI action passes.
  • You have fixed at least three unstable Composables.
  • The app works as expected and doesn't crash.

ILIYANGERMANOV avatar Feb 15 '24 08:02 ILIYANGERMANOV

Thank you @ILIYANGERMANOV for raising Issue #2965! 🚀 What's next? Read our Contribution Guidelines 📚.

Tagging @ILIYANGERMANOV for review & approval 👀

ivywallet avatar Feb 15 '24 08:02 ivywallet

Note: Even if you fix 2 Composables it's still very helpful! But... in general the more you can fix the better 😄

ILIYANGERMANOV avatar Feb 15 '24 08:02 ILIYANGERMANOV

I'm on it

AMHaroun avatar Feb 15 '24 12:02 AMHaroun

Thank you for your interest @AMHaroun! 🎉 Issue #2965 is assigned to you. You can work on it! ✅

If you don't want to work on it now, please un-assign yourself so other contributors can take it.

Also, make sure to read our Contribution Guidelines.

ivywallet avatar Feb 15 '24 12:02 ivywallet

@AMHaroun this guide by Google can help if you have any issues: https://developer.android.com/jetpack/compose/performance/stability/fix

ILIYANGERMANOV avatar Feb 15 '24 12:02 ILIYANGERMANOV

Thank you @AMHaroun for fixing some composables! 🙌 I re-opened the issue since there are more. Help is still wanted and much appreciated. I'll keep re-opening until we fix all composables and completely remove the baseline

ILIYANGERMANOV avatar Feb 17 '24 09:02 ILIYANGERMANOV

Bonus points: if we can prioritize the transactions list that's used on the Home screen. It's about replacing BigDecimal -> Double in the viewStates and the ViewModel + replacing LocalDateTime -> String and doing the date formatting in the ViewModel. It's a bigger change but this is the way to fix the scrolling performance

ILIYANGERMANOV avatar Feb 17 '24 09:02 ILIYANGERMANOV

I would like to work on this. Could you assign this to me if nobody is working on it? @ILIYANGERMANOV

devsarahgeo avatar Apr 02 '24 17:04 devsarahgeo

https://github.com/Ivy-Apps/ivy-wallet/blob/main/CONTRIBUTING.md

ILIYANGERMANOV avatar Apr 02 '24 17:04 ILIYANGERMANOV

No longer needed after K2 enables Stong Skipping

ILIYANGERMANOV avatar Jul 09 '24 22:07 ILIYANGERMANOV