From f5fe9300cdc148ae5dd86ef347ac301e9a54aa37 Mon Sep 17 00:00:00 2001 From: Veyndan Stuart Date: Mon, 11 Sep 2023 16:55:03 +0200 Subject: [PATCH] Reduce viewport updates in `LazyList` for improved perf with Zipline --- .../redwood/lazylayout/compose/LazyList.kt | 24 +++--- .../lazylayout/composeui/ComposeUiLazyList.kt | 4 +- .../redwood/lazylayout/dom/HTMLLazyList.kt | 2 +- .../app/cash/redwood/lazylayout/widgets.kt | 4 +- .../lazylayout/uiview/UIViewLazyList.kt | 1 - redwood-lazylayout-widget/build.gradle | 8 ++ .../lazylayout/widget/WindowedLazyList.kt | 22 ++++-- .../lazylayout/widget/WindowedLazyListTest.kt | 76 +++++++++++++++++++ 8 files changed, 115 insertions(+), 26 deletions(-) create mode 100644 redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyListTest.kt diff --git a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt index 308fc87ac0..74a5bd3663 100644 --- a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt +++ b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt @@ -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, @@ -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, diff --git a/redwood-lazylayout-composeui/src/commonMain/kotlin/app/cash/redwood/lazylayout/composeui/ComposeUiLazyList.kt b/redwood-lazylayout-composeui/src/commonMain/kotlin/app/cash/redwood/lazylayout/composeui/ComposeUiLazyList.kt index ac02f777de..00e0a9e3bc 100644 --- a/redwood-lazylayout-composeui/src/commonMain/kotlin/app/cash/redwood/lazylayout/composeui/ComposeUiLazyList.kt +++ b/redwood-lazylayout-composeui/src/commonMain/kotlin/app/cash/redwood/lazylayout/composeui/ComposeUiLazyList.kt @@ -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) @@ -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 } diff --git a/redwood-lazylayout-dom/src/commonMain/kotlin/app/cash/redwood/lazylayout/dom/HTMLLazyList.kt b/redwood-lazylayout-dom/src/commonMain/kotlin/app/cash/redwood/lazylayout/dom/HTMLLazyList.kt index fa2652c4b7..42869efa15 100644 --- a/redwood-lazylayout-dom/src/commonMain/kotlin/app/cash/redwood/lazylayout/dom/HTMLLazyList.kt +++ b/redwood-lazylayout-dom/src/commonMain/kotlin/app/cash/redwood/lazylayout/dom/HTMLLazyList.kt @@ -62,7 +62,7 @@ internal open class HTMLLazyList(document: Document) : LazyList { 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) { diff --git a/redwood-lazylayout-schema/src/main/kotlin/app/cash/redwood/lazylayout/widgets.kt b/redwood-lazylayout-schema/src/main/kotlin/app/cash/redwood/lazylayout/widgets.kt index fb4d6e3c46..7036168c24 100644 --- a/redwood-lazylayout-schema/src/main/kotlin/app/cash/redwood/lazylayout/widgets.kt +++ b/redwood-lazylayout-schema/src/main/kotlin/app/cash/redwood/lazylayout/widgets.kt @@ -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, @@ -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, diff --git a/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt b/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt index fb7a5c0294..592ed14f99 100644 --- a/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt +++ b/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt @@ -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() }, diff --git a/redwood-lazylayout-widget/build.gradle b/redwood-lazylayout-widget/build.gradle index 37aba9fb8c..a2c9d051dc 100644 --- a/redwood-lazylayout-widget/build.gradle +++ b/redwood-lazylayout-widget/build.gradle @@ -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 + } + } } } diff --git a/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyList.kt b/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyList.kt index 163e4b5f90..ee9d283136 100644 --- a/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyList.kt +++ b/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyList.kt @@ -19,9 +19,9 @@ public abstract class WindowedLazyList( private val listUpdateCallback: ListUpdateCallback, ) : LazyList { - 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 = WindowedChildren(listUpdateCallback) @@ -29,11 +29,17 @@ public abstract class WindowedLazyList( 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) } } diff --git a/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyListTest.kt b/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyListTest.kt new file mode 100644 index 0000000000..41c049ea0d --- /dev/null +++ b/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/WindowedLazyListTest.kt @@ -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( + private val scope: CoroutineScope, + override val value: W, +) : WindowedLazyList(NoOpListUpdateCallback) { + private val _viewportChanges = MutableSharedFlow() + val viewportChanges: SharedFlow = _viewportChanges + + override var modifier: Modifier = Modifier + override val placeholder: Widget.Children = 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 +}