Skip to content

cmux-tui: read the registry identity without the registry lock - #27780

Merged
cmux-bot[bot] merged 6 commits into
feat-cmux-nextfrom
daemon-registry-identity-cache
Nov 1, 2026
Merged

cmux-bot[bot] merged 6 commits into
feat-cmux-nextfrom
daemon-registry-identity-cache

Conversation

@austinywang

@austinywang austinywang commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Part of the cmux NEXT lag audit (#27698 has the connection-limit side).

On a loaded nightly-next Mac, server-stats over 3.7 hours showed the workspace registry lock as the daemon's bottleneck:

  • waits: p50 14 ms, p90 5.2 s, p99 16.8 s, max 144 s;
  • holds: p99 655 ms, max 94 s;
  • 67,948 stalls across 155,310 acquisitions.

The app's 2 s requests timed out behind it: put-frontend-projection (629 times), close-workspace (537), list-workspaces resyncs (181, with a 10 s budget), identify (281).

Mux::registry_identity() took that lock only to read the registry's id and generation, two values fixed when the registry opens. Generation changes only on a reopen, which builds a new Mux. About twenty request paths read them, including:

  • every resource mutation's reply (resource_router::mutation_result);
  • list-workspaces;
  • identify (cmd_server);
  • attach generation checks (cmd_attach);
  • terminal commands (cmd_terminals).

So each of those requests queued behind a durable commit a second time, just for two constants. The pair is now read once in Mux construction, the same way session_public_id is, and returned without the lock.

Testing

Regression test: registry_identity_does_not_wait_on_the_registry_lock in cmux-tui-core/src/mux/tests/workspaces.rs. It holds the registry lock on one thread and reads the identity from another, with a 5 s bound.

  • Red: ./scripts/verify-cmux-tui-hosted.sh --filter registry_identity_does_not_wait_on_the_registry_lock at af7d305 (test only): RESULT_RED
  • Green: the same command at the head: RESULT_GREEN

Observed / Not run

  • Observed: the server-stats and app error-journal numbers above, on the nightly-next app's session daemon.
  • Not run: no before/after latency measurement on a loaded daemon. This removes one lock acquisition per affected request. The registry lock's other holders (durable commits with a synced WAL write each, personal_snapshot) still contend, and are listed in the audit as follow-ups.

Performance

No perf numbers: this removes one registry-lock acquisition per identity read; no before/after harness was run on a loaded daemon. The server-stats lock wait and hold numbers in the Summary are the observed baseline that motivated it.

Changelog

Fixed: cmux NEXT requests that only read the session's identity no longer wait behind saves on a busy Mac.

🤖 Generated with Claude Code

austinywang and others added 2 commits October 29, 2026 17:52
…ock (red)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
registry_identity() locked the workspace registry to read its id and
generation, two values fixed when the registry opens. About twenty request
paths read them: every mutation's reply (mutation_result), list-workspaces,
identify, attach checks. On a loaded Mac the registry lock waited p90 5.2 s,
p99 16.8 s (server-stats after 3.7 h: 67,948 stalls, 155,310 acquisitions),
so the app's 2 s requests (put-frontend-projection, close-workspace,
list-workspaces) timed out partly behind a lock taken for two constants. The
pair is now read once at construction, like session_public_id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review (subagent): SHIP at 8227e9e

  • Core claim verified: WorkspaceRegistry.registry_id and generation are private and assigned once, in initialize (workspace_registry.rs:2903-2906). Nothing writes either field afterwards, and nothing swaps or reopens the registry inside a live Mux. Session reset runs before a registry opens. Authority rotation uses the provider's separate u64 authority_generation. The old code read the same in-memory fields, so behaviour is identical in every case.
  • Constructors: there is one Mux { … } literal (mux/construct.rs from_workspace_registry), and every constructor goes through it.
  • Test: it compiles by reading. It's red at af7d305 (the reader blocks on the held lock and recv_timeout fails) and green at the head.
  • Minor, not changed: in the red case the detached reader's tx.send().unwrap() panics after the receiver is gone. That's harmless. Left as is so the head and its hosted runs stay the same.
  • Nit: generation is a fresh UUID on each open. The doc comment is accurate for one Mux's lifetime.

Primary agent's own check: I read the diff. The field is set from the same registry value the old lock read, at the one construction site, and the getter returns a clone. There's no other caller-visible change.

Review (claude-subagent): SHIP at 8227e9e

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 0789b33: 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=pr27780 -f ref=daemon-registry-identity-cache -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on c4a9ee3 to compare; do not merge until green; see lane-rules

Shipped in:

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 8 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp5c6vcjjb/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpu3ono_4a/summary-fail.json (live):/summary-fail.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
imported web bundle inputs are not routed:/imported web bundle inputs are not routed: View Logs
test_every_part_of_bun_run_check_runs_in_the_warn_branch (main.WebviewsWarnSplitCov
ersCheck.test_every_part_of_bun_run_check_runs_in_the_warn_branch)/
test_every_part_of_bun_run_check_runs_in_the_warn_branch (main.WebviewsWarnSplitCov
ersCheck.test_every_part_of_bun_run_check_runs_in_the_warn_branch)
View Logs

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

@lawrencecchen

Copy link
Copy Markdown
Contributor

Overlap note from the lock work for the 10-29 report (#27698): branch fix/pa-b3, PR into feat-cmux-next opening after its fleet green.

It touches the same workspace registry lock as this PR:

  • It fixes a self-deadlock. Mux::lock_state_pinned settles the late answers of indeterminate registry commits (cx-g1fa.3) while it holds the registry and mux.state. The settle ran publish_resource_event, whose publish_pending_terminal_directories locks mux.state again: a lock-rank panic in debug, a deadlock of the whole daemon in release. The red test fails on the base with that lock-rank panic (fleet step a551489a).
  • It adds server-stats include:["locks"], which reports the registry and state locks by #[track_caller] site, and adds #[track_caller] to the Mux lock wrappers.
  • Files: diagnostics.rs, mux/signaled_mutex.rs, mux/journal.rs, server/server_stats.rs, mux/resource_effects/effect_commit.rs, about 20 wrapper files (#[track_caller] lines only, including mux/resource_effects.rs), mux/tests/writer_lock_order.rs, and the SDK schema and bindings.

registry_identity is one of the wrappers. My branch does not change its body, so the only textual contact with this PR is in mux/resource_effects.rs. @austinywang, please check the overlap. I expect either order to merge cleanly.

@lawrencecchen

Copy link
Copy Markdown
Contributor

Correction: my branch does not change registry_identity; the only shared file is mux/resource_effects.rs.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Review (codex gpt-6-astra high fast, drain lane): SHIP at 8227e9e

No blocking findings.

@austinywang

Copy link
Copy Markdown
Contributor Author

Sweep fix: (1) pr-perf-section-check: added a ## Performance section with a No perf numbers: reason to the description (checked locally against scripts/ci/pr_perf_section_check.py: passes). (2) clippy (changed cmux-tui packages) was this PR's own: unused_qualifications on std::time::Duration in the new test; fixed. (3) Merged origin/feat-cmux-next cleanly for the stale-base reds (Swift compile/fast tier, guard tests). New head 0789b33. The head needs CI and a review receipt before relabel.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Independent review for awareness (codex gpt-6-astra, high, fast; read-only, no builds), at head 0789b3388af6b7b4876a57561d4685cfcbbfc85a against the merge base with current feat-cmux-next:

No verified defects found. The cached identity preserves behavior and the regression test covers lock independence. No builds/tests were run, per instruction.

VERDICT: SHIP at 0789b33 — no concrete correctness, lifecycle, or concurrency defects found.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Review (codex gpt-6-astra high, next-review): SHIP at 0789b33

SHIP at 0789b33

Visual verdict: no images; Leo tries the build

No correctness findings. Both identity fields remain immutable after registry construction, and all Mux constructors use the shared initialization path. The regression test checks reads while the registry lock is held.

Tests were not rerun: this box lacks Cargo and Zig.

Follow-ups: none

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Review (next-review patch-id; carried): SHIP at 706c849

The independent SHIP at 0789b33 carries to this head: the PR's own change (git diff $(git merge-base feat-cmux-next HEAD) HEAD) has the same git patch-id --verbatim (ec01c41498ca71b5) at both heads, and no merge between them was resolved by hand. Nothing new to review. A newer BLOCK, or any change to the PR's own diff, ends the carry.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Review (next-review patch-id; carried): SHIP at 19cc5b4

The independent SHIP at 0789b33 carries to this head: the PR's own change (git diff $(git merge-base feat-cmux-next HEAD) HEAD) has the same git patch-id --verbatim (ec01c41498ca71b5) at both heads, and no merge between them was resolved by hand. Nothing new to review. A newer BLOCK, or any change to the PR's own diff, ends the carry.

@cmux-bot
cmux-bot Bot merged commit fe2c12f into feat-cmux-next Nov 1, 2026
190 of 192 checks passed
@cmux-bot
cmux-bot Bot deleted the daemon-registry-identity-cache branch November 1, 2026 23:24

This branch was previously deployed

3 inactive deployments
ci-routing-agent — 19cc5b41 Deployed Nov 1, 2026 by github-actions[bot] via cmux-next macOS placement #60369
artifacts — 19cc5b41 Deployed Nov 1, 2026 by cmux-bot[bot] via publish tree-addressed cmux-tui #97784
sentry-dsyms — 19cc5b41 Deployed Nov 1, 2026 by cmux-bot[bot] via upload cmux-tui dSYMs to Sentry #97784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants