acpmux: a shared, paced full session list for every client (3 stale clients 82% -> 4% of a core)

Merged
TE
9d ago
+986-31

Asked: "build the daemon-side guard: cache, or coalesce, full session-list replies per client and rate-limit them, so a stale or buggy client can never cost more than a few percent of a core again. Make it a system guard, not a one-off; a test drives one client at 1 Hz full-list requests and asserts bounded CPU. Codex xhigh review, then land."

Problem

A dogfood Mac's idle acpmux daemon sits at 30-41% of a core. Three cmux-tui servers built before #26255 reconnect about once a second and ask _acpmux/watch for the whole session list each time. In a 10 s sample of the daemon, about 36% of busy time goes to building every session summary, 44% to serializing and cloning the reply, and 12% to freeing it. #26255 fixed that client, but any old or buggy client can still do the same thing: every full-list request rebuilds and reserializes the whole list.

Fix: one guard in the daemon, for every client

New hub/list_guard.rs and server/list_reply.rs. _acpmux/sessions, _acpmux/watch and ACP session/list all read the list through the guard. No client name is special-cased.

  • One build, shared. A snapshot is the list serialized once ({"sessions":[...]}). A whole-list reply goes out as those bytes: no JSON tree and no deep copy per request. A page (limit/cursor) or a filter (peer/status) is cut from the same bytes. Each row keeps its sessionId, updatedAt, peer and status, and session_pages::session_list_reply picks the rows by them, so paging the whole list costs no extra build. The bytes are identical to the old replies; unit tests compare them byte for byte. A peer still gets masked summaries (peer_text.rs); the masked text is also made only once per build.

  • Never stale for a change a client is owed. A snapshot is reused only while nothing a watcher hears of has changed since it was built. Three kinds of change count:

    • every send on the hub's event channel (it is now a small Events wrapper that counts sends);
    • every attach, detach and visibility change;
    • every write to the peers' rows;
    • every change of a peer's link state (each peer row's peerState).

    It is reused only within its window, and never while the chat logs are still loading. A request that finds the snapshot out of date gets a build that starts after the request arrived. Fields that change with the clock alone (stalledSince, an expiring tag) are at most one window old. _acpmux/watch still turns the watch on before it reads the list.

  • A watcher never gets a list older than a notice it was sent. The reply is queued only if no session_changed notice was queued on that connection since the list was read. The check holds the connection's outbound lock, which every notice takes too. If a notice got there first, the list is read again once. If a notice overtakes it a second time, or the reply had to wait for room in a full outbound queue and a notice went first, the reply goes out followed by _acpmux/lagged {watch: true}, which tells the client to read the list again.

  • Paced. Up to 4 builds run back to back, then one per window. The window is 32 times what the last build took, capped at 15 s. Full-list builds then take about 1/32 of a core however many clients ask. A request shares any build that began after it arrived (once every chat log is read), so while chats keep changing, everyone waiting shares the next build.

  • Per connection, and all connections together. One connection reads up to 2 lists back to back, then one per window. All connections together read up to 8 per window; that covers a client that opens a new connection for each list. A page costs its share of the rows (limit / rows), so reading every page once costs one list. A page always costs at least 1/16 of a list, because finding it reads every row's key. The first list is built before any pacing, so the pace is known from the first request on. A request that would wait more than 30 s is refused with a retryable list.paced error that carries retryAfterMs, so a flood never builds an unbounded queue.

Timers and caches, with their stop conditions:

  • Each build spawns one task that sleeps for that build's window, drops the snapshot unless a newer build replaced it, and ends. It holds only a weak reference to the guard's state.
  • The snapshot itself lives at most until that task runs, or until the next build replaces it.
  • A connection's pace is a field of the connection and goes with it.
  • There is no periodic timer; daemon_idle still passes.

Memory. The guard keeps the list it just sent, serialized, for its window. In one job, five runs of tests/daemon_memory.rs read 158-160 MB on base and 160-164 MB with this PR, so its budget goes from 160 to 166 MB, with the reason in the test. The build copies what it keeps only after the summaries are freed, so glibc can give the summaries' heap back.

Before / after CPU (same harness, same job)

Linux 32 vCPU cmux-ci job at the final head (run 40645955696):

  • a release acpmux daemon over 2,800 seeded idle chats (a 14.5 MB list, like the dogfood Mac's 14.7 MB);
  • three clients like the pre-#26255 servers: connect, _acpmux/watch, read the reply, hang up, again a second later;
  • daemon CPU read from /proc/PID/stat over 180 s.

Base is feat-cmux-next 489e69743; after is this PR. Only this PR's files differ between the two runs.

daemon CPU, 3 stale clients, 180 slists servedidle, no clientsRSS
before47.3% of a core6000.00%620 MB
after4.2% of a core6000.00%569 MB

Base read 47-83% across five jobs on this harness; the guard read 3.7-4.5% in each. The base number moves between jobs; the guard's stays at about 4%. The clients still get their one list per second: the guard paces them, and none waits anywhere near 30 s, so none is refused.

Test

tests/daemon_list_guard.rs runs the real daemon binary over 400 chats with summaries of the real size (about 5 KB each). It measures the daemon's process CPU over 15 s per scenario and asserts at most 8% of a core in each. It then checks the replies are still correct:

  • after each of 6 renames, the next list shows the new name, and a watcher hears the change;
  • a watcher keeps asking for the list while another client renames a chat 60 times, and no list lacks a rename it was already told of;
  • reading the list a page of 64 at a time gives the whole list, in order.

Base and an earlier head in one job (run 40609013380), and the final head (run 40650191654); cmux-ci, Linux, debug build:

scenario (400 chats)basethis PR (final head)
one client reconnecting at 1 Hz3.4%2.2%
one connection asking at 1 Hz3.3%2.0%
three clients reconnecting at 1 Hz10.5%2.5%
one client reconnecting as fast as it reads61.5% (290 lists)5.1% (218 lists)
  • Base: the test fails (over 8% of a core on "three clients" and "as fast as it reads").
  • This PR: it passes. The other checks in the same job:
    • unit tests for:
      • the byte-identical whole list and pages;
      • the per-connection and global pace, the least a page costs, and the 30 s cap;
      • reuse until a change (never while logs load), and sharing a build that began after the request;
      • the window expiry;
    • hub_integration 511 passed, peer_integration 50 passed, daemon_idle and daemon_memory passed;
    • a peer's link state change rebuilds the list, and a build made while the chat logs load is never shared;
    • cargo clippy -p acpmux --all-targets -D warnings is clean.
  • agent_tools::tests::own_servers_run_under_the_daemon_prefix fails on base too, so it is not this PR's. cli::codex_auth_broker failed once in some runs and passed in others; this PR does not touch it.

Review

An independent Codex reviewer (gpt-6-astra; xhigh for round 1, high after) went five rounds. It said SHIP, no findings at the head, 768b4cbb8fc8. At that head, PR CI clippy (changed cmux-tui packages), rustfmt and rust workspaces are green, and so is a Linux cmux-ci job. Its earlier findings, all fixed:

  • a list built during a change could follow that change's notice;
  • the pace's queue had no bound;
  • a page of one row was charged next to nothing;
  • the first requests skipped the global pace;
  • a build made while the chat logs load could be shared;
  • peerState was missing from the key, and then was keyed by a hash sum where it needed an exact value;
  • a reply that waited for room in a full queue could follow a notice.

Risk

Medium: every full-list read now goes through the guard.

  • A client that floods list requests past a 30 s wait gets a retryable list.paced error (with retryAfterMs) instead of a list.
  • Correctness rests on every change a list shows counting as a change. Event sends, attaches and peer-row writes cover what clients are notified of. Clock-only fields can be up to one window old: 15 s at most, and about 5 s for 2,800 chats.
  • Under heavy load a client waits up to one window for a fresh list, instead of the daemon spending the CPU.

Overlap

pr-overlap lists open PRs that touch the same files (dispatch_request, server/mod.rs), none with the same fix: #25189, #26127, #26203, #24797, #22030. #23051 (open) removes a different per-reply copy in rpc.rs; this PR does not touch rpc.rs.

Takes back #25054: only in form. The _acpmux/sessions arm moves into server/list_reply.rs, and its peer/status filters (#25054) are kept: they are cut from the shared list by the same session_list_reply.

Size: one guard, its two callers and its daemon test: about 1,000 changed lines, about 420 of them tests. It stays one PR because the guard, its reply path and the test that fails on base only mean something together.

Agent session: agent session
šŸ¤– Generated with Claude Code