Optimize Unicode predicate checks - #3703
Conversation
16732ee to
77cf43b
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the encoding/utf8 package with a strict, allocation-free (non-JS) UTF-8 validation API and refactors shared malformed-byte detection to support JS error reporting, while also adding small fast paths for common Char Unicode predicate helpers to improve hot-path performance in core Unicode handling.
Changes:
- Added
@encoding/utf8.is_valid(BytesView, ignore_bom?) -> Boolwith target-specific implementations (JS viaTextDecoder(fatal: true), non-JS via a shared scalar malformed-byte finder). - Centralized strict malformed-byte scanning in
encoding/utf8/decode.mbtand reused it for the JSdecodemalformed-suffix path. - Added small integer fast paths for
Char::is_ascii_whitespace,Char::is_whitespace, andChar::is_numeric, plus targeted tests/docs and benchmark coverage.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| encoding/utf8/README.mbt.md | Documents the new is_valid API with a short example. |
| encoding/utf8/pkg.generated.mbti | Exposes is_valid in the generated public interface. |
| encoding/utf8/moon.pkg | Adds target gating for new is_valid_* files and imports bench for benchmarks. |
| encoding/utf8/is_valid_nonjs.mbt | Implements non-JS is_valid via BOM handling + shared malformed scan. |
| encoding/utf8/is_valid_js.mbt | Implements JS is_valid using TextDecoder with fatal: true. |
| encoding/utf8/is_valid_bench_test.mbt | Adds benchmarks comparing is_valid vs decode across ASCII/mixed inputs. |
| encoding/utf8/decode.mbt | Adds shared strict malformed-byte finder with an ASCII fast-skip. |
| encoding/utf8/decode_test.mbt | Adds validation tests for is_valid and BOM-related cases. |
| encoding/utf8/decode_js.mbt | Switches JS malformed-suffix computation to reuse the shared malformed finder. |
| char/char_test.mbt | Adds boundary/edge-case assertions for updated numeric/whitespace predicates. |
| builtin/char.mbt | Adds fast paths for whitespace/numeric helpers using integer codepoint checks. |
| builtin/char_predicate_bench_test.mbt | Adds benchmarks for the updated Char predicate helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
77cf43b to
f0e3929
Compare
e54e087 to
9b78131
Compare
Benchmark review of the
|
| ascii n=62000 | main (6 singletons) | this PR (int arith) | range pattern |
|---|---|---|---|
| native | 13.6 µs | 28.1 µs | 13.4 µs |
| js | 224.1 µs | 212.3 µs | 109–118 µs |
| wasm-gc | 78.3 µs | 58.8 µs | 68.7 µs |
On native the arithmetic is 2.1x slower than the pattern; on js the pattern is ~1.9x faster than the arithmetic. wasm-gc is the only backend where the arithmetic wins (by ~14%). Recommendation: take the range pattern, not the arithmetic.
2. is_numeric — 100% of the −95% win is the early-exit structure, and it's expressible in patterns
The gain comes from not entering the ~140-line table for ASCII, not from integers. This form keeps the fast path readable and the table untouched:
pub fn Char::is_numeric(self : Self) -> Bool {
if self is ('0'..='9') {
return true
}
if self < '\u{B2}' {
return false
}
self
is ('\u{B2}' // ... table unchanged, leading '0'..='9' range removed
)
}| main | this PR (int arith) | pattern fast path | |
|---|---|---|---|
| native ascii | 882.8 µs | 80.1 µs | 75.8 µs |
| native unicode | 246.1 µs | 152.5 µs | 150.3 µs |
| js ascii | 1.46 ms | 216.3 µs | 215.6 µs |
| js unicode | 750.1 µs | 395.5 µs | 402.8 µs |
| wasm-gc ascii | 2.56 ms | 153.9 µs | 161.6 µs |
| wasm-gc unicode | 830.8 µs | 444.7 µs | 457.1 µs |
Statistical tie with the arithmetic on all three backends (11.6x / 6.8x / 15.8x over main on ASCII). Recommendation: take the pattern form.
3. is_whitespace — the one place the arithmetic still earns its keep, and it exposes a codegen gap
I tried several readable shapes. Two are interesting:
// (a) same control flow as this PR, in char terms
if self <= '\u{20}' {
return self is ('\u{09}'..='\u{0D}' | ' ')
}
if self < '\u{85}' || self > '\u{3000}' {
return false
}
self
is ('\u{85}' | '\u{A0}' | '\u{1680}' | '\u{2000}'..='\u{200A}'
| '\u{2028}' | '\u{2029}' | '\u{202F}' | '\u{205F}' | '\u{3000}')
// (b) one ordered match
match self {
'\u{09}'..='\u{0D}' | ' ' => true
'\u{00}'..='\u{84}' => false
'\u{85}' | '\u{A0}' | '\u{1680}' | '\u{2000}'..='\u{200A}'
| '\u{2028}' | '\u{2029}' | '\u{202F}' | '\u{205F}' | '\u{3000}' => true
_ => false
}| ascii / unicode | main | this PR (int arith) | (a) char compares | (b) ordered match |
|---|---|---|---|---|
| native | 66.9 / 26.4 | 32.2 / 17.2 | 32.0 / 17.1 | 58.0 / 37.1 |
| js | 243.5 / 93.6 | 227.2 / 88.2 | 224.9 / 87.0 | 224.3 / 87.2 |
| wasm-gc | 146.0 / 53.0 | 102.2 / 52.4 | 177.7 / 72.9 | 102.0 / 49.7 |
Every cell is reproducible (re-runs within ~1%). The strange part: (a) exactly ties the arithmetic on native and js but is 1.7x slower on wasm-gc — slower than main, even — while (b) exactly ties the arithmetic on wasm-gc and js but is 1.8x slower on native. Two more data points on the sensitivity: pattern-based guards (if self is ('\u{00}'..='\u{20}') then guard self is ('\u{85}'..='\u{3000}')) land at 159.5 µs on wasm-gc, and adding a '\u{3001}'..='\u{10FFFF}' => false arm to (b) swings wasm-gc from 102 → 169 µs.
So for is_whitespace specifically, no single source-level pattern form matches the integer arithmetic on all three backends today — but the per-backend ties show the machine code is achievable from patterns; which shape wins is a backend codegen accident (char Compare vs pattern lowering on wasm-gc, match vs if-chains on native). That looks like a moonc issue worth filing: char range/or patterns and char comparisons should lower to the same decision tree regardless of surface syntax. The is_numeric table is further evidence — even after the ASCII exit, the ~80-range table costs ~7 ns/char (a linear-ish scan); a compiler-emitted binary search over the ranges would beat every variant measured here.
Suggested resolution for this PR
is_ascii_whitespace: useself is ('\u{09}'..='\u{0D}' | ' ')— faster than the arithmetic on native and js, and more readable than both current forms.is_numeric: use the pattern-based fast path — ties the arithmetic everywhere, keeps the table.is_whitespace: either keep the integer arithmetic with a comment noting it's a workaround for backend-dependent pattern codegen, or take form (a)/(b) and accept one backend's regression until the codegen gap is fixed upstream.
(The UTF-8 is_valid part of this PR is not affected by any of this. Measurements: moon bench --release per backend on darwin/arm64; baseline = current main, which this PR currently conflicts with — the three predicates are unchanged on main, so the comparison holds.)
Benchmarks and analysis by Claude Code on behalf of @bobzhang.
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>
|
done in #4112 |
Motivation
This PR is intended as another concrete example of what the core library can do if the
V128type is available to core implementations. Unicode and UTF-8 routines are common byte-oriented hot paths, and they are good candidates for target-specific fast paths hidden behind stable package APIs.The new UTF-8 validity API is allocation-free on non-JS targets and keeps the same strict contract as
decode. It does not useV128yet because the natural input type isBytesView; a zero-copy way to loadV128fromBytes/BytesViewwould make SIMD UTF-8 scanning practical for wasm-gc as well. The char predicate changes are included as helper-level fast paths around the same Unicode boundary.Summary
@encoding/utf8.is_valid(BytesView, ignore_bom?)for strict UTF-8 validation without decoding on non-JS targets.decodeerror suffix path.Char::is_ascii_whitespace,Char::is_whitespace, andChar::is_numeric.UTF-8 Validation
The new
@encoding/utf8.is_validAPI follows the same strict UTF-8 contract as@encoding/utf8.decode: it rejects overlong sequences, UTF-16 surrogates, code points above U+10FFFF, lone continuation bytes, truncated sequences, and invalid leading bytes.The non-JS path uses a
BytesViewscalar walker with an ASCII-prefix skip. I did not useV128here because the current public V128 load APIs takeFixedArray[Byte], while this API naturally works onBytesView; using V128 for wasm-gc would need a zero-copy way to load fromBytes/BytesView.There was no previous
is_validAPI, so the benchmark compares validation againstdecodeas the existing way to validate well-formed UTF-8.Command:
wasm-gc --release
is_validdecodewasm --release
is_validdecodejs --release
is_validdecodeChar Helper Predicate Benchmarks
The char predicate changes are helper-level fast paths for common low-codepoint checks used around Unicode handling. Baseline was measured on
upstream/mainwith only the new benchmark file added. After was measured on this branch.Command:
wasm-gc --release
Char::is_ascii_whitespaceascii n=62000Char::is_whitespaceascii n=62000Char::is_whitespaceunicode n=22000Char::is_numericascii n=62000Char::is_numericunicode n=22000wasm --release
Char::is_ascii_whitespaceascii n=62000Char::is_whitespaceascii n=62000Char::is_whitespaceunicode n=22000Char::is_numericascii n=62000Char::is_numericunicode n=22000Validation
moon test --enable-coveragepassed with 6594 tests.