Repository navigation
fix(cmux-tui-core): the public fold compares a restated tab decorated, as the journal writes it - #36212
fix(cmux-tui-core): the public fold compares a restated tab decorated, as the journal writes it#36212lawrencecchen wants to merge 3 commits into
Conversation
…its pane (red: every chat tab of the pane is journaled again)
… decorated, as the journal writes it The fold holds the decorated values the journal states (99e3260); the projection restates a pane's browser tabs raw. A raw upsert of a decorated tab (a chat tab: content_kind conversation, its agent session and browser profile in extra) never equalled its fold value, so every chat tab of a pane was journaled again on each create in it: with 400 tabs in a pane, about 400 journaled changes and a 41 ms journal step for one tab. An upsert the raw compare keeps and the fold holds is now decorated (state/values.rs decorate_value, as the journal append does) and compared again.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
web and size checks: green |
|
Found 3 test failures on Blacksmith runners: Failures
|
…lone could make it stated (review) A held tab, screen or workspace whose raw value differs from the fold in a field decoration does not set (a tab whose index moved after a close or drag) is kept without a query; only one that differs at most in content_kind and extra is decorated and compared again. Panes, terminals and browsers are never decorated.
|
Review (independent review subagent, Claude): SHIP at fe8a354 Pruning is exact: the prune decorates with the state before the commit and the journal after the patch; they can differ only if the same commit writes a state row of a held resource, and none of the four commit paths does (patch apply touches only new workspaces and closed browsers, whose tabs change raw too; group-close state concerns only closing members; terminal close adds deletes). Every decoration join reads a field the raw value carries. State writes without their own delta are still journaled (the prune compares the current decoration with the fold's old value). The connection lock is reentrant and every caller opens its transaction after the prune; a decoration error fails the commit as the journal append already did. Review point taken in fe8a354: decorate only a workspace, tab or screen whose held value differs from the raw one at most in Fleet at this head: Linux |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
Base red on
feat-cmux-next(Linux, in full runs):cmux-tui-core state::group_update_latency_tests::a_persons_requests_answer_while_busy_sessions_saturate_the_registryfails in 9 of 31 full runs I have logs of: "a person's request waited behind the busy sessions: new-conversation-tab ... p95=209 to 368 ms" (budget 200 ms). Only the chat tab misses its budget.Cause (product): a chat tab create journaled every other chat tab of its pane again. The public fold holds the decorated values the journal states (99e3260: the journal decorates every upsert with its state rows, so the fold folds the same decorated values). The projection restates a pane's browser tabs raw, and
prune_stated_topology_deltascompared them raw: a chat tab's stored value hascontent_kind: conversationandextra.conversation/extra.browser_profile_id, its raw restatementcontent_kind: browserand neither, so it never matched and was never pruned. A probe on the shrunk case showed it: with 100 restated rows in, about 20 tab upserts kept per create, each an unchanged chat tab (storedconversation, restatedbrowser, all else equal). Every create then paid the journal append, its subject index rows and the fold's decoration for every chat tab of the pane, under the registry lock that a person's next click waits on, and the 4cmux agent opencallers of the test each grew their own pane. (Workspaces already had a special case for the same mismatch:without_workspace_key.)Fix: an upsert that the raw compare keeps and the fold holds is decorated with
state::values::decorate_value(as the journal append decorates it) and compared again. The kept changes are journaled raw as before (the append decorates them). All callers take the registry connection after the prune.Measured (probe, a pane grown to 400 chat tabs, 850 creates, Linux): journaled changes per create mean 108 -> 4 (max 402 -> 64), journal step mean 8.3 -> 2.5 ms (max 41 -> 10 ms), whole commit mean 12.9 -> 6.8 ms (max 59 -> 28 ms); a chat tab in the 400-tab pane 117 -> 92 ms. The rest of the per-pane cost is the per-tab validation of the pane's order, fixed in its own PR (linked below), and the
tab-addedentity build.Commits: (1) red test
state::group_update_latency_tests::a_chat_tab_create_restates_no_other_chat_tab_of_its_pane(31 tabs restated by one create on the base); (2) the fix.Testing
dd4f43c0dc7ae07c66e38059) and macOS (jobbc1ebe45e05038393031a24b): "one chat tab create restated 31 tabs".allfilterstate::(job3a4db4b216c59e96d3c88332): 295 of 295 pass, fmt pass, clippy pass; macOS filterstate::(job6a59e6ca2ab921260dbac0b7): 295 of 295; panic ratchet (jobcbd24ba1cd0028736e4c4107) pass; projection tests filterprojection(job4a3f4afaae3b4182445e1c7a) 111 of 111, the debug cross-check included.b8986116df12ab276fdb86b4): the latency test passes; 4 reds outside this change (acpmux::session_pool a_dead_pooled_host...,acpmux::subsessions_safety ..., and the twomachine_update_journeyreds that test(cmux-tui): the machine journeys' older machine is older by its build time (#36044) #36151 fixes). More full runs for the latency test are in a comment below.python3 scripts/cmux-next/prepush-gate.py: no new violations.Observed / Not run
diag/brs-busy-nomerge,diag/brs-busy-fix-nomerge, not for merge).Changelog
Fixed: Opening a chat tab no longer slows down as its pane fills with chat tabs, and a busy session's writes keep less of the agent daemon's store lock.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes a latency regression where each chat tab create in a pane re-journaled every other chat tab of that pane, adding the journal and fold decoration cost for all of them under the registry lock.
The public fold holds decorated values, but the projection restated tabs raw, so upserts never matched and were never pruned. Kept upserts are now decorated before comparison, as the journal would write them, and journaled raw as before. Adds a regression test asserting one chat tab create restates at most its own tabs.
Written for commit 83bc15f. Summary will update on new commits.