LazyColumn memory leak (SnapshotMutableStateImpl$StateStateRecord) [239603305]

Fixed

Bug

Status Update

No update yet.

Description

br...@monzo.com

created issue #1

Jul 20, 2022 02:08PM

Jetpack Compose version: I've tested on 1.3.0-alpha01 and 1.2.0-beta03

Jetpack Compose component(s) used: LazyColumn

Android Studio Build: 221.3427.89.2211.8689873

Kotlin version: 1.7.10

Steps to Reproduce or Code Sample to Reproduce:

Create a LazyColumn
Scroll
Memory 📈

Context:

We have a rather large (10k+ items) LazyColumn where scrolling (eventually) leads to the application grinding to a halt, constantly trying to free memory. It can't, we just keep getting lots of these:

Background concurrent copying GC freed 51964(1414KB) AllocSpace objects, 6(120KB) LOS objects, 0% free, 190MB/192MB, paused 101us total 1.483s

I took at heap dump at this point and basically all of the heap is used up by instances of SnapshotMutableStateImpl$StateStateRecord.

Replace LazyColumn with a RecyclerView (with each item being a ComposeView) and the memory leak is gone. App performance is night and day better.

I've been trying for hours to reproduce this in a trivial sample project and haven't had any luck yet. Raising this anyway just in case you have any ideas about what might be causing it. Obviously this makes LazyColumn completely unusable for us, so we'll stick to RecyclerView for now!

dump.png

153 KB

View

Download

shortestpath.png

402 KB

View

Download

Comments

ma...@google.com <ma...@google.com> Jul 20, 2022 02:52PM

Reassigned to an...@google.com.

as...@google.com <as...@google.com> #2Jul 20, 2022 04:54PM

Project: platform/frameworks/support
Branch: androidx-main

commit a330c0d3bcdd41326f37968a60e6084ad4a2e32c
Author: Chet Haase <chet@google.com>
Date: Wed Jul 05 07:26:46 2023

Convert APIs using PointF to use Float instead

PointF is a convenient mechanism for passing around x.y values
representing 2D points. But there are downsides, including:
- Converting to PointF: You may not have the data in PointF form
to begin with, so using an API which takes PointF requires converting
the data to that form (including allocating a PointF object every time)
- Mutability: Point structures can be mutated internally, causing
unpredictability in what that mutation means. Should the library
react to those changes? Ignore them? Do defensive copies (requiring
even more allocations)? Using primitive types like Float make the
behavior more obvious (by making the data inherently immutable).
- Allocations: Whenever we use object types, there are necessarily
allocations on the Java heap for them. This puts pressure on the GC
at both allocation and collection time. Given the amount of points
being passed around (especially at morph creation time, when curves
are being split and created), this causes a lot of PointF objects to
be allocated (even temporarily). Using Float avoids that problem.

Also fixed bug with unclosed paths causing discontinuity at the
start/end point.

Bug: 276466399
Bug: 290254314
Test: integration and unit tests pass
Relnote: PointF parameters changed to Float pairs
Change-Id: Id4705d27c7be31b26ade8186b99fffe2e2f8450e

M graphics/graphics-shapes/api/current.txt
M graphics/graphics-shapes/api/restricted_current.txt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicShapeTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonMeasureTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/RoundedPolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/ShapesTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/TestUtils.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Cubic.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/CubicShape.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FeatureMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FloatMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Morph.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/PolygonMeasure.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/RoundedPolygon.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Shapes.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Utils.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/DebugDraw.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/ShapeEditor.kt
M graphics/integration-tests/testapp/src/main/java/androidx/graphics/shapes/test/MaterialShapes.kt

https://android-review.googlesource.com/2649119

br...@monzo.com <br...@monzo.com> #3Jul 20, 2022 05:04PM

PointF was the main (possibly only) mutability issue, marking this as fixed

dump.hprof

291 MB

Download

as...@google.com <as...@google.com> #4Jul 20, 2022 06:55PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.graphics:graphics-shapes:1.0.0-alpha04

Message last modified on Jul 20, 2022 06:58PM

br...@monzo.com <br...@monzo.com> #5Jul 20, 2022 09:19PM

I managed to repro it after a bunch of debugging. The bad news is that it's caused when using Coil within a LazyColumn, so that library is doing something that's causing you to leak snapshots... somehow.

Repro:

Add implementation io.coil-kt:coil-compose:2.1.0 to build.gradle
Add the following composable:

@Composable fun Leak() {
    val lazyListState = rememberLazyListState()
    val items by remember { mutableStateOf(List(10000) { it }) }

    LazyColumn(state = lazyListState) {
        items(items) {
            Box(Modifier.fillMaxWidth()) {
                AsyncImage(
                    model = "https://i.picsum.photos/id/104/200/200.jpg?hmac=3XxEVXVjwoI45-6sum_iMwNZ52GT-SJacVWr4fh4hqI",
                    contentDescription = null
                )
            }
        }
    }
}

Scroll
Memory 📈

ap...@google.com <ap...@google.com> #6Jul 21, 2022 08:10PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit e82b3518094e5451dd4697f31a412c10075b28c5
Author: Andrei Shikov <ashikov@google.com>
Date: Wed Jul 20 23:43:58 2022

Dispose nested snapshots created from transparent snapshots

Adds a flag to transparent snapshots to "manage" wrapped snapshots which forces dispose of the wrapped snapshot whenever transparent one is disposed.
This flag is only enabled for the snapshots taken inside transparent snapshots, fixing memory leaks in certain conditions.

Fixes a minor bug where transparent snapshot wasn't receiving reads from nested snapshots as well.

Fixes: 239603305
Test: SnapshotTests#testNestedWithinTransparentSnapshotDisposedCorrectly

Change-Id: I62eddd279c8cf44b032d852d646c9ba21ad08a39

M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/Snapshot.kt
M compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/snapshots/SnapshotTests.kt

https://android-review.googlesource.com/2160780

br...@monzo.com <br...@monzo.com> #7Jul 22, 2022 10:33AM

Thanks for the quick turnaround!

na...@google.com <na...@google.com> #8Oct 24, 2022 01:37PM

The following release(s) address this bug:

androidx.compose.runtime:runtime:1.3.0

Issue 239603305

Description

Issue summary

Comments

ma...@google.com <ma...@google.com> Jul 20, 2022 02:52PM

as...@google.com <as...@google.com> #2Jul 20, 2022 04:54PM

br...@monzo.com <br...@monzo.com> #3Jul 20, 2022 05:04PM

as...@google.com <as...@google.com> #4Jul 20, 2022 06:55PM

br...@monzo.com <br...@monzo.com> #5Jul 20, 2022 09:19PM

ap...@google.com <ap...@google.com> #6Jul 21, 2022 08:10PM

br...@monzo.com <br...@monzo.com> #7Jul 22, 2022 10:33AM

na...@google.com <na...@google.com> #8Oct 24, 2022 01:37PM

Add comment

Issue metadata