This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new 36026fb5a fix(rust): fix mulmurhash compute (#3824)
36026fb5a is described below
commit 36026fb5a794916b7fb49436b2116a24a9944b44
Author: Shawn Yang <[email protected]>
AuthorDate: Wed Jul 8 17:32:18 2026 +0530
fix(rust): fix mulmurhash compute (#3824)
## Why?
## What does this PR do?
## Related issues
## AI Contribution Checklist
- [ ] Substantial AI assistance was used in this PR: `yes` / `no`
- [ ] If `yes`, I included a completed [AI Contribution
Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs)
in this PR description and the required `AI Usage Disclosure`.
- [ ] If `yes`, my PR description includes the required `ai_review`
summary and screenshot evidence or equivalent persisted links of the
final clean AI review results from both fresh reviewers described in
`AI_POLICY.md`, the Fory-guided reviewer and the independent general
reviewer, on the current PR diff or current HEAD after the latest code
changes.
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
---
cpp/fory/thirdparty/BUILD | 12 +++++-
cpp/fory/thirdparty/MurmurHash3.cc | 56 +++++++++++--------------
cpp/fory/thirdparty/MurmurHash3_test.cc | 72 +++++++++++++++++++++++++++++++++
rust/fory-core/src/util/string_util.rs | 14 +++++--
4 files changed, 117 insertions(+), 37 deletions(-)
diff --git a/cpp/fory/thirdparty/BUILD b/cpp/fory/thirdparty/BUILD
index e951e391a..73816dfe6 100644
--- a/cpp/fory/thirdparty/BUILD
+++ b/cpp/fory/thirdparty/BUILD
@@ -1,4 +1,4 @@
-load("@rules_cc//cc:defs.bzl", "cc_library")
+load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
cc_library(
name = "libmmh3",
@@ -17,6 +17,16 @@ cc_library(
visibility = ["//visibility:public"],
)
+cc_test(
+ name = "murmur_hash3_test",
+ srcs = ["MurmurHash3_test.cc"],
+ deps = [
+ ":libmmh3",
+ "@googletest//:gtest",
+ "@googletest//:gtest_main",
+ ],
+)
+
cc_test(
name = "flat_hash_map_test",
srcs = ["flat_hash_map_test.cc"],
diff --git a/cpp/fory/thirdparty/MurmurHash3.cc
b/cpp/fory/thirdparty/MurmurHash3.cc
index ae4aa3617..2e64f1287 100644
--- a/cpp/fory/thirdparty/MurmurHash3.cc
+++ b/cpp/fory/thirdparty/MurmurHash3.cc
@@ -9,6 +9,8 @@
#include "fory/thirdparty/MurmurHash3.h"
+#include <cstring>
+
//-----------------------------------------------------------------------------
// Platform-specific functions and macros
@@ -58,28 +60,20 @@ inline uint64_t rotl64(uint64_t x, int8_t r) {
#endif // !defined(_MSC_VER)
//-----------------------------------------------------------------------------
-// Block read - on little-endian machines this is a single load,
-// while on big-endian or unknown machines the byte accesses should
-// still get optimized into the most efficient instruction.
-FORCE_INLINE uint32_t getblock(const uint32_t *p, int i) {
-#if defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
- return p[i];
-#else
- const uint8_t *c = (const uint8_t *)&p[i];
+// Block read. Fory callers may pass slices at arbitrary byte alignment, so
+// assemble little-endian words from bytes instead of aliasing the input as
+// uint32_t/uint64_t arrays.
+FORCE_INLINE uint32_t getblock32(const uint8_t *p, int i) {
+ const uint8_t *c = p + i * 4;
return (uint32_t)c[0] | (uint32_t)c[1] << 8 | (uint32_t)c[2] << 16 |
(uint32_t)c[3] << 24;
-#endif
}
-FORCE_INLINE uint64_t getblock(const uint64_t *p, int i) {
-#if defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
- return p[i];
-#else
- const uint8_t *c = (const uint8_t *)&p[i];
+FORCE_INLINE uint64_t getblock64(const uint8_t *p, int i) {
+ const uint8_t *c = p + i * 8;
return (uint64_t)c[0] | (uint64_t)c[1] << 8 | (uint64_t)c[2] << 16 |
(uint64_t)c[3] << 24 | (uint64_t)c[4] << 32 | (uint64_t)c[5] << 40 |
(uint64_t)c[6] << 48 | (uint64_t)c[7] << 56;
-#endif
}
//-----------------------------------------------------------------------------
@@ -121,10 +115,10 @@ void MurmurHash3_x86_32(const void *key, int len,
uint32_t seed, void *out) {
//----------
// body
- const uint32_t *blocks = (const uint32_t *)(data + nblocks * 4);
+ const uint8_t *blocks = data + nblocks * 4;
for (int i = -nblocks; i; i++) {
- uint32_t k1 = getblock(blocks, i);
+ uint32_t k1 = getblock32(blocks, i);
k1 *= c1;
k1 = ROTL32(k1, 15);
@@ -162,7 +156,7 @@ void MurmurHash3_x86_32(const void *key, int len, uint32_t
seed, void *out) {
h1 = fmix(h1);
- *(uint32_t *)out = h1;
+ std::memcpy(out, &h1, sizeof(h1));
}
//-----------------------------------------------------------------------------
@@ -185,13 +179,13 @@ void MurmurHash3_x86_128(const void *key, const int len,
uint32_t seed,
//----------
// body
- const uint32_t *blocks = (const uint32_t *)(data + nblocks * 16);
+ const uint8_t *blocks = data + nblocks * 16;
for (int i = -nblocks; i; i++) {
- uint32_t k1 = getblock(blocks, i * 4 + 0);
- uint32_t k2 = getblock(blocks, i * 4 + 1);
- uint32_t k3 = getblock(blocks, i * 4 + 2);
- uint32_t k4 = getblock(blocks, i * 4 + 3);
+ uint32_t k1 = getblock32(blocks, i * 4 + 0);
+ uint32_t k2 = getblock32(blocks, i * 4 + 1);
+ uint32_t k3 = getblock32(blocks, i * 4 + 2);
+ uint32_t k4 = getblock32(blocks, i * 4 + 3);
k1 *= c1;
k1 = ROTL32(k1, 15);
@@ -319,10 +313,8 @@ void MurmurHash3_x86_128(const void *key, const int len,
uint32_t seed,
h3 += h1;
h4 += h1;
- ((uint32_t *)out)[0] = h1;
- ((uint32_t *)out)[1] = h2;
- ((uint32_t *)out)[2] = h3;
- ((uint32_t *)out)[3] = h4;
+ uint32_t hash[4] = {h1, h2, h3, h4};
+ std::memcpy(out, hash, sizeof(hash));
}
//-----------------------------------------------------------------------------
@@ -341,11 +333,9 @@ void MurmurHash3_x64_128(const void *key, const int len,
const uint32_t seed,
//----------
// body
- const uint64_t *blocks = (const uint64_t *)(data);
-
for (int i = 0; i < nblocks; i++) {
- uint64_t k1 = getblock(blocks, i * 2 + 0);
- uint64_t k2 = getblock(blocks, i * 2 + 1);
+ uint64_t k1 = getblock64(data, i * 2 + 0);
+ uint64_t k2 = getblock64(data, i * 2 + 1);
k1 *= c1;
k1 = ROTL64(k1, 31);
@@ -431,8 +421,8 @@ void MurmurHash3_x64_128(const void *key, const int len,
const uint32_t seed,
h1 += h2;
h2 += h1;
- ((uint64_t *)out)[0] = h1;
- ((uint64_t *)out)[1] = h2;
+ uint64_t hash[2] = {h1, h2};
+ std::memcpy(out, hash, sizeof(hash));
}
//-----------------------------------------------------------------------------
diff --git a/cpp/fory/thirdparty/MurmurHash3_test.cc
b/cpp/fory/thirdparty/MurmurHash3_test.cc
new file mode 100644
index 000000000..4f39a11a9
--- /dev/null
+++ b/cpp/fory/thirdparty/MurmurHash3_test.cc
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "fory/thirdparty/MurmurHash3.h"
+
+#include <array>
+#include <cstdint>
+#include <cstring>
+
+#include "gtest/gtest.h"
+
+namespace fory {
+
+TEST(MurmurHash3Test, AcceptsUnalignedBuffers) {
+ const std::array<uint8_t, 32> payload = {
+ 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x61, 0x62,
+ 0x63, 0x64, 0x65, 0x66, 0x31, 0x71, 0x72, 0x73, 0x74, 0x75, 0x76,
+ 0x77, 0x78, 0x79, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x32,
+ };
+ std::array<uint8_t, 33> unaligned_input{};
+ std::memcpy(unaligned_input.data() + 1, payload.data(), payload.size());
+
+ uint32_t aligned32 = 0;
+ std::array<uint8_t, sizeof(uint32_t) + 1> unaligned32{};
+ MurmurHash3_x86_32(payload.data(), static_cast<int>(payload.size()), 47,
+ &aligned32);
+ MurmurHash3_x86_32(unaligned_input.data() + 1,
+ static_cast<int>(payload.size()), 47,
+ unaligned32.data() + 1);
+ EXPECT_EQ(std::memcmp(&aligned32, unaligned32.data() + 1, sizeof(aligned32)),
+ 0);
+
+ std::array<uint32_t, 4> aligned_x86_128{};
+ std::array<uint8_t, sizeof(aligned_x86_128) + 1> unaligned_x86_128{};
+ MurmurHash3_x86_128(payload.data(), static_cast<int>(payload.size()), 47,
+ aligned_x86_128.data());
+ MurmurHash3_x86_128(unaligned_input.data() + 1,
+ static_cast<int>(payload.size()), 47,
+ unaligned_x86_128.data() + 1);
+ EXPECT_EQ(std::memcmp(aligned_x86_128.data(), unaligned_x86_128.data() + 1,
+ sizeof(aligned_x86_128)),
+ 0);
+
+ std::array<uint64_t, 2> aligned_x64_128{};
+ std::array<uint8_t, sizeof(aligned_x64_128) + 1> unaligned_x64_128{};
+ MurmurHash3_x64_128(payload.data(), static_cast<int>(payload.size()), 47,
+ aligned_x64_128.data());
+ MurmurHash3_x64_128(unaligned_input.data() + 1,
+ static_cast<int>(payload.size()), 47,
+ unaligned_x64_128.data() + 1);
+ EXPECT_EQ(std::memcmp(aligned_x64_128.data(), unaligned_x64_128.data() + 1,
+ sizeof(aligned_x64_128)),
+ 0);
+}
+
+} // namespace fory
diff --git a/rust/fory-core/src/util/string_util.rs
b/rust/fory-core/src/util/string_util.rs
index 5e0d1ab35..800440f75 100644
--- a/rust/fory-core/src/util/string_util.rs
+++ b/rust/fory-core/src/util/string_util.rs
@@ -15,7 +15,6 @@
// specific language governing permissions and limitations
// under the License.
-use std::mem;
use std::ptr;
const MAX_HASH32: u64 = (1 << 31) - 1;
@@ -477,8 +476,11 @@ pub fn murmurhash3_x64_128(bytes: &[u8], seed: u64) ->
(u64, u64) {
let (mut h1, mut h2) = (seed, seed);
for i in 0..block_count as usize {
- let b64: &[u64] = unsafe { mem::transmute(bytes) };
- let (mut k1, mut k2) = (b64[i * 2], b64[i * 2 + 1]);
+ let offset = i * read_size as usize;
+ // Input byte slices can be arbitrarily aligned. Read MurmurHash3
blocks
+ // as explicit little-endian bytes instead of forming an aligned u64
slice.
+ let mut k1 = u64::from_le_bytes(bytes[offset..offset +
8].try_into().unwrap());
+ let mut k2 = u64::from_le_bytes(bytes[offset + 8..offset +
16].try_into().unwrap());
k1 = k1.wrapping_mul(c1);
k1 = k1.rotate_left(31);
@@ -650,6 +652,12 @@ mod test_hash {
assert!(murmurhash3_x64_128("Lorem ipsum dolor sit amet, consectetur
adipiscing elit. Etiam at consequat massa. Cras eleifend pellentesque ex, at
dignissim libero maximus ut. Sed eget nulla felis".as_bytes(), 0)
== (9455322759164802692, 17863277201603478371));
}
+
+ #[test]
+ fn test_unaligned_full_block() {
+ let data = b"x123456789abcdef1";
+ assert!(murmurhash3_x64_128(&data[1..], 0) == (9259082041050667785,
12459473952842597282));
+ }
}
#[cfg(test)]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
