Skip to content

perf(cmux-tui-core): a pane or tab list inside one workspace reads only that workspace (CLI-1) - #32177

Merged
lawrencecchen merged 4 commits into
feat-cmux-nextfrom
perf/cli-scoped-lists
Nov 2, 2026
Merged

lawrencecchen merged 4 commits into
feat-cmux-nextfrom
perf/cli-scoped-lists

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Under the L1 CLI storm against a 40-workspace / 200-terminal daemon (fleet run 360f44a009fb64208b8e3ab6, STATUS 6237), the largest holder of BOTH global daemon locks was public_session_snapshot (resource_api.rs:553): 22.2 s of registry hold and 22.2 s of Mux.state hold in a 60 s storm (37 %), 2,117 calls at about 10.5 ms each. Its callers were CLI tab list --pane P, pane list --workspace W and terminal list --pane P: each built the whole session snapshot (every workspace, screen, pane, tab, terminal, browser, notification, agent and frontend projection) and then kept one pane's rows. Idle, tab list and pane list grew from 6 ms at 1 workspace to 14 ms at 40; under the L6 app load (STATUS 6075) tab list was 50 ms p50 and every other daemon call waited behind these holds.

Fix: a pane.list or tab.list whose resolved path names a workspace (a --workspace, --screen or --pane scope) reads that workspace's stored subtree (resource_topology_scoped, the read the scoped projection already uses) under the registry lock only, not the state lock, and builds the same pane and tab values in the same order from one shared builder (public_pane_and_tab_values, now also used by the session snapshot), with the same tab decoration. A session-wide list keeps the snapshot.

Not in this PR: terminal.list --pane (a terminal value needs live runtime state; next PR), screen.list.

Owner: PL-daemon (cmux-tui daemon, cmux-tui/crates/cmux-tui-core/).

Changelog

none

Testing

Regression test: scoped_pane_and_tab_lists_read_only_their_workspace (cmux-tui-core, mux/tests/read_paths.rs). In a two-workspace session with a split and extra tabs, pane.list --workspace, tab.list --pane and tab.list --workspace must equal the session snapshot's rows for that scope, read no full topology, read no terminal host, projection or journal table, and write nothing.

  • Red at bf60e2d (test only): step 578ce041959f5ceafee6d050 FAILED as expected: pane.list {"workspace":...} read the whole topology (its value check passed on the base, so the expected rows are right).
  • Green and full cmux-tui suite at 6fd44a3: step aadb36b5a8e67ad9d51f110b: 15,151 of 15,154 passed, the new test among them. The 3 failures are the same base reds as on every head today, in files this PR does not touch (acpmux every_test_binary_runs_in_the_hermetic_environment and usage_stats::the_session_list_and_info_carry_what_the_chats_transcript_says, cmux-tui share_chat_waits_for_the_owners_answer).

Command: cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --ref SHA --os linux --arg test --arg scoped_pane_and_tab_lists_read_only

A/B: interleaved before (23de9c2) / after (6fd44a3) runs of perf-10x-daemon.sh --arg=scenarios=cli-storm are queued on one mini (6c5c4e8f, 6633e96b against b4c6acd1, 1cc4271f); the numbers are added here when they end.

Observed / Not run

Observed: the base lock split (fleet run 360f44a0), the red and suite steps above.
Not run: an L6 round in the real app with this change.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes pane.list and tab.list calls scoped to a workspace, screen, or pane building the entire session snapshot and holding both global daemon locks. A scoped list now reads only that workspace's stored subtree under the registry lock alone, producing the same pane and tab values in the same order as before. Session-wide lists still read the snapshot; terminal.list and screen.list are unchanged.

Testing

  • Adds a regression test asserting scoped lists match the session snapshot's rows and read no full topology, terminal, projection, or journal tables.
  • Adds a cli-storm scenario to bench-daemon-latency.py that runs the L1 CLI mix against an L5-sized session and reports daemon lock and topology projection splits.

Written for commit a7c4d74. Summary will update on new commits.

View guided diff Turn on auto-fix

…ed session with lock and projection split (CLI-1)
…ly that workspace (CLI-1)

Under the L1 CLI storm (STATUS 6237, fleet run 360f44a0) the largest
holder of both global daemon locks was public_session_snapshot: 22 s of
registry hold and 22 s of state hold in 60 s, from CLI `tab list`,
`pane list` and `terminal list` calls that built the whole session
snapshot (every workspace, terminal, notification, agent and projection)
and kept one pane's rows. A pane.list or tab.list whose path names a
workspace now reads that workspace's stored subtree under the registry
lock alone (as workspace.list already reads only workspace rows), with
the same pane and tab values in the same order, from one shared builder.
A session-wide list keeps the snapshot.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 6fd44a3: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr32177 -f ref=perf/cli-scoped-lists -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 6fd44a3

I read the code only: no build and no test run. Scope: resource_api.rs, resource_router/topology.rs and mux/tests/read_paths.rs. I skimmed the bench script.

I found no Blocker, High or Medium issues.

1. The values are the same as the snapshot's.

  • Pane focused: load_resource_topology_scoped starts from load_topology_frame. That function fills active_workspace from meta and active_screens for every live workspace. The scoped screens_by_id has every screen of the workspace, so screen_focused and active_pane give the same result.
  • Pane zoomed: it comes from the same screen row.
  • Tab focused: it is pane.active_tab, and panes_by_id has every pane of the workspace.
  • Tab value: it comes from the same RegistryTab::public_value. The browser URL and terminal id come from the same load_tabs LEFT JOIN.
  • Decoration: decorate_snapshot decorates only workspaces, screens and tabs. The scoped path decorates tabs with the same decorate_value(connection, "tab", ...), which also sets the conversation content_kind. Neither path decorates panes.
  • Per-connection projection: the projection that changes conversation, agent-session and page tabs back to browser for older clients (conversation_tabs_wire::project_conversation_tabs) runs on the outbound response text in message_writer.rs. Both paths return the same Value, so the projection acts the same way on both.

2. The order is the same.

  • Public ids derive Ord on String, which compares bytes. The ids are ASCII lowercase hex, so this order is the same as SQLite BINARY collation.
  • creation_ordinal is a non-negative i64 stored as u64, so its order does not change.
  • The pane sort key (screen_id, creation_ordinal, public_id) is the full load's ORDER BY. Filtering a globally sorted list to one workspace keeps the same relative order.
  • The tabs' (pane_id, position) has no ties: the unique partial index live_resource_tab_position covers it.

3. Consistency.

  • The scoped path holds the registry lock and pins the connection once. The registry connection is the only connection that writes. The journal writer also needs that connection lock, so all the SELECTs and the decoration read one committed cut. This is the same pattern as public_workspaces.
  • The snapshot's panes and tabs never read State. Only terminals and browsers read it. So dropping the state lock changes nothing for these collections.
  • Path resolution reads live state, then the scoped read uses the DB. The old path did the same, and SnapshotPathIndex still requires a workspace match. If a pane moved and state and DB differ for a moment, both paths return the same empty list. A workspace closed between the two steps gives an empty list on both paths.

4. Scope selection.

  • resolve_resource_selectors sets path.workspace for Workspace scope from the selector. For Screen and Pane scope it sets it through resource_indexes.
  • If an index entry is missing, path.workspace is None and the code falls back to the snapshot, which is correct.
  • SnapshotPathIndex only needs id and workspace_id from the screens. It gets those, and all of the workspace's panes, from the scoped value. The terminals and browsers collections are not routed to the scoped value.

5. The test.

  • The red at bf60e2d fails for the right reason: the snapshot increments FULL_TOPOLOGY_READS and reads terminal_hosts, projections and journal tables.
  • test_mux starts with no workspace, so panes[0] is always in a two-pane workspace. The pane_ids.len() >= 2 check is not flaky.
  • Each case asserts that its expected rows are not empty.
  • The thread-local counter is valid because handle_resource_message dispatches on the test thread.
  • The table list is correct for the fixed path. The fixed path reads meta, resource_workspaces/screens/panes/tabs, the screen side tables, resource_browsers, resource_terminals (from the join) and tab_presentation/tab_groups/tab_state. None of these is in NOT_A_TAB_TABLE.

Low findings

  • Low, test coverage (mux/tests/read_paths.rs, new test). Each workspace has one screen, so nothing exercises the pane re-sort in public_workspace_topology. The scoped loader walks screens by position, and the full load sorts by screen id. Also, the chosen workspace is the active one only when its random screen id sorts first, so focused: true is compared in only about half of the runs. A second screen in the workspace, and a scope pinned to the active workspace, would make both checks deterministic. Today a regression in the pane sort or in the frame's active-screen fill could pass.

  • Low, performance (resource_api.rs, public_workspace_topology).

    • pane.list decorates every tab of the workspace. That is three or more queries per tab under the registry lock, and the result is discarded.
    • tab.list --pane decorates every tab of the workspace, not only that pane's tabs.
    • The scoped loader also reads resource_browsers rows and the screen side tables, which neither list uses.
    • This is still much less work than the full snapshot. Decorating after the SnapshotPathIndex filter, or not at all for panes, would make the cost follow the result size. This PR exists to cut lock hold time, so a follow-up is worth it.
  • Low, doc (resource_api.rs, the public_workspace_topology doc comment). It says "It reads no ... terminals". But load_tabs LEFT JOINs resource_terminals and resource_browsers, and the scoped loader reads browser rows. Change it to "no terminal hosts", or similar.

  • Low, info (resource_api.rs, public_workspace_topology). The old path went through lock_state_pinned, which first waits up to a limit (drain_unsettled) for earlier indeterminate journal commits. The scoped read, like public_workspaces and session.ping, does not wait. A list right after an indeterminate commit can therefore show the pre-commit rows, where the old path could show the post-commit rows. That earlier request already got an indeterminate reply, so this breaks no read-your-writes promise. No change is needed.

  • Bench script (bench-daemon-latency.py). I found no obvious bugs. One note: if the probe connection drops, the probe's read_line loop has no exit except the socket timeout. That is acceptable for a bench.

@blacksmith-sh

blacksmith-sh Bot commented Nov 2, 2026

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpq1bqufji/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmpxodzdq_1/summary-missing.json (live):/summary-missing.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 2, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 11:33 Inactive
@lawrencecchen
lawrencecchen merged commit cfae5ba into feat-cmux-next Nov 2, 2026
161 of 165 checks passed
@lawrencecchen
lawrencecchen deleted the perf/cli-scoped-lists branch November 2, 2026 11:46
cmux-bot Bot pushed a commit that referenced this pull request Nov 2, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

A/B after merge (headless daemon at 40 workspaces / 200 terminals, perf-10x-daemon.sh --arg=scenarios=cli-storm, 8-worker CLI mix at 120/s, 7,200 calls per run, one worker cmux23-perf, interleaved before / #32157 / #32177 x2). p50/p95/max ms.

kind before r1 before r2 #32157 r1 #32157 r2 #32177 r1 #32177 r2
tab.close 45.8/116/199 42.5/121/170 32.4/96/134 35.8/96/144 31.7/69/159 34.0/87/155
tab.list 22.3/43/71 22.6/43/72 21.8/42/75 21.1/43/94 10.2/26/53 10.1/26/49
pane.list 21.2/43/68 22.0/45/70 21.3/44/82 21.3/43/70 10.1/25/49 10.0/26/52
workspace.list 14.9/36/62 15.2/37/52 14.4/36/61 13.9/38/65 10.6/25/59 10.7/26/76
all calls 19.4/57/199 19.6/58/170 18.9/53/153 19.3/55/146 12.3/37/159 12.5/39/155

Whole-snapshot hold of both global locks (resource_api.rs:553): 17.7 s of the 60 s storm before and with #32157; 4.2 s with #32177 (only terminal.list left; a follow-up scopes it). Steps: e1db8d4f, 1c92eb84 (before), f73bf071, 8872535e (#32157 at 9bd4028), de2b4396, 97476920 (#32177 at 6fd44a3).

This branch was previously deployed

3 inactive deployments
artifacts — a7c4d74e Deployed Nov 2, 2026 by cmux-bot[bot] via publish tree-addressed cmux-tui #100943
sentry-dsyms — a7c4d74e Deployed Nov 2, 2026 by cmux-bot[bot] via upload cmux-tui dSYMs to Sentry #100943
ci-routing-agent — a7c4d74e Deployed Nov 2, 2026 by cmux-bot[bot] via cmux-tui tree preflight #100943
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant