perf(acpmux): pick session pages without cloning 3,000 keys (SW-11)

Merged
TE
4d ago
+82-27

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 both Snapshot::page and Snapshot::bounded called session_list_reply(keys.clone(), ...): even limit: 1 cloned 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

  • Add session_list_reply_borrowed(&[Value], params), which filters and orders borrowed references, and clones only the selected rows into the eventual JSON reply. Keep session_list_reply(Vec<Value>,...) as a compatibility wrapper for existing call sites/tests.
  • Update the two list_guard call sites to pass their snapshot's key slice directly. The limit: 1 path 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).
  • Preserve filtering by peer/status/localOnly/excludeStatus, both timestamp and sessionId cursor modes, stable tie ordering, nextCursor and byte-bounded row serialization. Snapshot ownership and list pacing remain unchanged.
  • For unbounded/no-limit requests, every returned row is necessarily copied once to form the reply. A later cleanup could avoid even that intermediate JSON copy by paging the text spans directly; that larger change is outside this PR.

Regression

Added three_thousand_key_snapshot_pages_only_selected_rows in list_guard.rs. It builds 3,000 snapshot keys, checks limit:1 first 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 Value clones 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

  • Branched from feat-cmux-next@ab644ffb15dc73c46664abaec4087e044a7670c5.
  • Two Rust files, no new threads, timers, I/O or API fields. No code changes to #28831's byte limits or #28273's Rust-client pagination.
  • Source-only in this connected environment: run 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.
  • Review against newer mainline changes before landing. The PR remains draft until CI and owner checks.

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.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with 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::page and Snapshot::bounded previously called session_list_reply(keys.clone(), ...), so even limit: 1 deep-copied every key before filtering. Both now filter and order borrowed references via the new session_list_reply_borrowed(&[Value], params) and copy only the selected rows into the reply. The owned session_list_reply remains only as a test-only wrapper; filtering, cursor modes, tie ordering, and byte-bounded serialization are unchanged.

Testing

  • Adds 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.
  • Updates the Snapshot::new test for the live flag.