refactor(checksums): one owner for the SIMD batch input cap - #7787
Merged
Conversation
Eleven SIMD backends each declared their own `MAX_INPUT_SIZE`, and the MD4 AVX-512 path carried the same bound as a bare `1024 * 1024` literal with no name at all. All twelve sites held the identical value, so this was latent duplication rather than a live divergence, but nothing kept them in step. Collapse them onto a single `simd_batch::MAX_INPUT_SIZE` consumed by every MD4 and MD5 backend, so a per-backend value can no longer be written. The bound caps the transient padding allocation each backend makes (lanes x cap, 16 MiB on the AVX-512 path); batches above it fall back to the scalar digest. It has no upstream counterpart - upstream rsync has no multi-buffer MD4/MD5 - so the value is documented as an oc-side allocation-tuning choice. Add tests pinning the value against its allocation invariants and checking MD4/MD5 batch digests against the scalar reference on both sides of the cap. The bound had no test before this. No behaviour change: every backend reads the value it read previously.
oferchen
enabled auto-merge (squash)
September 9, 2026 12:04
oferchen
added a commit
that referenced
this pull request
Sep 9, 2026
The measurement-gate note's "Three that look mirrored and are not" section landed as #7779 and went stale within a day: all three entries have since been fixed, and one of them cited a file that is being deleted. Each closure verified in the tree, not taken from a changelog: - hash table floor/growth -> #7790. `TAG_TABLE_SIZE` now exists at exactly one site (`crates/matching/src/index/mod.rs:68`) and its own doc says the growth role "belongs to CompactLookup", which grows on upstream's `(count/8) * 10 + 11` rule. The entry's second cited site, `optimized_search.rs:12`, is dropped: that module is dead and its removal is in flight, and the live constant was never the growth-bearing one. - `MAX_XATTR_DIGEST_LEN` literal -> #7786. Now `<Md5 as OutputSizeUser>::OutputSize::USIZE` with `const _: () = assert!(MAX_XATTR_DIGEST_LEN == 16)` beneath it (`crates/protocol/src/xattr/mod.rs:76,81`), so a digest change cannot silently move a wire length. - `MAX_INPUT_SIZE` copies -> #7787. One definition at `crates/checksums/src/simd_batch/mod.rs:46`; every backend imports it. The entry said nine copies; the sweep found twelve sites, eleven constants plus one bare literal no grep for the name could reach. The count is corrected rather than left flattering. The section is kept rather than deleted: the lesson it teaches (value equality is not provenance) is the reason the gate exists, and each fix is evidence the reading was right. The upstream anchors are unchanged except `match.c:86-88` -> `match.c:84-88`, matching the range the code itself cites for the same rule. Also drops the deleted module from the zsync design note's file list and corrects that entry's crate directory (`crates/match/` -> `crates/matching/`).
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
1 participant
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.
The 1 MiB per-input cap that decides whether a SIMD batch backend hashes
in-lane or bails to the scalar digest was written out twelve times across
crates/checksums/src/simd_batch/— eleven namedconst MAX_INPUT_SIZEdeclarations plus one bare literal at
md4/simd/avx512.rs:59(
if max_len > 1024 * 1024) that a grep for the constant name cannot see.There is now one owner in
simd_batch/mod.rs, cfg-gated to the threearchitectures that have SIMD backends, consumed by all twelve former sites.
Why the bound exists
Stated in the doc comment, because it was nowhere before: every backend pads
each lane into a freshly allocated 64-byte-block multiple, so the transient
allocation is bounded by lanes x cap — 16 MiB on the widest, AVX-512 16-lane
path.
Upstream has no multi-buffer MD4/MD5 at all (its only checksum SIMD is
get_checksum1, the Adler rolling sum), so there is no upstream counterpart tomirror and the doc says so rather than inventing a citation. The one 1 MiB
constant in upstream's SIMD source,
simd-checksum-x86_64.cpp:512 BLOCK_LEN, isa benchmark-harness buffer and unrelated.
The bound was untested
Mutating both live aarch64 copies on the base tree killed 0 of 1158 tests.
Two tests now cover it:
cap_bounds_the_worst_case_padded_allocation— pins the value against theallocation reasoning that justifies it (whole 64-byte blocks; 16-lane worst
case), so a change to the value has to confront why.
digests_agree_with_scalar_on_both_sides_of_the_cap— atMAX_INPUT_SIZEandMAX_INPUT_SIZE + 1, MD4 and MD5 batch digests must equal the scalarreference. This is what makes a shared owner safe: a backend reading a
different bound than the scalar oracle assumes would still have to agree here.
Codegen neutrality
Proven past the test suite: LLVM IR diff is 18659 lines both sides, 22 differing
— the module ID and
core::panic::Locationline numbers shifting by exactly 3,the three deleted lines per file.
Gates
rustfmt 3430 files clean (whole-tree checker), clippy
-p checksums --all-targets --all-featuresclean, nextest 1160/1160 (1158before plus the 2 new).
Host limitation, stated rather than implied: aarch64-apple-darwin executes only
the NEON path. SSE2/SSSE3/SSE4.1/AVX2/AVX-512 are type-checked but not run
locally, and wasm32 is not compiled — CI covers those.