From 6447e6342ba2b1617335cd6182032a02652a8507 Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Tue, 15 Sep 2026 21:20:43 -0700 Subject: [PATCH] perf(tui): hand tool-output rows back as a shared handle (#6213 T1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rows cache exists so a finalized tool cell is not re-wrapped on every frame, but a hit still deep-copied every `String` and styled span in the payload — the one thing the cache was supposed to make free. The rows now live behind an `Arc` and a hit hands back a refcount bump. The caller only iterates or indexes the rows, so it derefs unchanged: `all_lines.iter()`, `&all_lines[idx]`, and `&all_lines` into `selected_output_indices` all still resolve. Part of #6213. T1 is not finished: the other half is keying the cache by tool-result id so the per-frame `hash_str` over the whole output disappears, and that needs the id plumbed down to the render call. Verification: cargo check -p codewhale-tui --all-targets --all-features --locked (clean) test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 12799 filtered out (tui::output_rows_cache) test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 12800 filtered out (tui::history::tool_output) Signed-off-by: CodeWhale Bot --- crates/tui/src/tui/output_rows_cache.rs | 33 ++++++++++++++++--------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/crates/tui/src/tui/output_rows_cache.rs b/crates/tui/src/tui/output_rows_cache.rs index adf3e6ddcd..e9b8892656 100644 --- a/crates/tui/src/tui/output_rows_cache.rs +++ b/crates/tui/src/tui/output_rows_cache.rs @@ -34,6 +34,7 @@ use std::cell::RefCell; use std::collections::{HashMap, VecDeque}; +use std::sync::Arc; use crate::tui::history::OutputRow; @@ -48,14 +49,17 @@ const DEFAULT_CAPACITY: usize = 256; /// `line_limit` changes; rows are shared across all line limits. #[derive(Debug, Clone)] struct CacheEntry { - rows: Vec, + /// Shared so a cache hit hands back a refcount bump. Every hit used to + /// deep-copy each `String` and styled span of a payload whose whole point + /// is that it does not change between frames (#6213 T1). + rows: Arc>, /// Map of `line_limit -> selected indices`. Bounded by the /// distinct line limits passed in by the renderer (typically 1–3). selected_by_limit: HashMap>, } impl CacheEntry { - fn new(rows: Vec) -> Self { + fn new(rows: Arc>) -> Self { Self { rows, selected_by_limit: HashMap::new(), @@ -102,14 +106,15 @@ impl OutputRowsCacheInner { } /// Get or compute the wrapped output rows for `output` at `width`. - /// On a hit, returns a clone of the cached `Vec` — the - /// caller can iterate without holding a lock. + /// On a hit, returns another handle on the cached rows — the caller can + /// iterate without holding a lock, and pays a refcount bump rather than a + /// deep copy. fn get_or_compute_rows( &mut self, content_hash: u64, width: u16, compute: F, - ) -> Vec + ) -> Arc> where F: FnOnce() -> Vec, { @@ -118,11 +123,11 @@ impl OutputRowsCacheInner { width, }; if let Some(entry) = self.by_key.get(&key) { - return entry.rows.clone(); + return Arc::clone(&entry.rows); } - let rows = compute(); - let entry = CacheEntry::new(rows.clone()); + let rows = Arc::new(compute()); + let entry = CacheEntry::new(Arc::clone(&rows)); if self.by_key.len() >= self.capacity && let Some(oldest) = self.insertion_order.pop_front() @@ -183,12 +188,12 @@ pub fn reset_for_tests() { } /// Look up (or compute) the wrapped output rows for `output` at `width`. -/// On a hit the cached `Vec` is cloned without re-running -/// the per-line ANSI strip or the wrap pass. +/// On a hit the cached rows are handed back behind a shared handle, so the +/// per-line ANSI strip and wrap pass are skipped without copying the rows. /// String-keyed convenience over [`get_or_compute_rows_with_hash`]. Only the /// tests use it now that production callers hash once and pass the hash. #[cfg(test)] -pub fn get_or_compute_rows(output: &str, width: u16, compute: F) -> Vec +pub fn get_or_compute_rows(output: &str, width: u16, compute: F) -> Arc> where F: FnOnce() -> Vec, { @@ -198,7 +203,11 @@ where /// As `get_or_compute_rows` but takes a precomputed content hash, so a /// caller that already hashed the output (e.g. to also key /// [`get_or_compute_indices`]) does not hash it a second time (#3757 review). -pub fn get_or_compute_rows_with_hash(content_hash: u64, width: u16, compute: F) -> Vec +pub fn get_or_compute_rows_with_hash( + content_hash: u64, + width: u16, + compute: F, +) -> Arc> where F: FnOnce() -> Vec, {