Work out the retained window once a frame, not once a row
The same mistake as the draw-phase culling, moved into another phase. Each row held a `derivedStateOf` over the scroll position to decide whether it stayed built. That reads correctly and costs the same shape: every one of those derived states is invalidated by every scroll frame and has to be re-evaluated to learn whether its answer changed, so the per-frame work grew with the number of loaded rows again -- this time landing in the recomposition pass, which the platform reports as the frame's animation phase. On a Pixel 9 Pro XL at 153 rows that was 11.6ms at the median, against an 8.3ms frame, and it was the largest term left. The window is now one range that every row reads, recomputed once a frame from one observer. And recomputed lazily: every row reads it, so every change to it disturbs all of them, which is affordable once every couple of screens and is not affordable at row boundaries, where a fling would cross one every few frames. The window is eight screens either side and it moves in steps of two, so the margin absorbs the staleness. Worth naming as a pattern, since this is the third time: a per-row answer to a question about the scroll position is O(rows) per frame wherever it is evaluated -- in draw, in a derived state, anywhere. The question has one answer and it belongs in one place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
718fb5320c
commit
2b24362cc4
1 file changed
+47
-12
@@ -14,13 +14,14 @@ import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.rememberScrollState
|
||||
import androidx.compose.foundation.verticalScroll
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.Stable
|
||||
import androidx.compose.runtime.derivedStateOf
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.key
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.runtime.snapshotFlow
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.drawWithContent
|
||||
@@ -257,7 +258,41 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
||||
* made a page of history land as one hundred-millisecond frame -- seventeen screens of markdown
|
||||
* shaped at once, because "not measured yet" described the whole page.
|
||||
*/
|
||||
fun retains(seq: Long): Boolean = rowIndex[seq]?.let { it in retainedRange() } ?: true
|
||||
/**
|
||||
* Which rows are built, as one piece of state every row reads rather than a question every row
|
||||
* asks.
|
||||
*
|
||||
* Each row used to hold a `derivedStateOf` over the scroll position. That reads correctly and
|
||||
* costs the same shape as the draw-phase culling it replaced: every one of those derived states
|
||||
* is invalidated by every scroll frame and has to be re-evaluated to find out whether its
|
||||
* answer changed, so the per-frame work grew with the number of loaded rows again -- this time
|
||||
* landing in the recomposition pass, which the platform reports as the frame's animation phase.
|
||||
* Measured on a Pixel 9 Pro XL at 153 rows: 11.6ms of it at the median, against an 8.3ms frame.
|
||||
*
|
||||
* One shared range costs one evaluation a frame, and rows are only disturbed when it actually
|
||||
* moves.
|
||||
*/
|
||||
var retained: IntRange by mutableStateOf(IntRange.EMPTY)
|
||||
private set
|
||||
|
||||
fun retains(seq: Long): Boolean = rowIndex[seq]?.let { it in retained } ?: true
|
||||
|
||||
/**
|
||||
* Recomputes [retained] if the view has moved far enough to be worth it.
|
||||
*
|
||||
* Deliberately lazy about it. Every row reads the range, so every change to it recomposes all
|
||||
* of them -- which is affordable once every couple of screens and is not affordable at the row
|
||||
* boundaries, where a fling would cross one every few frames. The window is eight screens
|
||||
* either side and this moves it in steps of two, so the margin absorbs the staleness.
|
||||
*/
|
||||
internal fun trackRetained() {
|
||||
val viewportTop = scroll.maxValue - scroll.value
|
||||
val step = (scroll.viewportSize * RETAIN_STEP_SCREENS).coerceAtLeast(1)
|
||||
val moved = viewportTop - rangeAt
|
||||
if (retained.isEmpty() || moved > step || moved < -step || topsVersion != rangeVersion) {
|
||||
retained = retainedRange(viewportTop)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Which rows stay built, as a range of indices rather than a test each row makes for itself.
|
||||
@@ -268,11 +303,9 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
||||
* what happened. Here the nearest row to the viewport is in the range by construction, whatever
|
||||
* the numbers say, so the worst a mistake can do is build too few rows or too many.
|
||||
*/
|
||||
private fun retainedRange(): IntRange {
|
||||
private fun retainedRange(viewportTop: Int): IntRange {
|
||||
refreshTops()
|
||||
if (order.isEmpty()) return IntRange.EMPTY
|
||||
val viewportTop = scroll.maxValue - scroll.value
|
||||
if (viewportTop == rangeAt && topsVersion == rangeVersion) return rangeFrom..rangeTo
|
||||
val margin = (scroll.viewportSize * RETAIN_SCREENS).coerceAtLeast(1)
|
||||
val from = viewportTop - margin
|
||||
val to = viewportTop + scroll.viewportSize + margin
|
||||
@@ -297,15 +330,11 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
||||
first = nearest
|
||||
last = nearest
|
||||
}
|
||||
rangeFrom = first
|
||||
rangeTo = last
|
||||
rangeAt = viewportTop
|
||||
rangeVersion = topsVersion
|
||||
return first..last
|
||||
}
|
||||
|
||||
private var rangeFrom = 0
|
||||
private var rangeTo = -1
|
||||
private var rangeAt = Int.MIN_VALUE
|
||||
private var rangeVersion = -1
|
||||
private var topsVersion = 0
|
||||
@@ -400,6 +429,11 @@ fun TranscriptColumn(
|
||||
)
|
||||
layoutDirection
|
||||
}
|
||||
// One evaluation of the window a frame, for the whole list; see [TranscriptScroll.retained].
|
||||
LaunchedEffect(state) {
|
||||
snapshotFlow { state.scroll.value to state.scroll.maxValue }
|
||||
.collect { state.trackRetained() }
|
||||
}
|
||||
Column(
|
||||
modifier
|
||||
.verticalScroll(state.scroll, reverseScrolling = true)
|
||||
@@ -462,9 +496,7 @@ private fun RetainedRow(
|
||||
) {
|
||||
// Derived, so a row hears about the scroll only when its own answer changes rather than on
|
||||
// every frame; see [TranscriptScroll.retains].
|
||||
val retained by
|
||||
remember(state, item.startSeq) { derivedStateOf { state.retains(item.startSeq) } }
|
||||
if (!retained) {
|
||||
if (!state.retains(item.startSeq)) {
|
||||
DebugStats.count("row stood down")
|
||||
Spacer(
|
||||
Modifier.fillMaxWidth()
|
||||
@@ -501,5 +533,8 @@ private fun RetainedRow(
|
||||
/** How far either side of the screen a row stays built; see [TranscriptScroll.retains]. */
|
||||
private const val RETAIN_SCREENS = 8
|
||||
|
||||
/** How far the view moves before the retained range is worked out again; see `trackRetained`. */
|
||||
private const val RETAIN_STEP_SCREENS = 2
|
||||
|
||||
/** What a row is assumed to be worth before any of them have been measured. */
|
||||
private const val ROW_GUESS = 800
|
||||
Reference in new issue
Block a user