https://kotlinlang.org logo
Join Slack
Powered by
Hi! I am working on a stock trading application an...
# compose
n
Hi! I am working on a stock trading application and we are currently migrating our real-time updated lists to compose. I have a 
LazyColumn
witch displays the stocks and each stock is real-time updated through web-socket events. I am testing the LazyColumn performance (using a release build) and it’s dropping frames. The performance is not as good as it was with the RecyclerView (with DiffUtil). I also used the Recomposition highlighter from the Google play team and verified that my LazyColumn is constantly recomposing. I will include the code in the thread. Any ideas on how to improve the performance?
👍 3
This is my `LazyColumn`:
Copy code
@Composable
private fun WatchlistColumn(
    listState: LazyListState,
    followedSecuritySecurities: FollowedSecurityListItems?,
...
) {
    val stocks = followedSecuritySecurities?.eqty.orEmpty()
    val etfs = followedSecuritySecurities?.etf.orEmpty()
    val cryptos = followedSecuritySecurities?.crypto.orEmpty()
    LazyColumn(
        state = listState,
        modifier = Modifier
            .testTag("watchlist_column"),
    ) {
        if (stocks.isNotEmpty()) {
            // Add header
            item(key = SecurityType.EQTY) {
                WatchlistSectionHeader(
                    headerTitle = R.string.watchlist_header_stocks,
                    displayMode = displayMode,
                    onDisplayModeChanged = onDisplayModeChanged,
                    showDisplayMode = true
                )
            }

            items(stocks, key = { listItem -> listItem.id }) {
                WatchlistItem(
                    item = it,
                    displayMode = displayMode,
                    numberFormats = numberFormats,
                    onSecurityClick = onSecurityClick,
                    onDisplayModeChanged = onDisplayModeChanged,
                    onItemUnfollowed = onItemUnfollowed
                )
            }
        }
...
The 
followedSecuritySecurities
comes from the 
ViewModel
Copy code
val followedSecuritySecurities by viewModel.followedSecuritySecurities.collectAsState(initial = null)
In the viewModel:
Copy code
private val _followedSecurities = MutableStateFlow<FollowedSecurities?>(null)
val followedSecuritySecurities = _followedSecurities.map {
    it?.toFollowedListItems()
}
And this is the function in the viewModel that updates the list when the price of an item changes:
Copy code
private fun handleFollowSecurityUpdate(securityQuote: SecurityQuote) {
    val followedSecurities = _followedSecurities.value ?: return
    val allFollowedSecurities = followedSecurities.all().toMutableList()

    val stockIndex = allFollowedSecurities.indexOfFirst { it.security.subscribingId == securityQuote.id }
    if (stockIndex == -1) {
        return
    }
    val newFollowedStock = allFollowedSecurities[stockIndex].copyWithQuote(securityQuote)
    allFollowedSecurities[stockIndex] = newFollowedStock

    _followedSecurities.value = FollowedSecurities(allFollowedSecurities)
}
How can I make sure that when one item is updated through web-socket event, only that item recomposes, not the whole list? Basically replicating the DiffUtil behavior.
d
Just sub here, but I think you’re already doing what’s neccessary, i.e. giving each item an unique ID via 
key = { listItem -> listItem.id }
, maybe it’s just not the right one / stable one?
s
When the entire list updates, if the LazyList is provided unique ids it should be able to do this efficiently. Are the ids provided unique?
Oh you beat me to it 😅
😄 1
n
The id’s provided are the id’s from the stocks themselves so yes unique. Each stock has a unique id.
s
I wonder if this has to do with the lines 
val stocks = followedSecuritySecurities?.eqty.orEmpty()
being defined in composition and basically happen on every recomposition. Might be stretch but what if you wrap each on of them like
Copy code
val stocks = remember(followedSecuritySecurities.eqty) {
  followedSecuritySecurities?.eqty.orEmpty()
}
n
Thanks for the suggestion! I will give it a try.
s
I was also thinking potentially you’re recreating the flow in composition, but 
collectAsState
seems to used 
produceState
under the hood so shouldn’t be an issue
n
I am unwrapping the state at the top level composable:
Copy code
@Composable
fun WatchlistScreen(
    viewModel: WatchlistViewModel,
) {
    val followedListItems by viewModel.followedListItems.collectAsState(initial = null)
    WatchlistScreen(
        listState = rememberLazyListState(),
        isLoading = isLoading,
        followedSecuritySecurities = followedListItems,
...
}
I wonder if this has to do with the lines val stocks = followedSecuritySecurities?.eqty.orEmpty() being defined in composition and basically happen on every recomposition.
I don’t see a difference when doing this unfortunately. I do think however that it is better to wrap it in a remember.
s
Maybe a good question is, what is it that is recomposing that you’re not expecting to? Do all individual items recompose when only the data of one of them changes? If it’s the entire LazyColumn doing a recomposition when 1 item changes, maybe that’s expected? And it’s performant since it depends on not having all the children recompose? Just an idea, not sure how LazyList really works
t
If you are instantiating some classes in WatchlistItem, it is better to reduce them. Recently, I worked on improving the performance of LazyColumn on a Nexus 7 2013. I am caching and reusing instances of Modifier, it is effective to improve the LazyColumn performance. Sample implementation: (Sorry in japanese) https://twitter.com/tomoya0x00/status/1503699937959477253
p
Annotate your security model as Immutable
💯 1
r
Verified my lazycolumn is constantly recomposing
IMO this should not be happening
e
I've seen this with 
LazyColumn
every time I use it. That is, unless each individual item observes it's own state, all visible items recompose when even one item in the list is updated. I just assumed that that's how it works.
p
It doesn't have to work like this, read my comment on immutable
m
The itens must be stable. The list must be stable. And the clicklisteners must not capture values. If this happens, it will completly skip the non changed items. Try to set empty listeners. Does the problem go away?
n
I did a couple of things which seem to help: Unwrap my state at the lowest level possible. So my 
LazyColumn
accepts a Flow as a parameter:
Copy code
@Composable
private fun WatchlistColumn(
    listState: LazyListState,
    followedSecuritySecurities: Flow<FollowedSecurityListItems>,
and unwraps the state itself, instead of unwrapping the state at the top level composable. The other thing is marking my data classes as 
@Immutable
like @Paul Woitaschek suggested. It seems to help, the 
recomposeHighlighter
is not red anymore. However I placed some logs and it seems that a web socket update which updates a price of one single item, still triggers recomposition for all visible items. Before, when I was unwrapping the list of items at the top-level composable, it triggered recomposition for the 
LazyColumn
too. I used this for logging:
Copy code
class Ref(var value: Int)

// Note the inline function below which ensures that this function is essentially
// copied at the call site to ensure that its logging only recompositions from the
// original call site.
@Composable
inline fun LogCompositions(tag: String, msg: String) {
    val ref = remember { Ref(0) }
    SideEffect { ref.value++ }
    Log.d(tag, "Compositions: $msg ${ref.value}")
}
This suggests to me that there is room for more optimisation.
Is this expected? That an update to one item’s price, triggers recomposition for all visible items? We have implemented a workaround for this. We isolated the price from the 
Security
model and pass the price through 
CompositionLocal
to the TextView that displays the price. This is very performant. It was tested with a real-time updated LazyColumn with 1000's of entries. But it is something I would hope we don’t have to do forever. I am seeking for a simpler solution which is still performant.
p
Please post the data model and the composable that is recomposing
m
you dont need to do any of that, read the flow directly before the lazyColumn. The problem is elsewhere. Can you paste your model here please, it would help
n
Copy code
@Immutable
data class FollowedSecurityListItem(
    val followedSecurity: FollowedSecurity,
    override val userAcceptedAgreements: Boolean
) : Identifiable, HideableSecurity {
    override val id: String
        get() = followedSecurity.id

    override val security: Security
        get() = followedSecurity.security
}
And the list item composable:
Copy code
@Composable
private fun WatchlistItem(
    item: FollowedSecurityListItem,
    displayMode: SecurityDisplayMode?,
    numberFormats: NumberFormats,
    onSecurityClick: (FollowedSecurityListItem) -> Unit,
    onDisplayModeChanged: () -> Unit,
    onItemUnfollowed: (FollowedSecurityListItem) -> Unit
) {
    val rememberedItem = remember(item) { item }

    LogCompositions("Message", "WatchlistItem ${item.id}")
    var itemDeleted by remember {
        mutableStateOf(false)
    }
    val dismissState = rememberDismissState(
        confirmStateChange = {
            if (it == DismissValue.DismissedToStart) {
                itemDeleted = true
            }
            true
        }
    )

    val color by animateColorAsState(
        targetValue = if (itemDeleted) {
            MaterialTheme.colors.background
        } else {
            MaterialTheme.colors.backgroundGray
        },
        finishedListener = {
            onItemUnfollowed(item)
        }
    )

    SwipeToDismiss(
        state = dismissState,
        directions = setOf(DismissDirection.EndToStart),
        dismissThresholds = { FractionalThreshold(0.15f) },
        modifier = Modifier
            .testTag("swipe_to_dismiss")
            .recomposeHighlighter(),
        background = {
            WatchlistItemBackground(
                modifier = Modifier.background(color = color)
            )
        },
        dismissContent = {
            WatchlistItemContent(rememberedItem, displayMode, numberFormats, onSecurityClick, onDisplayModeChanged)
        }
    )
}
p
Did by any chance you enable animateItemPlacement?
m
indeed. I have noticed as well that can cause unexpected recompositions
p
Are NumberFormats and SecurityDisplayMode Immutable as well?
m
remember the item is useless there i guess. It wont do anything, but it is almost sure not the cause
n
No they are not. 
SecurityDisplayMode
is an enum:
Copy code
enum class SecurityDisplayMode {
    VALUE,
    PERFORMANCE_TODAY,
    PERFORMANCE_ALL_TIME;
}
NumberFormats is a singleton with some helper function so that we can format the prices. I was thinking to provide it as a CompositionLocal instead of passing it down the tree. I don’t think that causes the performance problem though.
m
enum is stable
singleton probably is not!!
n
remember the item is useless there i guess. It wont do anything, but it is almost sure not the cause
This is a recent addition. Before I didn’t use 
remember
there. I thought it might help.
p
Yep you can write 
object Counter { var count : Int }
n
Is it a good idea to provide the Singleton (NumberFormats) as a CompositionLocal instead of passing it down the tree?
p
No, that should not be abused for sneaking in method parameters
m
won’t do any good. What you should do is make sure the data that is used in the composables is already formatted and mapped
n
I didn’t enable the 
animateItemPlacement
modifier. There is an animation when the price changes though. But we had the same performance issue with other similar long lists that didn’t have any animations.
m
if you can, extract the format stuff to viewmodel. If you cannot, try to make the formatter an interface and mark is as stable (if it is the case)
p
If it’s not stable, it’s expected to cause recompositions because if it changes, all items that depend on it should change too
👍 1
m
ideally, the data that you use to configure the composables should be already formatted/mapped
run the gradle command and it will tell you right away - it needs to be skippable and restartable, then you are fine
the formatter probably is unstable - that should be the apparent reason
n
ideally, the data that you use to configure the composables should be already formatted/mapped
You are right. I will work on that. However I have the feeling that this doesn’t cause the recompositions. This Singleton doesn’t contain any state. Just helper functions.
p
Then slap @Immutable on for a first try
👍 1
n
Is there a gradle command that tells you if a class is stable?
@Paul Woitaschek what will happen if you annotate a class as 
@Immutable
while it isn’t?
m
then it should work while being broken 😛
p
It will not recompose and update properly
m
not necessarily. It would depend i guess
p
I could never get that one working
m
In my experience you should check in this order: All params of the function are stable. The model class is immutable. There is a recompose scope there. Any listeners should not access something that is not stable
🧠 1
Normally, i solve recompose problems like above
The last one is optional i guess. You cannot always have that. For instance, imagine you want to scroll something on a button click. Then you would need to use a scope, which isn’t stable, so it causes some recomposition issues, which is completely normal in that case i guess
n
I miss the DiffUtil
p
I really don’t 😄
👍 4
👍🏼 1
m
did you try @Paul Woitaschek approach, did it work ?
n
I am working on formatting the prices in the viewModel and see if that helps.
m
ya, the idea is to get rid of the singleton 😛 It should help
n
Then slap @Immutable on for a first try
Wow @Paul Woitaschek that seems to work!
m
you should have params to format the composable, not the data!
ya, the singleton is not stable as we suspected
n
Thank you both! And everyone else who helped me troubleshooting this.
m
@Paul Woitaschek is not a solution. Just a test to check if the singleton is the problem 😮
i don’t think you should do that, only if you are sure its indeed stable. On a side note, if the singleton is indeed stable, it should me marked as Stable and not Immutable. I think it is more appropriate
p
I did not understand from the docs when you should use one over the other
m
Me neither. I don’t think it matters, but the name should mean something. Immutable i think is for models. In this case, does it make sense to mark the singleton as immutable? Probably not. For me is makes more sense to make it Stable
which is the problem here. The excess recomposition in this case happens because his singleton is not stable (which has nothing to do with it being a singleton. A simple class there wouldn’t also be stable in the same scenario
n
From the docs: 
One notable type that is stable but _is_ mutable is Compose's MutableState type. If a value is held in a MutableState, the state object overall is considered to be stable as Compose will be notified of any changes to the .value property of State.
I think 
@Immutable
is a stronger version of 
@Stable
.
Do you all think those annotations are here to stay? I feel it is a bit of a guesswork at least for me. It’s an extra thing you have to think about, adding the 
@Immutable
annotation to all your models.
m
you dont need to add to all the model. Just the ones that the compiler will define as unstable but they should be stable
example: 
data class Item(val date: LocalDate)
this is unstable. But it should be stable because LocalDate is immutable. But the compiler won’t guess that. So you should mark it
most of the cases you won’t need to annotate
More clearly: 
data class Item(val date: LocalDate)
@Composable
fun ItemComposable(item: Item){
otherComposable(item.date)
}
@Composable
fun otherComposable(date: LocalDate){}
As it is, it will recompose ItemComposable regardless os date being the same, but it will skip otherComposable. With stable, it will skip completely.
p
m
exactly. That is the purpose of those annotations
probably, File equals can differentiate the same File. But as the class is not Stable, it will recompose either way
p
Which makes sense as the file itself can change its contents
But it would be nice if you could for example slap an annotation to a composable function parameter instead of that ceremony
m
ya, for sure!
n
@myanmarking Thanks for the explanation.
23 Views
Open in Slack
PreviousNext
Powered by