From 268bb3224f4731fea0d9e897e3be7e6210aa5b7f Mon Sep 17 00:00:00 2001 From: simota Date: Sun, 6 Sep 2026 05:37:17 +0900 Subject: [PATCH] perf: reuse row-build buffers and scan auto-approve prompts once per feed - render: keep per-row instance capacity on a full pane rebuild (resize_with instead of fresh Vecs) and thread a RowBuildScratch (segment cells, RunPool of ShapeRuns, highlight buffer) through rebuild_row_instances so dirty rows stop reallocating per row/run. - auto-approve: detect_and_update_any_agent runs find_prompt at most once per feed, sharing the observed MatchedPrompt between the decision and the state update; two-match rule and cooldown rescans are unchanged. Tests: full_rebuild_keeps_per_row_buffer_capacity, run_pool_reuse_matches_fresh_segmentation, detect_and_update_scans_viewport_once_per_feed. Claude-Session: https://claude.ai/code/session_01GBe292wmocEXM8SjBzjGq7 --- crates/noa-app/src/auto_approve.rs | 115 +++++++++++++++--- crates/noa-render/src/renderer/cell.rs | 69 ++++++++--- crates/noa-render/src/renderer/mod.rs | 6 +- crates/noa-render/src/renderer/tests/cache.rs | 78 ++++++++++++ crates/noa-render/src/segment.rs | 102 ++++++++++++++-- 5 files changed, 325 insertions(+), 45 deletions(-) diff --git a/crates/noa-app/src/auto_approve.rs b/crates/noa-app/src/auto_approve.rs index 0e9208a8..d5274960 100644 --- a/crates/noa-app/src/auto_approve.rs +++ b/crates/noa-app/src/auto_approve.rs @@ -298,23 +298,34 @@ pub(crate) fn detect( detect_inner(rows, cursor, Some(agent), ctx, state) } -pub(crate) fn detect_any_agent( - rows: &[RowText], - cursor: Point, - ctx: DetectContext, - state: &AutoApproveState, -) -> Decision { - detect_inner(rows, cursor, None, ctx, state) -} - +/// Detect and advance the two-match state machine in one pass. The viewport +/// is scanned (lowercased + signature-matched) at most once per feed: the +/// same `MatchedPrompt` observation feeds both the decision and the state +/// update, instead of each re-running `find_prompt` on the same rows. pub(crate) fn detect_and_update_any_agent( rows: &[RowText], cursor: Point, ctx: DetectContext, state: &mut AutoApproveState, ) -> Decision { - let decision = detect_any_agent(rows, cursor, ctx, state); - apply_decision_state(rows, cursor, ctx, state, &decision); + let (decision, matched) = match suppression(ctx, state.disabled_by_runaway) { + Some(reason) => { + // Only the input-cooldown suppressions keep tracking the prompt + // (see `apply_decision_state`), so only they pay for a scan. + let matched = matches!( + reason, + SuppressReason::RecentUserInput | SuppressReason::PasteActive + ) + .then(|| find_prompt(rows, cursor, None)) + .flatten(); + (Decision::Suppressed(reason), matched) + } + None => { + let matched = find_prompt(rows, cursor, None); + (decide(matched.as_ref(), ctx, state), matched) + } + }; + apply_decision_state(rows, matched.as_ref(), ctx, state, &decision); decision } @@ -339,6 +350,7 @@ pub(crate) fn viewport_rows_from_terminal(terminal: &Terminal) -> Vec { .collect() } +#[cfg(test)] fn detect_inner( rows: &[RowText], cursor: Point, @@ -349,8 +361,17 @@ fn detect_inner( if let Some(reason) = suppression(ctx, state.disabled_by_runaway) { return Decision::Suppressed(reason); } + decide(find_prompt(rows, cursor, agent).as_ref(), ctx, state) +} - let Some(matched) = find_prompt(rows, cursor, agent) else { +/// The unsuppressed decision for one observed viewport: `matched` is the +/// prompt `find_prompt` found there (or `None`). +fn decide( + matched: Option<&MatchedPrompt>, + ctx: DetectContext, + state: &AutoApproveState, +) -> Decision { + let Some(matched) = matched else { return Decision::Hold; }; @@ -381,9 +402,12 @@ fn detect_inner( } } +/// Advance the state machine after `decision`. `matched` is the prompt the +/// caller observed on `rows` in the same feed (`None` when none was found, +/// or when the suppression reason makes tracking one pointless). fn apply_decision_state( rows: &[RowText], - cursor: Point, + matched: Option<&MatchedPrompt>, ctx: DetectContext, state: &mut AutoApproveState, decision: &Decision, @@ -405,7 +429,7 @@ fn apply_decision_state( }); } Decision::Hold => { - if let Some(matched) = find_prompt(rows, cursor, None) { + if let Some(matched) = matched { if state.pending_fire.is_some_and(|pending| { pending.signature != matched.signature || pending.region_hash != matched.region_hash @@ -448,7 +472,7 @@ fn apply_decision_state( reason, SuppressReason::RecentUserInput | SuppressReason::PasteActive ) { - find_prompt(rows, cursor, None).map(|matched| MatchKey { + matched.map(|matched| MatchKey { signature: matched.signature, region_hash: matched.region_hash, }) @@ -486,7 +510,16 @@ fn suppression(ctx: DetectContext, disabled_by_runaway: bool) -> Option = const { std::cell::Cell::new(0) }; +} + fn find_prompt(rows: &[RowText], cursor: Point, agent: Option) -> Option { + #[cfg(test)] + PROMPT_SCANS.with(|scans| scans.set(scans.get() + 1)); let lowercase_rows = lowercase_rows(rows); SIGNATURES .iter() @@ -772,6 +805,58 @@ mod tests { ]) } + fn prompt_scans() -> usize { + PROMPT_SCANS.with(|scans| scans.get()) + } + + /// One feed scans the viewport once, whether it holds a prompt (first + /// match: Hold), a second identical match (Fire), or nothing at all. + #[test] + fn detect_and_update_scans_viewport_once_per_feed() { + let now = Instant::now(); + let mut state = AutoApproveState::default(); + let prompt = rows(&[ + "Claude wants to edit crates/noa-app/src/lib.rs", + "❯ 1. Yes", + " 2. No", + ]); + let plain = rows(&["plain output", "no prompt here"]); + + let before = prompt_scans(); + assert_eq!( + detect_and_update_any_agent(&plain, cursor(1), base_ctx(now), &mut state), + Decision::Hold + ); + assert_eq!(prompt_scans() - before, 1, "no-prompt feed must scan once"); + + let before = prompt_scans(); + assert_eq!( + detect_and_update_any_agent(&prompt, cursor(1), base_ctx(now), &mut state), + Decision::Hold + ); + assert_eq!(prompt_scans() - before, 1, "first match must scan once"); + + let before = prompt_scans(); + assert!(matches!( + detect_and_update_any_agent(&prompt, cursor(1), base_ctx(now), &mut state), + Decision::Fire { .. } + )); + assert_eq!(prompt_scans() - before, 1, "firing feed must scan once"); + + let mut ctx = base_ctx(now); + ctx.guards.last_user_input_at = Some(now); + let before = prompt_scans(); + assert!(matches!( + detect_and_update_any_agent(&prompt, cursor(1), ctx, &mut state), + Decision::Suppressed(SuppressReason::RecentUserInput) + )); + assert_eq!( + prompt_scans() - before, + 1, + "input-cooldown feed must scan once" + ); + } + fn rows(input: &[&str]) -> Vec { input.iter().map(|line| (*line).to_string()).collect() } diff --git a/crates/noa-render/src/renderer/cell.rs b/crates/noa-render/src/renderer/cell.rs index b99afd8b..e3750b65 100644 --- a/crates/noa-render/src/renderer/cell.rs +++ b/crates/noa-render/src/renderer/cell.rs @@ -42,6 +42,7 @@ pub(super) fn rebuild_cell_instances( let mut bg_rows = Vec::with_capacity(snap.rows.len()); let mut glyph_rows = Vec::with_capacity(snap.rows.len()); let mut deco_rows = Vec::with_capacity(snap.rows.len()); + let mut scratch = RowBuildScratch::default(); for (row_idx, row) in snap.rows.iter().enumerate() { let mut bg = Vec::new(); let mut glyph = Vec::new(); @@ -54,6 +55,7 @@ pub(super) fn rebuild_cell_instances( theme, target_format_is_srgb, metrics, + &mut scratch, RowInstanceBuffers { bg: &mut bg, glyph: &mut glyph, @@ -75,6 +77,18 @@ pub(super) fn rebuild_cell_instances( (clear_color, (metrics.cell_w, metrics.cell_h)) } +/// Per-row working memory [`rebuild_row_instances`] refills for every row it +/// builds. One instance lives on the `Renderer` and is threaded through +/// [`update_pane_cache`], so the intermediate segmentation / highlight / +/// run buffers are allocated once and reused instead of being rebuilt from +/// zero capacity for each dirty row. +#[derive(Default)] +pub(super) struct RowBuildScratch { + segment_cells: Vec, + runs: RunPool, + highlights: Vec, +} + /// The three parallel output bands [`rebuild_row_instances`] fills in place, /// grouped into one param so the caller's `PaneRenderCache` row slots (or a /// scratch `Vec` in tests) pass as a single unit instead of three positional @@ -112,6 +126,7 @@ pub(super) fn rebuild_row_instances( theme: &Theme, target_format_is_srgb: bool, metrics: Metrics, + scratch: &mut RowBuildScratch, out: RowInstanceBuffers<'_>, ) { // In/out Vec params (not return-by-value): the caller owns the @@ -126,7 +141,12 @@ pub(super) fn rebuild_row_instances( bg_instances.clear(); glyph_instances.clear(); decoration_instances.clear(); - let mut segment_cells = Vec::with_capacity(row.cells.len()); + let RowBuildScratch { + segment_cells, + runs, + highlights, + } = scratch; + segment_cells.clear(); // Cursor shape only depends on pane-wide snapshot state (position, // DECSCUSR style, focus, blink phase), so it is resolved once per row @@ -137,7 +157,7 @@ pub(super) fn rebuild_row_instances( } else { cursor_visual_for(snap) }; - let row_highlights = RowHighlights::new(snap, y, row.cells.len()); + let row_highlights = RowHighlights::new(snap, y, row.cells.len(), highlights); // Restored-record gutter (`scrollback-persist` spec §5): this row's // session-absolute index falls inside a persisted-snapshot range the // caller restored, so column 0 gets an extra marker quad below — unless @@ -357,9 +377,10 @@ pub(super) fn rebuild_row_instances( // becomes an extra instance anchored at its base cell, positioned // by its own shaped offset instead of an independent per-char pen // bearing (REQ-SHAPE-4). - for run in segment_row(font, &segment_cells) { + segment_row_into(font, segment_cells, runs); + for run in runs.runs() { let shaped = font.shape_run(&run.cells); - emit_run_glyph_instances(glyph_instances, font, &run, &shaped, y, metrics); + emit_run_glyph_instances(glyph_instances, font, run, &shaped, y, metrics); } } @@ -370,17 +391,19 @@ pub(super) struct CellHighlight { search_match: bool, } -pub(super) struct RowHighlights { - cells: Option>, +pub(super) struct RowHighlights<'a> { + cells: Option<&'a [CellHighlight]>, } -impl RowHighlights { - fn new(snap: &FrameSnapshot, y: u16, cols: usize) -> Self { +impl<'a> RowHighlights<'a> { + fn new(snap: &FrameSnapshot, y: u16, cols: usize, buf: &'a mut Vec) -> Self { if cols == 0 || (snap.selection.is_none() && snap.search.matches().is_empty()) { return Self { cells: None }; } - let mut cells = vec![CellHighlight::default(); cols]; + buf.clear(); + buf.resize(cols, CellHighlight::default()); + let cells = buf.as_mut_slice(); let storage_y = snap.row_base + y as usize; if let Some(selection) = snap.selection { @@ -388,7 +411,7 @@ impl RowHighlights { if start.y <= storage_y && storage_y <= end.y { let start_x = if storage_y == start.y { start.x } else { 0 }; let end_x = if storage_y == end.y { end.x } else { u16::MAX }; - mark_highlight_span(&mut cells, start_x, end_x, |cell| { + mark_highlight_span(cells, start_x, end_x, |cell| { cell.selected = true; }); } @@ -406,7 +429,7 @@ impl RowHighlights { } else { u16::MAX }; - mark_highlight_span(&mut cells, start_x, end_x, |cell| { + mark_highlight_span(cells, start_x, end_x, |cell| { cell.search_match = true; cell.active_search |= active == Some(*search_match); }); @@ -417,7 +440,6 @@ impl RowHighlights { fn get(&self, idx: usize) -> CellHighlight { self.cells - .as_ref() .and_then(|cells| cells.get(idx)) .copied() .unwrap_or_default() @@ -498,7 +520,15 @@ pub(super) fn rebuild_pane_cached( theme: &Theme, target_format_is_srgb: bool, ) -> PaneRebuild { - let result = update_pane_cache(cache, snap, font, theme, target_format_is_srgb); + let mut scratch = RowBuildScratch::default(); + let result = update_pane_cache( + cache, + snap, + font, + theme, + target_format_is_srgb, + &mut scratch, + ); instances.extend_from_slice(&cache.flat); instances.extend_from_slice(&cache.overlays); result @@ -513,6 +543,7 @@ pub(super) fn update_pane_cache( font: &mut FontGrid, theme: &Theme, target_format_is_srgb: bool, + scratch: &mut RowBuildScratch, ) -> PaneRebuild { let metrics = font.metrics(); let clear_color = surface_output_rgba( @@ -662,10 +693,15 @@ pub(super) fn update_pane_cache( } } + // A full rebuild invalidates every row's CONTENT, not its storage: + // keep each row slot's capacity (every row is rebuilt below, and + // `rebuild_row_instances` clears the slot first) so a selection + // change or theme swap does not free and re-grow every row buffer. + // Only a row-count change touches the outer vectors. if full { - cache.bg = vec![Vec::new(); rows]; - cache.glyph = vec![Vec::new(); rows]; - cache.deco = vec![Vec::new(); rows]; + cache.bg.resize_with(rows, Vec::new); + cache.glyph.resize_with(rows, Vec::new); + cache.deco.resize_with(rows, Vec::new); cache.flat.clear(); } @@ -680,6 +716,7 @@ pub(super) fn update_pane_cache( theme, target_format_is_srgb, metrics, + scratch, RowInstanceBuffers { bg: &mut cache.bg[row_idx], glyph: &mut cache.glyph[row_idx], diff --git a/crates/noa-render/src/renderer/mod.rs b/crates/noa-render/src/renderer/mod.rs index 6d0857b0..0bcd01a2 100644 --- a/crates/noa-render/src/renderer/mod.rs +++ b/crates/noa-render/src/renderer/mod.rs @@ -16,7 +16,7 @@ use crate::draw_plan::{DrawOp, PaneId, PaneRect, build_draw_plan}; use crate::image_layer::{ImageDraw, ImageLayer}; use crate::instance::{BlendMode, CellInstance, PaneUniformParams, populate_pane_uniform}; use crate::pipeline::CellPipeline; -use crate::segment::{SegmentCell, ShapeRun, segment_row}; +use crate::segment::{RunPool, SegmentCell, ShapeRun, segment_row, segment_row_into}; use crate::snapshot::{ CommandPaletteSnapshot, ConfirmDialogSnapshot, FrameSnapshot, HoverLink, ImagePlacementSnapshot, PaletteRow, SnapshotImage, @@ -358,6 +358,8 @@ pub struct Renderer { /// pane's stable render-side identity so it survives split reordering /// across frames. pane_render_cache: HashMap, + /// Reused per-row build buffers shared by every pane rebuild. + row_scratch: RowBuildScratch, /// Total rows regenerated across all panes in the most recent /// `rebuild_panes` call (AC-WP4-02). rows_rebuilt_last_frame: u64, @@ -462,6 +464,7 @@ impl Renderer { target_format: format, target_format_is_srgb: format.is_srgb(), pane_render_cache: HashMap::new(), + row_scratch: RowBuildScratch::default(), rows_rebuilt_last_frame: 0, frame_unstable: false, background_opacity: 1.0, @@ -853,6 +856,7 @@ impl Renderer { font, theme, self.target_format_is_srgb, + &mut self.row_scratch, ); rows_rebuilt_total += rows_rebuilt; if !stable { diff --git a/crates/noa-render/src/renderer/tests/cache.rs b/crates/noa-render/src/renderer/tests/cache.rs index 3b6ee332..11ecf485 100644 --- a/crates/noa-render/src/renderer/tests/cache.rs +++ b/crates/noa-render/src/renderer/tests/cache.rs @@ -1410,3 +1410,81 @@ fn invalidate_pane_after_a_skipped_mixed_window_still_surfaces_the_in_place_edit skipped round" ); } + +#[test] +fn full_rebuild_keeps_per_row_buffer_capacity() { + // A full rebuild (here: a selection change, one of the pane-wide + // invalidation triggers) must refill the existing per-row slots rather + // than replace them with fresh zero-capacity `Vec`s — the row count is + // unchanged, so last frame's capacity is exactly what this frame needs. + let Some(mut font) = skip_font() else { return }; + let theme = Theme::new(); + let mut cache = PaneRenderCache::empty(); + let mut instances = Vec::new(); + + let snap_a = baseline_snapshot(['A', 'B', 'C']); + let first = rebuild_pane_cached( + &mut cache, + &mut instances, + &snap_a, + &mut font, + &theme, + false, + ); + assert_eq!(first.rows_rebuilt, 3); + let glyph_caps: Vec = cache.glyph.iter().map(Vec::capacity).collect(); + let glyph_ptrs: Vec<*const CellInstance> = cache.glyph.iter().map(Vec::as_ptr).collect(); + assert!( + glyph_caps.iter().all(|cap| *cap > 0), + "every baseline row emits a glyph, so every slot must hold capacity" + ); + let reference = instances.clone(); + instances.clear(); + + // Row 1, not row 0: the block cursor already owns cell (0, 0)'s + // background, so selecting it would leave the output unchanged. + let mut snap_b = baseline_snapshot(['A', 'B', 'C']); + snap_b.selection = Some(Selection::new( + SelectionPoint::new(0, 1), + SelectionPoint::new(0, 1), + )); + let second = rebuild_pane_cached( + &mut cache, + &mut instances, + &snap_b, + &mut font, + &theme, + false, + ); + assert_eq!( + second.rows_rebuilt, 3, + "a selection change is a full rebuild" + ); + assert_eq!( + cache.glyph.iter().map(Vec::capacity).collect::>(), + glyph_caps, + "a same-size full rebuild must keep each row slot's capacity" + ); + assert_eq!( + cache.glyph.iter().map(Vec::as_ptr).collect::>(), + glyph_ptrs, + "a same-size full rebuild must reuse each row slot's allocation" + ); + assert_ne!( + instances, reference, + "the selected cell's background must have changed the output" + ); + + // Rebuilding the pre-selection snapshot again yields the original output + // through the reused buffers. + instances.clear(); + rebuild_pane_cached( + &mut cache, + &mut instances, + &snap_a, + &mut font, + &theme, + false, + ); + assert_eq!(instances, reference); +} diff --git a/crates/noa-render/src/segment.rs b/crates/noa-render/src/segment.rs index a9437d28..b1e2a5ef 100644 --- a/crates/noa-render/src/segment.rs +++ b/crates/noa-render/src/segment.rs @@ -83,6 +83,41 @@ fn boundary_key(font: &mut FontGrid, cell: &SegmentCell) -> (BoundaryKey, StyleK (key, style) } +/// Reusable output slot for [`segment_row_into`]: the `ShapeRun`s (and +/// their inner `cells`/`cell_render` `Vec`s) from the previous row are kept +/// allocated and refilled, so a steady stream of dirty rows segments without +/// per-row / per-run heap churn. Only the first `live` entries of `runs` are +/// meaningful for the row most recently segmented. +#[derive(Default)] +pub struct RunPool { + runs: Vec, + live: usize, +} + +impl RunPool { + /// The runs produced by the most recent [`segment_row_into`] call. + pub fn runs(&self) -> &[ShapeRun] { + &self.runs[..self.live] + } + + fn open_run(&mut self, start_col: u16) -> &mut ShapeRun { + if self.live == self.runs.len() { + self.runs.push(ShapeRun { + start_col, + cells: Vec::new(), + cell_render: Vec::new(), + }); + } else { + let run = &mut self.runs[self.live]; + run.start_col = start_col; + run.cells.clear(); + run.cell_render.clear(); + } + self.live += 1; + &mut self.runs[self.live - 1] + } +} + /// Segment one row's cells into shapeable runs (REQ-SHAPE-6): breaks at /// font-face, style (bold/italic), selection, active-search-match, /// search-match, and cursor boundaries. A row never crosses into another @@ -92,8 +127,11 @@ fn boundary_key(font: &mut FontGrid, cell: &SegmentCell) -> (BoundaryKey, StyleK /// Takes `&mut FontGrid` because resolving a codepoint the curated font stack /// cannot map may lazily pull a system fallback face into the stack (macOS /// CoreText cascade — see [`noa_font::FontGrid::resolve_face_for_style`]). -pub fn segment_row(font: &mut FontGrid, cells: &[SegmentCell]) -> Vec { - let mut runs: Vec = Vec::new(); +/// +/// Writes into `out` (see [`RunPool`]) instead of returning a fresh `Vec`; +/// [`segment_row`] is the allocating convenience wrapper. +pub fn segment_row_into(font: &mut FontGrid, cells: &[SegmentCell], out: &mut RunPool) { + out.live = 0; let mut current_key: Option = None; for (idx, cell) in cells.iter().enumerate() { @@ -108,21 +146,26 @@ pub fn segment_row(font: &mut FontGrid, cells: &[SegmentCell]) -> Vec cursor: cell.cursor, }; - if current_key == Some(key) { - let run = runs.last_mut().expect("current_key implies an open run"); - run.cells.push(shape_cell); - run.cell_render.push(render_info); + let run = if current_key == Some(key) { + out.runs + .get_mut(out.live - 1) + .expect("current_key implies an open run") } else { - runs.push(ShapeRun { - start_col: idx as u16, - cells: vec![shape_cell], - cell_render: vec![render_info], - }); current_key = Some(key); - } + out.open_run(idx as u16) + }; + run.cells.push(shape_cell); + run.cell_render.push(render_info); } +} - runs +/// Allocating form of [`segment_row_into`] for one-off callers (overlays, +/// tests) that do not keep a [`RunPool`] around. +pub fn segment_row(font: &mut FontGrid, cells: &[SegmentCell]) -> Vec { + let mut pool = RunPool::default(); + segment_row_into(font, cells, &mut pool); + pool.runs.truncate(pool.live); + pool.runs } #[cfg(test)] @@ -239,6 +282,39 @@ mod tests { ); } + /// A reused `RunPool` yields exactly the runs a fresh segmentation would + /// (stale runs from a longer previous row never leak through `runs()`), + /// and keeps its `ShapeRun` slots allocated across rows. + #[test] + fn run_pool_reuse_matches_fresh_segmentation() { + let Some(mut font) = skip_font() else { return }; + let mut long = vec![plain_cell('a'), plain_cell('b'), plain_cell('c')]; + long[1].bold = true; + let short = vec![plain_cell('x')]; + + let mut pool = RunPool::default(); + segment_row_into(&mut font, &long, &mut pool); + assert_eq!(pool.runs().len(), 3); + let slots_after_long = pool.runs.len(); + + segment_row_into(&mut font, &short, &mut pool); + let fresh = segment_row(&mut font, &short); + assert_eq!(pool.runs().len(), fresh.len()); + for (pooled, fresh) in pool.runs().iter().zip(&fresh) { + assert_eq!(pooled.start_col, fresh.start_col); + assert_eq!(pooled.cells.len(), fresh.cells.len()); + assert_eq!(pooled.cells[0].ch, fresh.cells[0].ch); + } + assert_eq!( + pool.runs.len(), + slots_after_long, + "a shorter row must reuse the pool's existing run slots, not shrink them" + ); + segment_row_into(&mut font, &long, &mut pool); + assert_eq!(pool.runs().len(), 3); + assert_eq!(pool.runs.len(), slots_after_long); + } + /// `cell_render` stays parallel to `cells` and carries color/cursor /// context without polluting the `ShapeCell`s themselves. #[test]