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

Memory leak in SnapshotStateObserver? #1969

Closed
bassstorm opened this issue Mar 20, 2022 · 17 comments · Fixed by JetBrains/compose-multiplatform-core#325
Closed

Memory leak in SnapshotStateObserver? #1969

bassstorm opened this issue Mar 20, 2022 · 17 comments · Fixed by JetBrains/compose-multiplatform-core#325
Assignees
Labels
p:critical Critical priority performance
Milestone

Comments

@bassstorm
Copy link

Hi JB team,

I'm trying to get up with a desktop app which updates small part of UI quite frequently (e.g. a timer). As result of app's normal operation, memory consumption grows relatively quickly. Profiling points to SnapshotStateObserver, which seem doesn't release some state. However, I don't see a way how to control that or set a limit.

After just half an hour of work, memory trend is clear:
image

On below screenshot you may see Live Bytes and Objects, the max numbers there constantly grow:
image

Sample app source:

import androidx.compose.material.Text
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.window.Window
import androidx.compose.ui.window.application
import kotlinx.datetime.Clock
import kotlin.concurrent.fixedRateTimer

fun main() = application {
    remember {
        fixedRateTimer(period = 50) { Holder.refreshClock() }
    }

    Window(onCloseRequest = ::exitApplication) {
        Text(Holder.clock.value)
    }
}

object Holder {
    val clock = mutableStateOf(Clock.System.now().toString())

    fun refreshClock() {
        clock.value = Clock.System.now().toString()
    }
}

OS: Manjaro Linux
Compose: 1.1.0
OpenJDK: 11.0.15

Looking forward hearing back from you. Thanks!

@bassstorm
Copy link
Author

A bit of extra details to add.

In androidx.compose.runtime.snapshots.SnapshotStateObserver#observeReads, onValueChangedForScope does seem always to be a new instance for my mutable state change event. So it leads to a situation when applyMaps list grows infinitely.

@XilinJia
Copy link

This appears similar to a problem I have been investigating, though my problem occurs on Android with Jetpack Compose. Some details here

@mnonnenmacher
Copy link

There is a related bug report for Jetpack Compose: https://issuetracker.google.com/issues/223222717

@bassstorm
Copy link
Author

@mnonnenmacher thanks for sharing, that's very good to know! Hopefully will be fixed soon in upstream.

@pema4
Copy link

pema4 commented Apr 16, 2022

I believe the problem is in the UpdateEffect composable

On every invocation of the performUpdate snapshotObserver is subscribed with new lambda object:

fun performUpdate() {
    snapshotObserver.observeReads(
        Unit,
        onValueChangedForScope = { tasks.trySend(::performUpdate) }
    ) {
        currentUpdate()
    }
}

I replaced this declaration with the following and the memory leak disappeared:

lateinit var onValueChangedForScope: (Unit) -> Unit
fun performUpdate() {
    snapshotObserver.observeReads(
        Unit,
        onValueChangedForScope = onValueChangedForScope,
    ) {
        currentUpdate()
    }
}
onValueChangedForScope = { tasks.trySend(::performUpdate) }

@Thomas-Vos
Copy link
Contributor

I think I may be experiencing the same issue. My app slows down after time, so I started the Intellij debug view. After running the app for some time I paused it and took a look at the memory usage. I can see that there are lots of allocations of androidx.compose.runtime.collection.IdentityArraySet. See screenshot:
Screenshot 2022-04-22 at 14 31 57

Is this expected? I am not sure. When looking at the stack trace of one of the objects I see the following:

Screenshot 2022-04-22 at 14 32 39

@wznshuai
Copy link

wznshuai commented May 20, 2022

I ran into the same problem。
image

@wangjunqiangongithub
Copy link

image

I have a similar problem, the snapshotmutablestateimpl has a memory leak problem, I'm a novice, and I don't know how to continue the analysis

@bassstorm
Copy link
Author

There's no real movement in related Google ticket: https://issuetracker.google.com/issues/223222717
Please vote or comment there if you can

@Ic-ks
Copy link

Ic-ks commented Oct 14, 2022

I posted the following already at the google issue tracker:

The memory leak is still present in Compose 1.2.0 with Kotlin 1.7.20.
I profiled the code below (https://github.com/Ic-ks/compose-memory-leak) for 3 minutes with the Java Flight Recorder (the related .jfr file can be found here: https://issuetracker.google.com/action/issues/223222717/attachments/39334890?download=true). The overall allocated memory was more than 7GB. Heap usage steadily increased to 80 MB. It seems that SnapshotStateObserver does not clean its references properly.

val randomValueFlow = flow { while (true) emit(Math.random() * 100) }
    .onEach { delay(10) }
    .map { it.toInt() }
    .flowOn(Dispatchers.Default)

fun main() = application {
    Window(onCloseRequest = ::exitApplication) {
        val randomValue by randomValueFlow.collectAsState(0L)
        Text(text = randomValue.toString(), fontSize = 20.sp)
    }
}

jfr

PS: .jfr files can be viewed with JDK Mission Control (https://github.com/openjdk/jmc)

@yaroslavkulinich
Copy link

I have the same issue. Constant high-frequency UI updates cause memory leak.
If the app is designed to be used for hours - there's no way to make it work without a restart.

@yaroslavkulinich
Copy link

yaroslavkulinich commented Nov 4, 2022

@pema4 Does your solution with gradle task patch work with Compose 1.2.0 version (maybe you already migrated)?
Did you face any issues after patching UpdateEffect?
I'm thinking to apply your approach, but it's nice to have some information about possible side-effects (if exist).
Thanks

@yaroslavkulinich
Copy link

yaroslavkulinich commented Nov 7, 2022

@igordmn please, take a closer look at this problem.
The repository with the reproducer published here https://github.com/Ic-ks/compose-memory-leak shows the problem.
It would be perfect if the changes described by @pema4 were included directly in the jb-compose build.
The problem for long-running apps with high-frequency UI updates is HUGE.

@igordmn
Copy link
Collaborator

igordmn commented Nov 7, 2022

Thanks for pointing at this issue, we'll take a look.

@igordmn igordmn self-assigned this Nov 7, 2022
@igordmn igordmn added this to the 1.3 milestone Nov 7, 2022
@igordmn igordmn added the p:critical Critical priority label Nov 10, 2022
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Nov 10, 2022
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
@igordmn
Copy link
Collaborator

igordmn commented Nov 10, 2022

Thanks to all for investigating this issue! And especially @pema4 for the fix.

It is fixed, and the fix will be in 1.3.0 (or in 1.2.2, we will decide soon).

@bassstorm
Copy link
Author

Terrific! Can't wait for release!

igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Nov 10, 2022
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
eymar pushed a commit to JetBrains/compose-multiplatform-core that referenced this issue Nov 16, 2022
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
igordmn added a commit to JetBrains/compose-multiplatform-core that referenced this issue Dec 5, 2022
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
eymar pushed a commit to JetBrains/compose-multiplatform-core that referenced this issue Jan 13, 2023
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
MatkovIvan pushed a commit to MatkovIvan/compose-multiplatform that referenced this issue May 10, 2023
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains#2455
Fixes JetBrains#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
@okushnikov
Copy link
Collaborator

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

@JetBrains JetBrains locked and limited conversation to collaborators Dec 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p:critical Critical priority performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.