From c6907f3228ca198e9db0716624e2ec3e4c6c0631 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:05:56 +0900 Subject: [PATCH] Move tuple/frozenset hash algorithms into common::hash (#8213) * Move tuple/frozenset hash algorithms into common::hash Move the pure tuplehash fold (xxHash-based) into `hash::hash_tuple`, taking a fallible iterator of element hashes, and the frozenset XOR-fold into a `hash::FrozenSetHash` streaming accumulator. `builtins::tuple` and `builtins::set` keep only the element-hash loop and delegate the arithmetic. Assisted-by: Claude * common/hash: rename acc_pyhash to acc_py_hash Drop the now-unused `// cspell:ignore pyhash` from tuple.rs left behind by the tuple hash move; the only remaining occurrence was the acc_pyhash local. Renaming it lets cspell pass without an ignore. Assisted-by: Claude --- crates/common/src/hash.rs | 98 +++++++++++++++++++++++++++++++++ crates/vm/src/builtins/set.rs | 30 +++------- crates/vm/src/builtins/tuple.rs | 51 +---------------- 3 files changed, 108 insertions(+), 71 deletions(-) diff --git a/crates/common/src/hash.rs b/crates/common/src/hash.rs index 91fd5e1cb..56e6e0006 100644 --- a/crates/common/src/hash.rs +++ b/crates/common/src/hash.rs @@ -198,3 +198,101 @@ pub fn keyed_hash(key: u64, buf: &[u8]) -> u64 { buf.hash(&mut hasher); hasher.finish() } + +/// tuplehash: fold the element hashes of a tuple (xxHash-based). +/// +/// The caller supplies each element's hash lazily; a hash computation may fail, +/// in which case the error short-circuits the fold. +pub fn hash_tuple( + element_hashes: impl IntoIterator>, +) -> Result { + const PRIME1: PyUHash = cfg_select! { + target_pointer_width = "64" => 11400714785074694791, + target_pointer_width = "32" => 2654435761, + _ => unreachable!(), + }; + + const PRIME2: PyUHash = cfg_select! { + target_pointer_width = "64" => 14029467366897019727, + target_pointer_width = "32" => 2246822519, + _ => unreachable!(), + }; + + const PRIME5: PyUHash = cfg_select! { + target_pointer_width = "64" => 2870177450012600261, + target_pointer_width = "32" => 374761393, + _ => unreachable!(), + }; + + const ROTATE: u32 = cfg_select! { + target_pointer_width = "64" => 31, + target_pointer_width = "32" => 13, + _ => unreachable!(), + }; + + let mut acc = PRIME5; + let mut len: PyUHash = 0; + + for element_hash in element_hashes { + let lane = element_hash? as PyUHash; + acc = acc.wrapping_add(lane.wrapping_mul(PRIME2)); + acc = acc.rotate_left(ROTATE); + acc = acc.wrapping_mul(PRIME1); + len += 1; + } + + acc = acc.wrapping_add(len ^ (PRIME5 ^ 3527539)); + + let acc_py_hash = acc as PyHash; + if acc_py_hash == -1 { + return Ok(1546275796); + } + + Ok(acc_py_hash) +} + +/// frozenset_hash: order-independent XOR-fold of a frozenset's element hashes. +/// +/// The entry hashes are fed in one at a time via [`FrozenSetHash::add`], so the +/// caller keeps ownership of the iteration (which may hold a lock and compute +/// each element hash fallibly). The fold is commutative, so element order does +/// not affect the result. +pub struct FrozenSetHash { + hash: u64, +} + +impl FrozenSetHash { + #[must_use] + pub fn new(len: usize) -> Self { + // Factor in the number of active entries + Self { + hash: (len as u64 + 1).wrapping_mul(1927868237), + } + } + + pub fn add(&mut self, element_hash: PyHash) { + // Work to increase the bit dispersion for closely spaced hash values. + // This is important because some use cases have many combinations of a + // small number of elements with nearby hashes so that many distinct + // combinations collapse to only a handful of distinct hash values. + const fn shuffle_bits(h: u64) -> u64 { + ((h ^ 89869747) ^ (h.wrapping_shl(16))).wrapping_mul(3644798167) + } + // Xor-in shuffled bits from every entry's hash field because xor is + // commutative and a frozenset hash should be independent of order. + self.hash ^= shuffle_bits(element_hash as u64); + } + + #[must_use] + pub fn finish(self) -> PyHash { + let mut hash = self.hash; + // Disperse patterns arising in nested frozen-sets + hash ^= (hash >> 11) ^ (hash >> 25); + hash = hash.wrapping_mul(69069).wrapping_add(907133923); + // -1 is reserved as an error code + if hash == u64::MAX { + hash = 590923713; + } + hash as PyHash + } +} diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 8b38223fc..44dc9a221 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -442,28 +442,14 @@ impl PySetInner { } fn hash(&self, vm: &VirtualMachine) -> PyResult { - // Work to increase the bit dispersion for closely spaced hash values. - // This is important because some use cases have many combinations of a - // small number of elements with nearby hashes so that many distinct - // combinations collapse to only a handful of distinct hash values. - const fn _shuffle_bits(h: u64) -> u64 { - ((h ^ 89869747) ^ (h.wrapping_shl(16))).wrapping_mul(3644798167) - } - // Factor in the number of active entries - let mut hash: u64 = (self.len() as u64 + 1).wrapping_mul(1927868237); - // Xor-in shuffled bits from every entry's hash field because xor is - // commutative and a frozenset hash should be independent of order. - hash = self.content.try_fold_keys(hash, |h, element| { - Ok(h ^ _shuffle_bits(element.hash(vm)? as u64)) - })?; - // Disperse patterns arising in nested frozen-sets - hash ^= (hash >> 11) ^ (hash >> 25); - hash = hash.wrapping_mul(69069).wrapping_add(907133923); - // -1 is reserved as an error code - if hash == u64::MAX { - hash = 590923713; - } - Ok(hash as PyHash) + let hasher = self.content.try_fold_keys( + hash::FrozenSetHash::new(self.len()), + |mut hasher, element| { + hasher.add(element.hash(vm)?); + Ok(hasher) + }, + )?; + Ok(hasher.finish()) } // Run operation, on failure, if item is a set/set subclass, convert it diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 4606509fd..7ed815ce2 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -1,14 +1,8 @@ -// cspell:ignore pyhash - use super::{ PositionIterInternal, PyGenericAlias, PyStrRef, PyType, PyTypeRef, iter::builtins_iter, }; use crate::common::lock::LazyLock; -use crate::common::{ - hash::{PyHash, PyUHash}, - lock::PyMutex, - wtf8::wtf8_concat, -}; +use crate::common::{hash, hash::PyHash, lock::PyMutex, wtf8::wtf8_concat}; use crate::object::{Traverse, TraverseFn}; use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, @@ -727,46 +721,5 @@ pub(crate) fn init(context: &'static Context) { } pub(super) fn tuple_hash(elements: &[PyObjectRef], vm: &VirtualMachine) -> PyResult { - const PRIME1: PyUHash = cfg_select! { - target_pointer_width = "64" => 11400714785074694791, - target_pointer_width = "32" => 2654435761, - _ => unreachable!(), - }; - - const PRIME2: PyUHash = cfg_select! { - target_pointer_width = "64" => 14029467366897019727, - target_pointer_width = "32" => 2246822519, - _ => unreachable!(), - }; - - const PRIME5: PyUHash = cfg_select! { - target_pointer_width = "64" => 2870177450012600261, - target_pointer_width = "32" => 374761393, - _ => unreachable!(), - }; - - const ROTATE: u32 = cfg_select! { - target_pointer_width = "64" => 31, - target_pointer_width = "32" => 13, - _ => unreachable!(), - }; - - let mut acc = PRIME5; - let len = elements.len() as PyUHash; - - for val in elements { - let lane = val.hash(vm)? as PyUHash; - acc = acc.wrapping_add(lane.wrapping_mul(PRIME2)); - acc = acc.rotate_left(ROTATE); - acc = acc.wrapping_mul(PRIME1); - } - - acc = acc.wrapping_add(len ^ (PRIME5 ^ 3527539)); - - let acc_pyhash = acc as PyHash; - if acc_pyhash == -1 { - return Ok(1546275796); - } - - Ok(acc_pyhash) + hash::hash_tuple(elements.iter().map(|val| val.hash(vm))) }