From 9d39c9d539b60daa30eff577ea6166900602c4ea Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 21:50:33 +0000 Subject: [PATCH 1/8] perf(appstate): run LTHash lane math on fearless_simd instead of portable_simd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proof of concept for replacing `std::simd` with fearless_simd across the workspace, starting with the smallest of the three SIMD sites. The motivation is not throughput, it is the toolchain: `portable_simd` is what pins the whole workspace to nightly, and `wacore-appstate/simd` was part of that chain. fearless_simd resolves the SIMD level at runtime on stable (MSRV 1.89, below our 1.94), so this crate now builds and tests with SIMD enabled on stable 1.94.1. `wacore-appstate/simd` therefore no longer chains to `wacore-binary/simd`, which is still `portable_simd` and still nightly-only. `wacore/simd` enables both, so the feature's meaning is unchanged for consumers. Measured on this machine (Level::new() detects AVX-512): - appstate_benchmark/bench_lthash_subtract_then_add_812 fastest 992.8 µs -> 990.5 µs, i.e. parity. - demo binary: +2304 B stripped, +2048 B .text, against PR budgets of 64 KiB and 32 KiB. - fearless_simd adds one crates.io dependency with no transitive deps. Isolating the lane math from the HKDF it sits behind (812 operands x 128 B) explains the flat end-to-end number and is worth recording, because it bears on where SIMD is worth having at all: scalar 3.89 µs portable_simd 3.96 µs fearless_simd, dispatch! per operand 5.09 µs fearless_simd, dispatch! hoisted 3.84 µs Two things follow. The lane math is ~0.4% of LTHash cost -- HKDF dominates -- and LLVM already auto-vectorizes the scalar loop as well as either SIMD backend does, so neither backend was buying anything here. And the gap between the two fearless_simd rows is ~1.5 ns per `dispatch!`, not codegen quality: hoisted, it matches scalar. Dispatch belongs above a hot loop, not inside one. That constraint shapes the decoder/encoder port, where the win is real -- those `swizzle_dyn` calls do not currently lower to `pshufb` on the default x86-64 baseline. Dispatch is left per operand here: 1.5 ns against ~1250 ns of HKDF per operand is invisible, and hoisting it would monomorphize HKDF once per SIMD level for no gain. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- Cargo.lock | 7 ++++ Cargo.toml | 4 ++ wacore/Cargo.toml | 6 ++- wacore/appstate/Cargo.toml | 8 +++- wacore/appstate/src/lib.rs | 1 - wacore/appstate/src/lthash.rs | 72 +++++++++++++++++++++++------------ 6 files changed, 70 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02fe54b7e..d538aa4ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1356,6 +1356,12 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" +[[package]] +name = "fearless_simd" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0f5f895dac0865bc235f1364793899569a7db538137f1024dccea56bf41c78d" + [[package]] name = "ff" version = "0.13.1" @@ -3889,6 +3895,7 @@ dependencies = [ "anyhow", "buffa", "codspeed-divan-compat", + "fearless_simd", "hex", "hkdf 0.13.0", "hmac 0.13.0", diff --git a/Cargo.toml b/Cargo.toml index 92cb2e75e..eeedbb661 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -95,6 +95,10 @@ diesel_migrations = { version = "2.3.2", default-features = false, features = [" divan = { package = "codspeed-divan-compat", version = "5.0.1" } env_logger = { version = "0.11", default-features = false } event-listener = { version = "5", default-features = false } +# `std` (default) rather than the `libm` path: wacore-appstate is already +# std-only (LazyLock), and libm would add a dependency to serve float ops +# this crate never touches. +fearless_simd = { version = "0.6.0" } flate2 = { version = "1.1.9", default-features = false, features = ["zlib-rs"] } futures = { version = "0.3", default-features = false, features = ["alloc", "async-await"] } getrandom = { version = "0.4", default-features = false } diff --git a/wacore/Cargo.toml b/wacore/Cargo.toml index 375088036..6460251ff 100644 --- a/wacore/Cargo.toml +++ b/wacore/Cargo.toml @@ -18,7 +18,11 @@ ignored = ["getrandom"] [features] default = ["simd"] -simd = ["wacore-appstate/simd"] +# Two independent SIMD backends behind one switch: appstate's LTHash runs on +# fearless_simd (stable, runtime-dispatched), wacore-binary's packed codec +# still on `portable_simd` (nightly). Enabling both here keeps this feature's +# meaning — "SIMD everywhere it exists" — unchanged for consumers. +simd = ["wacore-appstate/simd", "wacore-binary/simd"] debug-snapshots = [] # Typed interop with the decoded libsignal SessionRecord v1 model. legacy-session-interop = ["wacore-libsignal/legacy-session-interop"] diff --git a/wacore/appstate/Cargo.toml b/wacore/appstate/Cargo.toml index fccbb0b5b..2e282805b 100644 --- a/wacore/appstate/Cargo.toml +++ b/wacore/appstate/Cargo.toml @@ -13,11 +13,17 @@ crate-type = ["rlib"] [features] default = ["simd"] -simd = ["wacore-binary/simd"] +# LTHash lane math via fearless_simd: runtime level detection on stable, so +# this feature no longer drags the crate onto nightly. It deliberately does +# NOT chain to `wacore-binary/simd` — that one is still `portable_simd` and +# nightly-only; `wacore/simd` enables both so the top-level surface is +# unchanged. +simd = ["dep:fearless_simd"] [dependencies] anyhow = { workspace = true } buffa = { workspace = true } +fearless_simd = { workspace = true, optional = true } hex = { workspace = true } hkdf = { workspace = true } hmac = { workspace = true } diff --git a/wacore/appstate/src/lib.rs b/wacore/appstate/src/lib.rs index 8bbef2762..1c73d4ace 100644 --- a/wacore/appstate/src/lib.rs +++ b/wacore/appstate/src/lib.rs @@ -1,4 +1,3 @@ -#![cfg_attr(feature = "simd", feature(portable_simd))] pub mod decode; pub mod encode; pub mod errors; diff --git a/wacore/appstate/src/lthash.rs b/wacore/appstate/src/lthash.rs index bfe4ac14b..b18c294ef 100644 --- a/wacore/appstate/src/lthash.rs +++ b/wacore/appstate/src/lthash.rs @@ -1,5 +1,5 @@ #[cfg(feature = "simd")] -use core::simd::u16x8; +use fearless_simd::{Level, Simd, SimdBase, dispatch, u16x8}; use hkdf::Hkdf; use hmac::digest::KeyInit; use hmac::{Hmac, Mac}; @@ -12,6 +12,12 @@ use std::sync::LazyLock; static EXTRACT_HMAC: LazyLock> = LazyLock::new(|| Hmac::::new_from_slice(&[0u8; 32]).expect("32-byte HMAC key")); +/// Probing CPU features is comparatively expensive and the answer cannot +/// change while the process runs, so the level is resolved once here: +/// `multiple_op` dispatches once per operand, and a patch carries hundreds. +#[cfg(feature = "simd")] +static SIMD_LEVEL: LazyLock = LazyLock::new(Level::new); + #[derive(Clone, Debug)] pub struct LTHash { pub hkdf_info: &'static [u8], @@ -77,30 +83,7 @@ fn perform_pointwise_with_overflow(base: &mut [u8], input: &[u8], subtract: bool let (base_chunks, base_rem) = base_remaining.as_chunks_mut::<16>(); let (input_chunks, input_rem) = input_remaining.as_chunks::<16>(); - for (base_chunk, input_chunk) in base_chunks.iter_mut().zip(input_chunks) { - // `from_le_bytes` per lane states the wire endianness directly, so - // the same code is correct on either host; on little-endian it - // lowers to the plain 16-byte load a transmute would have emitted. - let base_arr: [u16; 8] = core::array::from_fn(|i| { - u16::from_le_bytes([base_chunk[2 * i], base_chunk[2 * i + 1]]) - }); - let input_arr: [u16; 8] = core::array::from_fn(|i| { - u16::from_le_bytes([input_chunk[2 * i], input_chunk[2 * i + 1]]) - }); - let base_simd = u16x8::from_array(base_arr); - let input_simd = u16x8::from_array(input_arr); - - let result_simd = if subtract { - base_simd - input_simd - } else { - base_simd + input_simd - }; - - let out = result_simd.to_array(); - for (base_pair, lane) in base_chunk.as_chunks_mut::<2>().0.iter_mut().zip(out) { - *base_pair = lane.to_le_bytes(); - } - } + dispatch!(*SIMD_LEVEL, simd => pointwise_chunks(simd, base_chunks, input_chunks, subtract)); base_remaining = base_rem; input_remaining = input_rem; @@ -124,6 +107,45 @@ fn perform_pointwise_with_overflow(base: &mut [u8], input: &[u8], subtract: bool } } +/// The lane math, generic over the SIMD level `dispatch!` picked at runtime. +/// `#[inline(always)]` is load-bearing rather than a hint: it is what makes +/// each instantiation inherit its level's `target_feature` set, so the AVX2 +/// copy lowers to AVX2 instead of to the baseline the caller was compiled for. +#[cfg(feature = "simd")] +#[inline(always)] +fn pointwise_chunks( + simd: S, + base_chunks: &mut [[u8; 16]], + input_chunks: &[[u8; 16]], + subtract: bool, +) { + for (base_chunk, input_chunk) in base_chunks.iter_mut().zip(input_chunks) { + // `from_le_bytes` per lane states the wire endianness directly, so + // the same code is correct on either host; on little-endian it + // lowers to the plain 16-byte load a transmute would have emitted. + let base_arr: [u16; 8] = core::array::from_fn(|i| { + u16::from_le_bytes([base_chunk[2 * i], base_chunk[2 * i + 1]]) + }); + let input_arr: [u16; 8] = core::array::from_fn(|i| { + u16::from_le_bytes([input_chunk[2 * i], input_chunk[2 * i + 1]]) + }); + let base_simd = u16x8::from_slice(simd, &base_arr); + let input_simd = u16x8::from_slice(simd, &input_arr); + + let result_simd = if subtract { + base_simd - input_simd + } else { + base_simd + input_simd + }; + + let mut out = [0u16; 8]; + result_simd.store_slice(&mut out); + for (base_pair, lane) in base_chunk.as_chunks_mut::<2>().0.iter_mut().zip(out) { + *base_pair = lane.to_le_bytes(); + } + } +} + fn hkdf_sha256_into(key: &[u8], info: &[u8], out: &mut [u8]) { let mut extract = EXTRACT_HMAC.clone(); extract.update(key); From 7c02e87253d268f1289f959e813a8b182b7994e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 22:01:43 +0000 Subject: [PATCH 2/8] test(appstate): check the LTHash SIMD path against an independent reference `test_simd_determinism_and_consistency` called `perform_pointwise_with_overflow` twice and compared the results. Both calls take the same path, so despite the name it never compared SIMD against scalar; it only proved the path was deterministic. A lane-conversion or wrapping bug would have been invisible to it. Split it in two. The round-trip property it did check keeps its own test under a name that says so. Alongside it, a straight-line scalar reference written for obviousness rather than speed, compared against the real function over sizes that straddle the 16-byte chunk boundary (under one chunk, exactly one, chunk-plus-tail, several) with inputs seeded onto the wrap boundaries in both directions. Verified by mutation: flipping the SIMD path's lane store to `to_be_bytes` fails the new test. The old one passed with that same mutation applied. Raised in review on #1262. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- wacore/appstate/src/lthash.rs | 77 +++++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 16 deletions(-) diff --git a/wacore/appstate/src/lthash.rs b/wacore/appstate/src/lthash.rs index b18c294ef..7fb45a33c 100644 --- a/wacore/appstate/src/lthash.rs +++ b/wacore/appstate/src/lthash.rs @@ -192,27 +192,72 @@ mod tests { } #[test] - fn test_simd_determinism_and_consistency() { + fn add_then_subtract_returns_to_zero_across_sizes() { let test_sizes = [2, 4, 8, 16, 18, 32, 64, 128, 256]; for &size in &test_sizes { - let mut base_simd = vec![0u8; size]; - let mut base_scalar = vec![0u8; size]; + let mut base = vec![0u8; size]; let input = vec![1u8; size]; - perform_pointwise_with_overflow(&mut base_simd, &input, false); - perform_pointwise_with_overflow(&mut base_scalar, &input, false); - assert_eq!(base_simd, base_scalar, "Add failed for size {}", size); - - perform_pointwise_with_overflow(&mut base_simd, &input, true); - perform_pointwise_with_overflow(&mut base_scalar, &input, true); - assert_eq!(base_simd, base_scalar, "Subtract failed for size {}", size); - assert_eq!( - base_simd, - vec![0u8; size], - "Subtract result incorrect for size {}", - size - ); + perform_pointwise_with_overflow(&mut base, &input, false); + perform_pointwise_with_overflow(&mut base, &input, true); + assert_eq!(base, vec![0u8; size], "size {size}"); + } + } + + /// Straight-line reference: no chunking, no dispatch, no feature gate. The + /// point is to be obviously correct rather than fast, so that the test + /// below is an independent check on the SIMD path rather than a + /// comparison of that path against itself. + fn reference_pointwise(base: &mut [u8], input: &[u8], subtract: bool) { + for (b, i) in base.chunks_exact_mut(2).zip(input.chunks_exact(2)) { + let x = u16::from_le_bytes([b[0], b[1]]); + let y = u16::from_le_bytes([i[0], i[1]]); + let r = if subtract { + x.wrapping_sub(y) + } else { + x.wrapping_add(y) + }; + b.copy_from_slice(&r.to_le_bytes()); + } + } + + /// The SIMD path splits into 16-byte chunks and leaves a scalar tail, so + /// the sizes below straddle that boundary: under one chunk, exactly one, + /// chunk-plus-tail, and several chunks. Inputs are seeded to cover the + /// wrap boundaries in both directions, which is where a lane-width or + /// endianness mistake would show up rather than in round-number data. + #[test] + fn simd_path_matches_independent_scalar_reference() { + let sizes = [0usize, 2, 14, 16, 18, 32, 34, 128, 130, 256]; + // Deterministic LCG: reproducible failures, no dev-dependency. + let mut seed = 0x2545_F491u32; + let mut next = move || { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + (seed >> 24) as u8 + }; + + for size in sizes { + for subtract in [false, true] { + for edge in [0u8, 0xFF, 0x01] { + let base: Vec = (0..size) + .map(|i| if i % 3 == 0 { edge } else { next() }) + .collect(); + let input: Vec = (0..size) + .map(|i| if i % 5 == 0 { edge } else { next() }) + .collect(); + + let mut actual = base.clone(); + let mut expected = base.clone(); + perform_pointwise_with_overflow(&mut actual, &input, subtract); + reference_pointwise(&mut expected, &input, subtract); + + assert_eq!( + actual, expected, + "size {size}, subtract {subtract}, edge {edge:#04x}" + ); + } + } } } From 44303a84001a7abdfcfe9dae54d7f46804385cd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 22:28:48 +0000 Subject: [PATCH 3/8] perf(binary): replace the packed codec's SIMD with lookup tables Step 2 was meant to port `wacore-binary`'s decoder and encoder to fearless_simd, on the theory that their `swizzle_dyn` calls were losing a `pshufb` on the default x86-64 baseline and that runtime dispatch would win it back. Measuring first killed that theory, so this deletes the vectors instead of porting them. Wall clock on the bench host swings ~18% between identical runs, so the numbers below are callgrind instruction counts over 20k marshal/unmarshal rounds of an ack node, split by message-id length. 20 characters is the common id; 31 is where the decoder's chunk loop first engages at all, since it needs 16 packed bytes. scalar portable_simd this change encode, 20 chars 74.15M 69.93M 68.95M encode, 32 chars 83.05M 74.83M 75.99M decode, 20 chars 29.10M 30.41M 29.09M decode, 32 chars 32.88M 36.07M 32.87M The decoder's vector path was slower than the table it sat in front of, at every length. Rebuilding with `-Ctarget-cpu=x86-64-v2`, so the shuffles really did lower to `pshufb`, narrowed the 32-character loss from 9.7% to 7.0% and never turned it into a win: `HEX_PAIRS[byte]` is a single 2-byte load, and shuffle/interleave/store does not beat that. The encoder's vector path was a real win, but not against a fair opponent. It was competing with two `match` ladders reached through a `fn` pointer. Given the same 256-entry table treatment the decoder already had, scalar takes the common 20-character case outright and gives up 1.5% at 32. What that buys, beyond the instruction counts: `wacore-binary` no longer needs `portable_simd`, so its `simd` feature is gone and `cargo +stable build -p wacore` now works with default features for the first time. Combined with the appstate change earlier in this branch, the demo binary is 5.1 KiB smaller than main rather than 2.25 KiB larger. CI follows: the Miri matrix drops its scalar leg, which no longer names a distinct code path, and the stable job tests default features -- the configuration that ships and that it could never reach before -- keeping the scalar build as a compile check for wasm and ESP32. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- .github/workflows/main.yml | 35 ++++---- .github/workflows/miri.yml | 13 ++- Cargo.toml | 2 +- wacore/Cargo.toml | 10 +-- wacore/appstate/Cargo.toml | 5 +- wacore/binary/Cargo.toml | 2 - wacore/binary/src/decoder.rs | 82 +++--------------- wacore/binary/src/encoder.rs | 157 +++++++++++++++-------------------- wacore/binary/src/lib.rs | 2 - 9 files changed, 114 insertions(+), 194 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f56619ae4..1c3f5412d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -220,7 +220,8 @@ jobs: df -h / sudo rm -rf --one-file-system /usr/local/lib/android df -h / - # `simd` needs nightly, so the matrix runs on the pinned toolchain. + # Pinned toolchain: .cargo/config.toml carries nightly-only rustflags + # (-Zshare-generics) for this target, and this job does not clear them. - uses: dtolnay/rust-toolchain@master with: toolchain: nightly-2026-06-16 @@ -251,7 +252,7 @@ jobs: -p wacore-libsignal -p wacore-noise -p waproto test-stable: - name: Test Stable (no-simd) + name: Test Stable (MSRV) runs-on: ubuntu-latest # .cargo/config.toml sets nightly-only rustflags (-Zshare-generics) for the # x86_64-linux target; a set-but-empty RUSTFLAGS takes precedence over @@ -279,18 +280,24 @@ jobs: uses: Swatinem/rust-cache@v2 with: cache-targets: "true" - - name: Build wacore-binary (stable, no SIMD) - run: cargo build -p wacore-binary --no-default-features --verbose - - name: Test wacore-binary (stable, no SIMD) - run: cargo nextest run --profile ci -p wacore-binary --no-default-features --lib - - name: Build wacore-appstate (stable, no SIMD) - run: cargo build -p wacore-appstate --no-default-features --verbose - - name: Test wacore-appstate (stable, no SIMD) - run: cargo nextest run --profile ci -p wacore-appstate --no-default-features --lib - - name: Build wacore (stable, no SIMD) - run: cargo build -p wacore --no-default-features --verbose - - name: Test wacore (stable, no SIMD) - run: cargo nextest run --profile ci -p wacore --no-default-features --lib + # Default features, which include `simd`. That combination used to be + # nightly-only, so this job could only ever run the scalar half; both + # halves are stable now and the SIMD one is what ships, so it is the one + # worth testing here. + - name: Test wacore-binary (stable) + run: cargo nextest run --profile ci -p wacore-binary --lib + - name: Test wacore-appstate (stable) + run: cargo nextest run --profile ci -p wacore-appstate --lib + - name: Test wacore (stable) + run: cargo nextest run --profile ci -p wacore --lib + # `--no-default-features` turns the LTHash lane math back to scalar. It + # is the configuration wasm and ESP32 build, so it stays covered; a + # compile is enough, since the scalar path is what every test above + # already compares against. + - name: Build the scalar configuration (stable) + run: > + cargo build --verbose --no-default-features + -p wacore-binary -p wacore-appstate -p wacore # `rust-version` is published metadata for every member, but only the # three crates above are exercised at that toolchain. This compiles the # rest of the publishable set so the declared floor is a checked promise diff --git a/.github/workflows/miri.yml b/.github/workflows/miri.yml index 7b9c4d23b..4af579c1d 100644 --- a/.github/workflows/miri.yml +++ b/.github/workflows/miri.yml @@ -40,15 +40,14 @@ jobs: # inflate's uninitialized spare capacity in `zlib_pool`. Both are # invisible to clippy and to native tests — nothing observes the # aliasing violation or the uninit read until it miscompiles. + # + # One leg, not two: the crate used to carry portable-SIMD scanners in + # the decoder/encoder alongside scalar fallbacks, so `--no-default- + # features` reached a genuinely different code path. The packed codec + # is table-driven scalar now and there is only one path to check. - name: wacore-binary - cache-key: binary-simd + cache-key: binary args: -p wacore-binary --lib - # The portable-SIMD scanners in the decoder/encoder and their scalar - # fallbacks are separate code paths, and `--no-default-features` is the - # only way to reach the latter. - - name: wacore-binary (no simd) - cache-key: binary-scalar - args: -p wacore-binary --no-default-features --lib # No `unsafe` of its own, but it drives wacore-binary's zero-copy # decode over real Noise frames and pulls the crypto stack # (aes/sha2/curve25519), whose unsafe backends this exercises. diff --git a/Cargo.toml b/Cargo.toml index eeedbb661..b4a122fd1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,7 @@ default-members = [ ] # Minimum supported Rust version. Floor is the stable release CI's -# "Test Stable (no-simd)" job pins; raise both together (that job's +# "Test Stable (MSRV)" job pins; raise both together (that job's # `toolchain:` and this value) when a dependency or language feature needs it. [workspace.package] rust-version = "1.94" diff --git a/wacore/Cargo.toml b/wacore/Cargo.toml index 6460251ff..d4b271355 100644 --- a/wacore/Cargo.toml +++ b/wacore/Cargo.toml @@ -18,11 +18,11 @@ ignored = ["getrandom"] [features] default = ["simd"] -# Two independent SIMD backends behind one switch: appstate's LTHash runs on -# fearless_simd (stable, runtime-dispatched), wacore-binary's packed codec -# still on `portable_simd` (nightly). Enabling both here keeps this feature's -# meaning — "SIMD everywhere it exists" — unchanged for consumers. -simd = ["wacore-appstate/simd", "wacore-binary/simd"] +# appstate's LTHash is the only SIMD left in the tree; wacore-binary's packed +# codec is table-driven scalar, which measured faster than the vectors it +# replaced. This runs on fearless_simd, so the feature no longer implies +# nightly. +simd = ["wacore-appstate/simd"] debug-snapshots = [] # Typed interop with the decoded libsignal SessionRecord v1 model. legacy-session-interop = ["wacore-libsignal/legacy-session-interop"] diff --git a/wacore/appstate/Cargo.toml b/wacore/appstate/Cargo.toml index 2e282805b..f99e88395 100644 --- a/wacore/appstate/Cargo.toml +++ b/wacore/appstate/Cargo.toml @@ -14,10 +14,7 @@ crate-type = ["rlib"] [features] default = ["simd"] # LTHash lane math via fearless_simd: runtime level detection on stable, so -# this feature no longer drags the crate onto nightly. It deliberately does -# NOT chain to `wacore-binary/simd` — that one is still `portable_simd` and -# nightly-only; `wacore/simd` enables both so the top-level surface is -# unchanged. +# this feature does not drag the crate onto nightly. simd = ["dep:fearless_simd"] [dependencies] diff --git a/wacore/binary/Cargo.toml b/wacore/binary/Cargo.toml index 0531adc77..716daa462 100644 --- a/wacore/binary/Cargo.toml +++ b/wacore/binary/Cargo.toml @@ -12,8 +12,6 @@ description = "Binary data and constants for WhatsApp protocol" crate-type = ["rlib"] [features] -default = ["simd"] -simd = [] serde = ["dep:serde", "compact_str/serde", "smallvec/serde"] # Render raw phone numbers in `Jid::observe()` instead of the redacted `pn#`. # Local debugging only; never enable in production. diff --git a/wacore/binary/src/decoder.rs b/wacore/binary/src/decoder.rs index 24c273fac..0fea535f7 100644 --- a/wacore/binary/src/decoder.rs +++ b/wacore/binary/src/decoder.rs @@ -4,8 +4,6 @@ use crate::node::{AttrsRef, NodeContentRef, NodeRef, NodeStr, ValueRef}; use crate::token; use compact_str::CompactString; use std::borrow::Cow; -#[cfg(feature = "simd")] -use std::simd::{Simd, prelude::*, u8x16}; /// Format a JidRef directly into CompactString using direct push operations, /// bypassing `fmt::Display` and `dyn Write` dispatch entirely. @@ -374,30 +372,16 @@ impl<'a> Decoder<'a> { Ok(CompactString::from(s)) } + // Deliberately scalar. A vectorised version of this loop lived here until + // it was measured against the table: `HEX_PAIRS[byte]` is one 2-byte load + // per input byte, and a shuffle/interleave/store sequence does not beat + // that. Under callgrind the SIMD path cost 4.5% more instructions on a + // 20-character id and 9.7% more on a 32-character one, and enabling real + // `pshufb` (`-Ctarget-cpu=x86-64-v2`) only narrowed the loss to 7.0%. + // Packed payloads are ids and phone numbers, so the loop also needed a + // 31-character string before it engaged at all. #[inline] fn decode_packed_hex(packed_data: &[u8], out: &mut [u8], pos: &mut usize) { - #[cfg(feature = "simd")] - let packed_data = { - const HEX_LOOKUP: [u8; 16] = *b"0123456789ABCDEF"; - let lookup_table = Simd::from_array(HEX_LOOKUP); - let low_mask = Simd::splat(0x0F); - - let (chunks, remainder) = packed_data.as_chunks::<16>(); - for chunk in chunks { - let data = u8x16::from_array(*chunk); - let high_nibbles = (data >> 4) & low_mask; - let low_nibbles = data & low_mask; - let high_chars = lookup_table.swizzle_dyn(high_nibbles); - let low_chars = lookup_table.swizzle_dyn(low_nibbles); - let (lo, hi) = Simd::interleave(high_chars, low_chars); - out[*pos..*pos + 16].copy_from_slice(lo.as_array()); - *pos += 16; - out[*pos..*pos + 16].copy_from_slice(hi.as_array()); - *pos += 16; - } - remainder - }; - let written = packed_data.len() * 2; for (slot, &byte) in out[*pos..*pos + written] .chunks_exact_mut(2) @@ -408,54 +392,12 @@ impl<'a> Decoder<'a> { *pos += written; } + // Scalar for the same reason as `decode_packed_hex`, and more so: the + // vector version had to validate every lane against the two legal + // out-of-range nibbles before it could shuffle, then fall back to this + // loop anyway whenever a lane failed. #[inline] fn decode_packed_nibble(packed_data: &[u8], out: &mut [u8], pos: &mut usize) -> Result<()> { - #[cfg(feature = "simd")] - let packed_data = { - const NIBBLE_LOOKUP: [u8; 16] = *b"0123456789-.\x00\x00\x00\x00"; - let lookup_table = Simd::from_array(NIBBLE_LOOKUP); - let low_mask = Simd::splat(0x0F); - let le11 = Simd::splat(11); - let f15 = Simd::splat(15); - - let (chunks, remainder) = packed_data.as_chunks::<16>(); - for chunk in chunks { - let data = u8x16::from_array(*chunk); - - let high_nibbles = (data >> 4) & low_mask; - let low_nibbles = data & low_mask; - - let hi_valid = high_nibbles.simd_le(le11) | high_nibbles.simd_eq(f15); - let lo_valid = low_nibbles.simd_le(le11) | low_nibbles.simd_eq(f15); - if !(hi_valid & lo_valid).all() { - for byte in *chunk { - let high = (byte & 0xF0) >> 4; - let low = byte & 0x0F; - Self::unpack_nibble(high)?; - Self::unpack_nibble(low)?; - } - for byte in *chunk { - let high = (byte & 0xF0) >> 4; - let low = byte & 0x0F; - out[*pos] = Self::unpack_nibble(high)?; - *pos += 1; - out[*pos] = Self::unpack_nibble(low)?; - *pos += 1; - } - continue; - } - - let high_chars = lookup_table.swizzle_dyn(high_nibbles); - let low_chars = lookup_table.swizzle_dyn(low_nibbles); - let (lo, hi) = Simd::interleave(high_chars, low_chars); - out[*pos..*pos + 16].copy_from_slice(lo.as_array()); - *pos += 16; - out[*pos..*pos + 16].copy_from_slice(hi.as_array()); - *pos += 16; - } - remainder - }; - let written = packed_data.len() * 2; for (slot, &byte) in out[*pos..*pos + written] .chunks_exact_mut(2) diff --git a/wacore/binary/src/encoder.rs b/wacore/binary/src/encoder.rs index a8d78b6e2..fe3cf43ec 100644 --- a/wacore/binary/src/encoder.rs +++ b/wacore/binary/src/encoder.rs @@ -1,17 +1,47 @@ use std::io::Write; -#[cfg(feature = "simd")] -use core::simd::Select; -#[cfg(feature = "simd")] -use core::simd::prelude::*; -#[cfg(feature = "simd")] -use core::simd::{Simd, u8x16}; - use crate::error::{BinaryError, Result}; use crate::jid::{self, Jid, JidRef}; use crate::node::{Node, NodeContent, NodeContentRef, NodeRef, NodeValue, ValueRef}; use crate::token; +/// Marks a byte no packed encoding accepts. `validate_hex`/`validate_nibble` +/// gate every caller, so a hit means the caller skipped that check. +const PACK_INVALID: u8 = 0xFF; + +/// ASCII to nibble, the inverse of the decoder's `HEX_PAIRS`. Index 0 maps to +/// 15 because that is the pad an odd-length string writes as its second half. +static HEX_ENC: [u8; 256] = { + let mut table = [PACK_INVALID; 256]; + let mut c = b'0'; + while c <= b'9' { + table[c as usize] = c - b'0'; + c += 1; + } + let mut c = b'A'; + while c <= b'F' { + table[c as usize] = 10 + (c - b'A'); + c += 1; + } + table[0] = 15; + table +}; + +/// ASCII to nibble for `NIBBLE_8`: digits plus the two punctuation characters +/// a phone number can carry. +static NIBBLE_ENC: [u8; 256] = { + let mut table = [PACK_INVALID; 256]; + let mut c = b'0'; + while c <= b'9' { + table[c as usize] = c - b'0'; + c += 1; + } + table[b'-' as usize] = 10; + table[b'.' as usize] = 11; + table[0] = 15; + table +}; + pub trait ByteWriter { fn write_u8(&mut self, value: u8) -> Result<()>; fn write_bytes(&mut self, bytes: &[u8]) -> Result<()>; @@ -867,30 +897,20 @@ impl<'a, W: ByteWriter> Encoder<'a, W> { Ok(()) } + /// Two table loads and a shift. This replaced a pair of `match` ladders + /// reached through a `fn` pointer, which is what the vectorised loop that + /// used to sit in `write_packed_bytes` was really competing against. #[inline(always)] - fn pack_nibble(value: u8) -> u8 { - match value { - b'-' => 10, - b'.' => 11, - 0 => 15, - c if c.is_ascii_digit() => c - b'0', - _ => panic!("Invalid char for nibble packing: {value}"), - } - } - - #[inline(always)] - fn pack_hex(value: u8) -> u8 { - match value { - c if c.is_ascii_digit() => c - b'0', - c if (b'A'..=b'F').contains(&c) => 10 + (c - b'A'), - 0 => 15, - _ => panic!("Invalid char for hex packing: {value}"), - } - } - - #[inline(always)] - fn pack_byte_pair(packer: fn(u8) -> u8, part1: u8, part2: u8) -> u8 { - (packer(part1) << 4) | packer(part2) + fn pack_pair_table(table: &[u8; 256], part1: u8, part2: u8) -> u8 { + let hi = table[part1 as usize]; + let lo = table[part2 as usize]; + // `validate_hex`/`validate_nibble` gate every caller, so this is the + // same unreachable case the `match` arms panicked on. + assert!( + hi != PACK_INVALID && lo != PACK_INVALID, + "invalid char for packing" + ); + (hi << 4) | lo } fn write_packed_bytes(&mut self, value: &str, data_type: u8) -> Result<()> { @@ -906,67 +926,26 @@ impl<'a, W: ByteWriter> Encoder<'a, W> { } self.write_u8(rounded_len)?; - #[allow(unused_mut)] - let mut input_bytes = value.as_bytes(); - - if data_type == token::NIBBLE_8 { - #[cfg(feature = "simd")] - { - const NIBBLE_LOOKUP: [u8; 16] = - [10, 11, 255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 255, 255, 255]; - let lookup = Simd::from_array(NIBBLE_LOOKUP); - let nibble_base = Simd::splat(b'-'); - - while input_bytes.len() >= 16 { - let (chunk, rest) = input_bytes.split_at(16); - let input = u8x16::from_slice(chunk); - let indices = input.saturating_sub(nibble_base); - let nibbles = lookup.swizzle_dyn(indices); - - let (evens, odds) = nibbles.deinterleave(nibbles.rotate_elements_left::<1>()); - let packed: Simd = (evens << Simd::splat(4)) | odds; - let packed_bytes = packed.to_array(); - self.write_raw_bytes(&packed_bytes[..8])?; - - input_bytes = rest; - } - } - - let mut bytes_iter = input_bytes.iter().copied(); - while let Some(part1) = bytes_iter.next() { - let part2 = bytes_iter.next().unwrap_or(0); - self.write_u8(Self::pack_byte_pair(Self::pack_nibble, part1, part2))?; - } + let input_bytes = value.as_bytes(); + let table = if data_type == token::NIBBLE_8 { + &NIBBLE_ENC } else { - #[cfg(feature = "simd")] - { - let ascii_0 = Simd::splat(b'0'); - let ascii_a = Simd::splat(b'A'); - let ten = Simd::splat(10); - - while input_bytes.len() >= 16 { - let (chunk, rest) = input_bytes.split_at(16); - let input = u8x16::from_slice(chunk); - - let digit_vals = input - ascii_0; - let letter_vals = input - ascii_a + ten; - let is_letter = input.simd_ge(ascii_a); - let nibbles = is_letter.select(letter_vals, digit_vals); - - let (evens, odds) = nibbles.deinterleave(nibbles.rotate_elements_left::<1>()); - let packed: Simd = (evens << Simd::splat(4)) | odds; - let packed_bytes = packed.to_array(); - self.write_raw_bytes(&packed_bytes[..8])?; - - input_bytes = rest; - } - } + &HEX_ENC + }; - let mut bytes_iter = input_bytes.iter().copied(); - while let Some(part1) = bytes_iter.next() { - let part2 = bytes_iter.next().unwrap_or(0); - self.write_u8(Self::pack_byte_pair(Self::pack_hex, part1, part2))?; - } + // Whole pairs first, so the common even-length case carries no + // per-iteration "is there a second half" branch. `PACKED_MAX` is 127, + // so the buffer covers any string that reaches here. + let mut packed = [0u8; 64]; + let (pairs, tail) = input_bytes.as_chunks::<2>(); + for (slot, pair) in packed.iter_mut().zip(pairs) { + *slot = Self::pack_pair_table(table, pair[0], pair[1]); + } + self.write_raw_bytes(&packed[..pairs.len()])?; + + // Odd length: the low nibble is the 0 pad, which both tables map to 15. + if let [last] = tail { + self.write_u8(Self::pack_pair_table(table, *last, 0))?; } Ok(()) } diff --git a/wacore/binary/src/lib.rs b/wacore/binary/src/lib.rs index 1f6528743..ec0ee3ae5 100644 --- a/wacore/binary/src/lib.rs +++ b/wacore/binary/src/lib.rs @@ -1,5 +1,3 @@ -#![cfg_attr(feature = "simd", feature(portable_simd))] - pub mod attrs; pub mod builder; pub mod consts; From 8f253d985b3f899a533f65e3be15862e2d9d16ab Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 22:50:50 +0000 Subject: [PATCH 4/8] build(binary): keep a no-op `simd` feature so downstream manifests resolve `wacore-binary` is published, and a manifest that names `features = ["simd"]` fails to resolve against a crate that has no such feature. The error names the consumer's manifest rather than this change, which makes it a poor way to learn the feature is gone. The feature does nothing and stays out of `default`. Raised independently in review on #1262 by two reviewers. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- wacore/binary/Cargo.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/wacore/binary/Cargo.toml b/wacore/binary/Cargo.toml index 716daa462..61cb3cec6 100644 --- a/wacore/binary/Cargo.toml +++ b/wacore/binary/Cargo.toml @@ -12,6 +12,13 @@ description = "Binary data and constants for WhatsApp protocol" crate-type = ["rlib"] [features] +# Does nothing. The packed codec this used to gate is table-driven scalar now, +# which measured faster than the vectors it replaced. Kept, and out of +# `default`, only because the crate is published: a manifest that names +# `features = ["simd"]` fails to resolve against a crate that has no such +# feature, and that error points at the consumer rather than at this change. +# Delete it at the next major. +simd = [] serde = ["dep:serde", "compact_str/serde", "smallvec/serde"] # Render raw phone numbers in `Jid::observe()` instead of the redacted `pn#`. # Local debugging only; never enable in production. From b249e055312f47ca8713e94cbbf20fe1bb207738 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 22:57:54 +0000 Subject: [PATCH 5/8] perf(binary): take the validity branch out of the packed encode loop The lookup tables left the 32-character case 1.5% behind the vectors they replaced. The cause was the per-pair `assert!`: a conditional branch in the loop body, which stops LLVM unrolling it. The check does not need to be there. Legal table entries are 0..=15 and `PACK_INVALID` is 0xFF, so ORing every lookup into an accumulator and testing its high nibble once, after the loop, detects exactly the same inputs. The odd-length byte folds into the same accumulator, and the test still runs before anything reaches the writer, so an invalid character cannot escape onto the wire any more than it could before. Callgrind, 20k marshals of an ack node, against the same baselines: portable_simd branchy tables this change encode, 20 chars 69.93M 68.95M 68.09M encode, 32 chars 74.83M 75.99M 74.71M That closes the 32-character gap and turns it into a small win, so scalar is now ahead of the vectors at both lengths rather than trading. Decode is untouched and measures unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- wacore/binary/src/encoder.rs | 47 +++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/wacore/binary/src/encoder.rs b/wacore/binary/src/encoder.rs index fe3cf43ec..24d4d79fd 100644 --- a/wacore/binary/src/encoder.rs +++ b/wacore/binary/src/encoder.rs @@ -897,22 +897,6 @@ impl<'a, W: ByteWriter> Encoder<'a, W> { Ok(()) } - /// Two table loads and a shift. This replaced a pair of `match` ladders - /// reached through a `fn` pointer, which is what the vectorised loop that - /// used to sit in `write_packed_bytes` was really competing against. - #[inline(always)] - fn pack_pair_table(table: &[u8; 256], part1: u8, part2: u8) -> u8 { - let hi = table[part1 as usize]; - let lo = table[part2 as usize]; - // `validate_hex`/`validate_nibble` gate every caller, so this is the - // same unreachable case the `match` arms panicked on. - assert!( - hi != PACK_INVALID && lo != PACK_INVALID, - "invalid char for packing" - ); - (hi << 4) | lo - } - fn write_packed_bytes(&mut self, value: &str, data_type: u8) -> Result<()> { if value.len() > token::PACKED_MAX as usize { panic!("String too long to be packed: {}", value.len()); @@ -938,14 +922,37 @@ impl<'a, W: ByteWriter> Encoder<'a, W> { // so the buffer covers any string that reaches here. let mut packed = [0u8; 64]; let (pairs, tail) = input_bytes.as_chunks::<2>(); + + // The validity test is an OR accumulator checked once below, not a + // branch per pair. Legal table entries are 0..=15 and `PACK_INVALID` + // is 0xFF, so a set high nibble in `seen` means some character was + // rejected. Keeping the branch out is what lets LLVM unroll this. + let mut seen = 0u8; for (slot, pair) in packed.iter_mut().zip(pairs) { - *slot = Self::pack_pair_table(table, pair[0], pair[1]); + let hi = table[pair[0] as usize]; + let lo = table[pair[1] as usize]; + seen |= hi | lo; + *slot = (hi << 4) | lo; } - self.write_raw_bytes(&packed[..pairs.len()])?; // Odd length: the low nibble is the 0 pad, which both tables map to 15. - if let [last] = tail { - self.write_u8(Self::pack_pair_table(table, *last, 0))?; + let odd = if let [last] = tail { + let hi = table[*last as usize]; + let lo = table[0]; + seen |= hi | lo; + Some((hi << 4) | lo) + } else { + None + }; + + // Checked before anything reaches the writer. `validate_hex` and + // `validate_nibble` gate every caller, so this is the same unreachable + // case the `match` ladders this replaced used to panic on. + assert!(seen & 0xF0 == 0, "invalid char for packing"); + + self.write_raw_bytes(&packed[..pairs.len()])?; + if let Some(byte) = odd { + self.write_u8(byte)?; } Ok(()) } From 223f8d5188a0a317afb552cdf9a788b6810b3488 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 23:03:58 +0000 Subject: [PATCH 6/8] build(binary): put the no-op `simd` feature back in `default` I had kept the feature but moved it out of `default`, which cargo-semver-checks flags as `feature_not_enabled_by_default`: downstream crates relying on default features lose it silently. That defeats the point of keeping the name at all. The feature gates nothing, so its presence in `default` costs nothing and leaves the published feature surface exactly where it was. The workspace is unaffected either way -- every internal edge takes wacore-binary with `default-features = false`. This clears the one semver finding this branch introduced. The `enum_variant_added` on `BinaryError::UnexpectedFormatByte` that the same job reports predates the branch and comes from main. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- wacore/binary/Cargo.toml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/wacore/binary/Cargo.toml b/wacore/binary/Cargo.toml index 61cb3cec6..44b268e81 100644 --- a/wacore/binary/Cargo.toml +++ b/wacore/binary/Cargo.toml @@ -13,11 +13,13 @@ crate-type = ["rlib"] [features] # Does nothing. The packed codec this used to gate is table-driven scalar now, -# which measured faster than the vectors it replaced. Kept, and out of -# `default`, only because the crate is published: a manifest that names -# `features = ["simd"]` fails to resolve against a crate that has no such -# feature, and that error points at the consumer rather than at this change. -# Delete it at the next major. +# which measured faster than the vectors it replaced. It stays declared, and +# stays in `default`, purely to leave the published feature surface where it +# was: dropping the name breaks any manifest that says `features = ["simd"]`, +# and dropping it from `default` trips `feature_not_enabled_by_default`. Both +# errors would point at the consumer rather than at this change. Since the +# feature gates nothing, keeping it costs nothing. Delete at the next major. +default = ["simd"] simd = [] serde = ["dep:serde", "compact_str/serde", "smallvec/serde"] # Render raw phone numbers in `Jid::observe()` instead of the redacted `pn#`. From 703c5457b4be2e817a63cdc3375625112e895ab2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 23:09:56 +0000 Subject: [PATCH 7/8] test(binary): pin the packed-encode tables to the ladders they replaced `HEX_ENC` and `NIBBLE_ENC` were derived from the `match` ladders that `write_packed_bytes` used to call, but nothing held them to that. A review pass on #1262 flagged exactly this: the diff shows a new table-driven path with no visible proof it accepts and rejects the same bytes as the old one. The ladders now live in the test module as the specification, checked exhaustively over all 256 byte values: same accepted set, same nibble for every accepted byte, same rejected set. `None` in the reference is the case the old code panicked on and the tables mark with `PACK_INVALID`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- wacore/binary/src/encoder.rs | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/wacore/binary/src/encoder.rs b/wacore/binary/src/encoder.rs index 24d4d79fd..39d78c59d 100644 --- a/wacore/binary/src/encoder.rs +++ b/wacore/binary/src/encoder.rs @@ -994,6 +994,48 @@ mod tests { type TestResult = Result<()>; + /// The `match` ladders `HEX_ENC`/`NIBBLE_ENC` replaced, kept here as the + /// specification they are checked against. `None` is the case the old code + /// panicked on and the tables mark with `PACK_INVALID`. + fn reference_hex(value: u8) -> Option { + match value { + c if c.is_ascii_digit() => Some(c - b'0'), + c if (b'A'..=b'F').contains(&c) => Some(10 + (c - b'A')), + 0 => Some(15), + _ => None, + } + } + + fn reference_nibble(value: u8) -> Option { + match value { + b'-' => Some(10), + b'.' => Some(11), + 0 => Some(15), + c if c.is_ascii_digit() => Some(c - b'0'), + _ => None, + } + } + + /// Exhaustive over the byte domain, so the tables cannot drift from the + /// ladders they were derived from: same accepted set, same nibble for + /// every accepted byte, same rejected set. + #[test] + fn encode_tables_match_the_ladders_they_replaced() { + for byte in 0u8..=255 { + let i = byte as usize; + assert_eq!( + reference_hex(byte), + (HEX_ENC[i] != PACK_INVALID).then_some(HEX_ENC[i]), + "hex table disagrees at {byte:#04x}" + ); + assert_eq!( + reference_nibble(byte), + (NIBBLE_ENC[i] != PACK_INVALID).then_some(NIBBLE_ENC[i]), + "nibble table disagrees at {byte:#04x}" + ); + } + } + #[test] fn test_encode_node() -> TestResult { let node = Node::new( From 039912ea257c690529a19f20c9bc9b7650f122bb Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 23:32:34 +0000 Subject: [PATCH 8/8] refactor: drop the `simd` feature and the fearless_simd dependency The packed codec lost its vectors earlier in this branch because scalar lookup tables measured faster. That left LTHash as the only SIMD in the tree, and it does not earn its keep either: over an 812-MAC batch the feature moves the total by 0.28% (831.93M instructions scalar, 829.63M with SIMD), because HKDF above it dominates and LLVM already auto-vectorizes the lane loop about as well as the intrinsics did. Isolated, the lane math alone is scalar 3.89 us, portable_simd 3.96 us, fearless_simd 3.84 us over 812 operands -- roughly 0.4% of LTHash's cost. A dependency and a feature flag across four manifests is a poor trade for that, and we are pre-1.0, so the feature goes rather than lingering as a compatibility no-op. What this leaves: no `simd` feature anywhere, no fearless_simd, no `portable_simd`, and nothing in the tree that needs nightly to build. The demo binary is 7.8 KiB smaller than main, against 5.09 KiB before this commit. The LTHash test that compared the SIMD path against a scalar reference would now be comparing the implementation with a copy of itself, so its reference is rewritten to reach the same answer a different way: lanes assembled by hand from byte positions, arithmetic in `u32` and masked, sharing neither `from_le_bytes` nor `wrapping_*` with the code under test. The sizes it sweeps still straddle the 16-byte boundary a vectorised implementation would chunk on, so the coverage holds if one comes back. Also drops the stale `% 2` workaround in the length assert; `rust-version` is 1.94 and `is_multiple_of` stabilised in 1.87. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsMHsab1sPM3nZRbm6rBtf --- .github/workflows/main.yml | 16 +--- Cargo.lock | 7 -- Cargo.toml | 6 -- tests/bench-integration/Cargo.toml | 1 - tests/e2e/Cargo.toml | 1 - wacore/Cargo.toml | 6 -- wacore/appstate/Cargo.toml | 7 -- wacore/appstate/src/lthash.rs | 113 ++++++----------------------- wacore/binary/Cargo.toml | 9 --- 9 files changed, 28 insertions(+), 138 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1c3f5412d..281108181 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -280,24 +280,16 @@ jobs: uses: Swatinem/rust-cache@v2 with: cache-targets: "true" - # Default features, which include `simd`. That combination used to be - # nightly-only, so this job could only ever run the scalar half; both - # halves are stable now and the SIMD one is what ships, so it is the one - # worth testing here. + # These three used to be reachable here only with `--no-default-features`, + # because their default `simd` feature pulled in `portable_simd` and so + # needed nightly. There is no SIMD in the tree any more and no feature + # gating it, so this now runs what ships. - name: Test wacore-binary (stable) run: cargo nextest run --profile ci -p wacore-binary --lib - name: Test wacore-appstate (stable) run: cargo nextest run --profile ci -p wacore-appstate --lib - name: Test wacore (stable) run: cargo nextest run --profile ci -p wacore --lib - # `--no-default-features` turns the LTHash lane math back to scalar. It - # is the configuration wasm and ESP32 build, so it stays covered; a - # compile is enough, since the scalar path is what every test above - # already compares against. - - name: Build the scalar configuration (stable) - run: > - cargo build --verbose --no-default-features - -p wacore-binary -p wacore-appstate -p wacore # `rust-version` is published metadata for every member, but only the # three crates above are exercised at that toolchain. This compiles the # rest of the publishable set so the declared floor is a checked promise diff --git a/Cargo.lock b/Cargo.lock index d538aa4ff..02fe54b7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1356,12 +1356,6 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" -[[package]] -name = "fearless_simd" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0f5f895dac0865bc235f1364793899569a7db538137f1024dccea56bf41c78d" - [[package]] name = "ff" version = "0.13.1" @@ -3895,7 +3889,6 @@ dependencies = [ "anyhow", "buffa", "codspeed-divan-compat", - "fearless_simd", "hex", "hkdf 0.13.0", "hmac 0.13.0", diff --git a/Cargo.toml b/Cargo.toml index b4a122fd1..114a7895c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -95,10 +95,6 @@ diesel_migrations = { version = "2.3.2", default-features = false, features = [" divan = { package = "codspeed-divan-compat", version = "5.0.1" } env_logger = { version = "0.11", default-features = false } event-listener = { version = "5", default-features = false } -# `std` (default) rather than the `libm` path: wacore-appstate is already -# std-only (LazyLock), and libm would add a dependency to serve float ops -# this crate never touches. -fearless_simd = { version = "0.6.0" } flate2 = { version = "1.1.9", default-features = false, features = ["zlib-rs"] } futures = { version = "0.3", default-features = false, features = ["alloc", "async-await"] } getrandom = { version = "0.4", default-features = false } @@ -165,7 +161,6 @@ tracing-pii = ["wacore/tracing-pii", "wacore-binary/tracing-pii"] danger-skip-tls-verify = ["whatsapp-rust-tokio-transport?/danger-skip-tls-verify"] danger-skip-cert-chain-verify = ["wacore/danger-skip-cert-chain-verify"] default = [ - "simd", "sqlite-storage", "tokio-transport", "tokio-runtime", @@ -173,7 +168,6 @@ default = [ "tokio-native", "signal", ] -simd = ["wacore/simd"] ureq-client = ["dep:whatsapp-rust-ureq-http-client"] tokio-transport = ["dep:whatsapp-rust-tokio-transport"] tokio-runtime = ["dep:tokio"] diff --git a/tests/bench-integration/Cargo.toml b/tests/bench-integration/Cargo.toml index c393f6024..b8c239a44 100644 --- a/tests/bench-integration/Cargo.toml +++ b/tests/bench-integration/Cargo.toml @@ -12,7 +12,6 @@ env_logger = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync", "time"] } whatsapp-rust = { path = "../..", default-features = false, features = [ "danger-skip-tls-verify", - "simd", "tokio-runtime", "tokio-native", "signal", diff --git a/tests/e2e/Cargo.toml b/tests/e2e/Cargo.toml index 523597e00..90155200b 100644 --- a/tests/e2e/Cargo.toml +++ b/tests/e2e/Cargo.toml @@ -23,7 +23,6 @@ wacore-binary = { path = "../../wacore/binary" } whatsapp-rust = { path = "../..", default-features = false, features = [ "danger-skip-cert-chain-verify", "danger-skip-tls-verify", - "simd", "tokio-runtime", "tokio-native", "signal", diff --git a/wacore/Cargo.toml b/wacore/Cargo.toml index d4b271355..8029c99e4 100644 --- a/wacore/Cargo.toml +++ b/wacore/Cargo.toml @@ -17,12 +17,6 @@ autobenches = false ignored = ["getrandom"] [features] -default = ["simd"] -# appstate's LTHash is the only SIMD left in the tree; wacore-binary's packed -# codec is table-driven scalar, which measured faster than the vectors it -# replaced. This runs on fearless_simd, so the feature no longer implies -# nightly. -simd = ["wacore-appstate/simd"] debug-snapshots = [] # Typed interop with the decoded libsignal SessionRecord v1 model. legacy-session-interop = ["wacore-libsignal/legacy-session-interop"] diff --git a/wacore/appstate/Cargo.toml b/wacore/appstate/Cargo.toml index f99e88395..4617f280a 100644 --- a/wacore/appstate/Cargo.toml +++ b/wacore/appstate/Cargo.toml @@ -11,16 +11,9 @@ description = "Appstate for WhatsApp protocol" [lib] crate-type = ["rlib"] -[features] -default = ["simd"] -# LTHash lane math via fearless_simd: runtime level detection on stable, so -# this feature does not drag the crate onto nightly. -simd = ["dep:fearless_simd"] - [dependencies] anyhow = { workspace = true } buffa = { workspace = true } -fearless_simd = { workspace = true, optional = true } hex = { workspace = true } hkdf = { workspace = true } hmac = { workspace = true } diff --git a/wacore/appstate/src/lthash.rs b/wacore/appstate/src/lthash.rs index 7fb45a33c..06ac77b40 100644 --- a/wacore/appstate/src/lthash.rs +++ b/wacore/appstate/src/lthash.rs @@ -1,5 +1,3 @@ -#[cfg(feature = "simd")] -use fearless_simd::{Level, Simd, SimdBase, dispatch, u16x8}; use hkdf::Hkdf; use hmac::digest::KeyInit; use hmac::{Hmac, Mac}; @@ -12,12 +10,6 @@ use std::sync::LazyLock; static EXTRACT_HMAC: LazyLock> = LazyLock::new(|| Hmac::::new_from_slice(&[0u8; 32]).expect("32-byte HMAC key")); -/// Probing CPU features is comparatively expensive and the answer cannot -/// change while the process runs, so the level is resolved once here: -/// `multiple_op` dispatches once per operand, and a patch carries hundreds. -#[cfg(feature = "simd")] -static SIMD_LEVEL: LazyLock = LazyLock::new(Level::new); - #[derive(Clone, Debug)] pub struct LTHash { pub hkdf_info: &'static [u8], @@ -63,36 +55,19 @@ impl LTHash { } } +/// Deliberately scalar. A hand-vectorised version of this loop lived here +/// until it was measured: over an 812-MAC batch it moved the total by 0.28%, +/// because HKDF above it dominates and LLVM already auto-vectorizes this loop +/// about as well as the intrinsics did. fn perform_pointwise_with_overflow(base: &mut [u8], input: &[u8], subtract: bool) { assert_eq!(base.len(), input.len(), "length mismatch"); - // Use `% 2` instead of `.is_multiple_of(2)` for stable Rust compatibility. - #[allow(clippy::manual_is_multiple_of)] - { - assert!(base.len() % 2 == 0, "slice lengths must be even"); - } - - #[allow(unused_mut, unused_assignments)] - let (mut base_remaining, mut input_remaining): (&mut [u8], &[u8]) = (base, input); + assert!(base.len().is_multiple_of(2), "slice lengths must be even"); // WA Web treats the accumulator as little-endian u16 lanes // (`new DataView(...).getUint16(off, true)` in WA/Crypto/LtHash.js). // Snapshot/patch MACs are HMACs over the accumulator bytes, so the lane // endianness is part of the wire spec. - #[cfg(feature = "simd")] - { - let (base_chunks, base_rem) = base_remaining.as_chunks_mut::<16>(); - let (input_chunks, input_rem) = input_remaining.as_chunks::<16>(); - - dispatch!(*SIMD_LEVEL, simd => pointwise_chunks(simd, base_chunks, input_chunks, subtract)); - - base_remaining = base_rem; - input_remaining = input_rem; - } - - for (base_pair, input_pair) in base_remaining - .chunks_exact_mut(2) - .zip(input_remaining.chunks_exact(2)) - { + for (base_pair, input_pair) in base.chunks_exact_mut(2).zip(input.chunks_exact(2)) { let x = u16::from_le_bytes([base_pair[0], base_pair[1]]); let y = u16::from_le_bytes([input_pair[0], input_pair[1]]); @@ -101,48 +76,7 @@ fn perform_pointwise_with_overflow(base: &mut [u8], input: &[u8], subtract: bool } else { x.wrapping_add(y) }; - let bytes = result.to_le_bytes(); - base_pair[0] = bytes[0]; - base_pair[1] = bytes[1]; - } -} - -/// The lane math, generic over the SIMD level `dispatch!` picked at runtime. -/// `#[inline(always)]` is load-bearing rather than a hint: it is what makes -/// each instantiation inherit its level's `target_feature` set, so the AVX2 -/// copy lowers to AVX2 instead of to the baseline the caller was compiled for. -#[cfg(feature = "simd")] -#[inline(always)] -fn pointwise_chunks( - simd: S, - base_chunks: &mut [[u8; 16]], - input_chunks: &[[u8; 16]], - subtract: bool, -) { - for (base_chunk, input_chunk) in base_chunks.iter_mut().zip(input_chunks) { - // `from_le_bytes` per lane states the wire endianness directly, so - // the same code is correct on either host; on little-endian it - // lowers to the plain 16-byte load a transmute would have emitted. - let base_arr: [u16; 8] = core::array::from_fn(|i| { - u16::from_le_bytes([base_chunk[2 * i], base_chunk[2 * i + 1]]) - }); - let input_arr: [u16; 8] = core::array::from_fn(|i| { - u16::from_le_bytes([input_chunk[2 * i], input_chunk[2 * i + 1]]) - }); - let base_simd = u16x8::from_slice(simd, &base_arr); - let input_simd = u16x8::from_slice(simd, &input_arr); - - let result_simd = if subtract { - base_simd - input_simd - } else { - base_simd + input_simd - }; - - let mut out = [0u16; 8]; - result_simd.store_slice(&mut out); - for (base_pair, lane) in base_chunk.as_chunks_mut::<2>().0.iter_mut().zip(out) { - *base_pair = lane.to_le_bytes(); - } + base_pair.copy_from_slice(&result.to_le_bytes()); } } @@ -205,30 +139,31 @@ mod tests { } } - /// Straight-line reference: no chunking, no dispatch, no feature gate. The - /// point is to be obviously correct rather than fast, so that the test - /// below is an independent check on the SIMD path rather than a - /// comparison of that path against itself. + /// Reference for the test below. It reaches the same answer by a + /// different route than the implementation: lanes are assembled by hand + /// from byte positions and the arithmetic is done in `u32` and masked, so + /// it shares neither `from_le_bytes` nor `wrapping_*` with the code under + /// test. A reference that mirrors the implementation proves nothing. fn reference_pointwise(base: &mut [u8], input: &[u8], subtract: bool) { - for (b, i) in base.chunks_exact_mut(2).zip(input.chunks_exact(2)) { - let x = u16::from_le_bytes([b[0], b[1]]); - let y = u16::from_le_bytes([i[0], i[1]]); + for i in (0..base.len()).step_by(2) { + let x = base[i] as u32 | ((base[i + 1] as u32) << 8); + let y = input[i] as u32 | ((input[i + 1] as u32) << 8); let r = if subtract { - x.wrapping_sub(y) + x.wrapping_sub(y) & 0xFFFF } else { - x.wrapping_add(y) + (x + y) & 0xFFFF }; - b.copy_from_slice(&r.to_le_bytes()); + base[i] = (r & 0xFF) as u8; + base[i + 1] = (r >> 8) as u8; } } - /// The SIMD path splits into 16-byte chunks and leaves a scalar tail, so - /// the sizes below straddle that boundary: under one chunk, exactly one, - /// chunk-plus-tail, and several chunks. Inputs are seeded to cover the - /// wrap boundaries in both directions, which is where a lane-width or - /// endianness mistake would show up rather than in round-number data. + /// Sizes straddle the 16-byte boundary a vectorised implementation would + /// chunk on, so the coverage still holds if one ever comes back. Inputs + /// are seeded onto the wrap boundaries in both directions, which is where + /// a lane-width or endianness mistake shows up rather than in round data. #[test] - fn simd_path_matches_independent_scalar_reference() { + fn pointwise_matches_independent_reference() { let sizes = [0usize, 2, 14, 16, 18, 32, 34, 128, 130, 256]; // Deterministic LCG: reproducible failures, no dev-dependency. let mut seed = 0x2545_F491u32; diff --git a/wacore/binary/Cargo.toml b/wacore/binary/Cargo.toml index 44b268e81..716daa462 100644 --- a/wacore/binary/Cargo.toml +++ b/wacore/binary/Cargo.toml @@ -12,15 +12,6 @@ description = "Binary data and constants for WhatsApp protocol" crate-type = ["rlib"] [features] -# Does nothing. The packed codec this used to gate is table-driven scalar now, -# which measured faster than the vectors it replaced. It stays declared, and -# stays in `default`, purely to leave the published feature surface where it -# was: dropping the name breaks any manifest that says `features = ["simd"]`, -# and dropping it from `default` trips `feature_not_enabled_by_default`. Both -# errors would point at the consumer rather than at this change. Since the -# feature gates nothing, keeping it costs nothing. Delete at the next major. -default = ["simd"] -simd = [] serde = ["dep:serde", "compact_str/serde", "smallvec/serde"] # Render raw phone numbers in `Jid::observe()` instead of the redacted `pn#`. # Local debugging only; never enable in production.