perf(tui): render child transcript by viewport window - #327
Conversation
Repaint used to render every transcript row and clip afterwards, so a child page paid for its whole history on every tick. openpi-dev#232 stopped the pairing index from being rebuilt per frame; this stops the rows themselves from being rebuilt. - AgentTranscriptRenderer.beginFrame() resolves the row total, then renders only the rows a window asks for. render() stays the full-transcript contract and is now that same path with a full-height window, so WorkflowTranscript- Renderer and existing callers are unchanged. - A per-items layout caches item heights as prefix sums, so a window locates its first item by binary search instead of re-deriving preceding rows. Appends extend the sums; a rebuilt pairing index, a moved live-tool set, a theme generation bump, or a changed key prefix rebuilds them. - AgentToolRenderLedger reports monotonic per-tool and ledger-wide revisions, so streaming native output invalidates only the items that show that tool. revision() is optional: a renderer without it is re-measured every frame, which preserves today's behaviour for third-party renderers. - findResult() binary searches the ascending result indices, so a wide fan of parallel calls whose results all land later cannot degrade into a quadratic pairing scan. - cwd and the openpi-dev#292 tool expansion state join the cache key. Both change tool rows, and neither was previously discriminated; without them a warm cache could serve one child's or one view's row to another. Fixes two row-total bugs found while measuring, both cases where the total was published before the rows were known: a same-length in-place item replacement, and an unrevisioned renderer whose native output grew. Each dropped rows off the end of the viewport for one frame. This is a scaling fix, not a fix for visible lag today. Warm repaint cost used to grow with history length and is now flat, but at the 512-item transcript ceiling the old cost was already only 0.1 ms, well inside a frame budget. Cold open and post-invalidate repaint are deliberately unchanged: a scrollable view must know its total row count, and knowing that means measuring every item once. Measured (benchmarks/child-transcript-viewport.ts, viewport 40, 100 repaints, counting item-array index reads per repaint): items before after 32 102 89 128 414 89 512 1662 89 At 512 items the cost now follows viewport height (10/20/40/80 rows -> 34/53/89/164 reads) instead of history length. The 64-way separated call/result shape is flat too: 65/93/93 for 32/128/512 items. benchmarks/child-transcript-repaint-demo.ts prints the same comparison as wall time over realistic Markdown-heavy turns, separating cold open, warm repaint, and post-invalidate so the regimes this does not improve stay visible. Refs openpi-dev#185
The cwd cache-key test hardcoded "D:/works/openpi", which path.isAbsolute only accepts on Windows. On Linux CI the value was relative, displayPath returned it unchanged for both cwd values, and the assertion that cwd changes the rendered row failed. Build both cwd values and the tool argument with path.resolve/join so they are absolute on whatever platform runs the suite, and relativization actually happens. Verified under posix semantics: the old fixture renders identically for both cwds, the new one does not.
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
我建议当前版本先不要合并。实现和测试本身很认真，性能数字也可复现；阻塞点不是正确性，而是当前收益不足以支撑新增的缓存架构。
当前证据：
- 在 512 项生产上限下，warm repaint 约从 0.082 ms 降到 0.002 ms；两者都远低于 16 ms 帧预算，PR 也明确说明目前没有可见卡顿。
- 更接近帧预算的 cold open 约 16.6 ms，而本方案不改善 cold open。
- 本次增加约 590 行生产逻辑，引入 items/pairing identity、行高和前缀和、theme generation、cwd、expanded、live tool、per-tool revision 和 ledger generation 等缓存失效契约。今后 transcript、compaction 或工具渲染发生变化时，这些都成为维护责任。
- PR 中修复的 same-length replacement 和 unrevisioned renderer row-total 问题，是窗口缓存成立后必须处理的新失效场景；旧的全量渲染路径没有这一类缓存一致性风险。
建议缩小范围：
- 保留 findResult() 的二分查找及相应回归测试；这是小而明确的改进。
- 保留 benchmark 作为后续证据工具。
- 完整 viewport/revision 缓存请在真实 AgentSessionPage 端到端 profile 证明当前实现明显占用帧预算，或 transcript 上限准备显著提高时再引入；届时也请先评估“稳定 document 的整段正文缓存 + 动态 live tail”这种更小机制。
我在 exact head 9691b00 上验证：bun run check 通过；专项测试 24/24；全量 1092 通过、1 项平台跳过；GitHub Node 22/24 和 Windows checks 全绿；另外对 1875 组 transcript 组合做了旧/新 render 输出比对，结果一致。没有发现模型上下文、消息、压缩或持久化状态影响。这里请求修改的是方案边界和投入产出比，不是声称存在未证实的运行时 bug。
|
感谢评审，三条意见全部接受。收缩已完成。过程中的测量改变了我对 #185 配对这一半问题的理解，其中一部分与我原先的 PR 描述矛盾，一并说明。 当前范围 已完全移除：beginFrame/窗口渲染、前缀和 layout 缓存、per-tool revision、cwd/expanded 入缓存键、AgentSessionPage 改动。 bun run check 通过；配对测试 7/7；相关套件 42/43（1 项为既有的 Windows 路径分隔符差异）。 更正原 PR 描述中的一处错误 我原先写本 PR "fixes two row-total bugs found while measuring"，这个表述是错的，我撤回。 旧路径的 rowCount 取自已完成的全量渲染（agent-session-page.ts:243-249，this.rowCount = transcript.length），结构上不可能出现行总数与实际不符。那两个 bug 是我引入窗口缓存后才产生 我也验证了自己新增的 cwd/expanded 缓存键是否在修复既有问题：不是。在干净的 ba550bc 上，expanded 切换（含/不含 native renderer）和 cwd 变化下，warm 与 cold 渲染结果一致，无陈旧行 关于「保留二分查找」 同意保留，但依据与我原先给出的不同 —— 因为测量结果与我的预期相反。 在实际的 per-id 索引长度下，二分查找的数组读取次数多于线性扫描： ┌──────────────────────┬──────────────────┬────────────┬────────────┐ #185 点名的 64-way 并行分离形态不是退化情形：并行调用各自持有独立 id，per-id 索引长度为 1，线性与二分均为 O(1)。只有 id 被复用时索引才会变长。 那么 id 会复用吗？扫描本机 170 个 Pi session transcript，按线性化路径统计（会话是 parentId 树，按文件统计会混淆兄弟分支）： 零复用。compaction 记录（全库 2 条）存储 summary 与 firstKeptEntryId，是截断而非消息重放，不会重新引入 id。 但构造上可达。直接驱动 workflowTranscriptDocument： 合成路径（workflow-tool-${index}）按下标唯一，安全。第 4 行是不需要 provider 出错的情形：当 legacy Workflow 历史中部分条目携带 call id、同名的其他条目没有时，fallback 配对的 也就是说，call id 唯一性是一项由 provider 保证、而 OpenPI 从不校验的不变量 —— 此处没有任何类型、断言或测试约束它。这才是二分查找的真实依据：它移除了对未受守卫的外部行为的隐式依 我此前用「慢一倍」描述 256 vs 512，是用比例掩盖了可忽略的绝对差值，表述不够诚实。 调用点注释已改写为可被数据检验的版本：说明唯一性来源、实测语料结论、可达的重复路径、以及代价。新增测试构造长度 40 的 per-id 索引，先断言 fixture 确实建出该长度（避免测试空转）， 按要求推迟的部分 viewport/revision 缓存已移出本 PR。若将来重开，需要真实 AgentSessionPage 端到端 profile 证明确实占用帧预算，或 transcript 上限显著提高；并且按您的建议，先评估「稳定 document 的 供将来参考的测量：512 项上限下 warm repaint 0.082 ms → 0.002 ms，而 cold open 约 16 ms 且不被窗口化方案改善。这与您「当前收益不足以支撑新增失效契约」的判断一致。 也感谢您在 9691b00 上的独立验证，尤其是 1875 组新旧 render 输出比对 —— 比我本地能做的更充分，因为完整套件在我的 Windows 机器上跑不完。 |
2 participants
Repaint used to render every transcript row and clip afterwards, so a child page paid for its whole history on every tick. #232 stopped the pairing index from being rebuilt per frame; this stops the rows themselves from being rebuilt.
Fixes two row-total bugs found while measuring, both cases where the total was published before the rows were known: a same-length in-place item replacement, and an unrevisioned renderer whose native output grew. Each dropped rows off the end of the viewport for one frame.
This is a scaling fix, not a fix for visible lag today. Warm repaint cost used to grow with history length and is now flat, but at the 512-item transcript ceiling the old cost was already only 0.1 ms, well inside a frame budget. Cold open and post-invalidate repaint are deliberately unchanged: a scrollable view must know its total row count, and knowing that means measuring every item once.
Measured (benchmarks/child-transcript-viewport.ts, viewport 40, 100 repaints, counting item-array index reads per repaint):
items before after
32 102 89
128 414 89
512 1662 89
At 512 items the cost now follows viewport height (10/20/40/80 rows -> 34/53/89/164 reads) instead of history length. The 64-way separated call/result shape is flat too: 65/93/93 for 32/128/512 items.
benchmarks/child-transcript-repaint-demo.ts prints the same comparison as wall time over realistic Markdown-heavy turns, separating cold open, warm repaint, and post-invalidate so the regimes this does not improve stay visible.
Refs #185