cmux-tui: the terminal revision is read without the registry lock; a progress report reads only its own tabs

LA
7d ago
+205-15

Summary

On Austin's day (9 runs of the pa-d harness, /work/tmp/pa-d/ab-28147), the workspace registry lock is held about 78 % of the time. On the hook-only load that the shipped agent hooks make, two of its largest holders take the lock for work that does not need it:

  • terminal_registry_revision (mux/frontend_projection.rs:9): one committed value that identify and every list-workspaces reply read. 151,415 acquisitions in 9 runs, 7.0 % of the run time, mean 1.7 ms, max 1.9 s, spent mostly waiting for the connection behind writer commits.
  • publish_terminal_progress (mux/terminal_progress.rs:18): a terminal's progress report (OSC 9;4, program status) read the whole session topology (every screen, pane, tab and browser, with their side tables) to find that one terminal's tabs. 7.1 % of the run time, mean 5.4 ms, max 2.1 s.

This PR:

  • Gives the registry a read-only connection to its own file (RegistryConnection::read_committed). The database runs in WAL mode, so this connection reads the state committed when its read starts. It never returns a partial or uncommitted write, and an open write transaction never blocks it. Its own mutex is a leaf lock, held only for the read. An in-memory registry (no file) has no reader, and callers keep the locked read.
  • Reads terminal_registry_revision on that connection, with no registry lock. If no reader exists, or the read fails, it falls back to the locked read.
  • Makes publish_terminal_progress read only this terminal's own tab rows (resource_tabs_of_content). It still holds the registry for its commit.
  • Adds an agents-hooks scenario to the diagnostic bench (bench-daemon-latency.py --scenarios agents-hooks). It is the agents load with no CLI calls: no shipped hook, plugin or status line calls the cmux CLI on each turn. The bench is not a CI step. scripts/measure/perf-10x-daemon.sh stays at its reviewed version.

This is the next PR of the #27698 lock series. It is stacked on #28147 (head d8f9ee99c02a), so review only the commits after it. 1e (an agent hook that changes no projection takes no registry lock) builds on this PR's reader connection. #27780 (Austin) touches the same lock.

Owner: PL-daemon (session cmux-b6).

Testing

Tests, in cmux-tui-core/src/mux/tests/registry_reads.rs:

  • the_terminal_revision_read_does_not_wait_for_the_registry_lock holds the registry lock and reads the revision from another thread, with a 5 s bound.
  • a_progress_report_reads_only_its_own_terminals_tabs records each table the report reads, using a SQLite authorizer on the registry connection. It checks that the report reads none of the per-screen side tables that only a whole-topology load reads (resource_screen_rows, resource_column_docks, resource_lone_docks, resource_parked_splits), and that the report still publishes a revision.
  • a_committed_read_during_an_open_write_transaction_sees_only_committed_data holds a write transaction open that changes the revision, and reads on the reader connection from another thread. The read must not wait, must return the committed value and not the uncommitted one, and must return the new value after the commit. It guards the reader; it is not a red test.

Red, fleet step 1e49666eaf503381f65e5869, cmux-tui-rust-check.sh test registry_reads at 044b745d0dcf (the tests, without the fix):

  • the_terminal_revision_read_does_not_wait_for_the_registry_lock FAILED: "the terminal revision read waited for the held registry lock" (registry_reads.rs:40, after 5.07 s).
  • a_progress_report_reads_only_its_own_terminals_tabs FAILED. At that commit the test still had an earlier assertion, which was too strict. Its failure printed every table the report read (log line at 20:10:20Z):
    a progress report read resource_screens: {"json_each", "meta", "resource_browsers", "resource_column_docks", "resource_lone_docks", "resource_mutations", "resource_panes", "resource_parked_splits", "resource_screen_rows", "resource_screens", "resource_tabs", "resource_terminals", "resource_workspaces", "sqlite_master", "stash_capture_workspaces", "state_pending_changes", "terminal_hosts"}
    All four side tables that the shipped assertion checks are in it. So the shipped assertion also fails without the fix.

Green: cmux-tui-rust-check.sh all at the head 5b321e61f852, fleet step 1323ed5be0f10f315cc33d30. It compiled, fmt and clippy pass, and 12970 of 12983 tests passed. All three registry_reads tests passed. None of the 13 failures is this PR's:

  • Base reds, the same as at this PR's base (#28147's merge-head run aa49f608): the CLI noun-first path (#28119); the acpmux hermetic guard (#28150); 8 feat-cmux-next reds (agents_chat_tools x5, reply_in_place, pane-policy known params, integrity_check_scope), which feat-tip-reds is fixing.
  • Unexplained reds, listed in the handoff followups for a fixer (not cleared as flaky). This PR changes no file in their crates or paths:
    • graphics_pointer::evicted_known_undelivered_motion_allows_same_cell_retry: 6 single runs passed, one attempt each, 3 at the head and 3 at the base (e364c319, 44e68d12, f3a4ab25, 58063d9e, 8be434c3, 0479c3ff).
    • local_resize_bounds_the_drain_to_a_queued_snapshot.
    • apps_store::the_customize_folder_installs_itself_and_reloads_when_it_changes.
  • The gate skipped every alone re-run because 13 tests failed, more than its limit of 12.

Observed / Not run

  • Observed: the red and green above. The holder numbers come from the 9 Austin's-day runs of the pa-d harness, from the registry's top 8 holders. Those runs did not read the locks section, so they have no p99 per holder.
  • Not run: the before/after A/B of the two holders on the hook-only load. pa-d runs it on one host (before b4726639bf49, a scratch ref of this tree with the fix reverted; after fee4db7999f2; the product code is unchanged at this head). It will report the LockStats p99 of both holders, each round's difference, and a paired t-interval with n.

Performance

No perf numbers: perf claim: pending A/B (pa-d). This PR claims no measured speedup until the A/B above is posted here.

Changelog

Fixed: On a busy Mac, agents' terminal progress and cmux NEXT's identify requests no longer wait behind other saves.

Proof

No capture: daemon-only change, no UI.

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

Summary by cubic

Reduces time waiting on the shared workspace registry lock by moving two of its largest read holders onto paths that don't take it, and fixes a self-deadlock on the settle of a late registry commit.

The terminal registry revision, read by identify and every list-workspaces reply, now reads on a new read-only WAL connection to the registry's own file, with no registry lock; an in-memory registry or a failed read keeps the locked read. Terminal progress reports now read only their own terminal's tab rows instead of the whole session topology.

Bug Fixes

  • Settling a late indeterminate registry receipt no longer relocks state: the settle publishes only the journal event, avoiding a self-deadlock while it holds both the registry and the state lock.
  • Replaced the Option/expect() StateGuard with a direct MutexGuard, and removed panic-ratchet hits in the new histogram's indexing and arithmetic.

New Features

  • server-stats include=["locks"] reports the registry and state lock holders by site, with #[track_caller] naming the request path that holds the lock.
  • Added an agents-hooks scenario to the daemon latency bench (hook-only load, no CLI calls); diagnostic use only, not a CI step.
  • New tests cover the lock-free revision read, the narrowed progress-report read, and committed reads during open write transactions.