perf: optimize FFI boundary and SIMD hot paths; fix AVX-512 string overflow - #48
Merged
Merged
Conversation
…erflow
Python FFI (python.rs):
- Add GIL_RELEASE_THRESHOLD (4096): short hex strings now compute on
borrowed bytes without a redundant to_vec() or GIL release/acquire
round-trip. ~1.7-2.1x faster for short strings, ~1.2-1.3x for bytes.
- Wire check_bytes_arrays_{first,best,all}_within_dist to the rayon
api.rs implementations so batch calls use multiple cores, while
preserving Python-side validation, error messages, and sentinels.
NEON (neon_simd.rs):
- Add pack32_xor_neon helper (immediate vshlq_n_u8 shift, dedup parse+pack).
- Batch popcounts into a vector accumulator and OR validation masks,
reducing/validating once per batch. ~14-18% faster on large strings,
neutral on small. _with_max checks invalid-hex before the max_dist
sentinel to preserve original semantics.
x86 (x86_simd.rs):
- Fix hamming_distance_string_avx512 silent overflow: epi8 lanes were
accumulated unbounded and produced wrong results for strings >~4032
chars. Now flushes via SAD into a wide accumulator every 32 iterations.
- Add #[inline] to popcnt128_shuffle / popcnt256_shuffle.
Tests (tests.rs):
- Add long-string overflow regression tests, including an AVX-512-gated case.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📊 Benchmark Comparison Results
➖ 40 benchmarks within noise (click to expand)
Legend: ✅ Faster (>5%) · |
The CI benchmark bot caught a severe regression: parallelizing bytes_array_first_within_dist with par_chunks().filter_map().min() is a non-short-circuiting full scan, so it evaluated every element to find the minimum matching index. This was catastrophic for early matches (510x slower for a match at index 0, 33% slower for a mid match) since the serial path returns immediately on the first hit. No parallel strategy can beat serial for an early match (rayon scheduling overhead alone dwarfs the ~0.2us serial cost), so always use the serial early-exit path for `first`. best/all keep the rayon parallel path since they must examine every element anyway (+27-32% at 16384 elems). Verified: match-at-0 back to ~0.0001ms/call (was 0.0875ms in the PR). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Lets serial_{first,best,all}_within_dist fold into their callers (the
Python allow_threads closures and the rayon wrappers), matching the
original inlined-closure codegen and removing the cross-function-call
boundary on the small/below-threshold hot path.
Note: an A/B on this aarch64 host showed the best[small] delta the CI
bot reported is within noise — at 512/1024 elems the ~1us Python call
overhead dominates the few-hundred-ns Rust loop, so the effect is not
locally measurable. This change is low-risk hygiene that can only help.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Performance review across the FFI boundary and SIMD hot paths, plus a correctness fix for AVX-512. Driven by a multi-subagent review of the Python/FFI, NEON, x86 SIMD, and dispatch/batch axes.
Changes
Python FFI (
src/python.rs)GIL_RELEASE_THRESHOLD(4096): short hex strings now compute on borrowed bytes without a redundantto_vec()or GIL release/acquire round-trip.check_bytes_arrays_{first,best,all}_within_distto the rayonapi.rsimplementations so batch calls use multiple cores, preserving Python-side validation, error messages, and sentinels.NEON (
src/neon_simd.rs)pack32_xor_neonhelper (immediatevshlq_n_u8shift, dedup parse+pack)._with_maxchecks invalid-hex before themax_distsentinel to preserve original semantics.x86 (
src/x86_simd.rs)hamming_distance_string_avx512accumulated epi8 lanes unbounded, producing silently wrong results for strings >~4032 chars. Now flushes via SAD into a wide accumulator every 32 iterations.#[inline]topopcnt128_shuffle/popcnt256_shuffle.Tests (
src/tests.rs)Measured impact (Apple Silicon / NEON host)
hamming_distance_string(short)hamming_distance_bytes(short)check_bytes_arrays_*x86 changes (AVX-512 fix,
#[inline]) validated by cross-compilation + regression tests; not runtime-benchmarked (no x86 host available).Validation
cargo fmt --check,ruff check,ruff format --checkall green.