Repository navigation
Conversation
The crate refuses to build for big-endian targets (apache#28, apache#31) because the Java library requires a little-endian native byte order. The Rust code does not need that for serialization: SketchBytes and SketchSlice read and write with an explicit byte order, and the crate has no native-endian loads or stores. The one place where the host byte order leaked in was hashing. Both MurmurHash3X64128 and XxHash64 implement only Hasher::write, so the default write_u16/u32/u64/u128/usize methods (and the signed variants, which forward to them) hash the native-endian bytes of the integer. On a big-endian host every integer item therefore hashes to a different value than on a little-endian host or in the Java, C++ and Go implementations. With only the compile_error removed, 17 integration tests fail on FreeBSD powerpc64 (big-endian): the bloom, CPC, HLL and XOR serialization compatibility tests against the Java, C++ and Go reference images, several theta set-operation tests and three HLL tests. Override the fixed-width Hasher methods in both hashers to hash the little-endian representation. This changes nothing on little-endian targets. With it, `cargo x test` passes on powerpc64 with the same results as on powerpc64le, including all serialization tests against the TCK snapshots, and stable clippy with -D warnings is clean. Drop the compile_error and the README note, and add unit tests that pin the contract: hashing an integer equals hashing its little-endian bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.
The crate refuses to build for big-endian targets (#28, #31) because the
Java library requires a little-endian native byte order. The Rust code
does not need that for serialization: SketchBytes and SketchSlice read
and write with an explicit byte order, and the crate has no
native-endian loads or stores.
The one place where the host byte order leaked in was hashing. Both
MurmurHash3X64128 and XxHash64 implement only Hasher::write, so the
default write_u16/u32/u64/u128/usize methods (and the signed variants,
which forward to them) hash the native-endian bytes of the integer. On
a big-endian host every integer item therefore hashes to a different
value than on a little-endian host or in the Java, C++ and Go
implementations. With only the compile_error removed, 17 integration
tests fail on FreeBSD powerpc64 (big-endian): the bloom, CPC, HLL and
XOR serialization compatibility tests against the Java, C++ and Go
reference images, several theta set-operation tests and three HLL
tests.
Override the fixed-width Hasher methods in both hashers to hash the
little-endian representation. This changes nothing on little-endian
targets. With it,
cargo x testpasses on powerpc64 with the sameresults as on powerpc64le, including all serialization tests against
the TCK snapshots, and stable clippy with -D warnings is clean.
Drop the compile_error and the README note, and add unit tests that pin
the contract: hashing an integer equals hashing its little-endian bytes.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com