Take every whole-message parse off the composing thread, and count the rest
Bryan's report after the last change read no worse in the numbers -- medians unchanged, worst stall halved -- but scrolling felt bumpier while messages loaded, and instrumenting the split paths found the feeling's likely source: work the counters never saw. The live reply's block split (BlockedMarkdown) recomputed on the composing thread at every delta -- a whole-message parse of the growing text, 815 of them and 2.9 seconds inside one streamed reply, a few milliseconds per delta on the thread that draws, plus a cache entry per partial text that nothing reads again. It now works the way parsedMarkdown already did one file over: first split inline so the row has its height, every later one off-thread, drawing one split behind, cached nowhere. Emulator, same streamed fixture: anim p90 7.3 -> 3.4ms, p99 9.3 -> 4.7ms. The settle moment had the same shape: nothing warms live deltas, so the just-finished reply's split parse ran inside the flatten, uncounted, in a frame. The flatten now splits a reply only when ParsedReplies.splitReady says warm() has made its parses; the session screen warms the one cold row off-thread and re-flattens (warmedTick), so the whole-to-blocks swap always composes against ready parses. Readiness is an explicit mark set by warm() rather than a peek into the blocks cache, because a message with memory notes is warmed as its parts -- inferred readiness left it unsplittable forever and re-warmed on every fold. Also: user slices shrink to ~1000 chars (about one viewport, so a slice composing mid-fling costs a few milliseconds, not sixty), and the split and flatten paths are all timed -- "units flattened", "markdown split into blocks", "message cut into parts", "user message cut into slices", "blocks split while streaming" -- so the next "it feels bumpier" report names its cause instead of hiding it in anim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
333d2de92b
commit
356dee65f1
5 files changed
+113
-8
No files matched your search
@@ -291,12 +291,40 @@ class ParsedReplies {
|
|||||||
|
|
||||||
private val chunks = ConcurrentHashMap<String, List<String>>()
|
private val chunks = ConcurrentHashMap<String, List<String>>()
|
||||||
|
|
||||||
fun blocksOf(text: String): List<String> = blocks.computeIfAbsent(text) { markdownBlocks(it) }
|
private val ready = ConcurrentHashMap.newKeySet<String>()
|
||||||
|
|
||||||
|
fun blocksOf(text: String): List<String> =
|
||||||
|
blocks.computeIfAbsent(text) {
|
||||||
|
DebugStats.timed("markdown split into blocks") { markdownBlocks(it) }
|
||||||
|
}
|
||||||
|
|
||||||
/** How a long user message divides into slices; cached for the same reason as [blocksOf]. */
|
/** How a long user message divides into slices; cached for the same reason as [blocksOf]. */
|
||||||
fun chunksOf(text: String): List<String> = chunks.computeIfAbsent(text) { userChunks(it) }
|
fun chunksOf(text: String): List<String> =
|
||||||
|
chunks.computeIfAbsent(text) {
|
||||||
|
DebugStats.timed("user message cut into slices") { userChunks(it) }
|
||||||
|
}
|
||||||
|
|
||||||
fun partsOf(text: String): List<MessagePart> = parts.computeIfAbsent(text) { messageParts(it) }
|
/**
|
||||||
|
* Whether [warm] has made everything drawing [text] as blocks will look up.
|
||||||
|
*
|
||||||
|
* What the flatten asks before drawing a reply that way. Splitting costs a parse of the whole
|
||||||
|
* message and the flatten runs on the composing thread -- so a reply not marked yet stays
|
||||||
|
* whole, drawing the parse it already has, until the screen has warmed it and re-flattens. An
|
||||||
|
* explicit mark rather than a peek into [blocksOf]'s cache, because a message with memory notes
|
||||||
|
* is warmed as its *parts*: nothing ever splits its full text, and inferring readiness from the
|
||||||
|
* cache left exactly that message unsplittable forever, re-warmed on every fold.
|
||||||
|
*/
|
||||||
|
fun splitReady(text: String): Boolean = text in ready
|
||||||
|
|
||||||
|
/** The other half of [splitReady]; [warm] calls it once a message's parses exist. */
|
||||||
|
fun markSplitReady(text: String) {
|
||||||
|
ready.add(text)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun partsOf(text: String): List<MessagePart> =
|
||||||
|
parts.computeIfAbsent(text) {
|
||||||
|
DebugStats.timed("message cut into parts") { messageParts(it) }
|
||||||
|
}
|
||||||
|
|
||||||
/** The parse of [text] -- the one made ahead, or one made now. */
|
/** The parse of [text] -- the one made ahead, or one made now. */
|
||||||
fun of(text: String): State =
|
fun of(text: String): State =
|
||||||
@@ -326,5 +354,6 @@ class ParsedReplies {
|
|||||||
blocks.clear()
|
blocks.clear()
|
||||||
parts.clear()
|
parts.clear()
|
||||||
chunks.clear()
|
chunks.clear()
|
||||||
|
ready.clear()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -4,6 +4,8 @@ import androidx.compose.foundation.layout.Arrangement
|
|||||||
import androidx.compose.foundation.layout.Column
|
import androidx.compose.foundation.layout.Column
|
||||||
import androidx.compose.foundation.layout.fillMaxWidth
|
import androidx.compose.foundation.layout.fillMaxWidth
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
|
import androidx.compose.runtime.LaunchedEffect
|
||||||
|
import androidx.compose.runtime.mutableStateOf
|
||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.draw.drawWithContent
|
import androidx.compose.ui.draw.drawWithContent
|
||||||
@@ -11,6 +13,8 @@ import androidx.compose.ui.graphics.graphicsLayer
|
|||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
import com.mikepenz.markdown.model.State
|
import com.mikepenz.markdown.model.State
|
||||||
import com.mikepenz.markdown.model.parseMarkdown
|
import com.mikepenz.markdown.model.parseMarkdown
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.withContext
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A message's top-level markdown blocks, cut where the parser says the blocks are.
|
* A message's top-level markdown blocks, cut where the parser says the blocks are.
|
||||||
@@ -70,7 +74,23 @@ fun BlockedMarkdown(
|
|||||||
modifier: Modifier = Modifier,
|
modifier: Modifier = Modifier,
|
||||||
live: Boolean = false,
|
live: Boolean = false,
|
||||||
) {
|
) {
|
||||||
val blocks = remember(text) { replies.blocksOf(text) }
|
// The first split is inline for the reason [parsedMarkdown]'s first parse is: the row must
|
||||||
|
// have its real height in its first frame. Every split after that is a delta landing, and it
|
||||||
|
// runs off the composing thread with the message drawing the split it already has until the
|
||||||
|
// new one arrives -- when this recomputed wherever composition ran, one streamed reply cost
|
||||||
|
// 815 whole-message parses and 2.9 seconds of them, a few milliseconds per delta, on the
|
||||||
|
// thread that draws. Not through [replies]: a reply mid-stream is a different text per
|
||||||
|
// delta, and each would leave a cache entry nothing reads again.
|
||||||
|
val split = remember { mutableStateOf(text to markdownBlocks(text)) }
|
||||||
|
LaunchedEffect(text) {
|
||||||
|
if (split.value.first == text) return@LaunchedEffect
|
||||||
|
split.value =
|
||||||
|
text to
|
||||||
|
withContext(Dispatchers.Default) {
|
||||||
|
DebugStats.timed("blocks split while streaming") { markdownBlocks(text) }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
val blocks = split.value.second
|
||||||
if (blocks.size == 1) {
|
if (blocks.size == 1) {
|
||||||
MarkdownText(blocks.first(), replies, modifier)
|
MarkdownText(blocks.first(), replies, modifier)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -343,7 +343,23 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: ()
|
|||||||
// What is actually drawn: the transcript with runs of adjacent tool
|
// What is actually drawn: the transcript with runs of adjacent tool
|
||||||
// calls folded into one row each, flattened into the list's units.
|
// calls folded into one row each, flattened into the list's units.
|
||||||
val rows = remember(items) { groupToolRuns(items) }
|
val rows = remember(items) { groupToolRuns(items) }
|
||||||
val units = remember(rows, expandedNotes) { transcriptUnits(rows, replies, expandedNotes) }
|
// Bumped when a cold reply's parses become ready, so the flatten runs again and can split
|
||||||
|
// it; see [unwarmedReplies].
|
||||||
|
var warmedTick by remember { mutableIntStateOf(0) }
|
||||||
|
val units =
|
||||||
|
remember(rows, expandedNotes, warmedTick) { transcriptUnits(rows, replies, expandedNotes) }
|
||||||
|
// The reply that just finished streaming is the one row whose parses nobody has made: pages
|
||||||
|
// warm before their fold lands, but nothing warms live deltas. Off the composing thread,
|
||||||
|
// then the tick re-flattens -- so settling never costs a whole-message parse in a frame.
|
||||||
|
// Re-launched per fold and almost always finds nothing; during streaming the last row is
|
||||||
|
// unsettled and not wanted.
|
||||||
|
LaunchedEffect(rows) {
|
||||||
|
val cold = unwarmedReplies(rows, replies)
|
||||||
|
if (cold.isNotEmpty()) {
|
||||||
|
warm(replies, cold)
|
||||||
|
warmedTick++
|
||||||
|
}
|
||||||
|
}
|
||||||
// The same list, readable from effects launched before this composition: an effect's closure
|
// The same list, readable from effects launched before this composition: an effect's closure
|
||||||
// keeps the values of the composition that launched it, and both the anchor saver and the
|
// keeps the values of the composition that launched it, and both the anchor saver and the
|
||||||
// restore need the units as they are *now*.
|
// restore need the units as they are *now*.
|
||||||
|
|||||||
@@ -576,5 +576,8 @@ suspend fun warm(replies: ParsedReplies, rows: List<TranscriptItem>) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (texts.isNotEmpty()) replies.warm(texts)
|
if (texts.isNotEmpty()) replies.warm(texts)
|
||||||
|
// After the parses exist, not before: [ParsedReplies.splitReady] is the flatten's
|
||||||
|
// licence to draw these as blocks on the composing thread.
|
||||||
|
rows.forEach { if (it is TranscriptItem.AssistantMsg) replies.markSplitReady(it.text) }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -164,6 +164,7 @@ fun transcriptUnits(
|
|||||||
replies: ParsedReplies,
|
replies: ParsedReplies,
|
||||||
openNotes: Set<Long>,
|
openNotes: Set<Long>,
|
||||||
): List<TranscriptUnit> {
|
): List<TranscriptUnit> {
|
||||||
|
val started = System.nanoTime()
|
||||||
val units = ArrayList<TranscriptUnit>(rows.size)
|
val units = ArrayList<TranscriptUnit>(rows.size)
|
||||||
rows.forEachIndexed { index, row ->
|
rows.forEachIndexed { index, row ->
|
||||||
val rowGap = if (index == 0) 0.dp else TRANSCRIPT_SPACING
|
val rowGap = if (index == 0) 0.dp else TRANSCRIPT_SPACING
|
||||||
@@ -204,7 +205,9 @@ fun transcriptUnits(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
} else if (
|
} else if (
|
||||||
item is TranscriptItem.AssistantMsg && (item.settled || index != rows.lastIndex)
|
item is TranscriptItem.AssistantMsg &&
|
||||||
|
splitWanted(item, index, rows.lastIndex) &&
|
||||||
|
replies.splitReady(item.text)
|
||||||
) {
|
) {
|
||||||
var ordinal = 0
|
var ordinal = 0
|
||||||
fun gap() = if (ordinal == 0) rowGap else BLOCK_SPACING
|
fun gap() = if (ordinal == 0) rowGap else BLOCK_SPACING
|
||||||
@@ -227,9 +230,37 @@ fun transcriptUnits(
|
|||||||
}
|
}
|
||||||
units.reverse()
|
units.reverse()
|
||||||
reportDuplicateKeys(units)
|
reportDuplicateKeys(units)
|
||||||
|
// Timed because this runs per fold on the composing thread: "loading messages feels bumpy"
|
||||||
|
// is this number growing, and it was invisible until it was written down.
|
||||||
|
DebugStats.record("units flattened", System.nanoTime() - started)
|
||||||
return units
|
return units
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether this reply should be drawn as blocks: settled, or anywhere but the newest row.
|
||||||
|
*
|
||||||
|
* Wanting is not being ready -- the flatten also asks [ParsedReplies.splitReady], and the two
|
||||||
|
* questions are separate because they are answered by different things: this one by the fold, the
|
||||||
|
* other by whether [warm] has run for the text. [unwarmedReplies] is the gap between them.
|
||||||
|
*/
|
||||||
|
private fun splitWanted(item: TranscriptItem.AssistantMsg, index: Int, lastIndex: Int) =
|
||||||
|
item.settled || index != lastIndex
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The replies among [rows] that should draw as blocks but whose parses are not made yet.
|
||||||
|
*
|
||||||
|
* Normally empty: every page's rows are warmed before the fold lands. The one row that can be cold
|
||||||
|
* is the reply that just finished streaming -- nothing warms live deltas, so at the moment its turn
|
||||||
|
* ends its split would cost a whole-message parse on the composing thread. The session screen warms
|
||||||
|
* what this returns off-thread and re-flattens, so the whole-to-blocks swap always composes against
|
||||||
|
* ready parses.
|
||||||
|
*/
|
||||||
|
fun unwarmedReplies(rows: List<TranscriptRow>, replies: ParsedReplies): List<TranscriptItem> =
|
||||||
|
rows.mapIndexedNotNull { index, row ->
|
||||||
|
val item = (row as? TranscriptRow.Single)?.item as? TranscriptItem.AssistantMsg
|
||||||
|
item?.takeIf { splitWanted(it, index, rows.lastIndex) && !replies.splitReady(it.text) }
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Above this many characters, a user message is drawn in slices rather than as one bubble.
|
* Above this many characters, a user message is drawn in slices rather than as one bubble.
|
||||||
*
|
*
|
||||||
@@ -240,8 +271,14 @@ fun transcriptUnits(
|
|||||||
*/
|
*/
|
||||||
const val USER_SPLIT_CHARS = 4000
|
const val USER_SPLIT_CHARS = 4000
|
||||||
|
|
||||||
/** Roughly how much text one slice holds -- bounded, like a markdown block, is the whole point. */
|
/**
|
||||||
private const val USER_CHUNK_CHARS = 2500
|
* Roughly how much text one slice holds -- bounded, like a markdown block, is the whole point.
|
||||||
|
*
|
||||||
|
* About one viewport of wrapped text: a slice is composed whole in the frame it scrolls into, so
|
||||||
|
* its size is a frame-budget decision, and one screenful keeps that to a few milliseconds on the
|
||||||
|
* phone. Smaller buys nothing -- the seams are free -- but the units multiply.
|
||||||
|
*/
|
||||||
|
private const val USER_CHUNK_CHARS = 1000
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A long user message cut at line starts into slices of roughly [USER_CHUNK_CHARS].
|
* A long user message cut at line starts into slices of roughly [USER_CHUNK_CHARS].
|
||||||
|
|||||||
Reference in new issue
Block a user