Skip to content

Typing under load (F4 follow-up B): a key takes no session lock when a workspace has a size default - #32255

Merged
cmux-bot[bot] merged 5 commits into
feat-cmux-nextfrom
perf/typing-fix-2-size-default
Nov 2, 2026
Merged

cmux-bot[bot] merged 5 commits into
feat-cmux-nextfrom
perf/typing-fix-2-size-default

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up B of #31940 (TYPING UNDER LOAD F4). Stacked on #32249 (base branch perf/typing-fix-2-scoped-route; it retargets to feat-cmux-next when #32249 lands). Only the last two commits are this PR.

Each key notes its view's activity in the terminal sizing engine (note_terminal_activity_of). That mutation went through mutate_terminal_sizing, which always re-expands the host size policy, because a mutation may add, remove or rename a participant. The re-expansion first asks whether the terminal follows a workspace default (follows_host_policy -> workspace_default_of), which looks the terminal's workspace up under the session lock, and may walk every surface for its placements (runtime_placements). So as soon as any workspace had a size default, every key took the session lock again, behind automation.

A key's activity changes no participant and no identity, and the host policy's expansion depends only on those. Activity now goes through mutate_terminal_sizing_keeping_participants: the same mutation, grid apply and publications, without the host policy refresh. Every mutation that adds, removes or renames a participant still refreshes it. A workspace default, terminal override or host policy change still re-applies to the terminals it covers (set_workspace_size_policy, adopt_host_size_policy).

Testing

Regression test (cmux-tui-core server::tests::input_lane): a_workspace_size_default_keeps_the_key_off_the_session_lock. With the settings owner's host policy and a size default on the typing terminal's workspace, the second key of an attached client must take the session lock 0 times (a lock-take counter on the key's thread; no timing).

Command: cmux-ci run --class isolated --os linux --script scripts/ci/cmux-tui-rust-check.sh --arg test --arg input_lane --ref <sha>

  • RED at the test commit 3496e49, 5/5 (left: 1, right: 0): steps 69cc2ac5fb3ef248808816da, f4f3e7ff6e6a5bec4159ddd2, 841b858ce4ef613a6267a6f1, 8a215f32c596cd120a66b7ea, e95e360d3d9e98db3637b2d7.
  • GREEN at the fix 589a571, 5/5 (input_lane 6 of 6): steps 40fd3541e29b06b184606daf, 362a515619bb35e5bdbc3c5a, 1336a18c48f8e373de0c5314, ce36f763a2285fa8f5ea7409, 38f2bb5e0a2b652dd0641a48.
  • Sizing suites at 589a571 (test sizing): 57 of 57 pass (step 38481cb32fc72fbca15efed9), including host_size_policy (latest follows input, a workspace default of any placement, policy revisions).

Review note (non-blocking, from the SHIP review): a key no longer re-resolves the host policy, so a tab moved out of a workspace with a size default gets the host policy back at its next attach, report, detach or focus click, not at its next key. A move into a workspace with a default was never re-resolved; a follow-up can re-resolve the policy on a move.

Observed / Not run

Changelog

Fixed: Typing in a terminal no longer waits behind scripts and agents when a workspace has a terminal size default

Proof

No capture: daemon-only change, no UI.

…he session lock (TYPING FIX 2, #31940 follow-up A)

The terminal kept for the person's keys (#31940) is checked against one
process-wide route generation that every routing write moves. A tab
create or close in any workspace (an agent's tab storm) therefore sent
every attached client's next key back to the session lock: O(clients)
lock takes per routing change, during the very load the lane exists for.

The test counts the session lock acquisitions of the key's turn on its
own thread (a test-only counter in StateMutex), so it needs no timing:
after a new workspace, a tab in it and that tab's close, the next key
must take the lock 0 times. Red at this commit: it takes it once.
…nges (TYPING FIX 2, #31940 follow-up A)

#31940 checked the terminal kept for the person's keys against one
process-wide route generation that every routing write moved. Every tab
create or close anywhere (an agent's tab storm, a CLI new-workspace)
therefore sent every attached client's next key back to the session
lock: O(clients) lock takes per change, under the load the lane is for.

Now each surface id has its own route generation (a fixed array of 1024
slots, id % 1024: bounded memory, nothing to remove, lock-free reads).
A RouteTable write moves only the ids whose route it changes (RouteKey):
a surfaces or terminal_catalog_by_runtime write its key, a
terminal_catalog write the runtime ids of the owner it removes and the
owner it puts in. terminal_catalog_by_runtime is a RouteTable now too,
so every table an id routes through moves that id. Two ids in one slot
only make a key look its terminal up once more, never use a stale one.
The respawn, rollback and mutation tests of #31940 keep their meaning.
…n lock (TYPING FIX 2, #31940 follow-up B)

Each key notes its view's activity in the sizing engine. That mutation
re-expanded the host policy, which asks whether the terminal follows a
workspace default (workspace_default_of), which looks the terminal's
workspace up under the session lock, and may walk every surface for its
placements, as soon as any workspace has a size default. So with a
workspace default every key took the session lock again.

Red at this commit: with the settings owner's host policy and a default
on the typing terminal's workspace, the second key takes the lock once.
…FIX 2, #31940 follow-up B)

Every sizing mutation re-expanded the host policy for its terminal,
because participants may have come, gone or changed identity. A key's
activity note changes none of them, and the expansion depends on
nothing else a key changes. But the refresh first asks whether the
terminal follows a workspace default (workspace_default_of), which looks
the terminal's workspace up under the session lock and may walk every
surface for its placements; with any workspace size default set, every
key took the session lock again, behind automation.

Activity now goes through mutate_terminal_sizing_keeping_participants,
which is mutate_terminal_sizing without that refresh. Every mutation
that adds, removes or renames a participant still refreshes it, and a
workspace default, terminal override or host policy change still
re-applies to the terminals it covers.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-subagent, correctness first): SHIP at 589a571

What I checked (code reading only, no local build or test):

  • host_policy_for reads the host policy, terminal overrides, workspace defaults and host_policy_participants, which gives (id, priority_key, device_kind -> mac, device_id, member view/client). TerminalSizingEngine::note_activity changes only entries[i].activity. publish rebuilds participants in the order of entries, which is attach order, so activity never reorders them. priority_key comes from identity. So a key cannot change the expansion. Latest expands to TerminalSizingPolicy::new(Latest, [], None) with no participant input, and "latest" itself is engine-level (activity clock), so skipping the refresh does not change the grid. host_policy_prefer_the_active_device_follows_input goes through note-size-activity -> note_terminal_activity -> note_terminal_activity_of (the new path), and it still holds.
  • With a workspace default, follows_host_policy is false, so the skipped refresh was a no-op that only took the lock.
  • Every participant, identity, override, default and host-policy change still refreshes: attach/sync, detach, client disconnect, refresh_terminal_client_identity, set_workspace_size_policy and adopt_host_size_policy.

Non-blocking notes:

  • Before this change, a key's refresh also healed one case. A tab moved from a workspace with a size default to one without kept the old default until the next refresh: nothing re-resolves the sizing policy on a workspace move (workspace_size_policies is read only in workspace_default_of). Now only a focus claim, attach, report or detach heals it. The move into a defaulted workspace was never healed. This is a gap in the move path that existed before this PR. A follow-up could re-resolve the policy on a move.
  • When ownership flips under latest (two devices alternating), apply_terminal_grid and publish_size_states still call Mux::surface (the state lock) on that key. A single typing device in steady state takes no lock. This is out of scope here.

@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
Base automatically changed from perf/typing-fix-2-scoped-route to feat-cmux-next November 2, 2026 13:07
…-fix-2-size-default

# Conflicts:
#	cmux-tui/crates/cmux-tui-core/src/server/tests/input_lane.rs
@lawrencecchen lawrencecchen removed 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
@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 24cb4f6: 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=pr32255 -f ref=perf/typing-fix-2-size-default -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-subagent, correctness first): SHIP at 24cb4f6

Re-check after the merge of origin/feat-cmux-next (627d40b). I read the code only and did not build.

  • git diff origin/feat-cmux-next...24cb4f6 touches only mux/terminal_sizing.rs and server/tests/input_lane.rs. Without index and hunk-header lines, it is byte-identical to the reviewed b7bc10a..589a571.
  • Typing under load (F4 follow-up A): a tab opened or closed elsewhere no longer sends every client's next key to the session lock #32249 landed as 6fdfe98, and its patch is identical to the reviewed one.
  • No other feat commit since the reviewed base 1ae75c7 touches terminal_sizing*, cmux-terminal-sizing, cmd_terminal_io, client_registry*, route_table, signaled_mutex or creating_input. None changes mutate_terminal_sizing, note_terminal_activity_of, refresh_host_policy_locked, input_surface or send_key.
  • The new scoped close projection (close_effects.rs, resource_effect_projection_scoped_locked) and the respawn gate (terminal_respawn.rs, supervisor.rs) are still built and installed under the same state guard. They add no route-table write outside that guard.

The non-blocking notes of my first review are unchanged.

@cmux-bot
cmux-bot Bot merged commit ae55c24 into feat-cmux-next Nov 2, 2026
213 of 217 checks passed
@cmux-bot
cmux-bot Bot deleted the perf/typing-fix-2-size-default branch November 2, 2026 14:03
@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

This branch was previously deployed

3 inactive deployments
ci-routing-agent — 24cb4f62 Deployed Nov 2, 2026 by github-actions[bot] via cmux-next macOS placement #63041
artifacts — 24cb4f62 Deployed Nov 2, 2026 by lawrencecchen via publish tree-addressed cmux-tui #101515
sentry-dsyms — 24cb4f62 Deployed Nov 2, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #101515
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