Repository navigation
perf(acpmux): track only live process slots for idle census (SW-11) - #30712
Conversation
GitHub Actions
Inactive
|
SW-11 CI triage: |
GitHub Actions
Failure
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
web and size checks: green Shipped in:
|
|
Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over. Taking this ChatGPT draft over for Leo's ask ask-e06a62154f (acpmux / mux-host batch): reviewing against current feat-cmux-next, then carrying it to land on a lane/ branch or closing it. |
097dd79 to
cd0e3bb
Compare
GitHub Actions
Inactive
|
Found 14 test failures on Blacksmith runners: Failures
...and 4 more test failures. View all on Blacksmith: run 41372930222, run 41372930013
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
|
For Leo (teamleaderleo): this PR made the Rust panic ratchet red on feat-cmux-next (acpmux clippy::indexing_slicing 687 > 686: processes.rs lose_running |
|
Base red from this PR (author: Leo, teamleaderleo). |
…ratchet indexing_slicing 687 > 686, #30712) (#31695) #30712 (5a0fd07) ends each running process through self.processes[index], one clippy::indexing_slicing hit over the acpmux baseline (687 > 686), so the Rust panic ratchet is red on feat-cmux-next. It now reads the slot with get_mut and skips a slot with no row; a running slot always names a row (ProcessTable::insert), so nothing changes. Fleet panic step (cmux-tui-rust-check.sh panic): RED at origin/feat-cmux-next 34446e1. Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
…uspicious_open_options, #31462) (#31697) * base red: cmux-tui Cargo.lock matches #31462's fs4 dependency #31462 added `fs4.workspace = true` to crates/cmux-tui/Cargo.toml, but its Cargo.lock change put "fs4" in the dependency list of cmux-server, not cmux-tui. Every `--locked` cargo step on feat-cmux-next at 765d3c4 fails with "cannot update the lock file cmux-tui/Cargo.lock because --locked was passed". The lock here is what `cargo metadata --manifest-path cmux-tui/Cargo.toml` writes (no hand edit): "fs4" moves from cmux-server to cmux-tui. Red at d3aaa26: cargo metadata --locked --manifest-path cmux-tui/Cargo.toml -> error: cannot update the lock file ... because --locked was passed Green on this commit: the same command exits 0. (cherry picked from commit d59b646) * base red: acpmux lose_running reads its slot without an index (panic ratchet indexing_slicing 687 > 686, #30712) #30712 (5a0fd07) ends each running process through self.processes[index], one clippy::indexing_slicing hit over the acpmux baseline (687 > 686), so the Rust panic ratchet is red on feat-cmux-next. It now reads the slot with get_mut and skips a slot with no row; a running slot always names a row (ProcessTable::insert), so nothing changes. Fleet panic step (cmux-tui-rust-check.sh panic): RED at origin/feat-cmux-next 34446e1. (cherry picked from commit 7bf145e) * base red: plugin update lock opens without truncation (clippy suspicious_open_options, #31462) #31462 (765d3c4) opens .plugin-update.lock with create(true) and no truncate setting. clippy::suspicious_open_options is deny under -D warnings, so the Linux clippy step of cmux-tui-rust-check.sh fails to compile cmux-tui (exit 101). The broken Cargo.lock from the same PR hid it: every --locked step stopped before clippy. The file only carries the advisory lock, so truncate(false) keeps what it does today and says so. Fleet clippy step (Linux) at 5fbcfca (lock + panic fix, before this commit): RED, crates/cmux-tui/src/plugin_manager.rs:233:43 (hq run 41379993466). * base red: acpmux process pages read rows without an index (panic ratchet indexing_slicing 696 > 686, #30726) #30726 (fbaa7a5, "byte-bound background-process history with opt-in pages") added ten clippy::indexing_slicing hits in acpmux hub/processes.rs: the page window self.processes[start..end] in ProcessTable::page_snapshot and nine serde_json indexes row["..."] in the too-large row of process_page_values. With #31695 merged, acpmux is 696 > baseline 686, so the Rust panic ratchet is red on feat-cmux-next. The window is now processes.get(start..end): start is an ID's slot + 1 (at most len) and end = min(start + limit, len), so the range is always in bounds and the fallback never applies. The row fields use row.get(..), which serializes a missing field as null, the same as an index on a missing key. The output does not change. No baseline change and no allow. Fleet panic step: RED at tip 7da73fe (indexing_slicing 697 > 686, hq run 41384159581) and at 4a94625 with the get_mut fix (696 > 686, hq run 41384159646, hits at processes.rs:327 and 384-388). * Move the acpmux process pages fix (#30726) to its own PR The lead asked for it in a separate PR (base-red/acpmux-process-pages-index). This PR keeps only the truncate(false) Linux clippy fix for #31462. This reverts commit 9b650d5. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
SW-11: keep an ordered set of live background processes for idle checks and host exit
Root cause
A daemon session's
ProcessTableretains completed background-process history for its list/tail contract. The frequentrunning_processes_readcensus andany_process_runningcheck still searched every retained entry to find active work;lose_runninglikewise scanned all entries whenever an agent host ended. At 3,000 historical processes with only 2 live tasks, idle checks did 3,000 status tests per request despite #29277's process-id and tool-call indexes.Change
BTreeSet<usize>of running process slots. Slots refer to the existing append-orderedVec<Process>; the process vector remains the authoritative record and keeps its original order.fold, replay/upsert (in either direction) and host exit. A new table reconstructed from its log rebuilds the active set automatically.any_process_runningnow tests!running.is_empty(), andrunning_processes_readanswersrunning.len(), both O(1) regardless of completed-history length.lose_runningvisits only active slots, in original append order, preserving the order and content of emittedprocess_updatenotifications. No new background scan, timer, disk state or wire field.Regression
The added Rust test module
processes_active_tests.rsexercises 3,000 completed entries plus three live ones, out-of-order replay updates, live→completed→live transitions, lost-notification order and idempotent repeat host exits. Existing Claude/Codexhub_integrationprocess tests remain the end-to-end contract.Performance
No perf numbers: count/idle checks become O(1) reads instead of O(N) scans, and loss propagation visits only R live entries (BTreeSet lookup O(log R) when a state changes). The test verifies the 3,000-record scale and preserved identities, but same-host CPU/RSS/wakeup measurements have not run. The live index retains a small extra entry for each currently running process.
Validation and ownership
feat-cmux-next@fd34ef95aa2c8f741aa23da7789e0b564732087a, which already includes perf(acpmux): read background-process output sizes after releasing the session lock (SW-11) #29238 and perf(acpmux): index background process identities for 3,000-entry replay (SW-11) #29277. This PR extends their daemon-owned process table and does not modify any other SW-11 worktree.cd cmux-tui && cargo test -p acpmux --lib process_active_tests,cargo test -p acpmux --lib process_index_tests,cargo test -p acpmux --test hub_integration -- process,cargo fmt --check, Clippy and the NEXT fast-tier gate before marking ready. Compare base/head at 1/100/3,000 historical entries with 0/1/30 live processes.Capture
No capture: daemon-only bookkeeping with identical process-list and status responses.
Changelog: Background-process idle checks use a live set, avoiding full-history scans and ending only currently running processes on host exit.
SW-11: HQ investigation.