Warm a fence's highlighting like a parse, and page the restore by rows
Two things the measurements for the previous commit turned up. Highlighting a fence cost 174ms for a two-hundred-line Kotlin block, and the lazy list charged it again every time that block scrolled back into composition -- six times in one bench run, with the scroll's draw phase at 1.29ms per frame. So it is warmed and cached where parses already are: `highlight` is a plain function taking no colour from the theme, `warm` fills `ParsedReplies.highlighted` from `fences(parse)` off the drawing thread, and `fenceContent` extracts the code here rather than through the library's composable, so the string warmed is the string drawn. And the anchor restore asked for a span counted in events, which goes negative when the anchor's row is the oldest half-row and was coerced to one -- a request per delta, six hundred round trips walking one reply back a word at a time with the spinner up. It asks for a page of rows now. Clean pairs, fresh sessions each side, same gestures. Fence scroll (transcript-bench.sh): draw phase 0.74ms per frame before highlighting existed, 0.77ms after, no lexing in the window either side. Streaming forty linked items (stream-bench.sh): 2412ms of reparsing before, 674ms after; mean 5.0ms to 1.4ms, worst 8.9ms to 7.9ms. Bullet glyphs, fence colours, the image links and the reference link checked on the emulator; lint clean on AGP 9.4.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6892dc7caf
commit
ab6a797941
6 files changed
+344
-148
No files matched your search
+136
-75
@@ -162,92 +162,153 @@ and are the reason several tempting simplifications were rejected.
|
||||
|
||||
## Session of 2026-09-03: the list above, worked through
|
||||
|
||||
Items 1-5 of the previous list are implemented and build; the AGP bump from
|
||||
item 7 is in. What is *not* done is in "What is next" below, and the state
|
||||
of each measurement is stated honestly here so nothing has to be re-derived.
|
||||
Items 1-5 of the previous list are done and checked on the emulator, and so
|
||||
is the AGP bump from item 7. Two things were found while measuring them: a
|
||||
174ms stall this work introduced and then removed, and a restore bug that
|
||||
predates it. Both are described below with their numbers.
|
||||
|
||||
**1. Styled markers per depth -- done, unverified on screen.**
|
||||
`MarkdownListItem`'s `Marker` draws `•`, `◦`, `▪` by depth (cycling) in
|
||||
`listMarkerColor` (Theme.kt, Lavender: the scheme's secondary accent, which
|
||||
nothing else used, so it now means "structure"). Ordered numbers take the
|
||||
same colour. Still to check on the emulator: that `◦` and `▪` are in the
|
||||
system fonts rather than drawing as boxes -- the comment on `BULLETS` claims
|
||||
they were checked, and that check is what the next session owes it.
|
||||
**1. Styled markers per depth.** `MarkdownListItem`'s `Marker` draws `•`,
|
||||
`◦`, `▪` by depth, cycling past the third, in `listMarkerColor` (Theme.kt,
|
||||
Lavender -- the scheme's secondary accent, which nothing else used, so it
|
||||
now means "structure"). Ordered numbers take the same colour. All three
|
||||
glyphs were checked on screen at four depths: they render from the system
|
||||
fonts, no missing-glyph boxes. The colour is the same at every depth on
|
||||
purpose -- depth is said by the glyph and the indent, and a colour per depth
|
||||
would make a difference in degree look like one in kind.
|
||||
|
||||
**2. Syntax highlighting inside fences -- done, measured before only.**
|
||||
`CodeFence.kt` holds `highlighted` (moved out of `ToolInput.kt`, timed as
|
||||
`code highlighted`), the `fenceLanguage` alias table (extension or name to
|
||||
the highlights lexer; unknown words stay plain on purpose), and `CodeFence`
|
||||
/ `CodeBlock`, registered as the component table's `codeFence`/`codeBlock`.
|
||||
The library's `MarkdownCodeFence` still finds the code inside the node; the
|
||||
drawing is ours (same background, corner, padding and sideways scroll, minus
|
||||
the shadow, border and empty pointer handler). Baseline `transcript-bench.sh`
|
||||
on the fixture below, before the change: draw phase 0.72ms/frame, transcript
|
||||
0.36ms, the 200-line kotlin fence one 10,700px block. The after run has not
|
||||
been taken.
|
||||
**2. Syntax highlighting inside fences.** `CodeFence.kt` holds it:
|
||||
`highlight` (moved out of `ToolInput.kt`, so a reply's code and a tool
|
||||
call's command are the same colours), the `fenceLanguage` alias table
|
||||
(extension or name to the highlights lexer; a word not in it stays plain,
|
||||
because a fence coloured by the wrong language's rules looks highlighted and
|
||||
is wrong in a way the reader cannot see), `fenceContent`, and the
|
||||
`codeFence`/`codeBlock` entries of the component table.
|
||||
|
||||
The measurement is the reason there is a cache. Highlighted naively, with
|
||||
the answer held by a `remember` inside the fence, a two-hundred-line Kotlin
|
||||
fence cost **174ms to lex** and the lazy list charged it again every time
|
||||
the block scrolled back into composition -- six times in one bench run,
|
||||
1043ms of lexing, and the scroll's draw phase up at 1.29ms per frame. So
|
||||
highlighting is now warmed and cached exactly as parsing is
|
||||
(`ParsedReplies.highlighted`, filled by `warm` from `fences(parse)`), and
|
||||
`highlight` is a plain function taking no colour from the theme, which is
|
||||
what lets it run off the drawing thread.
|
||||
|
||||
Because the warming has to ask for the same string the drawing does,
|
||||
`fenceContent` extracts the code and the language word itself -- the rule
|
||||
copied from the library's `MarkdownCodeFence`, which is a composable and so
|
||||
cannot be called from `warm`. Two extractions would be two keys, and the
|
||||
warmed answer would be missed at every fence with nothing saying so.
|
||||
|
||||
Clean pair, two fresh sessions of the same 200-line Kotlin fence, no saved
|
||||
anchor, same gestures (`transcript-bench.sh`):
|
||||
|
||||
| | before (no highlighting) | after (warmed) |
|
||||
|---|---|---|
|
||||
| draw phase per frame | 0.74ms | 0.77ms |
|
||||
| the transcript's share | 0.33ms | 0.33ms |
|
||||
| lexing during the scroll | none | none |
|
||||
|
||||
**3. `MarkdownRoot` no longer calls the library's `Markdown()`.** It
|
||||
provides the eight locals itself (`LocalReferenceLinkHandler` from the
|
||||
parse, padding, dimens, colours, typography, a no-op image transformer,
|
||||
animations, components).
|
||||
provides the locals itself -- reference links from the parse, padding,
|
||||
dimens, colours, typography, a no-op image transformer, animations,
|
||||
components. Nothing between a piece and the screen is the library's now
|
||||
except the leaf composables named in the component table.
|
||||
|
||||
**4. Paragraphs with images -- done differently from the plan.** The plan
|
||||
said draw the image as its own piece; the app has no image loader and the
|
||||
renderer's transformer was the no-op one, so an image in a reply drew as
|
||||
*nothing*. An `IMAGE` node is now appended by `appendPlainLink` as a link
|
||||
carrying its alt text (the address when there is none), and with no image to
|
||||
place the `hasImage` branch and the renderer's `MarkdownText` are gone:
|
||||
every paragraph is the platform `BasicText`.
|
||||
said draw the image as its own piece; measuring first showed the app has no
|
||||
image loader and the renderer's transformer was the no-op one, so an image
|
||||
in a reply drew as *nothing at all*. An `IMAGE` node is now appended by
|
||||
`appendPlainLink` as a link carrying its alt text (the address when there is
|
||||
none), which says what was there and opens it. With no image to place, the
|
||||
`hasImage` branch and the renderer's `MarkdownText` are gone: every
|
||||
paragraph is the platform `BasicText`.
|
||||
|
||||
**5. Per-item units for a streaming list -- done, before measured, after
|
||||
not.** `LiveParse.advanceTo` cuts at the last item of a multi-item list
|
||||
(`openPiece`), provided that item has content beyond its marker (a bare `-`
|
||||
may still become a paragraph line); the cut is at the start of the item's
|
||||
line so indentation survives the reparse. `Segment.continues` marks a tail
|
||||
that carries on a list, and `MarkdownPiece`'s `continuesList`/`listContinues`
|
||||
keep the padding of an inner item at the seam so nothing moves when it
|
||||
does. Measured with the new `app/stream-bench.sh` streaming
|
||||
`/tmp/longlist.md` (forty bullet items with a link each) on the old build:
|
||||
`markdown reparsed while streaming: 483, 3.9ms mean, 11.6ms worst`,
|
||||
`record: one block` worst 1.6ms. The same run on the new build printed an
|
||||
empty report -- the first thing to look at (the screen showed the list
|
||||
drawn with `•` markers, so the build runs; the report tap or the idle wait
|
||||
may have misfired).
|
||||
**5. Per-item units for a streaming list.** `LiveParse.advanceTo` now cuts
|
||||
at the last item of a multi-item list (`openPiece`), provided that item has
|
||||
content beyond its marker -- a bare `-` is an empty item now and the first
|
||||
character of a paragraph line once `-x` arrives, so cutting on it would draw
|
||||
that line as a new item. The cut is at the start of the item's line, so the
|
||||
indentation the reparse reads its nesting from survives.
|
||||
`Segment.continues` marks a tail that carries on a list, and
|
||||
`MarkdownPiece`'s `continuesList`/`listContinues` keep an inner item's
|
||||
padding at the seam, so nothing moves when the seam does.
|
||||
|
||||
Clean pair, two fresh sessions, forty linked bullet items streamed word at a
|
||||
time (`stream-bench.sh /tmp/longlist.md`):
|
||||
|
||||
| | before | after |
|
||||
|---|---|---|
|
||||
| reparses while streaming | 482 | 483 |
|
||||
| total time in them | 2412ms | 674ms |
|
||||
| mean / worst | 5.0ms / 8.9ms | 1.4ms / 7.9ms |
|
||||
| `record: one block` worst | 1.8ms | 0.7ms |
|
||||
|
||||
The worst case moves least, which is the shape to expect: the first reparse
|
||||
of a tail still covers whatever has arrived, and the last item can be long.
|
||||
What changes is that every reparse after it covers one item instead of the
|
||||
whole list.
|
||||
|
||||
**The restore walked back one event per request.** Found while benching, and
|
||||
older than this work. `savedAnchor`'s loop asked for
|
||||
`oldestSeq - anchor.seq + RESTORE_PAGE_CUSHION` events; when the anchor's row
|
||||
is already loaded but is the oldest half-row (which `anchorRow` refuses,
|
||||
correctly -- it grows when the page behind it lands), that span is negative
|
||||
and was coerced to 1. So the restore fetched one event, then one more, at a
|
||||
round trip each: six hundred requests walking a long reply back a word at a
|
||||
time, with the screen on its spinner the whole way and the sandbox log
|
||||
printing `limit=1` once a second. It now asks for a page counted in rows,
|
||||
which is the only kind that can promise to reach the row behind the anchor.
|
||||
|
||||
**Harness.** `app/stream-bench.sh [-k] FILE` is `transcript-bench.sh` for a
|
||||
reply still arriving: opens the first session, resets the report, sends
|
||||
FILE through `ui-sandbox.sh send`, waits for idle, prints the report.
|
||||
Fixtures used this session, all in `/tmp` (regenerate from the shapes
|
||||
named): `fixture.md` (lists three deep, ordered and nested, fences in
|
||||
reply still arriving: opens the first session, taps the app's own "Jump to
|
||||
latest" so the list is pinned to the newest end, resets the report, sends
|
||||
FILE, waits for the transcript to stop growing, prints the report. Both of
|
||||
those last two are corrections to a first version that measured nothing:
|
||||
a transcript parked further back never redraws while a reply streams into it
|
||||
(the list must not move under a reader), and a session is idle at *both*
|
||||
ends of a turn, so polling for idle answers before the turn has started.
|
||||
Fixtures live in `/tmp` and are regenerated from the shapes named here:
|
||||
`fixture.md` (lists four deep, ordered and nested, fences in
|
||||
kotlin/rust/sh/none, a table with a link, a quote with a list, an inline and
|
||||
a standalone image, a reference link), `longfence.md` (200-line kotlin
|
||||
fence), `longlist.md` (40 linked items), `numlist.md`.
|
||||
a standalone image, a reference link), `longfence.md` (200-line Kotlin
|
||||
fence), `longlist.md` (40 linked items).
|
||||
|
||||
**Seen and not chased: a reconnect loop.** On the *old* build, restarting
|
||||
the app onto the fixture session with a saved anchor mid-transcript while a
|
||||
600-delta reply was streaming left the screen on a spinner, reconnecting
|
||||
every 1.5s (`RECONNECT_DELAY_MS`) with `session screen recomposed: 26` and
|
||||
the fence message re-warmed each time, until the sandbox server was
|
||||
restarted. `events?after=N` more than `CATCH_UP_LIMIT` (200) behind answers
|
||||
`reset` plus the newest 200 *raw* deltas, i.e. a window starting
|
||||
mid-message; the restore loop and that reset clearing `items` look like the
|
||||
two halves. Reproduce with `stream-bench.sh` (restart form) after a
|
||||
`transcript-bench.sh` run has left an anchor mid-fence.
|
||||
**Two traps in the emulator loop**, both of which cost a bench run here.
|
||||
`adb shell pm clear` removes the enrolment and the notification permission
|
||||
along with the saved anchors, so the next run measures a permission dialog;
|
||||
re-enrol with the command `ui-sandbox.sh` prints and
|
||||
`pm grant … POST_NOTIFICATIONS`. And a saved anchor is per session id, so
|
||||
the only way two builds start a scroll from the same place is a *fresh
|
||||
session for each*.
|
||||
|
||||
## What is next, in order
|
||||
|
||||
1. **Look at the fixture on the emulator** (session `fixture2` in the
|
||||
sandbox holds only `fixture.md`): bullet glyphs at three depths, fence
|
||||
colours, the image drawn as a link, the reference link at the foot.
|
||||
2. **Take the after measurements**: `transcript-bench.sh` for the fence,
|
||||
`stream-bench.sh /tmp/longlist.md` for the list, and put both pairs in
|
||||
the commit message. Find out why the after run's report was empty.
|
||||
3. **Lint** (`./gradlew :androidApp:lint`) on AGP 9.4.0; the bump is in
|
||||
`libs.versions.toml` and the build passed, lint has not been run.
|
||||
4. **The reconnect loop above.**
|
||||
5. **File the highlights range bug upstream** -- no `gh` in this VM and no
|
||||
GitHub credential, so it needs Bryan or a token. One-line repro: lexing
|
||||
`x '*/a/*'` as `SyntaxLanguage.SHELL` in highlights 1.1.0 returns a
|
||||
highlight whose `location.end` precedes its `location.start`.
|
||||
6. Everything from the earlier list that still stands: regression runs
|
||||
before and after any change to these files, pasted into the commit.
|
||||
1. **A fence still arriving is lexed per delta.** `warm` covers settled
|
||||
messages; the live tail's fence is highlighted on the composing thread by
|
||||
`ParsedReplies.highlighted`'s inline miss, once per delta, and at 174ms
|
||||
for a long one that is the stall above wearing a different hat. The tail
|
||||
is short while it is being written, so this may already be cheap -- but
|
||||
it is unmeasured, and `stream-bench.sh /tmp/longfence.md` is the run that
|
||||
says. Cheapest fix if it bites: highlight the live tail only when it is
|
||||
under some length, or move the miss off-thread the way `LiveParse` moved
|
||||
parsing, keeping the plain text until the answer lands.
|
||||
2. **The reconnect loop.** Restarting the app onto a session with a saved
|
||||
anchor while a long reply was streaming left it reconnecting every 1.5s
|
||||
(`RECONNECT_DELAY_MS`), spinner up, until the server was restarted.
|
||||
`events?after=N` more than `CATCH_UP_LIMIT` (200) behind answers `reset`
|
||||
plus the newest 200 *raw* deltas -- a window starting mid-message -- and
|
||||
the reset clears `items`, which is the state the restore loop then pages
|
||||
against. The one-event-per-request bug above was part of what made it so
|
||||
visible; whether it survives that fix is the first thing to find out.
|
||||
3. **File the highlights range bug upstream.** There is no `gh` and no
|
||||
GitHub credential in this VM, so it needs Bryan or a token. One-line
|
||||
repro: lexing `x '*/a/*'` as `SyntaxLanguage.SHELL` in highlights 1.1.0
|
||||
returns a highlight whose `location.end` precedes its `location.start`.
|
||||
Until then `highlight` drops such spans, which is why the shell fence in
|
||||
`fixture.md` -- whose command contains `'*/.git/*'` -- draws plain while
|
||||
an ordinary shell fence colours.
|
||||
4. **Regression runs.** `transcript-bench.sh` and `stream-bench.sh` before
|
||||
and after any change to the files above, with the report in the commit.
|
||||
The numbers to watch are the worst `record: one block`, the reparse mean
|
||||
while streaming, and the draw phase's accounting line.
|
||||
Reference in new issue
Block a user