Repository navigation
perf(acpmux): pick session pages without cloning 3,000 keys (SW-11) - #30706
Conversation
|
SW-11 CI triage: the early |
|
Found 11 test failures on Blacksmith runners: Failures
...and 1 more test failures. View all on Blacksmith
|
|
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)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on dff069f to compare; do not merge until green; see lane-rules 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. |
|
Owned by an agent session · session |
404409e to
d920a75
Compare
d920a75 to
6ae8d96
Compare
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
6ae8d96 to
088e6cc
Compare
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
SW-11: session-list paging selects borrowed keys before cloning rows
Problem
A large daemon keeps the session summaries as shared serialized text plus a compact
Vec<Value>of paging keys. But bothSnapshot::pageandSnapshot::boundedcalledsession_list_reply(keys.clone(), ...): evenlimit: 1cloned all 3,000 keys before filtering down to one row. The owned reply implementation then discarded the unused copies. This runs on a frequently requested, paced daemon list path.Change
session_list_reply_borrowed(&[Value], params), which filters and orders borrowed references, and clones only the selected rows into the eventual JSON reply. Keepsession_list_reply(Vec<Value>,...)as a compatibility wrapper for existing call sites/tests.list_guardcall sites to pass their snapshot's key slice directly. Thelimit: 1path now creates a borrowed-pointer selection over the N keys and deep-copies only the one selected key, rather than N (source-code-derived clone counts).Regression
Added
three_thousand_key_snapshot_pages_only_selected_rowsinlist_guard.rs. It builds 3,000 snapshot keys, checkslimit:1first and next cursor,localOnly/excludeStatus/orderBy, and byte-for-byte equivalence of the existing owned API with the borrowed API for multiple filter combinations. Existing session-pages tests exercise removed/moving sessions and oversized rows. No wire/schema changes.Performance
No perf numbers: the code removes N deep
Valueclones from a narrow page and copies only selected returned rows, but there is no same-harness before/after RSS, CPU or elapsed-time A/B measurement yet. The regression checks output and cursor correctness on 3,000 keys, not timing.Validation
feat-cmux-next@ab644ffb15dc73c46664abaec4087e044a7670c5.cd cmux-tui && cargo test -p acpmux --lib three_thousand_key_snapshot_pages_only_selected_rows,cargo test -p acpmux --lib hub::session_pages,cargo fmt --check, Clippy and the NEXT daemon guard before marking ready. A/B 1/100/3000 rows, page size 1/200, include heap allocation/CPU and preserve every row.Capture
No capture: daemon session-list internal allocation change with identical JSON and UI.
Changelog: Paged session lists select borrowed snapshot keys, avoiding whole-keyset copies for small result pages.
SW-11 continued findings: HQ #5599.
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Speeds up daemon session-list paging by cloning only the rows actually returned instead of the full 3,000-key snapshot.
Snapshot::pageandSnapshot::boundedpreviously calledsession_list_reply(keys.clone(), ...), so evenlimit: 1deep-copied every key before filtering. Both now filter and order borrowed references via the newsession_list_reply_borrowed(&[Value], params)and copy only the selected rows into the reply. The ownedsession_list_replyremains only as a test-only wrapper; filtering, cursor modes, tie ordering, and byte-bounded serialization are unchanged.Testing
three_thousand_key_snapshot_pages_only_selected_rows, verifying filters, cursors, and byte-for-byte parity between the owned and borrowed APIs on a 3,000-key snapshot.Snapshot::newtest for the live flag.Written for commit 088e6cc. Summary will update on new commits.