perf(ascii): vectorize decode on non-JS targets - #3838
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes encoding/ascii decoding on non-JS targets by adding a SIMD (v128) fast path while preserving the existing JavaScript implementation and scalar performance for small inputs.
Changes:
- Add a non-JS SIMD implementation for
@ascii.decode, with a 64-byte crossover to keep short inputs on the scalar path. - Add whitebox tests covering non-zero
BytesViewoffsets and correct error slicing when an invalid byte occurs inside a SIMD block. - Add decode micro-benchmarks across a range of input sizes (15 bytes to 1,000,000 bytes).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| encoding/ascii/moon.pkg | Adds core/v128 dependency for SIMD decode and core/bench for benchmark tests; gates a wbtest file off JS. |
| encoding/ascii/decode.mbt | Introduces non-JS decode_scalar + decode_v128 and selects between them; keeps JS decode intact via #cfg. |
| encoding/ascii/decode_v128_wbtest.mbt | Adds whitebox tests for view offsets + first-invalid-byte reporting within a SIMD block. |
| encoding/ascii/ascii_bench_test.mbt | Adds benchmark tests for multiple sizes to validate the intended crossover behavior and speedups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9d12cb3 to
054fe0e
Compare
054fe0e to
7793b06
Compare
|
Rebased onto latest main (clean, no conflicts) and pushed a follow-up commit ( ReviewThe SIMD design is correct and matches the scalar decoder exactly:
Possible follow-up (not blocking): Added tests (
|
| Target | n=100,000 before | after | n=1,000,000 before | after | Change |
|---|---|---|---|---|---|
| native | 27.57 µs | 11.53 µs | 274.82 µs | 80.03 µs | 2.4–3.4x |
| wasm | 75.47 µs | 15.16 µs | 759.01 µs | 149.31 µs | 5.0–5.1x |
| wasm-gc | 236.47 µs | 166.59 µs | 2.35 ms | 1.55 ms | 1.4–1.5x |
| js | 478.89 µs | 473.83 µs | 5.48 ms | 5.46 ms | parity (path unchanged) |
At the crossover, n=64: native 31.1 → 26.6 ns, wasm 86.7 → 64.4 ns, wasm-gc 167.1 → 101.2 ns; n=15/16/32 are at parity on every backend (scalar path retained).
|
Ran the full branch through Codex CLI review (
Its probing included the With the earlier comment's rebase, property tests, and 4-backend benchmark verification, this looks ready from my side. |
Add quickcheck properties pinning decode/decode_lossy against a byte-by-byte model on poisoned views (invalid bytes just outside the view bounds), the Malformed view against the first invalid byte, the SIMD decoder against the scalar decoder on arbitrary payloads and unaligned view offsets, and finish_string truncation; plus an exhaustive invalid-byte position sweep around SIMD block and crossover boundaries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7793b06 to
7e84e0e
Compare
Summary
@ascii.decode.Correctness
The SIMD path uses
i8x16_bitmaskto locate the first byte with its high bit set. On failure it raisesMalformedwith the same remainingBytesViewas the scalar decoder.Benchmarks
moon bench -p encoding/ascii -f ascii_bench_test.mbt --target <target> --release --no-parallelizen=15,n=16, andn=32remain on the scalar path and stayed within measurement variance of the baseline.Validation
moon test -p encoding/ascii --target all --no-rendermoon check encoding/ascii --target all --no-rendermoon info -p encoding/asciimoon fmt --checkfor all changed filesgit diff --checkThe installed compiler rejects an unrelated pattern in
builtin/iterator.mbt:789on an unmodified upstream checkout. The test and check commands above used a temporary equivalent rewrite of that pattern; it is not included in this PR.moon bench --profileis not available in the installed Moon CLI, so the validation uses release benchmarks and generated-C inspection instead.