perf(hex): vectorize encode/decode on the linear-memory backends - #4122
Conversation
`encode` and `decode` gain v128 fast paths on native and wasm, mirroring the structure already used by `encoding/base64`: the scalar codec is retained on every backend as the reference implementation, and the public entry points dispatch with `#cfg`. Encoding processes 16 source bytes per iteration: split each byte into its nibbles, interleave them into output order with two `i8x16_shuffle`s, and map nibbles to ASCII with a register-resident `i8x16_swizzle` table. The 32 ASCII characters are widened to UTF-16 before being stored. Decoding processes 32 characters per iteration: narrow the UTF-16 code units to bytes with `i8x16_narrow_i16x8_u`, whose signed-lane saturation sends every non-ASCII code unit to 255 or 0 and so keeps one from masquerading as a hex digit by way of its low byte; validate the three hex ranges vectorially; and fold to nibbles with wrapping adds. Anything the fast path will not take -- short, odd-length, or invalid input -- falls back to the scalar decoder, which stays the single authority on the `Malformed` rules. Benchmarks (4 KiB payload, scalar vs public entry point): | target | encode scalar | encode | speedup | decode scalar | decode | speedup | | ------ | ------------- | ------- | ------- | ------------- | ------- | ------- | | native | 16.33 us | 634 ns | 25.8x | 7.56 us | 815 ns | 9.3x | | wasm | 35.64 us | 947 ns | 37.6x | 21.17 us | 1.33 us | 15.9x | | js | 38.60 us | 35.93 us| 1.07x | 43.38 us | 44.01 us| 0.99x | js keeps the scalar path by `#cfg` and is unchanged, as expected. Correctness is covered by differential white-box tests -- fast path vs scalar for every length from 0 to 79, for views that start and end inside a larger buffer, and for an invalid character planted at every lane position -- and by quickcheck properties: decode-encode round-trips against an independent reference encoder, case-mixture round-trips, corruption at any position raising `Malformed`, and odd-length input raising `Malformed`. A further test plants raw UTF-16 code units that alias a hex digit in their low byte, lone surrogates included, which a truncating narrow would wrongly accept. Both block loops bound themselves by subtraction and the encoder refuses an input whose output size would not fit in an `Int`, so no vector store can be reached through a wrapped index.
Codex CLI review (
|
There was a problem hiding this comment.
Pull request overview
This PR adds SIMD (v128) fast paths for encoding/hex on the linear-memory backends (native + wasm), while preserving the scalar implementation as the reference and fallback. It mirrors the structure used by encoding/base64, using #cfg to dispatch at the public entry points.
Changes:
- Add
encode_v128/decode_v128implementations and dispatchencode/decodeto them on#cfg(any(target="native", target="wasm")). - Add differential white-box tests plus QuickCheck properties to pin vector behavior against scalar behavior and error rules.
- Add white-box benchmarks to measure scalar vs public entry points side-by-side.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| encoding/hex/encode.mbt | Adds #cfg dispatch so encode uses scalar on non-linear-memory targets and v128 on native/wasm. |
| encoding/hex/encode_v128.mbt | New v128 encoder implementation for 16-byte blocks with UTF-16 widening stores. |
| encoding/hex/decode.mbt | Adds #cfg dispatch so decode uses scalar on non-linear-memory targets and v128-with-fallback on native/wasm. |
| encoding/hex/decode_v128.mbt | New v128 decoder implementation for 32-code-unit blocks with vector validation + nibble folding. |
| encoding/hex/v128_wbtest.mbt | New differential white-box tests validating v128 paths vs scalar paths and view offset handling. |
| encoding/hex/quickcheck_test.mbt | New randomized property tests covering round-trips, case mixtures, corruption, and odd-length failure. |
| encoding/hex/v128_bench_wbtest.mbt | New white-box benchmarks comparing scalar vs public entry points at multiple sizes. |
| encoding/hex/moon.pkg | Adds v128 dependency plus test/wbtest imports consistent with SIMD-enabled encoding packages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let bytes_view = padded[pad:pad + length] | ||
| @test.assert_eq(encode(bytes_view), encode_scalar(bytes_view)) | ||
| let body = encode_scalar(bytes_view) | ||
| let sb = StringBuilder(size_hint=4 * (pad + body.length() + pad)) |
| (input : (Bytes, Array[Bool])) => { | ||
| let (bytes, flips) = input | ||
| let lower = @hex.encode(bytes) | ||
| let sb = StringBuilder(size_hint=4 * lower.length()) |
Coverage Report for CI Build 6261Coverage increased (+0.02%) to 90.795%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
The hint is measured in bytes and each UTF-16 code unit occupies two of them, so a hint derived from a code-unit count wants a factor of two, not four. The factor of four is right only where the count is source bytes, each of which becomes two code units. Addresses review comments from Copilot on #4122. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Addressed the Copilot review in b77d659. All three are correct.
Tests still pass 20/20 on native and wasm, 19/19 on wasm-gc and js. |
The block loops tested `index + 16 <= end`, which wraps to a true comparison for a view sitting at the very end of a large backing buffer, and `encode_v128` sized its UTF-16 buffer as `char_count * 2` without checking that the product fits in an `Int`. Neither is reachable through any realistic caller -- both need an allocation close to two gigabytes -- but the vector loads and stores they guard are not bounds checked, so a wrapped index reads and writes outside the buffer rather than failing. Both loops now bound themselves by subtraction, and the encoder hands an input whose buffer size would wrap to the scalar encoder, which builds the string without ever materializing that buffer. Found by Codex CLI while reviewing the equivalent code in #4122, which fixes the same two defects in `encoding/hex`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to #3625, which landed
encoding/hexwith a scalar-only codec. This adds v128 fast paths forencodeanddecodeon the linear-memory backends, structured exactly like the siblingencoding/base64: the scalar codec stays on every backend as the reference implementation, and the public entry points dispatch with#cfg.How it works
Encode — 16 source bytes per iteration. Split each byte into its nibbles (
i8x16_shr_uand a mask), interleave them into output order with twoi8x16_shuffles, and convert nibbles to ASCII with one register-residenti8x16_swizzletable. A MoonBitStringis UTF-16, so the 32 characters are widened withi16x8_extend_low/high_i8x16_ubefore being stored.Decode — 32 characters per iteration. Four
v128_load_i16x8are narrowed pairwise withi8x16_narrow_i16x8_u; that instruction reads its lanes as signed and saturates, so a code unit in0x0100..=0x7FFFbecomes 255 and one in0x8000..=0xFFFF(surrogate halves included) becomes 0. Neither is a hex digit, so no non-ASCII code unit can masquerade as one by way of its low byte. The three hex ranges are then validated vectorially and folded to nibbles with wrapping adds. Anything the fast path will not take — short, odd-length, or invalid input — falls back todecode_scalar, which remains the single authority on theMalformedrules, sodecoderaises on exactly the inputs it did before.Performance
moon bench encoding/hex, 4 KiB payload, scalar path vs. the public entry point on the same run:js keeps the scalar path by
#cfg, so both columns measure the same code and confirm no regression there. Other sizes, public entry point: 64 B encode is 34.5 ns native / 94.5 ns wasm / 608 ns js; 64 KiB is 16.0 µs / 15.7 µs / 687 µs encode and 12.7 µs / 20.5 µs / 746 µs decode.Correctness
Differential white-box tests pin both fast paths against the scalar ones: every length from 0 to 79 (so every block count and tail residue), views that start and end inside a larger buffer whose padding is itself valid hex, an invalid character planted at every lane position, and raw UTF-16 code units — lone surrogates included — planted at every position of a full block. A
#cfg-gated test callsencode_v128anddecode_v128directly, so the suite fails if the fast path is ever silently disabled rather than exercised.Four quickcheck properties cover decode∘encode against an independent reference encoder, case-mixture round-trips, corruption at any position raising
Malformed, and odd-length input raisingMalformed.Each of these mutations makes the suite fail, which is how I know the tests discriminate: an interleave shuffle index, the
-7nibble adjustment constant, eitherstart_offset()replaced by0, the saturating narrow replaced by a truncating shuffle, anddecode_v128returningNoneunconditionally.Both block loops bound themselves by subtraction, and the encoder declines an input whose UTF-16 byte buffer would not fit in an
Int, so no vector store can be reached through a wrapped index.moon testpasses 7512/7512;encoding/hexis 20/20 on native and wasm, 19/19 on wasm-gc and js.pkg.generated.mbtiis unchanged — this is an implementation change only.Review
Reviewed by Codex CLI at
ultrareasoning effort over two rounds. Round 1 withheld approval over two P1 memory-safety findings — a wrappinglength * 4output size and aindex + 16block bound that could wrap for a view nearInt::MAX— plus P2s on test discrimination and a P3 documentation error about the narrow's signed-lane behaviour. All are fixed; round 2 verdict and sign-off are posted in a comment below.