Actually run the amortization, and stop paying for history twice
Two things committed in9052e5fand94b1509were never wired up. The row count was tracked and nothing observed it, so the fix for the gap when sending was inert; `standUpSome` and `growing` were written and nothing called them, so the window still jumped in one step. Both are connected now, which is what the last two commit messages already claimed. The third thing is what the counters were pointing at all along. The frame's draw phase sat at a flat 7.7ms -- p99 only 1.8x p50, so not shaping, which is spiky -- while the transcript's own recording ran 43 times in 3638 frames and accounted for a fortieth of it. A stood-down row drew nothing but was still a layout node, and the framework's per-frame bookkeeping after a scroll walks live nodes rather than visible ones. So the cost grew with the conversation instead of with the screen, which is exactly the step the reader felt at each page loaded. The rows outside the window are now two spacers rather than one per row, which they can be because the window is a range: what is not in it is one run before and one run after. Live nodes are bounded by the window now. The gaps between collapsed rows are added into the spacer, because `spacedBy` puts a gap between children and a run drawn as one child is short by all of them -- which would not read as a spacing bug, it would move everything under the reader. Measure and place are timed separately now. The draw phase carries Compose's measurement, so "draw is high while nothing is recorded" could not say which half, and the two have different fixes. Diagnosis from the ai-app-2-6d session: foundation already places scrolled content with `placeRelativeWithLayer`, so the scroll is a layer transform today and re-placing children was never the cost -- the node-count walk is, and collapsing the runs is the experiment that settles it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
94b1509733
commit
f497d1f3ad
1 file changed
+88
-23
@@ -22,6 +22,7 @@ import androidx.compose.runtime.mutableStateOf
|
|||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
import androidx.compose.runtime.setValue
|
import androidx.compose.runtime.setValue
|
||||||
import androidx.compose.runtime.snapshotFlow
|
import androidx.compose.runtime.snapshotFlow
|
||||||
|
import androidx.compose.runtime.withFrameNanos
|
||||||
import androidx.compose.ui.Alignment
|
import androidx.compose.ui.Alignment
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.draw.drawWithContent
|
import androidx.compose.ui.draw.drawWithContent
|
||||||
@@ -281,7 +282,35 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
|||||||
var retained: IntRange by mutableStateOf(IntRange.EMPTY)
|
var retained: IntRange by mutableStateOf(IntRange.EMPTY)
|
||||||
private set
|
private set
|
||||||
|
|
||||||
fun retains(seq: Long): Boolean = rowIndex[seq]?.let { it in retained } ?: true
|
/**
|
||||||
|
* How tall rows [run] stand in for as a single spacer, the gaps between them included.
|
||||||
|
*
|
||||||
|
* The gaps have to be counted here because collapsing a run removes children from the column,
|
||||||
|
* and `Arrangement.spacedBy` puts a gap between children rather than after each one -- so a run
|
||||||
|
* of `k` rows drawn as one spacer is `k - 1` gaps shorter than the rows were unless it says so.
|
||||||
|
* Getting that wrong does not look like a spacing bug; it shortens the content, which moves
|
||||||
|
* everything the reader is looking at.
|
||||||
|
*/
|
||||||
|
fun runHeight(run: IntRange): Int {
|
||||||
|
if (run.isEmpty() || order.isEmpty()) return 0
|
||||||
|
var total = 0
|
||||||
|
for (index in run) total += assumed(order[index])
|
||||||
|
return total + (run.last - run.first) * spacing
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* [retained] clamped to a list of [count] rows, or empty if nothing should be built yet.
|
||||||
|
*
|
||||||
|
* The range is worked out against the order from the last layout, and a page of history landing
|
||||||
|
* changes that order in the same composition that reads this -- so an index from before it can
|
||||||
|
* be past the end. Clamping here rather than at the read sites keeps one answer to it.
|
||||||
|
*/
|
||||||
|
fun window(count: Int): IntRange {
|
||||||
|
if (count == 0 || retained.isEmpty()) return IntRange.EMPTY
|
||||||
|
val first = retained.first.coerceIn(0, count - 1)
|
||||||
|
val last = retained.last.coerceIn(first, count - 1)
|
||||||
|
return first..last
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Recomputes [retained] if the view has moved far enough to be worth it.
|
* Recomputes [retained] if the view has moved far enough to be worth it.
|
||||||
@@ -407,9 +436,6 @@ class TranscriptScroll(internal val scroll: ScrollState) {
|
|||||||
private var rangeVersion = -1
|
private var rangeVersion = -1
|
||||||
private var topsVersion = 0
|
private var topsVersion = 0
|
||||||
|
|
||||||
/** The height a row was last measured at, for the spacer that stands in for it. */
|
|
||||||
fun heightOf(seq: Long): Int = assumed(seq)
|
|
||||||
|
|
||||||
/** The height of the visible area, 0 until the first measurement. */
|
/** The height of the visible area, 0 until the first measurement. */
|
||||||
val viewport: Int
|
val viewport: Int
|
||||||
get() = scroll.viewportSize
|
get() = scroll.viewportSize
|
||||||
@@ -497,10 +523,25 @@ fun TranscriptColumn(
|
|||||||
layoutDirection
|
layoutDirection
|
||||||
}
|
}
|
||||||
// One evaluation of the window a frame, for the whole list; see [TranscriptScroll.retained].
|
// One evaluation of the window a frame, for the whole list; see [TranscriptScroll.retained].
|
||||||
|
// The row count is in here because a row arriving is the one case that does not move the view:
|
||||||
|
// without it a sent message falls outside the window and stands in as a spacer, which is a
|
||||||
|
// screen of blank between the last message and the box it was typed in.
|
||||||
LaunchedEffect(state) {
|
LaunchedEffect(state) {
|
||||||
snapshotFlow { state.scroll.value to state.scroll.maxValue }
|
snapshotFlow { Triple(state.scroll.value, state.scroll.maxValue, state.rowCount) }
|
||||||
.collect { state.trackRetained() }
|
.collect { state.trackRetained() }
|
||||||
}
|
}
|
||||||
|
// Walks the window towards its target a few rows a frame; see [TranscriptScroll.standUpSome].
|
||||||
|
// Driven from a frame callback rather than a plain loop so the rows stand up between frames
|
||||||
|
// instead of all inside one, which is the entire point of doing it gradually.
|
||||||
|
LaunchedEffect(state) {
|
||||||
|
snapshotFlow { state.growing }
|
||||||
|
.collect {
|
||||||
|
while (state.growing) {
|
||||||
|
withFrameNanos {}
|
||||||
|
state.standUpSome()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Column(
|
Column(
|
||||||
modifier
|
modifier
|
||||||
.verticalScroll(state.scroll, reverseScrolling = true)
|
.verticalScroll(state.scroll, reverseScrolling = true)
|
||||||
@@ -514,10 +555,22 @@ fun TranscriptColumn(
|
|||||||
// the whole transcript was being subcomposed again for each of those frames, which is
|
// the whole transcript was being subcomposed again for each of those frames, which is
|
||||||
// what made bringing the keyboard up cost more than anything else on the screen. Read
|
// what made bringing the keyboard up cost more than anything else on the screen. Read
|
||||||
// here it is a relayout, and the rows keep the measurements they already have.
|
// here it is a relayout, and the rows keep the measurements they already have.
|
||||||
|
//
|
||||||
|
// Timed in two halves because the frame's draw phase is where Compose's measurement
|
||||||
|
// lands, and "draw is high while nothing is being recorded" does not say which half.
|
||||||
|
// Measuring the transcript and placing it are different costs with different fixes:
|
||||||
|
// one is shaping text that has changed, the other is O(rows) whether or not anything
|
||||||
|
// has.
|
||||||
.layout { measurable, constraints ->
|
.layout { measurable, constraints ->
|
||||||
|
val started = System.nanoTime()
|
||||||
val placeable =
|
val placeable =
|
||||||
measurable.measure(constraints.copy(minHeight = state.scroll.viewportSize))
|
measurable.measure(constraints.copy(minHeight = state.scroll.viewportSize))
|
||||||
layout(placeable.width, placeable.height) { placeable.place(0, 0) }
|
DebugStats.record("measure: the whole transcript", System.nanoTime() - started)
|
||||||
|
layout(placeable.width, placeable.height) {
|
||||||
|
val placing = System.nanoTime()
|
||||||
|
placeable.place(0, 0)
|
||||||
|
DebugStats.record("place: the whole transcript", System.nanoTime() - placing)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
.fillMaxWidth()
|
.fillMaxWidth()
|
||||||
// Once for the whole list, not once per row: this is where a saved position is put
|
// Once for the whole list, not once per row: this is where a saved position is put
|
||||||
@@ -545,11 +598,25 @@ fun TranscriptColumn(
|
|||||||
},
|
},
|
||||||
verticalArrangement = Arrangement.spacedBy(spacing, Alignment.Bottom),
|
verticalArrangement = Arrangement.spacedBy(spacing, Alignment.Bottom),
|
||||||
) {
|
) {
|
||||||
rows.forEach { item ->
|
// Everything outside the window is two spacers rather than one per row, and that is what
|
||||||
|
// stops the cost of a frame growing with the conversation. A stood-down row was still a
|
||||||
|
// layout node, and the framework's own per-frame bookkeeping after a scroll -- the position
|
||||||
|
// dispatch, the cached screen rect each node keeps -- walks live nodes rather than visible
|
||||||
|
// ones. So the transcript got slower with every page loaded even though the extra rows drew
|
||||||
|
// nothing at all: on a Pixel 9 Pro XL, a flat 7.7ms in the frame's draw phase while our own
|
||||||
|
// recording accounted for a fortieth of it. Two spacers, because [retained] is a range: the
|
||||||
|
// rows that are not in it are always one run before it and one run after.
|
||||||
|
val window = state.window(rows.size)
|
||||||
|
val ahead = if (window.isEmpty()) rows.indices else 0..<window.first
|
||||||
|
val behind = if (window.isEmpty()) IntRange.EMPTY else (window.last + 1)..<rows.size
|
||||||
|
key("above") { StoodDownRun(state, ahead) }
|
||||||
|
for (index in window) {
|
||||||
|
val item = rows[index]
|
||||||
// Keyed so that a row keeps its composition -- and so the state inside it, an open
|
// Keyed so that a row keeps its composition -- and so the state inside it, an open
|
||||||
// tool call, stays with the row rather than with the position.
|
// tool call, stays with the row rather than with the position.
|
||||||
key(item.key) { RetainedRow(state, item, row) }
|
key(item.key) { RetainedRow(state, item, row) }
|
||||||
}
|
}
|
||||||
|
key("below") { StoodDownRun(state, behind) }
|
||||||
below()
|
below()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -560,30 +627,28 @@ val TRANSCRIPT_SPACING: Dp = 8.dp
|
|||||||
val TRANSCRIPT_PADDING: PaddingValues = PaddingValues(16.dp)
|
val TRANSCRIPT_PADDING: PaddingValues = PaddingValues(16.dp)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* One row, kept built while it is near the screen and stood in for by its own height when it is
|
* One row, built because it is near the screen.
|
||||||
* not.
|
|
||||||
*
|
*
|
||||||
* A composable of its own rather than a block inside the list, and that is what makes the window
|
* Whether a row is built is decided by the list rather than by the row, which is what lets the rows
|
||||||
* affordable: whether a row is retained is read here, so Compose can invalidate this row alone when
|
* that are not built stop being nodes; see the window in [TranscriptColumn]. The cost of that is
|
||||||
* the answer changes. Read from the list's own body instead, every row would recompose every time
|
* that the list's body recomposes when the window moves, which is why the window is deliberately
|
||||||
* any row crossed the edge of the window.
|
* lazy about moving -- see [TranscriptScroll.trackRetained].
|
||||||
*/
|
*/
|
||||||
|
@Composable
|
||||||
|
private fun StoodDownRun(state: TranscriptScroll, run: IntRange) {
|
||||||
|
if (run.isEmpty()) return
|
||||||
|
DebugStats.count("rows stood down as one spacer")
|
||||||
|
Spacer(
|
||||||
|
Modifier.fillMaxWidth().height(with(LocalDensity.current) { state.runHeight(run).toDp() })
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
private fun RetainedRow(
|
private fun RetainedRow(
|
||||||
state: TranscriptScroll,
|
state: TranscriptScroll,
|
||||||
item: TranscriptRow,
|
item: TranscriptRow,
|
||||||
row: @Composable (TranscriptRow) -> Unit,
|
row: @Composable (TranscriptRow) -> Unit,
|
||||||
) {
|
) {
|
||||||
// Derived, so a row hears about the scroll only when its own answer changes rather than on
|
|
||||||
// every frame; see [TranscriptScroll.retains].
|
|
||||||
if (!state.retains(item.startSeq)) {
|
|
||||||
DebugStats.count("row stood down")
|
|
||||||
Spacer(
|
|
||||||
Modifier.fillMaxWidth()
|
|
||||||
.height(with(LocalDensity.current) { state.heightOf(item.startSeq).toDp() })
|
|
||||||
)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
Column(
|
Column(
|
||||||
Modifier.fillMaxWidth()
|
Modifier.fillMaxWidth()
|
||||||
// A layer of its own, which is the piece of a lazy list this had not rebuilt.
|
// A layer of its own, which is the piece of a lazy list this had not rebuilt.
|
||||||
|
|||||||
Reference in new issue
Block a user