Skip to content

Numfmt: TEXT() and chart labels parse each format code once (#806) - #808

Merged
h-a-n-a-s-h-i merged 2 commits into
storytold:mainfrom
vangel76:perf/numfmt-text-parse-cache
Oct 13, 2026
Merged

h-a-n-a-s-h-i merged 2 commits into
storytold:mainfrom
vangel76:perf/numfmt-text-parse-cache

Conversation

@vangel76

Copy link
Copy Markdown
Contributor

Fixes #806

What was redundant

text_function called NumberFormat::parse(code) on every call, and the evaluator calls it once per element, so =TEXT(A1:A100000,"0.00") parsed the same code 100,000 times. chart/src/resolve.rs did the same per resolved value, and Fmt::new in chart/src/axis.rs per label helper call. A parsed format depends on the code text only (date system and value are applied when formatting), so one parse per distinct code is enough. No timings claimed.

What changed

  • numfmt: NumberFormat::parse_cached(code), a per-thread cache (thread_local, no lock, no unsafe, no dependency, wasm-safe). At most 64 codes; a full cache is emptied. Codes over 256 bytes are never stored. If the cache is unavailable (thread shutdown, re-entrant borrow) it falls back to a plain parse; no unwrap/expect.
  • text_function, chart/resolve.rs and chart/axis.rs use it. The calc evaluator gets it through text_function. NumberFormat::parse is unchanged.

Tests (crates/numfmt/src/tests.rs)

  • Equality: 45 codes (numbers, dates, fractions, text sections, conditions, scientific, fill, invalid codes) x 14 values x both date systems. text_function equals a copy of the old uncached implementation with the cache cold and warm, and format_value with parse_cached equals format_value with parse.
  • Counter (cfg(test) parse counter): 10,000 values with one code parse once; two alternating codes parse twice; capacity overflow evicts (old code reparsed, newest still cached); a long code is parsed every time.

Commands run

cargo test -p gridcraft-numfmt, cargo test -p gridcraft-calc -p gridcraft-functions -p gridcraft-chart, cargo test -p gridcraft-engine --lib display, cargo clippy -p gridcraft-numfmt -p gridcraft-chart --all-targets -- -D warnings, cargo fmt on numfmt and chart, cargo xtask layers, cargo check -p gridcraft-numfmt --target wasm32-unknown-unknown.

Left out

  • The engine's global Mutex<HashMap> cache in display.rs is untouched (it holds the lock while parsing).
  • Per-call allocations in formatting are not part of this PR.
  • Overlap with M12: regional formats (typed and displayed numbers, dates and times) #447 checked: its cache is for constant region codes keyed by &'static str; this one is keyed by user code. Possible textual conflict in the lib.rs imports only (mine adds a self-contained module and uses full paths).

🤖 Generated with Claude Code

…d#806)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
# Conflicts:
#	ROADMAP.md
@h-a-n-a-s-h-i
h-a-n-a-s-h-i merged commit 8e60c2a into storytold:main Oct 13, 2026
1 check failed
@echelon

echelon commented Oct 13, 2026

Copy link
Copy Markdown
Member

Merged — thank you @vangel76 for the contribution!

I merged main into your branch to resolve conflicts with other PRs that landed first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Perf: TEXT() and chart labels parse the format code for every value

3 participants