perf(string): accelerate ASCII character sets - #3842
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes common string character-set operations by introducing an ASCII fast path that uses a compact bitmap representation, improving performance for typical delimiter/whitespace trimming and scanning workloads in the MoonBit standard library.
Changes:
- Added a 128-bit ASCII character-set bitmap representation (four scalar
UInts) and helper routines to test membership and scan/traverse strings efficiently. - Routed
StringView::contains_any,trim_start,trim_end, andtrimthrough the ASCII bitmap path when the provided character set is ASCII-only, while preserving the existing non-ASCII fallback behavior. - Added regression coverage in
builtin/string_methods.mbtand introduced new benchmarks inbuiltin/string_char_set_bench_test.mbt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| builtin/string_methods.mbt | Implements ASCII bitmap fast paths for contains_any/trim* and adds regression tests to validate ASCII vs non-ASCII behavior. |
| builtin/string_char_set_bench_test.mbt | Adds benchmarks for contains_any and trimming on long inputs to quantify performance improvements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cee41c8 to
bc2767f
Compare
bc2767f to
4dc8400
Compare
|
Reviewed the bitmap approach and pushed two follow-up commits ( Review of the bitmap baselineCorrectness is solid: the ASCII bitmap is equivalent to
What the follow-up commits do
Benchmarks (Apple Silicon; before = PR head
|
| Benchmark | target | before | after | Change |
|---|---|---|---|---|
contains_any miss n=100000 |
native | 76.40 µs | 14.60 µs | 5.2x |
trim_start n=100000 |
native | 307.02 µs | 14.72 µs | 20.9x |
trim_end n=100000 |
native | 244.57 µs | 14.70 µs | 16.6x |
trim n=100000 |
native | 565.59 µs | 29.46 µs | 19.2x |
| trim short inputs | native | 149.92 ns | 108.90 ns | 1.4x |
contains_any miss n=100000 |
wasm-gc | 140.10 µs | 99.43 µs | 1.4x |
trim_start n=100000 |
wasm-gc | 591.40 µs | 105.91 µs | 5.6x |
trim n=100000 |
wasm-gc | 1.36 ms | 216.61 µs | 6.3x |
| trim short inputs | wasm-gc | 209.73 ns | 89.84 ns | 2.3x |
contains_any miss n=100000 |
js | 211.32 µs | 211.16 µs | parity |
trim_start n=100000 |
js | 522.51 µs | 351.60 µs | 1.5x |
trim n=100000 |
js | 977.83 µs | 724.59 µs | 1.3x |
| trim short inputs | js | 198.08 ns | 94.71 ns | 2.1x |
Short default trim() is now also faster than pre-PR main (108.9 ns vs ~114 ns), so the short-input regression is gone. The only measured cost is js short contains_any at 83.7 ns vs 78.6 ns (~1.2 ns/call).
Validation
moon fmt,moon info(no public API changes),moon check --target allmoon test -p moonbitlang/core/builtin --target all: 2957 (wasm) / 2957 (wasm-gc) / 2934 (js) / 2916 (native), all passing
Coverage Report for CI Build 6250Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.02%) to 90.763%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
Follow-up on the review loop for the SIMD commits. Round 1 (Codex CLI, Round 2 re-review came back clean. Its signoff:
It also benchmarked the wasm (linear) backend independently, where the SIMD path is active as well: Final state: |
Scan eight UTF-16 code units at a time on native and wasm, comparing each block against every set member broadcast to a vector (at most eight members; larger sets keep the scalar bitmap scan). The scalar paths now scan raw code units instead of decoded characters, which is equivalent for ASCII-only sets since an ASCII code unit is never half of a surrogate pair; the JavaScript backend keeps the character iterator for contains_any, where it compiles to a faster loop. Inlining build_ascii_char_set removes the short-input trim overhead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add quickcheck properties pinning contains_any and the trims against a character-by-character model on adversarial strings and offset views, an exhaustive sweep across SIMD block boundaries for every set size class, lone-surrogate cases, and short-input benchmarks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An empty set reached ascii_char_set_splat, which read code unit 0 beyond the set view's bounds; an empty view over a longer backing string then broadcast the backing character as a set member and trimmed data that should have been kept. Found by adversarial review. Guard the SIMD scans on a non-empty set, cover the empty-set and empty-view cases directly, and pass generated sets through offset views in the quickcheck properties so out-of-view reads see sentinel characters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
aeac20f to
ad51fd2
Compare
Summary
UIntbitmaps.contains_any,trim_start,trim_end, andtrimthrough the bitmap scan.contains_charimplementation for sets containing non-ASCII characters.Why
The previous multi-character paths scanned
charsfor every character in the input (O(n × m)). ASCII delimiter sets are common in trimming and token scanning; building the set once makes the scanO(n + m).The bitmap is passed as scalar words, so the optimized path introduces no temporary heap allocation.
Benchmarks
Input size: 100,000 characters. Lower is better.
contains_anyASCII misscontains_anymatch at endtrim_starttrim_endtrimValidation
moon test builtin/string_methods.mbt --target native --quietmoon test builtin/string_methods.mbt --target wasm-gc --quietmoon test builtin/string_methods.mbt --target js --quietmoon check builtin --target allmoon info builtinbuiltin/string_char_set_bench_test.mbtNo public API changes.