Merge pull request #808 from vangel76:perf/numfmt-text-parse-cache

ROADMAP.md

@@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
 
 | Date | What landed |
 |---|---|
+| 2026-10-12 | `TEXT()` over a range and chart labels parse each number format code once instead of once per value (#806) |
 | 2026-10-12 | SUBTOTAL(109) over hidden rows recalculates after Row Height, AutoFit Row Height and Ungroup reveal rows (found by the command-sequence fuzz) |
 | 2026-10-12 | Page Layout view: drag the margin edges on the rulers to set the page margins (resize cursor, live preview, value tooltip, one undo step, Esc cancels) (#792) |
 | 2026-10-12 | Forecast Sheet gets Forecast Start: begin the forecast inside the history to compare it with the actuals (#785) |

crates/chart/src/axis.rs

@@ -122,7 +122,7 @@ impl Fmt {
     pub fn new(code: &str) -> Fmt {
         let t = code.trim();
         let general = t.is_empty() || t.eq_ignore_ascii_case("general");
-        Fmt { general, fmt: NumberFormat::parse(if general { "General" } else { t }) }
+        Fmt { general, fmt: NumberFormat::parse_cached(if general { "General" } else { t }) }
     }
     /// Formats `v`; `step` (axis labels) fixes the decimals for General.
     pub fn format(&self, v: f64, step: Option<f64>) -> String {

crates/chart/src/resolve.rs

@@ -147,7 +147,7 @@ fn category_texts(wb: &Workbook, sheet: usize, formula: &str) -> Vec<String> {
                 if let Some(c) = at {
                     let code = &wb.styles.get(s.style_id(c)).num_fmt.0;
                     if !code.eq_ignore_ascii_case("general") {
-                        return format_value(v, &NumberFormat::parse(code), wb.date_system).text;
+                        return format_value(v, &NumberFormat::parse_cached(code), wb.date_system).text;
                     }
                 }
             }

crates/numfmt/src/lib.rs

@@ -101,6 +101,8 @@ impl Default for NumberFormat {
 impl NumberFormat {
     /// Parses a format code. Never fails: unknown or malformed parts degrade to literal text.
     pub fn parse(code: &str) -> NumberFormat {
+        #[cfg(test)]
+        parse_cache::count_parse();
         let trimmed = code.trim();
         // `checkbox` marks a checkbox cell (Insert › Checkbox): TRUE/FALSE draw as a box, any other
         // value displays as General.
@@ -295,6 +297,78 @@ pub fn format_value(v: &Value, fmt: &NumberFormat, sys: DateSystem) -> Formatted
     }
 }
 
+/// A small per-thread cache of parsed format codes for callers that format many values with the
+/// same code (`TEXT` over a range, chart labels). A parsed format depends on the code text only
+/// (the date system and values are applied when formatting), so the code is the whole key.
+mod parse_cache {
+    use std::cell::RefCell;
+    use std::collections::HashMap;
+
+    use super::NumberFormat;
+
+    /// Most codes kept per thread. A full cache is emptied, so memory stays bounded.
+    pub(super) const CAPACITY: usize = 64;
+    /// Longer codes are parsed every time and never stored.
+    pub(super) const MAX_CODE_LEN: usize = 256;
+
+    thread_local! {
+        static CACHE: RefCell<HashMap<String, NumberFormat>> = RefCell::new(HashMap::new());
+    }
+
+    pub(super) fn get(code: &str) -> NumberFormat {
+        if code.len() > MAX_CODE_LEN {
+            return NumberFormat::parse(code);
+        }
+        // Not usable (thread shutting down, re-entered): parse without the cache.
+        let cached = CACHE
+            .try_with(|c| {
+                let mut c = c.try_borrow_mut().ok()?;
+                if let Some(f) = c.get(code) {
+                    return Some(f.clone());
+                }
+                let f = NumberFormat::parse(code);
+                if c.len() >= CAPACITY {
+                    c.clear();
+                }
+                c.insert(code.to_string(), f.clone());
+                Some(f)
+            })
+            .ok()
+            .flatten();
+        cached.unwrap_or_else(|| NumberFormat::parse(code))
+    }
+
+    #[cfg(test)]
+    thread_local! {
+        static PARSES: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+    }
+
+    #[cfg(test)]
+    pub(super) fn count_parse() {
+        let _ = PARSES.try_with(|p| p.set(p.get() + 1));
+    }
+
+    /// Parses done on this thread so far (tests only).
+    #[cfg(test)]
+    pub(super) fn parses() -> usize {
+        PARSES.try_with(std::cell::Cell::get).unwrap_or(0)
+    }
+
+    /// Empties this thread's cache (tests only).
+    #[cfg(test)]
+    pub(super) fn clear() {
+        let _ = CACHE.try_with(|c| c.borrow_mut().clear());
+    }
+}
+
+impl NumberFormat {
+    /// Like [`NumberFormat::parse`], but remembers recently parsed codes (per thread, bounded) so
+    /// formatting many values with one code parses it once. Same result as `parse`.
+    pub fn parse_cached(code: &str) -> NumberFormat {
+        parse_cache::get(code)
+    }
+}
+
 /// The `TEXT()` worksheet function: formats a value with a format code. Numeric text is
 /// converted to a number first; other text and booleans (as `TRUE`/`FALSE`) go through the text
 /// section, or are returned as is.
@@ -302,7 +376,7 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result<String, C
     if code.is_empty() {
         return Ok(String::new());
     }
-    let fmt = NumberFormat::parse(code);
+    let fmt = NumberFormat::parse_cached(code);
     let n = match v.scalar() {
         Value::Error(e) => return Err(e),
         Value::Empty => 0.0,

crates/numfmt/src/tests.rs

@@ -614,3 +614,174 @@ fn decimal_digits_keeps_fifteen_digits_and_truncates_a_tie() {
     let (d, e) = decimal_digits(999999999999999.6);
     assert_eq!((d, e), ([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 15));
 }
+
+// ---- parse cache ----
+
+/// `TEXT()` as it was before the cache: parse the code, then format.
+fn text_uncached(v: &Value, code: &str, sys: DateSystem) -> Result<String, CellError> {
+    if code.is_empty() {
+        return Ok(String::new());
+    }
+    let fmt = NumberFormat::parse(code);
+    let n = match v.scalar() {
+        Value::Error(e) => return Err(e),
+        Value::Empty => 0.0,
+        Value::Number(n) => n,
+        Value::Bool(b) => return Ok(fmt.format_text(if b { "TRUE" } else { "FALSE" }).text),
+        Value::Text(t) => match gridcraft_core::parse::parse_number_text(&t) {
+            Some(n) => n,
+            None => return Ok(fmt.format_text(&t).text),
+        },
+        Value::Array(_) => return Err(CellError::Value),
+    };
+    if !n.is_finite() {
+        return Err(CellError::Num);
+    }
+    let (f, overflow) = fmt.format_number(n, sys);
+    if overflow { Err(CellError::Value) } else { Ok(f.text) }
+}
+
+const CACHE_CODES: &[&str] = &[
+    "0",
+    "0.00",
+    "#,##0.00",
+    "#,##0.00;[Red](#,##0.00)",
+    "$#,##0.00",
+    "0%",
+    "0.0%",
+    "0.00E+00",
+    "##0.0E+0",
+    "General",
+    "general",
+    "checkbox",
+    "@",
+    "0;@",
+    "0;-0;;@",
+    "00000",
+    "# ?/?",
+    "# ??/16",
+    "?/?",
+    "m/d/yyyy",
+    "yyyy-mm-dd hh:mm:ss",
+    "dddd, mmmm d, yyyy",
+    "h:mm AM/PM",
+    "[h]:mm:ss",
+    "mm:ss.000",
+    "[Red]0;[Blue]-0",
+    "[>100]0.0;[<5]0.000;0",
+    "[<=9999999]###-####;(###) ###-####",
+    "[$-409]d mmm yy",
+    "\"abc\"0",
+    "\\x0",
+    "*-0",
+    "_(* #,##0_)",
+    "0.0.0",
+    "[",
+    "\"abc",
+    "0;0;0;0;0",
+    "#,##0.00;[Red",
+    "[$-]0",
+    "  0.00  ",
+    "[Color99]0",
+    "0.00_);(0.00)",
+    "#,,\"M\"",
+    "yyyy",
+    "",
+];
+
+fn cache_values() -> Vec<Value> {
+    vec![
+        Value::Number(0.0),
+        Value::Number(1234.5678),
+        Value::Number(-42.5),
+        Value::Number(0.5),
+        Value::Number(45000.75),
+        Value::Number(1e15),
+        Value::Number(1e-7),
+        Value::Number(-0.0001),
+        Value::Number(f64::INFINITY),
+        Value::from("abc"),
+        Value::from("12.5"),
+        Value::Bool(true),
+        Value::Empty,
+        Value::Error(CellError::NA),
+    ]
+}
+
+#[test]
+fn parse_cached_gives_the_same_results_cold_and_warm() {
+    assert!(CACHE_CODES.len() >= 40);
+    parse_cache::clear();
+    let values = cache_values();
+    for sys in [DateSystem::D1900, DateSystem::D1904] {
+        // Cold on the first system, warm on the second.
+        for code in CACHE_CODES {
+            let direct = NumberFormat::parse(code);
+            let cached = NumberFormat::parse_cached(code);
+            assert_eq!(cached.code(), direct.code(), "{code:?}");
+            assert_eq!(
+                (cached.kind(), cached.is_date(), cached.is_text(), cached.is_general()),
+                (direct.kind(), direct.is_date(), direct.is_text(), direct.is_general())
+            );
+            for v in &values {
+                let want = text_uncached(v, code, sys);
+                assert_eq!(text_function(v, code, sys), want, "TEXT({v:?}, {code:?})");
+                // Again, now certainly warm.
+                assert_eq!(text_function(v, code, sys), want, "warm TEXT({v:?}, {code:?})");
+                assert_eq!(format_value(v, &cached, sys), format_value(v, &direct, sys), "{v:?} {code:?}");
+            }
+        }
+    }
+}
+
+#[test]
+fn parse_cached_parses_each_code_once() {
+    parse_cache::clear();
+    let before = parse_cache::parses();
+    for i in 0..10_000 {
+        assert!(text_function(&Value::Number(f64::from(i) + 0.5), "0.00", S).is_ok());
+    }
+    assert_eq!(parse_cache::parses() - before, 1);
+
+    let before = parse_cache::parses();
+    for i in 0..10_000 {
+        let code = if i % 2 == 0 { "0.00" } else { "#,##0.0" };
+        assert!(text_function(&Value::Number(f64::from(i)), code, S).is_ok());
+    }
+    // "0.00" is still cached from above: only the second code is new.
+    assert_eq!(parse_cache::parses() - before, 1);
+
+    parse_cache::clear();
+    let before = parse_cache::parses();
+    for i in 0..10_000 {
+        let code = if i % 2 == 0 { "0.00" } else { "#,##0.0" };
+        assert!(text_function(&Value::Number(f64::from(i)), code, S).is_ok());
+    }
+    assert_eq!(parse_cache::parses() - before, 2);
+}
+
+#[test]
+fn parse_cache_is_bounded() {
+    parse_cache::clear();
+    // More distinct codes than the capacity: all still format correctly...
+    let n = parse_cache::CAPACITY + 10;
+    for i in 0..n {
+        let c = format!("0.00\"{i}\"");
+        assert_eq!(text_function(&Value::Number(1.0), &c, S), Ok(format!("1.00{i}")));
+    }
+    // ...the first ones were evicted (parsed again), the newest is still cached.
+    let before = parse_cache::parses();
+    let newest = format!("0.00\"{}\"", n - 1);
+    assert!(text_function(&Value::Number(1.0), &newest, S).is_ok());
+    assert_eq!(parse_cache::parses() - before, 0);
+    assert!(text_function(&Value::Number(1.0), "0.00\"0\"", S).is_ok());
+    assert_eq!(parse_cache::parses() - before, 1);
+
+    // A very long code is parsed on every call and not stored.
+    let long = format!("0\"{}\"", "x".repeat(parse_cache::MAX_CODE_LEN));
+    let before = parse_cache::parses();
+    for _ in 0..3 {
+        assert!(text_function(&Value::Number(1.0), &long, S).is_ok());
+    }
+    assert_eq!(parse_cache::parses() - before, 3);
+}