perf(builtin): pattern-based fast paths for char predicates - #4112
Conversation
Restructure Char::is_ascii_whitespace, Char::is_whitespace and Char::is_numeric for speed while keeping every check in the char pattern DSL — no .to_int() arithmetic. Follow-up to the benchmark review of #3703, which proposed integer-comparison rewrites of the same three functions; the measurements there showed most of that PR's win comes from early-exit structure, not from integers, and that the arithmetic actually loses to patterns on some backends. - is_ascii_whitespace: six singleton alternatives become one range plus space: `self is ('\u{09}'..='\u{0D}' | ' ')`. - is_numeric: an ASCII fast path (`if self is ('0'..='9') { return true }`) and a below-table reject (`if self < '\u{B2}' { return false }`) in front of the unchanged range table (leading digit range removed from the table since the guard owns it). - is_whitespace: one ASCII branch (`if self <= '\u{20}'`) that decides via the same range pattern, with the non-ASCII table unchanged. Chosen over faster-on-one-backend alternatives because it is the only readable form that regresses nowhere: guard-based variants that tie the integer arithmetic on native are 1.7x slower on wasm-gc, and a single ordered match that ties on wasm-gc is 1.8x slower on native — a backend codegen sensitivity worth fixing in moonc rather than working around with integer code. Benchmarks (#3703's char_predicate_bench_test.mbt methodology, moon bench --release, same-day interleaved baseline on current main; main -> this branch): is_numeric ascii n=62000: native 886us -> 79us (11.2x), js 1.44ms -> 224us (6.4x), wasm-gc 2.65ms -> 164us (16.2x) is_numeric unicode n=22000: native 250us -> 156us, js 758us -> 415us, wasm-gc 807us -> 460us is_ascii_whitespace ascii n=62000: native 13.7us -> 13.9us (par), js 226us -> 112us (2.0x), wasm-gc 77.8us -> 70.5us (1.10x) is_whitespace ascii/unicode: native 67.4/26.7us -> 68.3/25.8us (par), js par, wasm-gc 147/53.1us -> 126/46.0us (1.16x/1.15x) For comparison, #3703's integer versions of the same functions measure 28.1us (native is_ascii_whitespace, 2.0x SLOWER than this pattern) and 212us on js (1.9x slower); its is_numeric ties this version on all three backends. Only is_whitespace keeps an arithmetic edge on native (32.2us vs 68.3us) at the cost of readability everywhere. Tests: char/predicate_reference_test.mbt keeps the previous flat patterns verbatim as reference functions and checks all three predicates against them for every Unicode scalar value (~1.1M code points). Mutation-verified: shifting the numeric reject bound by one fails at code point 178; shortening the ASCII whitespace range fails at code point 13. Codex CLI review (xhigh, first-round sign-off): "All three rewrites are set-equivalent; B2 is the numeric table minimum, and the whitespace guard fully decides every value through U+0020. Reference patterns match origin/main byte-for-byte. The inclusive sweep covers 1,112,064 scalar values; Int::to_char excludes exactly the surrogate range." Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Codex CLI <codex@openai.com>
Codex CLI reviewAdversarial review by Codex CLI ( Round 1 — approved, no blocking findings
Posted on behalf of Codex CLI by Claude Code. Validation outside Codex's read-only sandbox: full repo |
There was a problem hiding this comment.
Pull request overview
This PR restructures three hot-path Char predicate implementations in builtin to introduce pattern-based fast paths (early exits) while keeping the underlying Unicode classification tables expressed in the existing char pattern DSL. It also adds an exhaustive reference test to ensure the refactor is behavior-preserving across all Unicode scalar values.
Changes:
- Rewrote
Char::is_ascii_whitespaceto use a compact range-pattern fast path ('\u{09}'..='\u{0D}' | ' '). - Added an ASCII fast path and an early “below table minimum” reject for
Char::is_numeric, keeping the existing non-ASCII table intact. - Added an ASCII/low-codepoint fast path for
Char::is_whitespace, leaving the non-ASCII table unchanged. - Added an exhaustive equivalence test comparing the restructured predicates against the previous flat-pattern references for every Unicode scalar value.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
builtin/char.mbt |
Introduces pattern-based fast paths and early rejects for whitespace/numeric predicates without changing the non-ASCII classification tables. |
char/predicate_reference_test.mbt |
Adds exhaustive, full-Unicode scalar equivalence tests against the prior flat-pattern reference implementations to prevent off-by-one/table-regression bugs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Coverage Report for CI Build 6204Coverage decreased (-0.009%) to 90.708%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Addendum: why not
|
Summary
Pattern-based fast paths for
Char::is_ascii_whitespace,Char::is_whitespace, andChar::is_numeric— the speed of #3703's integer rewrites where patterns can deliver it, with every check staying in the char pattern DSL. Follow-up to the benchmark review posted on #3703 (thanks @mizchi for the bench methodology and for identifying the hot spots), which showed that most of that PR's win is early-exit structure, not integer arithmetic — and that the arithmetic actually loses to patterns on some backends.Changes
Benchmarks
#3703's
char_predicate_bench_test.mbtmethodology,moon bench --release, same-day interleaved baselines, branch cells are means of two runs (repeats within ~2%):is_numericascii n=62000is_numericunicode n=22000is_ascii_whitespaceasciiis_whitespaceascii / unicodeVersus #3703's integer versions: its
is_ascii_whitespacearithmetic measures 28.1 µs on native — 2.0x slower than this pattern — and 1.9x slower on js; itsis_numericties this version on all three backends. Onlyis_whitespacekeeps an arithmetic edge on native (32 vs 68 µs): every pattern form that reaches it there regresses another backend (char-compare guards are 1.7x slower on wasm-gc than the arithmetic and slower than main; a single orderedmatchties wasm-gc but is 1.8x slower on native). This PR takes the only readable form measured to regress nowhere, and treats the remaining gap as a moonc codegen issue (char patterns and comparisons should lower to the same decision tree regardless of surface syntax) rather than a reason to hand-write integer code in core.Tests
char/predicate_reference_test.mbtkeeps the previous flat patterns verbatim as reference functions and checks all three predicates against them for every Unicode scalar value (1,112,064 code points;Int::to_charexcludes exactly the surrogates). Mutation-verified: shifting the numeric reject bound by one fails at code point 178; shortening the ASCII whitespace range fails at code point 13.Test plan
moon test: 7462/7462charsuite 41/41 on wasm-gc, native, and jsmoon check --warn-list +unnecessary_annotationclean,moon fmtappliedmoon info: no.mbtichange (implementation-only)xhigh): first-round sign-off — set-equivalence, table-minimum claim, byte-identical references, and surrogate coverage all independently verified; see review comment🤖 Generated with Claude Code