Skip to content

Commit

Permalink
Reduce viewport updates in LazyList for improved perf with Zipline
Browse files Browse the repository at this point in the history
  • Loading branch information
veyndan committed Sep 12, 2023
1 parent 1eec9f9 commit f5fe930
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,24 @@ internal fun LazyList(
content: LazyListScope.() -> Unit,
) {
val itemProvider = rememberLazyListItemProvider(content)
var lastVisibleItemIndex by remember { mutableStateOf(0) }
var lastPagedItemIndex by remember { mutableStateOf(0) }
val itemsBefore = remember(state.firstVisibleItemIndex) { (state.firstVisibleItemIndex - OffscreenItemsBufferCount / 2).coerceAtLeast(0) }
val itemsAfter = remember(lastVisibleItemIndex, itemProvider.itemCount) { (itemProvider.itemCount - (lastVisibleItemIndex + OffscreenItemsBufferCount / 2).coerceAtMost(itemProvider.itemCount)).coerceAtLeast(0) }
val itemsAfter = remember(lastPagedItemIndex, itemProvider.itemCount) { (itemProvider.itemCount - (lastPagedItemIndex + OffscreenItemsBufferCount / 2).coerceAtMost(itemProvider.itemCount)).coerceAtLeast(0) }
val scrollItemIndex = remember(state.scrollToItemTriggeredId) { ScrollItemIndex(state.scrollToItemTriggeredId, state.firstVisibleItemIndex) }
var placeholderPoolSize by remember { mutableStateOf(20) }
LazyList(
isVertical,
itemsBefore = itemsBefore,
itemsAfter = itemsAfter,
onViewportChanged = { localFirstVisibleItemIndex, localLastVisibleItemIndex ->
val visibleItemCount = localLastVisibleItemIndex - localFirstVisibleItemIndex
onViewportChanged = { localFirstPagedItemIndex, localLastPagedItemIndex ->
val visibleItemCount = localLastPagedItemIndex - localFirstPagedItemIndex
val proposedPlaceholderPoolSize = visibleItemCount + visibleItemCount / 2
// We only ever want to increase the pool size.
if (placeholderPoolSize < proposedPlaceholderPoolSize) {
placeholderPoolSize = proposedPlaceholderPoolSize
}
state.firstVisibleItemIndex = localFirstVisibleItemIndex
lastVisibleItemIndex = localLastVisibleItemIndex
state.firstVisibleItemIndex = localFirstPagedItemIndex
lastPagedItemIndex = localLastPagedItemIndex
},
width = width,
height = height,
Expand Down Expand Up @@ -97,24 +97,24 @@ internal fun RefreshableLazyList(
content: LazyListScope.() -> Unit,
) {
val itemProvider = rememberLazyListItemProvider(content)
var lastVisibleItemIndex by remember { mutableStateOf(0) }
var lastPagedItemIndex by remember { mutableStateOf(0) }
val itemsBefore = remember(state.firstVisibleItemIndex) { (state.firstVisibleItemIndex - OffscreenItemsBufferCount / 2).coerceAtLeast(0) }
val itemsAfter = remember(lastVisibleItemIndex, itemProvider.itemCount) { (itemProvider.itemCount - (lastVisibleItemIndex + OffscreenItemsBufferCount / 2).coerceAtMost(itemProvider.itemCount)).coerceAtLeast(0) }
val itemsAfter = remember(lastPagedItemIndex, itemProvider.itemCount) { (itemProvider.itemCount - (lastPagedItemIndex + OffscreenItemsBufferCount / 2).coerceAtMost(itemProvider.itemCount)).coerceAtLeast(0) }
val scrollItemIndex = remember(state.scrollToItemTriggeredId) { ScrollItemIndex(state.scrollToItemTriggeredId, state.firstVisibleItemIndex) }
var placeholderPoolSize by remember { mutableStateOf(20) }
RefreshableLazyList(
isVertical,
itemsBefore = itemsBefore,
itemsAfter = itemsAfter,
onViewportChanged = { localFirstVisibleItemIndex, localLastVisibleItemIndex ->
val visibleItemCount = localLastVisibleItemIndex - localFirstVisibleItemIndex
onViewportChanged = { localFirstPagedItemIndex, localLastPagedItemIndex ->
val visibleItemCount = localLastPagedItemIndex - localFirstPagedItemIndex
val proposedPlaceholderPoolSize = visibleItemCount + visibleItemCount / 2
// We only ever want to increase the pool size.
if (placeholderPoolSize < proposedPlaceholderPoolSize) {
placeholderPoolSize = proposedPlaceholderPoolSize
}
state.firstVisibleItemIndex = localFirstVisibleItemIndex
lastVisibleItemIndex = localLastVisibleItemIndex
state.firstVisibleItemIndex = localFirstPagedItemIndex
lastPagedItemIndex = localLastPagedItemIndex
},
refreshing = refreshing,
onRefresh = onRefresh,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ internal class ComposeUiLazyList :
LazyList<@Composable () -> Unit>,
RefreshableLazyList<@Composable () -> Unit> {
private var isVertical by mutableStateOf(false)
private var onViewportChanged: ((firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit)? by mutableStateOf(null)
private var onViewportChanged: ((firstPagedItemIndex: Int, lastPagedItemIndex: Int) -> Unit)? by mutableStateOf(null)
private var itemsBefore by mutableStateOf(0)
private var itemsAfter by mutableStateOf(0)
private var isRefreshing by mutableStateOf(false)
Expand All @@ -78,7 +78,7 @@ internal class ComposeUiLazyList :
this.isVertical = isVertical
}

override fun onViewportChanged(onViewportChanged: (firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit) {
override fun onViewportChanged(onViewportChanged: (firstPagedItemIndex: Int, lastPagedItemIndex: Int) -> Unit) {
this.onViewportChanged = onViewportChanged
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal open class HTMLLazyList(document: Document) : LazyList<HTMLElement> {
value.style.flexDirection = if (isVertical) "column" else "row"
}

override fun onViewportChanged(onViewportChanged: (firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit) {
override fun onViewportChanged(onViewportChanged: (firstPagedItemIndex: Int, lastPagedItemIndex: Int) -> Unit) {
}

override fun itemsBefore(itemsBefore: Int) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import app.cash.redwood.ui.Margin
@Widget(1)
public data class LazyList(
@Property(1) val isVertical: Boolean,
@Property(2) val onViewportChanged: (firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit,
@Property(2) val onViewportChanged: (firstPagedItemIndex: Int, lastPagedItemIndex: Int) -> Unit,
@Property(3) val itemsBefore: Int,
@Property(4) val itemsAfter: Int,
@Property(5) val width: Constraint,
Expand All @@ -41,7 +41,7 @@ public data class LazyList(
@Widget(2)
public data class RefreshableLazyList(
@Property(1) val isVertical: Boolean,
@Property(2) val onViewportChanged: (firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit,
@Property(2) val onViewportChanged: (firstPagedItemIndex: Int, lastPagedItemIndex: Int) -> Unit,
@Property(3) val itemsBefore: Int,
@Property(4) val itemsAfter: Int,
@Property(5) val refreshing: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ internal open class UIViewLazyList(
val visibleIndexPaths = collectionView.indexPathsForVisibleItems()

if (visibleIndexPaths.isNotEmpty()) {
// TODO: Optimize this for less operations
updateViewport(
visibleIndexPaths.minOf { (it as NSIndexPath).item.toInt() },
visibleIndexPaths.maxOf { (it as NSIndexPath).item.toInt() },
Expand Down
8 changes: 8 additions & 0 deletions redwood-lazylayout-widget/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ kotlin {
api projects.redwoodLazylayoutApi
}
}
commonTest {
dependencies {
implementation libs.assertk
implementation libs.kotlin.test
implementation libs.kotlinx.coroutines.test
implementation libs.turbine
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,27 @@ public abstract class WindowedLazyList<W : Any>(
private val listUpdateCallback: ListUpdateCallback,
) : LazyList<W> {

private var firstVisibleItemIndex = 0
private var lastVisibleItemIndex = 0
private var onViewportChanged: ((firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit)? = null
private var firstPagedItemIndex = 0
private var lastPagedItemIndex = 0
private var onViewportChanged: ((firstPagedItemIndex: Int, lastPagedItemIndex: Int) -> Unit)? = null

final override val items: WindowedChildren<W> = WindowedChildren(listUpdateCallback)

final override fun onViewportChanged(onViewportChanged: (Int, Int) -> Unit) {
this.onViewportChanged = onViewportChanged
}

protected fun updateViewport(firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) {
if (firstVisibleItemIndex != this.firstVisibleItemIndex || lastVisibleItemIndex != this.lastVisibleItemIndex) {
this.firstVisibleItemIndex = firstVisibleItemIndex
this.lastVisibleItemIndex = lastVisibleItemIndex
onViewportChanged?.invoke(firstVisibleItemIndex, lastVisibleItemIndex)
public fun updateViewport(firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) {
// Paginate the results so `onViewportChanged` isn't blasted with a bunch of updates. This is
// particularly important when using `LazyList` in Treehouse (compared to plain Redwood), as
// the serialization cost between the host-guest bridge can be expensive.
val viewportItemCount = lastVisibleItemIndex - firstVisibleItemIndex + 1
val firstPagedItemIndex = (((firstVisibleItemIndex / viewportItemCount) * viewportItemCount) - viewportItemCount).coerceAtLeast(0)
val lastPagedItemIndex = ((lastVisibleItemIndex / viewportItemCount) * viewportItemCount) + viewportItemCount * 2
if (firstPagedItemIndex != this.firstPagedItemIndex || lastPagedItemIndex != this.lastPagedItemIndex) {
this.firstPagedItemIndex = firstPagedItemIndex
this.lastPagedItemIndex = lastPagedItemIndex
onViewportChanged?.invoke(firstPagedItemIndex, lastPagedItemIndex)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (C) 2023 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package app.cash.redwood.lazylayout.widget

import app.cash.redwood.Modifier
import app.cash.redwood.layout.api.Constraint
import app.cash.redwood.layout.api.CrossAxisAlignment
import app.cash.redwood.lazylayout.api.ScrollItemIndex
import app.cash.redwood.ui.Margin
import app.cash.redwood.widget.MutableListChildren
import app.cash.redwood.widget.Widget
import app.cash.turbine.test
import assertk.assertThat
import assertk.assertions.isEqualTo
import kotlin.test.Test
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.runTest

class WindowedLazyListTest {
@Test
fun updateViewPort() = runTest {
val windowedLazyList = FakeWindowedLazyList(this, "foo")
windowedLazyList.updateViewport(29, 38)
windowedLazyList.viewportChanges.test {
assertThat(awaitItem()).isEqualTo(10..50)
}
}
}

private class FakeWindowedLazyList<W : Any>(
private val scope: CoroutineScope,
override val value: W,
) : WindowedLazyList<W>(NoOpListUpdateCallback) {
private val _viewportChanges = MutableSharedFlow<IntRange>()
val viewportChanges: SharedFlow<IntRange> = _viewportChanges

override var modifier: Modifier = Modifier
override val placeholder: Widget.Children<W> = MutableListChildren()

init {
onViewportChanged { firstPagedItemIndex, lastPagedItemIndex ->
scope.launch {
_viewportChanges.emit(firstPagedItemIndex..lastPagedItemIndex)
}
}
}

override fun isVertical(isVertical: Boolean) = TODO()
override fun width(width: Constraint) = TODO()
override fun height(height: Constraint) = TODO()
override fun margin(margin: Margin) = TODO()
override fun crossAxisAlignment(crossAxisAlignment: CrossAxisAlignment) = TODO()
override fun scrollItemIndex(scrollItemIndex: ScrollItemIndex) = TODO()
}

private object NoOpListUpdateCallback : ListUpdateCallback {
override fun onInserted(position: Int, count: Int) = Unit
override fun onMoved(fromPosition: Int, toPosition: Int, count: Int) = Unit
override fun onRemoved(position: Int, count: Int) = Unit
}

0 comments on commit f5fe930

Please sign in to comment.