Status Update
Comments
si...@google.com <si...@google.com> #2
May I ask why P3? This is blocking for me to go to prod, I'd really like some inputs about what is going on so I can either fix or build a repro to have this fixed.
si...@google.com <si...@google.com>
si...@google.com <si...@google.com> #3
For the ideas on what to take a look, how do you use LazyListState.layoutInfo in your app? Will it continue growing if you comment out such usages?
mo...@google.com <mo...@google.com> #4
It happens even on screens that does not use the layoutInfo it just require many up / down flings.
LazyColumn(state = rememberLazyListState())
The only thing all screen have in common in that they have multiple item {} item{} block :(
Just in case the app is in open beta at
My main question before trying to do complex repro, is how much will it be relevant with all the recent changes in alpha 8 about Lazy stuff? And side question when
si...@google.com <si...@google.com> #5
I can't guarantee as I don't understand the issue yet, but I assume this issue is still not fixed, changes in alpha 8 were about different things.
si...@google.com <si...@google.com> #6
Ok so the good news is that the other issue is really fixed :) But this one not at all.
It seems 100% of the LazyInfo versions are kept in memory see new attachement.
I've sent you the full prof by mail as you probably can more easily find the root of the reference in the states to have some clues about what I need to do to reproduce.
mo...@google.com <mo...@google.com> #7
Ok so I speak too soon. There's an insane performance issue with 8506895 a simple lazycolumn with a few lazyrow now lags insanely.
Got lots of
Davey! duration=842ms; Flags=0, FrameTimelineVsyncId=30733, IntendedVsync=721976611111, Vsync=722518277756, InputEventId=0, HandleInputStart=722523309190, AnimationStart=722523309963, PerformTraversalsStart=722808544297, DrawStart=722808615383, FrameDeadline=722001544444, FrameInterval=722523205186, FrameStartTime=8333333, SyncQueued=722811912136, SyncStart=722812117702, IssueDrawCommandsStart=722812801825, SwapBuffers=722816362046, FrameCompleted=722819089423, DequeueBufferDuration=23560, QueueBufferDuration=2647257, GpuCompleted=722818621812, SwapBuffersCompleted=722819089423, DisplayPresentTime=0,
in logcat that I don't remember having seen before.
I guess that'll make a lot of repro to make :(
mo...@google.com <mo...@google.com> #8
According to profiler most is spent in compose:lazylist:prefetch:measure anything changed about that ?
Sorry for all the questions, but since huge time zone difference I try to gather the most info to be efficient tomorrow on both repro, as poor indie dev this is 100% unpaid time and I hate repro without any idea what to look after as it can take many hours.
se...@google.com <se...@google.com> #9
There was some internal refactoring in the prefetching logic recently, but I though it shouldn't change the logic at all. Also I tried to run a demo with simple LazyRows inside LazyColumn and didn't notice it being slower than before I guess. So it would be nice if you can share your code which is lagging.
se...@google.com <se...@google.com> #10
Hum did not thought I always see you answer late, I'm at the point where I would give you access to the code but it's a 100k lloc + tons of deps only available on a private gitlab server this would be as a pain than rebuilding a full repro.
I guess this will make my day tomorrow :(
About the snapshot version and the lag we are talking about 1 - 1.5 second complete lock of UI during scroll and loading the next item, item that is a text and a lazyrow of 15 item not even large.
ma...@google.com <ma...@google.com> #11
Ok my wife is not happy but found the culprit in case it's enough and I can avoid a repro.
val contentPadding = PaddingValues(horizontal = 16.dp, vertical = 8.dp)
LazyRow(
state = lazyListState,
contentPadding = contentPadding,
flingBehavior = rememberSnapperFlingBehavior(lazyListState, snapOffsetForItem = SnapOffsets.Start, endContentPadding = contentPadding.calculateEndPadding(LayoutDirection.Ltr)),
horizontalArrangement = Arrangement.spacedBy(12.dp)
) {
The lazyrows use Chris Banes Snapper 0.2.0
If it's not enough leave a message else I'll focus on the mem leak, removing snapping as temporary workaround is not a major issue.
ra...@google.com <ra...@google.com>
ma...@google.com <ma...@google.com> #12
LazyColumn {
items(1000) { x ->
val contentPadding = PaddingValues(horizontal = 16.dp, vertical = 8.dp)
val lazyListState = rememberLazyListState()
LazyRow(
state = lazyListState,
contentPadding = contentPadding,
flingBehavior = rememberSnapperFlingBehavior(
lazyListState,
snapOffsetForItem = SnapOffsets.Start,
endContentPadding = contentPadding.calculateEndPadding(
LayoutDirection.Ltr
)
),
horizontalArrangement = Arrangement.spacedBy(12.dp)
) {
items(100) { y ->
Text("Item x=$x y=$y", Modifier.height(300.dp))
}
}
}
}
Tried this code, and didn't notice any difference if I comment out the flingBehavior. not sure why it affects you
ap...@google.com <ap...@google.com> #13
Ok so I'll try to do both.
Seeing all the issues do you accept a large repro that I'll keep as a base matching most of my code and not directly trying to do small repros ?
ap...@google.com <ap...@google.com> #14
So I'm far from having a proper repro but:
- Wrapping the LazyList in a Box with nestedScroll have high performance impact and triggers the below issue way faster. Even an empty one like
.nestedScroll(remember<NestedScrollConnection> {
object : NestedScrollConnection {
override fun onPostScroll(consumed: Offset, available: Offset, source: NestedScrollSource): Offset {
return Offset.Zero
}
}
})
- The real underlying issue seems to be that LazyColums makes a tons of copy of the whole backed data, even in the non leak scenario if the backed data is large it relatively quickly generate memory fragmentation and prevent the further allocations. See attached screenshot of when it happens, the GC starts to panic and the app quickly crash after that.
For the snapper / perf regression this seems to be a Android Studio issue when it happens clear cache, restart, rebuild fix it.
For the nestedScroll I'm using it to move a top floating bar with and applying .offset { IntOffset(x = 0, y = topButtonBarState.offsetState.value.roundToInt()) },
@Stable
data class TopButtonBarState(val offsetState: State<Float>, val nestedScrollConnection: NestedScrollConnection, private val topBarScrollDistancePx: Float) {
fun onManualScrollDirection(direction: Int) {
nestedScrollConnection.onPostScroll(Offset(0f, topBarScrollDistancePx * -direction), Offset(0f, 0f), NestedScrollSource.Fling)
}
}
@Composable
fun rememberTopButtonBarState(): TopButtonBarState {
val statusBarPadding = WindowInsets.statusBars.asPaddingValues()
val topBarScrollDistance = 56.dp + statusBarPadding.calculateTopPadding()
val topBarScrollDistancePx = with(LocalDensity.current) { topBarScrollDistance.roundToPx().toFloat() }
val topBarOffsetPx = rememberSaveable { mutableStateOf(0f) }
val nestedScrollConnectionOffset = remember<NestedScrollConnection> {
object : NestedScrollConnection {
override fun onPostScroll(consumed: Offset, available: Offset, source: NestedScrollSource): Offset {
val delta = consumed.y
val newOffset = topBarOffsetPx.value + delta
topBarOffsetPx.value = newOffset.coerceIn(-topBarScrollDistancePx, 0f)
return Offset.Zero
}
}
}
return remember {
TopButtonBarState(topBarOffsetPx, nestedScrollConnectionOffset, topBarScrollDistancePx)
}
}
Is there another way to achieve that from the lazyliststate without the nested scroll?
Edit: found
ap...@google.com <ap...@google.com> #15
So got some kind of repro based on a previous one I made for you.
Just start the app and it will crash after some time.
The scroll/refresh simulation is made via
var reload by remember { mutableStateOf(0) }
LaunchedEffect(key1 = Unit) {
while (true) {
reload = Random.nextInt()
delay(100)
}
}
val items = remember(lazyPagingItems.size, stepSize, reload) {
derivedStateOf {
List(lazyPagingItems.size) { it }.windowed(stepSize, stepSize, true)
}
}
ni...@google.com <ni...@google.com> #16
And last comment to confirm the underlying issue with the full data copy.
Switching from:
items(lazyPagingItems) { item ->
to
items(lazyPagingItems.itemCount) { index ->
val item = lazyPagingItems[index]
vastly reduce the memory constraints and the issue on the main app.
ra...@google.com <ra...@google.com> #17
Can confirm your sample project is crashing for me, thanks. I will investigate it
ap...@google.com <ap...@google.com> #18
Nice.
Tell me if you need more data from the real app, the repro is a bit hackish but leads to same result so I suppose it's still valid enough about the root cause.
si...@google.com <si...@google.com>
so...@google.com <so...@google.com> #19
Chuck, I need your help with investigating this. The smallest repro
var reload by remember { mutableStateOf(0) }
LaunchedEffect(Unit) {
while (true) {
reload++
delay(100)
}
}
val items = remember(reload) {
derivedStateOf {
List(1000000) { it }
}
}
Text("List of size ${items.value.size}")
After a few seconds the app will crash with out of memory exception. Seems like there is some kind of a memory leak in the derivedstate implementation.
ap...@google.com <ap...@google.com> #20
For the record the main app crash when scrolling the lazy list without any refresh of any derivedState. So if no derived inside Lazy it's maybe 2 different issues and I repro the wrong one.
ap...@google.com <ap...@google.com> #21
We use DerivedState as an implementation detail in lazy lists as well, so most likely it is the same issue. Otherwise we first try to fix DerivedState and then test with your app again
da...@gmail.com <da...@gmail.com> #22
The issue is that there is an assumption that between the start of a function and the end that the Snapshot.current.id
does not change. However, reading a derivedStateOf
value will change the id of a snapshot so the derived state of value can know when to correctly recalculate the value if the values to calculate it change in the same snapshot.
This causes a memory leak because the RecomposeScope
tries to only forget about object that it is no longer observing if it the changes are actually applied for that scope. It does this by recording a lambda in the changes
list of composition. It further checks if the change that is being applied is from the last composition, if not, the next composition will clean the scope so it can ignore the notification. The recompose scope uses the Snapshot.id
of the composition snapshot as a token to identify which composition it is in. This is not valid to do as the snapshot id
can change during the composition. Because it changed the RecomposeScope
thinks it is receiving an out-of-date notification to forget its unused objects and ignores the notification.
The fix for this is uncertain yet. We can either copy the snapshot id at the beginning of composition and use that instead of snapshot.id
or always assume that start()
and end()
for the recompose scope are paired and trust that when end()
of the RecomposeScope is called it is for the same composition for which start()
was called. The check might also not be needed as the case it is checking probably never occurs.
Description
What the function does is:
- calls requestRectangle on View.
Therefore it is something that we can bubble up from current node to parents and then the View system until the provided area is visible on screen.
Not sure how compose scrollers would react to this and if it is possible.
Related tickets