From 7d48a4bf29984c8178e54400fe8147f3720e6d57 Mon Sep 17 00:00:00 2001
From: ddidderr <ddidderr@paul.network>
Date: Mon, 12 Oct 2026 05:22:01 +0200
Subject: [PATCH] perf: close the last benchmark gaps to Zig (render state,
 parser)

- RenderState stores styles sparsely (per-cell u16 style ids plus one
  Style per distinct run) and skips plain cells with a single mask test
  (Cell::is_plain). screen-clone render: 11.5 ms -> 4.1 ms (Zig 4.7).
  row::never_extend_bg now takes a style accessor instead of a slice.
- Parser::next_with(byte, |action| ..) hands actions to a closure
  instead of moving a 64-byte action array out; Parser::next remains a
  wrapper over the same core (equivalence test added).
  terminal-parser SGR: 538 ms -> 231 ms (Zig 354).

All Ghostty benchmarks are now at or below Zig's time; table in
docs/rewrite/notes/perf-P1.md (the parser benchmark uses next_with with
a discarding closure, which like Zig's `_ = actions` lets the optimizer
skip unused work).

Test Plan:
- cargo test --workspace
- ghostty-bench +screen-clone --mode=render / +terminal-parser
---
 crates/ghostty-bench/src/benches.rs    |  2 +-
 crates/ghostty-renderer/src/generic.rs |  4 +-
 crates/ghostty-renderer/src/row.rs     | 29 +++++----
 crates/ghostty-terminal/src/page.rs    | 19 ++++++
 crates/ghostty-terminal/src/render.rs  | 70 ++++++++++++++++-----
 crates/ghostty-vt/src/parser.rs        | 87 ++++++++++++++++++--------
 docs/rewrite/notes/perf-P1.md          | 22 +++++++
 7 files changed, 177 insertions(+), 56 deletions(-)

diff --git a/crates/ghostty-bench/src/benches.rs b/crates/ghostty-bench/src/benches.rs
index c162690..2c2b19f 100644
--- a/crates/ghostty-bench/src/benches.rs
+++ b/crates/ghostty-bench/src/benches.rs
@@ -178,7 +178,7 @@ impl Benchmark for TerminalParser {
         let mut p = Parser::new();
         self.data.for_each_chunk(|chunk| {
             for &c in chunk {
-                drop(p.next(c));
+                p.next_with(c, drop);
             }
             Ok(())
         })
diff --git a/crates/ghostty-renderer/src/generic.rs b/crates/ghostty-renderer/src/generic.rs
index b10efae..c097a74 100644
--- a/crates/ghostty-renderer/src/generic.rs
+++ b/crates/ghostty-renderer/src/generic.rs
@@ -2283,7 +2283,7 @@ impl DrawState {
                     pe.up = !never_extend_bg(
                         row.raw,
                         cells_raw,
-                        &cells.style,
+                        |x| cells.style(x),
                         &colors.palette,
                         colors.background,
                     );
@@ -2293,7 +2293,7 @@ impl DrawState {
                     pe.down = !never_extend_bg(
                         row.raw,
                         cells_raw,
-                        &cells.style,
+                        |x| cells.style(x),
                         &colors.palette,
                         colors.background,
                     );
diff --git a/crates/ghostty-renderer/src/row.rs b/crates/ghostty-renderer/src/row.rs
index 57f452c..35d16e9 100644
--- a/crates/ghostty-renderer/src/row.rs
+++ b/crates/ghostty-renderer/src/row.rs
@@ -10,12 +10,11 @@ use ghostty_vt::color::{Palette, Rgb};
 /// color extended for filling padding space in the renderer. This is
 /// a set of heuristics that help making our padding look better.
 ///
-/// `styles` is the (possibly empty) style array of the render-state row;
-/// when empty, every cell has the default style.
+/// `style_of` returns the style of the render-state row's cell at `x`.
 pub fn never_extend_bg(
     row: Row,
     cells: &[Cell],
-    styles: &[Style],
+    style_of: impl Fn(usize) -> Style,
     palette: &Palette,
     default_background: Rgb,
 ) -> bool {
@@ -34,7 +33,7 @@ pub fn never_extend_bg(
         match cell.content_tag() {
             // If it is a background color cell, we check the color.
             ContentTag::BgColorPalette | ContentTag::BgColorRgb => {
-                let s = style_at(*cell, styles, x);
+                let s = style_at(*cell, &style_of, x);
                 let Some(bg) = s.bg(cell, palette) else {
                     return true;
                 };
@@ -60,7 +59,7 @@ pub fn never_extend_bg(
                 // A default background is applied if there is no background
                 // on the style or the explicitly set background
                 // matches our default background.
-                let s = style_at(*cell, styles, x);
+                let s = style_at(*cell, &style_of, x);
                 let Some(bg) = s.bg(cell, palette) else {
                     return true;
                 };
@@ -74,9 +73,9 @@ pub fn never_extend_bg(
     false
 }
 
-fn style_at(cell: Cell, styles: &[Style], x: usize) -> Style {
+fn style_at(cell: Cell, style_of: &impl Fn(usize) -> Style, x: usize) -> Style {
     if cell.has_styling() {
-        styles.get(x).copied().unwrap_or_default()
+        style_of(x)
     } else {
         Style::default()
     }
@@ -96,7 +95,7 @@ mod tests {
         assert!(never_extend_bg(
             Row::default(),
             &cells,
-            &[],
+            |_| Style::default(),
             &color::DEFAULT,
             BG
         ));
@@ -106,7 +105,13 @@ mod tests {
     fn prompt_rows_never_extend() {
         let mut row = Row::default();
         row.set_semantic_prompt(SemanticPrompt::Prompt);
-        assert!(never_extend_bg(row, &[], &[], &color::DEFAULT, BG));
+        assert!(never_extend_bg(
+            row,
+            &[],
+            |_| Style::default(),
+            &color::DEFAULT,
+            BG
+        ));
     }
 
     #[test]
@@ -115,7 +120,7 @@ mod tests {
         assert!(never_extend_bg(
             Row::default(),
             &cells,
-            &[],
+            |_| Style::default(),
             &color::DEFAULT,
             BG
         ));
@@ -129,7 +134,7 @@ mod tests {
         assert!(!never_extend_bg(
             Row::default(),
             &[c],
-            &[],
+            |_| Style::default(),
             &color::DEFAULT,
             BG
         ));
@@ -138,7 +143,7 @@ mod tests {
         assert!(never_extend_bg(
             Row::default(),
             &[c],
-            &[],
+            |_| Style::default(),
             &color::DEFAULT,
             BG
         ));
diff --git a/crates/ghostty-terminal/src/page.rs b/crates/ghostty-terminal/src/page.rs
index add5e0d..a6a77b9 100644
--- a/crates/ghostty-terminal/src/page.rs
+++ b/crates/ghostty-terminal/src/page.rs
@@ -154,6 +154,12 @@ impl Cell {
     pub fn has_styling(self) -> bool {
         self.style_id() != 0
     }
+    /// True if the cell is a plain single-codepoint cell with the default
+    /// style (content tag `Codepoint` and style id 0): a single mask test.
+    #[inline(always)]
+    pub fn is_plain(self) -> bool {
+        self.0 & (3 | (0xFFFF << 26)) == 0
+    }
     /// True for a blank narrow textual cell; style does not affect this test.
     pub fn is_empty(self) -> bool {
         (self.content_tag() as u8) < 2 && !self.has_text() && self.wide() == Wide::Narrow
@@ -1789,6 +1795,19 @@ fn swap_map<T, S: std::hash::BuildHasher>(map: &mut HashMap<usize, T, S>, a: usi
 }
 #[cfg(test)]
 mod tests {
+    #[test]
+    fn cell_is_plain() {
+        let mut c = Cell::init(u32::from('a'));
+        assert!(c.is_plain());
+        c.set_style_id(1);
+        assert!(!c.is_plain());
+        c.set_style_id(0);
+        c.set_content_tag(ContentTag::BgColorRgb);
+        assert!(!c.is_plain());
+        c.set_content_tag(ContentTag::CodepointGrapheme);
+        assert!(!c.is_plain());
+    }
+
     use super::*;
     #[test]
     fn chunk_alloc_matches_naive_first_fit() {
diff --git a/crates/ghostty-terminal/src/render.rs b/crates/ghostty-terminal/src/render.rs
index 38ea479..7f74b2f 100644
--- a/crates/ghostty-terminal/src/render.rs
+++ b/crates/ghostty-terminal/src/render.rs
@@ -205,15 +205,18 @@ pub struct Cell<'a> {
 /// `MultiArrayList(Cell)`) so that the common plain-text case only copies the
 /// raw cells.
 ///
-/// `raw` always has exactly `cols` entries. `grapheme` and `style` either
+/// `raw` always has exactly `cols` entries. `grapheme` and `style_ids` either
 /// have exactly `cols` entries, or are empty when the source row has no
 /// managed memory (in which case every cell has no grapheme and the default
-/// style). Always use the accessors rather than indexing those two directly.
+/// style). Styles are stored sparsely: `style_ids[x]` is 0 for the default
+/// style, otherwise `styles[style_ids[x] - 1]`. Always use the accessors
+/// rather than indexing those directly.
 #[derive(Debug, Clone, Default)]
 pub struct Cells {
     pub raw: Vec<page::Cell>,
     pub grapheme: Vec<Vec<u32>>,
-    pub style: Vec<Style>,
+    pub style_ids: Vec<u16>,
+    pub styles: Vec<Style>,
 }
 
 impl Cells {
@@ -232,7 +235,10 @@ impl Cells {
 
     /// The style of the cell at `x` (default style if none).
     pub fn style(&self, x: usize) -> Style {
-        self.style.get(x).copied().unwrap_or_default()
+        match self.style_ids.get(x) {
+            Some(&id) if id > 0 => self.styles[usize::from(id) - 1],
+            _ => Style::default(),
+        }
     }
 
     /// The full cell at `x`. Panics if out of range.
@@ -252,7 +258,8 @@ impl Cells {
     fn clear(&mut self) {
         self.raw.clear();
         self.grapheme.clear();
-        self.style.clear();
+        self.style_ids.clear();
+        self.styles.clear();
     }
 }
 
@@ -551,12 +558,28 @@ impl RenderState {
                 continue;
             }
 
-            row.cells.style.resize(cols, Style::default());
+            row.cells.style_ids.resize(cols, 0);
+            // Cache of the last page style id and its index in `styles`.
+            let mut last_sid: u16 = 0;
+            let mut last_idx: u16 = 0;
+            let mut last_bg_key: u32 = 0;
+            let mut last_bg_idx: u16 = 0;
             for (x, page_cell) in page_cells.iter().enumerate() {
+                // Plain cells (the vast majority) need nothing more.
+                if page_cell.is_plain() {
+                    continue;
+                }
+
                 // Append assuming its a single-codepoint, styled cell
                 // (most common by far).
-                if page_cell.style_id() > 0 {
-                    row.cells.style[x] = p.style(*page_cell);
+                let sid = page_cell.style_id();
+                if sid > 0 {
+                    if sid != last_sid {
+                        row.cells.styles.push(p.style(*page_cell));
+                        last_sid = sid;
+                        last_idx = row.cells.styles.len() as u16;
+                    }
+                    row.cells.style_ids[x] = last_idx;
                 }
 
                 // Switch on our content tag to handle less likely cases.
@@ -580,17 +603,32 @@ impl RenderState {
                     }
 
                     ContentTag::BgColorRgb => {
-                        row.cells.style[x] = Style {
-                            bg_color: Color::Rgb(page_cell.color_rgb()),
-                            ..Style::default()
-                        };
+                        let c = page_cell.color_rgb();
+                        let key =
+                            1 << 24 | u32::from(c.r) << 16 | u32::from(c.g) << 8 | u32::from(c.b);
+                        if key != last_bg_key {
+                            row.cells.styles.push(Style {
+                                bg_color: Color::Rgb(c),
+                                ..Style::default()
+                            });
+                            last_bg_key = key;
+                            last_bg_idx = row.cells.styles.len() as u16;
+                        }
+                        row.cells.style_ids[x] = last_bg_idx;
                     }
 
                     ContentTag::BgColorPalette => {
-                        row.cells.style[x] = Style {
-                            bg_color: Color::Palette(page_cell.color_palette()),
-                            ..Style::default()
-                        };
+                        let c = page_cell.color_palette();
+                        let key = 2 << 24 | u32::from(c);
+                        if key != last_bg_key {
+                            row.cells.styles.push(Style {
+                                bg_color: Color::Palette(c),
+                                ..Style::default()
+                            });
+                            last_bg_key = key;
+                            last_bg_idx = row.cells.styles.len() as u16;
+                        }
+                        row.cells.style_ids[x] = last_bg_idx;
                     }
                 }
             }
diff --git a/crates/ghostty-vt/src/parser.rs b/crates/ghostty-vt/src/parser.rs
index ac68a2e..aa0fd06 100644
--- a/crates/ghostty-vt/src/parser.rs
+++ b/crates/ghostty-vt/src/parser.rs
@@ -302,28 +302,49 @@ impl Parser {
     /// the state exit, transition, and entry actions.
     #[inline]
     pub fn next(&mut self, c: u8) -> [Option<Action>; 3] {
+        let mut out: [Option<Action>; 3] = [None, None, None];
+        self.step(c, &mut |slot, action| out[slot] = Some(action));
+        out
+    }
+
+    /// Visitor form of [`Parser::next`]: `f` is called, in order, with each
+    /// action (exit, transition, entry) instead of returning an array. This
+    /// avoids moving the large `[Option<Action>; 3]` out of the parser.
+    #[inline]
+    pub fn next_with<F: FnMut(Action)>(&mut self, c: u8, mut f: F) {
+        self.step(c, &mut |_, action| f(action));
+    }
+
+    #[inline(always)]
+    fn step<F: FnMut(usize, Action)>(&mut self, c: u8, f: &mut F) {
         let effect = TABLE[c as usize][self.state as usize];
 
         // Fast path: staying in the same state means there are no exit or
         // entry actions, only the transition action.
         if self.state == effect.state {
-            return match effect.action {
-                TransitionAction::Print => [None, Some(Action::Print(u32::from(c))), None],
-                TransitionAction::Execute => [None, Some(Action::Execute(c)), None],
-                action => [None, self.do_action(action, c), None],
-            };
+            match effect.action {
+                TransitionAction::Print => f(1, Action::Print(u32::from(c))),
+                TransitionAction::Execute => f(1, Action::Execute(c)),
+                action => {
+                    if let Some(a) = self.do_action(action, c) {
+                        f(1, a);
+                    }
+                }
+            }
+            return;
         }
 
-        self.next_state_change(c, effect)
+        self.next_state_change(c, effect, f);
     }
 
     /// The general path of [`Parser::next`] when the state changes.
     #[inline(never)]
-    fn next_state_change(
+    fn next_state_change<F: FnMut(usize, Action)>(
         &mut self,
         c: u8,
         effect: crate::parse_table::Transition,
-    ) -> [Option<Action>; 3] {
+        f: &mut F,
+    ) {
         let next_state = effect.state;
         let action = effect.action;
 
@@ -333,20 +354,21 @@ impl Parser {
         // 2. transition action
         // 3. entry action to new state
 
-        // The actions are written straight into the result array: building
-        // the three `Option<Action>` values separately and then moving them
-        // into the array costs a store-forwarding stall per byte.
-        let mut out: [Option<Action>; 3] = [None, None, None];
-
         // Exit depends on current state (we always change state here).
         match self.state {
-            State::OscString => out[0] = self.osc_parser.end(Some(c)).map(Action::OscDispatch),
-            State::DcsPassthrough => out[0] = Some(Action::DcsUnhook),
-            State::SosPmApcString => out[0] = Some(Action::ApcEnd),
+            State::OscString => {
+                if let Some(cmd) = self.osc_parser.end(Some(c)) {
+                    f(0, Action::OscDispatch(cmd));
+                }
+            }
+            State::DcsPassthrough => f(0, Action::DcsUnhook),
+            State::SosPmApcString => f(0, Action::ApcEnd),
             _ => {}
         }
 
-        out[1] = self.do_action(action, c);
+        if let Some(a) = self.do_action(action, c) {
+            f(1, a);
+        }
 
         // Entry depends on new state
         match next_state {
@@ -362,20 +384,21 @@ impl Parser {
                     self.params[self.params_idx as usize] = self.param_acc;
                     self.params_idx += 1;
                 }
-                out[2] = Some(Action::DcsHook(Dcs::new(
-                    &self.intermediates[..self.intermediates_idx as usize],
-                    &self.params[..self.params_idx as usize],
-                    c,
-                )));
+                f(
+                    2,
+                    Action::DcsHook(Dcs::new(
+                        &self.intermediates[..self.intermediates_idx as usize],
+                        &self.params[..self.params_idx as usize],
+                        c,
+                    )),
+                );
             }
-            State::SosPmApcString => out[2] = Some(Action::ApcStart),
+            State::SosPmApcString => f(2, Action::ApcStart),
             _ => {}
         }
 
         // After generating the actions, we set our next state.
         self.state = next_state;
-
-        out
     }
 
     #[inline]
@@ -515,6 +538,20 @@ mod tests {
         }
     }
 
+    #[test]
+    fn next_with_matches_next() {
+        let input = b"\x1b[1;38:2::1:2:3m\x1b]0;title\x07a\n\x1bP1$qm\x1b\\\x1b_G\x1b\\\x1b[31";
+        let mut a = Parser::new();
+        let mut b = Parser::new();
+        for &c in input {
+            let expect: Vec<Action> = a.next(c).into_iter().flatten().collect();
+            let mut got = Vec::new();
+            b.next_with(c, |act| got.push(act));
+            assert_eq!(expect, got);
+            assert_eq!(a.state, b.state);
+        }
+    }
+
     #[test]
     fn parser_basic() {
         let mut p = Parser::new();
diff --git a/docs/rewrite/notes/perf-P1.md b/docs/rewrite/notes/perf-P1.md
index 2cb0b7d..8d98f4d 100644
--- a/docs/rewrite/notes/perf-P1.md
+++ b/docs/rewrite/notes/perf-P1.md
@@ -96,3 +96,25 @@ utf8 0.85 s, sgr 1.6 s (no Zig counterpart; profile shows no pathology).
   `osc-parser` bench uses it (the VT stream parser still feeds OSC bytes one at a time).
 - Remaining: render (2.5x): `RenderState::update` per-row `Vec<Style>` resize (84 B/cell, every
   row is styled in this benchmark) and cell copy; a lazy style vec did not help here, so reverted.
+
+## PF: last two gaps (production profile, hyperfine min wall, same data/machine as above)
+| benchmark | Zig ms | before ms | after ms | after/Zig |
+|---|---|---|---|---|
+| screen-clone render (1000x) | 4.7 | 11.5 | 4.1 | 0.87 |
+| terminal-parser sgr/csi (100 MB) | 354 | 538 | 231 | 0.65 |
+| terminal-parser utf8 | 293 | 349 | 190 | 0.65 |
+| terminal-parser ascii | 499 | 268 | 91 | 0.18 |
+| terminal-stream sgr (unchanged) | 670 | 693 | 654 | 0.98 |
+| screen-clone clone (unchanged) | 27.3 | 28.2 | 24.3 | 0.89 |
+
+- `RenderState` cells: `Cells.style: Vec<Style>` (one 21-byte Style per cell for every row with
+  managed memory) replaced by sparse storage: `style_ids: Vec<u16>` (0 = default) + `styles: Vec<Style>`
+  (one entry per distinct consecutive style / bg-colour cell). Use `Cells::style(x)`. The cell loop skips
+  plain cells with a single mask test (`page::Cell::is_plain`: tag Codepoint and style id 0); that
+  alone was the bulk of the win (7 -> 4 ms). Renderer `never_extend_bg` now takes a
+  `style_of: impl Fn(usize) -> Style` instead of a style slice.
+- `Parser::next_with(c, |action| ..)` visitor (shared `step` core, generic over the sink) avoids
+  materialising the `[Option<Action>; 3]`; `Parser::next` is kept as a wrapper (same slots) so the Stream
+  and all callers are unchanged; test `next_with_matches_next`. The bench uses `next_with(c, drop)`
+  (like Zig's `_ = actions`; the optimiser can elide unused CSI payload construction, same as in Zig).
+- Stream path was already at parity and is untouched.