Skip to content

perf(ui): incrementally prepare streamed Markdown math - #4207

Open
Astro-Han wants to merge 2 commits into
mainfrom
perf/streaming-markdown-preserve-math-source
Open

perf(ui): incrementally prepare streamed Markdown math#4207
Astro-Han wants to merge 2 commits into
mainfrom
perf/streaming-markdown-preserve-math-source

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Streaming math previously transformed both the full Markdown source and settledText on every React render, before Astryx applied its own streaming cursor and incremental block parser. This change keeps Runtime Host text authoritative, lets Astryx smooth the raw text first, and incrementally prepares only the parser-facing math tail from the last safe boundary.

The implementation remains one path: MarkdownBody → useStreamingText → math source transform → Astryx incremental parser. It removes the dynamic token namespace, two per-render token maps, and the separate transformed settledText path. It does not touch Desktop transcript range, virtualization, scroll ownership, or content visibility.

Results

  • Math preparation CPU fell by 80.8% at 64 KiB and 83.2% at 256 KiB, with B winning 7/7 pairs at both sizes.
  • On this machine, that local CPU reduction did not become an end-to-end latency win: median event-to-rendered-frame changed by +0.9% / +63.4%, with 2/7 and 4/7 wins.
  • Total Renderer busy CPU changed by -1.9% / +18.0%. First-token and final-flush results were mixed, so no whole-renderer improvement is claimed.
  • At 256 KiB, LoAF count improved 4 → 2 (5/7 wins), while LoAF-duration P95 regressed 9.2%. Post-GC heap was flat; transient 64 KiB peak-heap median was +26.7% with only 1/7 wins, so neither memory nor long-frame tails are claimed as wins.
  • Correctness passed in all 28 A/B trials. This PR records a proven reduction in math-preparation work, but it does not meet the original merge gate for end-to-end performance on this CPU.
Size / metric A median B median Δ A P95 / P99 B P95 / P99 MAD / median A → B B wins
64 KiB rendered-frame median (ms) 16.50 16.65 +0.9% 16.65 / 16.65 17.77 / 18.11 0.6% → 0.3% 2/7
256 KiB rendered-frame median (ms) 54.65 89.30 +63.4% 95.82 / 96.08 93.89 / 94.86 7.0% → 6.5% 4/7
64 KiB math preparation CPU (ms) 9.90 1.90 -80.8% 12.75 / 13.59 2.96 / 3.15 4.0% → 21.1% 7/7
256 KiB math preparation CPU (ms) 73.70 12.40 -83.2% 77.92 / 78.78 14.47 / 14.73 2.3% → 10.5% 7/7
64 KiB Renderer busy CPU (ms) 257.95 252.95 -1.9% 347.74 / 351.64 334.30 / 359.47 1.7% → 1.1% 5/7
256 KiB Renderer busy CPU (ms) 1302.02 1536.52 +18.0% 1797.01 / 1801.05 1753.14 / 1775.97 19.1% → 9.8% 3/7

A/B method and limits

A is baseline bb1256241eddb2384715f4131fb16c7599f92a8d; B is the product code in fffe591a2203b60019a499be9912112543c01ba5. Both modes ran in the same minified production React/Markdown fixture bundle (sha256:d790dff180e4baa9054eb4873033e3bc28c27caf8c9f063eebcc71111bfd805a; recorded result sha256:bdc6cab7cdd08ac0ca1ade74963f93b8320fcba88e573e41f018bb8a18908bea). The 64 KiB and 256 KiB fixtures each exercised ordinary Markdown, inline/display math, a long growing code fence, settled math plus a live tail, and final non-streaming flush. Seven interleaved paired runs alternated A/B and size order. CDP and the React hook were read-only; the temporary mode switch only selected A or B in the throwaway measurement bundle.

The harness is intentionally not published with the product change: doing so would retain a benchmark-only product mode and a second fixture protocol. The recorded temporary command was:

./node_modules/.bin/esbuild /private/tmp/pr4207-perf-entry.tsx --bundle --minify --keep-names --platform=browser --format=iife --define:process.env.NODE_ENV='"production"' --outfile=/private/tmp/pr4207-perf-bundle.js
PR4207_PERF_PAIRS=7 PR4207_PERF_OUTPUT=/private/tmp/pr4207-final-ab-v3.json node /private/tmp/pr4207-perf-runner.mjs

These paths are not present in a fresh checkout. This is a bare minified React/Markdown composition fixture, not a Desktop or Runtime Host E2E. The rendered-frame metric waits two requestAnimationFrame callbacks and is not labeled literal next paint. math preparation CPU is direct transform instrumentation; Renderer busy CPU, GC, heap, long tasks, and LoAF come from CDP. The current full-string input contract still requires source.startsWith(previousSource) to detect rewrites, so this removes repeated full math lexing and the second settledText pass, but does not claim that every source identity check is tail-only.

Raw 7 paired groups (14 size rows)
Pair KiB Order frame median A / B ms math CPU A / B ms busy CPU A / B ms first A / B ms flush A / B ms peak heap A / B MiB
1 64 A→B 16.40 / 16.75 13.80 / 3.20 352.62 / 252.39 42.40 / 15.00 23.20 / 19.20 39.49 / 39.67
1 256 A→B 51.55 / 89.85 72.60 / 14.80 1302.02 / 1624.98 51.70 / 48.10 68.60 / 67.90 218.59 / 231.79
2 64 B→A 16.65 / 16.50 8.80 / 1.80 253.51 / 260.89 13.60 / 13.10 21.10 / 18.50 39.19 / 45.72
2 256 B→A 54.65 / 50.20 75.40 / 12.40 1268.40 / 1236.01 53.90 / 50.20 88.50 / 74.60 240.97 / 254.41
3 64 A→B 16.50 / 16.65 9.90 / 2.40 336.34 / 252.95 28.00 / 18.20 19.40 / 18.90 70.75 / 48.89
3 256 A→B 96.15 / 95.10 71.30 / 13.70 1802.06 / 1781.68 102.80 / 91.40 73.00 / 71.20 217.37 / 227.93
4 64 B→A 16.45 / 18.20 9.50 / 2.10 254.77 / 365.76 13.80 / 36.50 20.80 / 18.30 39.12 / 66.40
4 256 B→A 54.20 / 73.40 79.00 / 11.10 1276.26 / 1397.42 54.50 / 49.80 72.30 / 79.90 249.52 / 244.81
5 64 A→B 16.60 / 16.60 10.30 / 1.90 256.96 / 249.20 17.60 / 17.70 19.90 / 18.70 48.33 / 51.54
5 256 A→B 50.85 / 91.05 73.70 / 11.70 1053.32 / 1536.52 61.00 / 50.80 71.80 / 73.10 203.19 / 188.05
6 64 B→A 16.65 / 16.65 9.30 / 1.10 271.83 / 252.96 19.50 / 11.50 20.60 / 18.10 39.17 / 52.29
6 256 B→A 95.05 / 50.50 75.10 / 10.50 1785.23 / 1256.24 137.20 / 50.20 70.00 / 71.00 214.16 / 228.25
7 64 A→B 15.95 / 16.70 9.90 / 1.50 257.95 / 250.28 17.20 / 17.40 19.50 / 18.10 47.93 / 50.05
7 256 A→B 91.25 / 89.30 71.50 / 13.30 1588.18 / 1686.55 57.80 / 131.30 74.00 / 67.50 233.71 / 215.34

Verification

npm ci
npm test --workspace @maka/ui                  # 287/287
npm run typecheck --workspace @maka/ui
npm run format:check
git diff --check bb1256241eddb2384715f4131fb16c7599f92a8d...fffe591a2203b60019a499be9912112543c01ba5

Coverage includes empty formula fallback; URL/email/Markdown markers inside math; display math across blank, heading, list, and table-looking lines; inline/fenced code isolation including split fence openers through final flush; literal and malformed transport syntax; rewritten/restored text; and real client rerenders that preserve settled KaTeX DOM identity through a growing tail and final flush. A fresh install reapplies the Astryx patch, and a source-condition bundle contains the same transformSource seam as the default dist build.

Entropy ledger

Dimension Before After
Authority Runtime Host text; Astryx parser unchanged
Math runtime state per-render registry + two Maps one bounded, discardable four-field cache per MarkdownBody mount
Representations raw + transformed source/settled + dynamic tokens + AST raw displayed source + one parser transport + AST
Scheduler / parser / observer useStreamingText; one incremental parser; existing observers unchanged owners and counts

The required additions are the post-scheduler/pre-parser transform seam, the safe source/text boundary pair, and collision-safe literal transport. They replace the old double full-source transform and map registry in this diff; no second store, parser, scheduler, protocol, observer, or transcript authority is added. The cumulative diff is +624/-178; the added volume is primarily the incremental lexer, behavior tests, and duplicated source/dist dependency patch, while owner and representation counts decrease as shown above.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex diagnosed the repeated preprocessing path, implemented the owner-aligned transform and incremental lexer, added tests, ran the production A/B, and prepared this PR. The commits carry Generated-by: Codex trailers.

Checklist

  • Tests cover the change and fail without it
  • Format, typecheck, dependency patch application, and affected tests pass locally

Does this PR entail a change in behavior?

  • Yes — parser-owned math source is now prepared after the streaming cursor and incrementally reused
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 29, 2026
Move math source preparation behind Astryx streaming and reuse only safe transformed prefixes while preserving the existing incremental parser and scheduler.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the perf/streaming-markdown-preserve-math-source branch from 860892c to a217481 Compare August 29, 2026 19:18
@Astro-Han Astro-Han changed the title perf(ui): reuse incremental Markdown parsing for math perf(ui): incrementally prepare streamed Markdown math Aug 29, 2026
@Astro-Han
Astro-Han marked this pull request as ready for review August 29, 2026 19:37
@github-actions github-actions Bot added effort/L Under 1000 readable lines and removed effort/M Under 500 readable lines labels Aug 29, 2026

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the latest head a2174817f. No P0 or P1 — approving. Two P3 notes are left inline.

The incremental design holds up. The safety of only re-scanning the tail rests on canMarkSafe latching off permanently at any unterminated construct — an unclosed fence, an unclosed backtick run, a pending delimiter, or a trailing \/$/` at end of input. Because that flag never resets within a scan, the safe boundary can never advance past a point whose interpretation a later append could change, so re-entering at that boundary with only "am I at a line start?" as carried context is sufficient. I checked each of those five branches individually.

Coverage is real rather than incidental. ${settled}growing${settled}growing live tail is a genuine prefix extension with math sitting in the settled portion, and it asserts the rendered math node keeps its identity across the two renders; the formula-rewrite test covers the cache-reset branch; and the unterminated fence, backtick, and delimiter cases each have behavioural tests.

One note on gating: test was still running when I reviewed, with label and windows_recovery green. The required checks still gate the merge, so this approval does not let anything through ahead of them.

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

// Reset incremental cache when the autolink option toggles — cached
// settled blocks were parsed with the previous setting.
@@ -1139,17 +1142,25 @@ export function Markdown({
- // Reset incremental cache when the autolink option toggles — cached

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — this silently drops a documented invariant guard.

The patch now removes Astryx's autolink-toggle cache reset. That reset existed for a stated reason: settled blocks already in the incremental cache were parsed under the previous autolink setting, so toggling it without invalidating them leaves stale parses on screen.

It is safe today only because of a single fact outside this file — markdown-body.tsx:161 passes autolink="gfm" as a hardcoded literal, and there is no other call site, so the flag cannot change at runtime and the reset was dead code here.

What I would ask for is a line in the PR body or the patch saying that, because the removal is currently unexplained. As it stands, the comment explaining why the guard existed is deleted along with the guard, so whoever later makes autolink dynamic gets a stale-cache bug with nothing left to warn them.

text += run;
index = runEnd;
atLineStart = false;
canMarkSafe = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — the one coverage combination that is missing.

Each half of this is tested: the unterminated cases (unclosed fence, unmatched backtick, pending delimiter) have behavioural tests, and the append path is exercised by the growing-tail streaming test.

What no test covers is the interaction — an append that closes a construct which was unterminated in the previous render. That is exactly the case this latch exists for: the boundary must have stayed behind the opener so the closer is re-scanned in context.

A cheap direct test would prepare a \b(unclosed backtick), then preparea `b` c, and assert the result equals a single one-shot preparation of the full string. The same shape for (` and for a fence would pin the invariant at the unit level instead of relying on it holding through the rendered output.

@zhiiw zhiiw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at exact head a2174817 (verified unchanged at review time; test 25m, windows_recovery, and label all completed/success on this head).

Incremental math preparation, verified rather than assumed:

  • The incremental cache boundary is conservative in the right places: the safe frontier only advances past closed constructs — closed fences, closed backtick runs, fully matched math tokens, and ordinary characters that cannot begin a delimiter (\, $, ` at the tail defer the mark). An unmatched backtick suppresses marking for the rest of that pass, so a later ) can never retroactively change how a settled prefix tokenizes. Line-start anchoring for fence detection is carried across the cut via source[sourceStart - 1] === '\n', and any non-prefix rewrite resets the whole cache.
  • The fixed token namespace is collision-safe by construction: literal \uE000MAKA_MATH:…\uE001 text in the source is re-encoded as a kind-2 literal token that renders as itself (the old dynamic-namespace loop existed for exactly this; the new scheme answers it structurally instead of by probing). Partial transport sequences at the stream tail are treated as pending and copied verbatim — the new tests pin that transport syntax never becomes visible across the display cursor, including through a formula rewrite.
  • One path, honestly bounded: the transform now runs on Astryx's smoothed display text immediately before its incremental parser (transformSource seam in the patch), with raw settledText on both sides of the cursor check, so host and renderer compare the same strings. The PR claims the math-preparation CPU reduction (85%/84%, 7/7 paired wins) and explicitly does not claim a whole-renderer latency win — the tables back exactly that claim.
  • Noted, not blocking: the patch update drops the previous autolink-toggle reset of the incremental parser cache. In this codebase autolink="gfm" is a constant prop at the only call site, so the removed guard is unreachable here; worth remembering if the prop ever becomes dynamic.

Executed on a real Windows machine at this head: re-applied the updated Astryx patch (deleted the stale patched package and reinstalled — the build catches this correctly, since transformSource is a new prop), clean forced rebuild, then the markdown-body + streaming-text suites 40/40 — including the new adversarial cases (literal token as prose, malformed prefix recovery, unmatched backtick/delimiter recovery, display math across Markdown-looking block boundaries, transport syntax never exposed mid-stream, formula rewrite).


Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

增量数学预处理,全部核实而非假设:安全前沿只在闭合构造后推进(闭合围栏/反引号/完整数学 token/不可能起始定界符的普通字符),未配对反引号会抑制本趟标记,尾部 \ $ ` 延迟标记——settled 前缀不会因后续增长改变切分;行首锚定跨切口正确传递;非前缀重写整体重置。固定命名空间用 kind-2 字面量重编码结构性解决碰撞(替代旧的动态探测);流尾半个传输序列按 pending 原样透传,新测试钉住传输语法永不越出显示光标。transformSource 缝让变换作用于 Astryx 平滑后的显示文本、紧贴其增量解析器前,settledText 两侧都是原始串。PR 只声称 math CPU 下降(85%/84%、7/7),不声称端到端延迟收益——表格与声明严格一致。注意项(不阻断):patch 删掉了 autolink 切换时重置增量缓存的守卫,本库唯一调用点 autolink 是常量字面量,不可达。本机真 Windows:重打 patch 后干净重建,markdown-body + streaming-text 40/40(含全部新对抗用例)。

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on exact head a2174817fc45bff7bb88c8921bdf229da4fb3003: I found no P0 or P1. The incremental preparation matches one-shot preparation across ordinary growth, rewrites, shrink/restore, math, literal transport syntax, and complete backtick-fence paths. One non-blocking P2 remains inline for a tilde fence whose opener is split across displayed provider chunks.

The exact-head test, windows_recovery, and label checks are green. Locally, the complete UI suite passed 286/286, the affected builds and Biome check passed, and the conflict-free current-main merge tree does not alter any reviewed file.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

index < source.length ||
(character !== '\\' && character !== '$' && character !== '`')
) {
markSafe();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Keep a partial line-start tilde fence behind the safe boundary

A normal streaming split can leave ~~ as the currently displayed source before the third ~ arrives. This branch marks both tildes safe because only trailing \, $, and backticks are held back. The next preparation therefore resumes after ~~ with startsAtLineStart=false, never recognizes the eventual ~~~ opener, and encodes math-looking text inside the code block as an internal transport token.

I reproduced this through the production MarkdownBody and Astryx path: render ~~ as the settled first target, then stream ~~~ts\n\\(not math\\)\n~~~, advance the real animation frames, and finally render the same text non-streaming. The code block still contains \uE000MAKA_MATH:0:6e6f74206d617468\uE001 instead of \(not math\), so the corruption survives the final flush. Existing fence tests provide the whole opener in one input and cannot exercise this boundary.

Please keep a one- or two-character line-start ~ run behind the safe boundary until a third tilde or a non-tilde decides whether it is a fence, and add this split-opener plus final-flush regression.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

@ARE404 ARE404 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving perf(ui): incrementally prepare streamed Markdown math at head a2174817.

I reviewed the incremental prepareMarkdownMath/protectMarkdownMath redesign and its tests, and verified the boundary-safety of the cache:

  • Only fully closed constructs (closed fences, matched literal tokens, closed delimiters) advance safeSourceEnd/safeTextEnd; any unterminated/pending construct leaves the tail unmarked, so the incremental re-parse always operates on the unsafe tail and reuses only provably-settled prefix. A math token still mid-stream is never cached as safe, so it is re-parsed fresh until it closes — the incremental slicing does not corrupt a token that straddles the boundary.
  • The startsWith prefix check forces a full re-parse on any rewrite rather than reusing a stale prefix, so a mid-stream edit cannot reuse wrong cached state.
  • test is green on this exact head (run 33270540247, 25m01s). Two non-blocking [P3] review threads remain open (the Astryx autolink-toggle cache reset removal in the dependency patch, and a missing boundary-coverage combination); they do not block, but the author should still reconcile them.

No P0–P2.

简体中文

批准 perf(ui): incrementally prepare streamed Markdown math,head a2174817
审查增量 prepareMarkdownMath/protectMarkdownMath 重设计与测试,核实缓存的边界安全:只有完全闭合的构造(闭合围栏、匹配的 literal token、闭合分隔符)才会推进 safeSourceEnd/safeTextEnd;任何未闭合/pending 构造会让尾段保持不安全,于是增量重解析始终只作用于不安全尾段、复用确定沉淀的前缀。还在流式中间态的 math token 永不标记为安全,直到闭合才重新解析——跨边界的 token 不会被增量切片破坏。startsWith 前缀检查保证任何改写都触发全量重解析、不复用过期前缀。test 在 exact head 绿(run 33270540247,25m01s)。两条非阻塞 [P3] 线程仍开着(依赖补丁里移除 Astryx autolink 缓存重置、以及一处边界覆盖缺失),不构成阻塞但建议作者另行统一。无 P0–P2。

Keep incomplete line-start fence markers behind the math cache boundary so later streaming chunks cannot turn code into math transport tokens.

Generated-by: Codex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants