From 04f1ebb3e5a19bf2b890ee4c9e66f07db492b965 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 9 Feb 2023 19:25:39 +0100 Subject: [PATCH 1/5] Add bevy_reflect/fixed_hash for deterministic Reflect::reflect_hash() --- crates/bevy_reflect/Cargo.toml | 1 + .../bevy_reflect_derive/src/container_attributes.rs | 4 ++-- crates/bevy_reflect/src/array.rs | 2 +- crates/bevy_reflect/src/enums/helpers.rs | 2 +- crates/bevy_reflect/src/impls/std.rs | 4 ++-- crates/bevy_reflect/src/reflect.rs | 11 ++++++++++- crates/bevy_utils/src/lib.rs | 2 +- 7 files changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index f74889c5ab06b..aded85a533668 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -15,6 +15,7 @@ default = [] bevy = ["glam", "smallvec", "bevy_math"] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] +fixed_hash = [] [dependencies] # bevy diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 94749f0e8a4e7..67323d8444ce5 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -5,7 +5,7 @@ //! the derive helper attribute for `Reflect`, which looks like: //! `#[reflect(PartialEq, Default, ...)]` and `#[reflect_value(PartialEq, Default, ...)]`. -use crate::fq_std::{FQAny, FQDefault, FQOption}; +use crate::fq_std::{FQAny, FQOption}; use crate::utility; use proc_macro2::{Ident, Span}; use quote::quote_spanned; @@ -225,7 +225,7 @@ impl ReflectTraits { &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> fn reflect_hash(&self) -> #FQOption { use ::core::hash::{Hash, Hasher}; - let mut hasher: #bevy_reflect_path::ReflectHasher = #FQDefault::default(); + let mut hasher = #bevy_reflect_path::reflect_hasher(); Hash::hash(&#FQAny::type_id(self), &mut hasher); Hash::hash(self, &mut hasher); #FQOption::Some(Hasher::finish(&hasher)) diff --git a/crates/bevy_reflect/src/array.rs b/crates/bevy_reflect/src/array.rs index 005e6f6b7e497..286fe0cfc9c2a 100644 --- a/crates/bevy_reflect/src/array.rs +++ b/crates/bevy_reflect/src/array.rs @@ -334,7 +334,7 @@ impl<'a> ExactSizeIterator for ArrayIter<'a> {} /// Returns the `u64` hash of the given [array](Array). #[inline] pub fn array_hash(array: &A) -> Option { - let mut hasher = crate::ReflectHasher::default(); + let mut hasher = crate::reflect_hasher(); std::any::Any::type_id(array).hash(&mut hasher); array.len().hash(&mut hasher); for value in array.iter() { diff --git a/crates/bevy_reflect/src/enums/helpers.rs b/crates/bevy_reflect/src/enums/helpers.rs index d1ea359f8d6d8..2e3d647e41308 100644 --- a/crates/bevy_reflect/src/enums/helpers.rs +++ b/crates/bevy_reflect/src/enums/helpers.rs @@ -5,7 +5,7 @@ use std::hash::{Hash, Hasher}; /// Returns the `u64` hash of the given [enum](Enum). #[inline] pub fn enum_hash(value: &TEnum) -> Option { - let mut hasher = crate::ReflectHasher::default(); + let mut hasher = crate::reflect_hasher(); std::any::Any::type_id(value).hash(&mut hasher); value.variant_name().hash(&mut hasher); value.variant_type().hash(&mut hasher); diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 7e53a46a7ac53..26a795afec043 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -995,7 +995,7 @@ impl Reflect for Cow<'static, str> { } fn reflect_hash(&self) -> Option { - let mut hasher = crate::ReflectHasher::default(); + let mut hasher = crate::reflect_hasher(); Hash::hash(&std::any::Any::type_id(self), &mut hasher); Hash::hash(self, &mut hasher); Some(hasher.finish()) @@ -1103,7 +1103,7 @@ impl Reflect for &'static Path { } fn reflect_hash(&self) -> Option { - let mut hasher = crate::ReflectHasher::default(); + let mut hasher = crate::reflect_hasher(); Hash::hash(&std::any::Any::type_id(self), &mut hasher); Hash::hash(self, &mut hasher); Some(hasher.finish()) diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 3857d8f3cd84b..067bbf75c2fe8 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -6,10 +6,19 @@ use crate::{ use std::{ any::{self, Any, TypeId}, fmt::Debug, + hash::BuildHasher, }; use crate::utility::NonGenericTypeInfoCell; -pub use bevy_utils::AHasher as ReflectHasher; + +#[cfg(feature = "fixed_hash")] +pub use bevy_utils::FixedState as ReflectBuildHasher; +#[cfg(not(feature = "fixed_hash"))] +pub use bevy_utils::RandomState as ReflectBuildHasher; + +pub fn reflect_hasher() -> bevy_utils::AHasher { + ReflectBuildHasher::default().build_hasher() +} /// An immutable enumeration of "kinds" of reflected type. /// diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 3ac14689731f5..0aa6295366e3f 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -30,7 +30,7 @@ pub use thiserror; pub use tracing; pub use uuid::Uuid; -use ahash::RandomState; +pub use ahash::RandomState; use hashbrown::hash_map::RawEntryMut; use std::{ fmt::Debug, From 9b48dfd414743719076d0e7613016cce504d218a Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Fri, 10 Feb 2023 07:50:17 +0100 Subject: [PATCH 2/5] Always use a fixed hash for bevy_reflect --- crates/bevy_reflect/Cargo.toml | 1 - crates/bevy_reflect/src/reflect.rs | 8 +++----- crates/bevy_utils/src/lib.rs | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index aded85a533668..f74889c5ab06b 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -15,7 +15,6 @@ default = [] bevy = ["glam", "smallvec", "bevy_math"] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] -fixed_hash = [] [dependencies] # bevy diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 067bbf75c2fe8..75a5b3aff2a0b 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -11,13 +11,11 @@ use std::{ use crate::utility::NonGenericTypeInfoCell; -#[cfg(feature = "fixed_hash")] -pub use bevy_utils::FixedState as ReflectBuildHasher; -#[cfg(not(feature = "fixed_hash"))] -pub use bevy_utils::RandomState as ReflectBuildHasher; +use bevy_utils::FixedState; +#[inline] pub fn reflect_hasher() -> bevy_utils::AHasher { - ReflectBuildHasher::default().build_hasher() + FixedState.build_hasher() } /// An immutable enumeration of "kinds" of reflected type. diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 0aa6295366e3f..3ac14689731f5 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -30,7 +30,7 @@ pub use thiserror; pub use tracing; pub use uuid::Uuid; -pub use ahash::RandomState; +use ahash::RandomState; use hashbrown::hash_map::RawEntryMut; use std::{ fmt::Debug, From 03f96d20da241d9a620b868d041554936f6214c6 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 14 Feb 2023 22:28:16 +0100 Subject: [PATCH 3/5] Move reflect_hasher to bevy_reflect::utility --- .../bevy_reflect_derive/src/container_attributes.rs | 2 +- crates/bevy_reflect/src/array.rs | 6 +++--- crates/bevy_reflect/src/enums/helpers.rs | 4 ++-- crates/bevy_reflect/src/impls/std.rs | 6 +++--- crates/bevy_reflect/src/reflect.rs | 8 -------- crates/bevy_reflect/src/utility.rs | 12 ++++++++++-- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 67323d8444ce5..87940c689a387 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -225,7 +225,7 @@ impl ReflectTraits { &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> fn reflect_hash(&self) -> #FQOption { use ::core::hash::{Hash, Hasher}; - let mut hasher = #bevy_reflect_path::reflect_hasher(); + let mut hasher = #bevy_reflect_path::utility::reflect_hasher(); Hash::hash(&#FQAny::type_id(self), &mut hasher); Hash::hash(self, &mut hasher); #FQOption::Some(Hasher::finish(&hasher)) diff --git a/crates/bevy_reflect/src/array.rs b/crates/bevy_reflect/src/array.rs index 286fe0cfc9c2a..98cb462cd5ca4 100644 --- a/crates/bevy_reflect/src/array.rs +++ b/crates/bevy_reflect/src/array.rs @@ -1,6 +1,6 @@ use crate::{ - utility::NonGenericTypeInfoCell, DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, - TypeInfo, Typed, + utility::{reflect_hasher, NonGenericTypeInfoCell}, + DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, }; use std::{ any::{Any, TypeId}, @@ -334,7 +334,7 @@ impl<'a> ExactSizeIterator for ArrayIter<'a> {} /// Returns the `u64` hash of the given [array](Array). #[inline] pub fn array_hash(array: &A) -> Option { - let mut hasher = crate::reflect_hasher(); + let mut hasher = reflect_hasher(); std::any::Any::type_id(array).hash(&mut hasher); array.len().hash(&mut hasher); for value in array.iter() { diff --git a/crates/bevy_reflect/src/enums/helpers.rs b/crates/bevy_reflect/src/enums/helpers.rs index 2e3d647e41308..09a3516c79d46 100644 --- a/crates/bevy_reflect/src/enums/helpers.rs +++ b/crates/bevy_reflect/src/enums/helpers.rs @@ -1,11 +1,11 @@ -use crate::{Enum, Reflect, ReflectRef, VariantType}; +use crate::{utility::reflect_hasher, Enum, Reflect, ReflectRef, VariantType}; use std::fmt::Debug; use std::hash::{Hash, Hasher}; /// Returns the `u64` hash of the given [enum](Enum). #[inline] pub fn enum_hash(value: &TEnum) -> Option { - let mut hasher = crate::reflect_hasher(); + let mut hasher = reflect_hasher(); std::any::Any::type_id(value).hash(&mut hasher); value.variant_name().hash(&mut hasher); value.variant_type().hash(&mut hasher); diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 26a795afec043..48182ead5612c 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -8,7 +8,7 @@ use crate::{ VariantInfo, VariantType, }; -use crate::utility::{GenericTypeInfoCell, NonGenericTypeInfoCell}; +use crate::utility::{reflect_hasher, GenericTypeInfoCell, NonGenericTypeInfoCell}; use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value}; use bevy_utils::{Duration, Instant}; use bevy_utils::{HashMap, HashSet}; @@ -995,7 +995,7 @@ impl Reflect for Cow<'static, str> { } fn reflect_hash(&self) -> Option { - let mut hasher = crate::reflect_hasher(); + let mut hasher = reflect_hasher(); Hash::hash(&std::any::Any::type_id(self), &mut hasher); Hash::hash(self, &mut hasher); Some(hasher.finish()) @@ -1103,7 +1103,7 @@ impl Reflect for &'static Path { } fn reflect_hash(&self) -> Option { - let mut hasher = crate::reflect_hasher(); + let mut hasher = reflect_hasher(); Hash::hash(&std::any::Any::type_id(self), &mut hasher); Hash::hash(self, &mut hasher); Some(hasher.finish()) diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 75a5b3aff2a0b..90c57f7d5bbd6 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -6,18 +6,10 @@ use crate::{ use std::{ any::{self, Any, TypeId}, fmt::Debug, - hash::BuildHasher, }; use crate::utility::NonGenericTypeInfoCell; -use bevy_utils::FixedState; - -#[inline] -pub fn reflect_hasher() -> bevy_utils::AHasher { - FixedState.build_hasher() -} - /// An immutable enumeration of "kinds" of reflected type. /// /// Each variant contains a trait object with methods specific to a kind of diff --git a/crates/bevy_reflect/src/utility.rs b/crates/bevy_reflect/src/utility.rs index 8474bcb6e8595..4e07ea1583b1b 100644 --- a/crates/bevy_reflect/src/utility.rs +++ b/crates/bevy_reflect/src/utility.rs @@ -1,10 +1,13 @@ //! Helpers for working with Bevy reflection. use crate::TypeInfo; -use bevy_utils::HashMap; +use bevy_utils::{FixedState, HashMap}; use once_cell::race::OnceBox; use parking_lot::RwLock; -use std::any::{Any, TypeId}; +use std::{ + any::{Any, TypeId}, + hash::BuildHasher, +}; /// A container for [`TypeInfo`] over non-generic types, allowing instances to be stored statically. /// @@ -147,3 +150,8 @@ impl GenericTypeInfoCell { }) } } + +#[inline] +pub fn reflect_hasher() -> bevy_utils::AHasher { + FixedState.build_hasher() +} From d4475d1676d259ec445b47ce57083becf19ff149 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 14 Feb 2023 22:32:31 +0100 Subject: [PATCH 4/5] Add doc comment for bevy_reflect::utility::reflect_hasher --- crates/bevy_reflect/src/utility.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_reflect/src/utility.rs b/crates/bevy_reflect/src/utility.rs index 4e07ea1583b1b..e10e6b0483cfb 100644 --- a/crates/bevy_reflect/src/utility.rs +++ b/crates/bevy_reflect/src/utility.rs @@ -151,6 +151,11 @@ impl GenericTypeInfoCell { } } +/// Deterministic fixed state hasher to be used by implementors of [`Reflect::reflect_hash`]. +/// +/// Hashes should be deterministic across processes so hashes can be used as +/// checksums for saved scenes, rollback snapshots etc. This function returns +/// such a hasher. #[inline] pub fn reflect_hasher() -> bevy_utils::AHasher { FixedState.build_hasher() From 6dc4bc87f47aeae5537d25d74f36685fdc21fd52 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 15 Feb 2023 06:50:07 +0100 Subject: [PATCH 5/5] Fix doc link --- crates/bevy_reflect/src/utility.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_reflect/src/utility.rs b/crates/bevy_reflect/src/utility.rs index e10e6b0483cfb..3af535a1925ca 100644 --- a/crates/bevy_reflect/src/utility.rs +++ b/crates/bevy_reflect/src/utility.rs @@ -156,6 +156,8 @@ impl GenericTypeInfoCell { /// Hashes should be deterministic across processes so hashes can be used as /// checksums for saved scenes, rollback snapshots etc. This function returns /// such a hasher. +/// +/// [`Reflect::reflect_hash`]: crate::Reflect #[inline] pub fn reflect_hasher() -> bevy_utils::AHasher { FixedState.build_hasher()