From 74ed44b980dac08f2b3a7721868b6b21fb716579 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Sun, 7 Jun 2026 17:59:11 +0200 Subject: [PATCH] Drop MemoHash's std Hash impl and route cache-identity hashing through CacheHash --- node-graph/graph-craft/src/document.rs | 2 +- node-graph/graph-craft/src/proto.rs | 14 ++++----- node-graph/libraries/core-types/src/memo.rs | 30 ++++++++++++++----- .../libraries/core-types/src/transform.rs | 2 +- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index 9c7dafd393..40c0304ae5 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -172,7 +172,7 @@ impl DocumentNode { } /// Represents the possible inputs to a node. -#[derive(Debug, Clone, PartialEq, Hash, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, PartialEq, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)] pub enum NodeInput { /// A reference to another node in the same network from which this node can receive its input. Node { node_id: NodeId, output_index: usize }, diff --git a/node-graph/graph-craft/src/proto.rs b/node-graph/graph-craft/src/proto.rs index 12a8214c46..2fa7eb93a3 100644 --- a/node-graph/graph-craft/src/proto.rs +++ b/node-graph/graph-craft/src/proto.rs @@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::hash::Hash; -#[derive(Debug, Default, PartialEq, Clone, Hash, Eq, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Default, PartialEq, Clone, Eq, serde::Serialize, serde::Deserialize)] /// A list of [`ProtoNode`]s, which is an intermediate step between the [`crate::document::NodeNetwork`] and the `BorrowTree` containing a single flattened network. pub struct ProtoNetwork { // TODO: remove this since it seems to be unused? @@ -90,7 +90,7 @@ impl PartialEq for ConstructionArgs { use std::hash::Hasher; let hash = |input: &Self| { let mut hasher = rustc_hash::FxHasher::default(); - input.hash(&mut hasher); + input.cache_hash(&mut hasher); hasher.finish() }; hash(self) == hash(other) @@ -99,8 +99,8 @@ impl PartialEq for ConstructionArgs { } } -impl Hash for ConstructionArgs { - fn hash(&self, state: &mut H) { +impl CacheHash for ConstructionArgs { + fn cache_hash(&self, state: &mut H) { core::mem::discriminant(self).hash(state); match self { Self::Nodes(nodes) => { @@ -108,7 +108,7 @@ impl Hash for ConstructionArgs { node.hash(state); } } - Self::Value(value) => value.hash(state), + Self::Value(value) => value.cache_hash(state), Self::Inline(inline) => inline.hash(state), } } @@ -124,7 +124,7 @@ impl ConstructionArgs { } } -#[derive(Debug, Clone, PartialEq, Hash, Eq, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] /// A proto node is an intermediate step between the `DocumentNode` and the boxed struct that actually runs the node (found in the [`BorrowTree`]). /// At different stages in the compilation process, this struct will be transformed into a reduced (more restricted) form acting as a subset of its original form, but that restricted form is still valid in the earlier stage in the compilation process before it was transformed. pub struct ProtoNode { @@ -157,7 +157,7 @@ impl ProtoNode { let mut hasher = rustc_hash::FxHasher::default(); self.identifier.as_str().hash(&mut hasher); - self.construction_args.hash(&mut hasher); + self.construction_args.cache_hash(&mut hasher); if self.skip_deduplication { self.original_location.path.hash(&mut hasher); } diff --git a/node-graph/libraries/core-types/src/memo.rs b/node-graph/libraries/core-types/src/memo.rs index 17a618c9a6..80766fc860 100644 --- a/node-graph/libraries/core-types/src/memo.rs +++ b/node-graph/libraries/core-types/src/memo.rs @@ -11,12 +11,34 @@ pub struct IORecord { pub output: O, } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] +#[derive(Clone, Debug)] pub struct MemoHash { hash: u64, value: Arc, } +// Compare the value, not the cache `hash`: `CacheHash` is not guaranteed to be an equivalence relation, +// so it can't back `eq`/`cmp`. Cache-identity hashing goes through `CacheHash`. +impl PartialEq for MemoHash { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + +impl Eq for MemoHash {} + +impl PartialOrd for MemoHash { + fn partial_cmp(&self, other: &Self) -> Option { + self.value.partial_cmp(&other.value) + } +} + +impl Ord for MemoHash { + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + self.value.cmp(&other.value) + } +} + impl<'de, T: serde::Deserialize<'de> + CacheHash> serde::Deserialize<'de> for MemoHash { fn deserialize(deserializer: D) -> Result where @@ -67,12 +89,6 @@ impl From for MemoHash { } } -impl Hash for MemoHash { - fn hash(&self, state: &mut H) { - self.hash.hash(state) - } -} - impl CacheHash for MemoHash { fn cache_hash(&self, state: &mut H) { self.hash.hash(state); diff --git a/node-graph/libraries/core-types/src/transform.rs b/node-graph/libraries/core-types/src/transform.rs index ac892d4362..90193eadd4 100644 --- a/node-graph/libraries/core-types/src/transform.rs +++ b/node-graph/libraries/core-types/src/transform.rs @@ -31,7 +31,7 @@ pub trait Transform { /// - `skew`: the horizontal shear coefficient (the raw matrix value, not an angle) /// /// The original transform can be reconstructed as: - /// ``` + /// ```ignore /// DAffine2::from_scale_angle_translation(scale, rotation, translation) * DAffine2::from_cols_array(&[1., 0., skew, 1., 0., 0.]) /// ``` #[inline(always)]