reworks

Author: itsibitzi

crates/geo_filters/docs/choosing-a-hash-function.md

@@ -22,6 +22,8 @@ to `BuildHasher::hash_one`.
 The following is an example of some incorrect code which produces nonsense results:
 
 ```rust
+use std::hash::RandomState;
+
 // Implement our marker trait for `RandomState`.
 // You should _NOT_ do this as `RandomState::default` does not produce
 // reproducible hashers.

crates/geo_filters/src/build_hasher.rs

@@ -22,16 +22,16 @@ pub trait ReproducibleBuildHasher: BuildHasher + Default + Clone {
         debug_assert_eq!(
             Self::default().build_hasher().finish(),
             Self::default().build_hasher().finish(),
-            "Hashers produced by GeoFilterBuildHasher do not produce the same output with the same input"
+            "Hashers produced by ReproducibleBuildHasher do not produce the same output with the same input"
         );
     }
 }
 
-// Note that this `BuildHasher` has a consistent implementation of `Default`
-// but is NOT stable across releases of Rust. It is therefore dangerous
-// to use if you plan on serializing the geofilters and reusing them due
-// to the fact that you can serialize a filter made with one version and
-// deserialize with another version of the hasher factor.
+/// Note that this `BuildHasher` has a consistent implementation of `Default`
+/// but is NOT stable across releases of Rust. It is therefore dangerous
+/// to use if you plan on serializing the geofilters and reusing them due
+/// to the fact that you can serialize a filter made with one version and
+/// deserialize with another version of the hasher factor.
 pub type DefaultBuildHasher = BuildHasherDefault<DefaultHasher>;
 
 impl ReproducibleBuildHasher for DefaultBuildHasher {}

crates/geo_filters/src/config.rs

@@ -355,12 +355,10 @@ pub(crate) fn take_ref<I: Iterator>(iter: &mut I, n: usize) -> impl Iterator<Ite
 pub(crate) mod tests {
     use rand::{RngCore, SeedableRng};
 
-    use crate::{build_hasher::DefaultBuildHasher, Count, Method};
+    use crate::{Count, Method};
 
     /// Runs estimation trials and returns the average precision and variance.
-    pub(crate) fn test_estimate<M: Method, C: Count<M, DefaultBuildHasher>>(
-        f: impl Fn() -> C,
-    ) -> (f32, f32) {
+    pub(crate) fn test_estimate<M: Method, C: Count<M>>(f: impl Fn() -> C) -> (f32, f32) {
         let mut rnd = rand::rngs::StdRng::from_os_rng();
         let cnt = 10000usize;
         let mut avg_precision = 0.0;

crates/geo_filters/src/diff_count.rs

@@ -305,7 +305,9 @@ pub(crate) fn masked<C: GeoConfig<Diff>>(
 /// This operation corresponds to computing the symmetric difference of the two
 /// sets represented by the GeoDiffCounts.
 ///
-/// SAFETY: The two GeoDiffCounts must have the same configuration and hash builder.
+/// # Panics
+///
+/// Panics if the configuration of the geofilters is not identical.
 pub(crate) fn xor<C: GeoConfig<Diff>>(
     diff_count: &GeoDiffCount<'_, C>,
     other: &GeoDiffCount<'_, C>,
@@ -321,7 +323,9 @@ pub(crate) fn xor<C: GeoConfig<Diff>>(
     )
 }
 
-impl<C: GeoConfig<Diff>> Count<Diff, C::BuildHasher> for GeoDiffCount<'_, C> {
+impl<C: GeoConfig<Diff>> Count<Diff> for GeoDiffCount<'_, C> {
+    type BuildHasher = C::BuildHasher;
+
     fn push_hash(&mut self, hash: u64) {
         self.xor_bit(self.config.hash_to_bucket(hash));
     }

crates/geo_filters/src/distinct_count.rs

@@ -128,7 +128,9 @@ impl<C: GeoConfig<Distinct>> GeoDistinctCount<'_, C> {
     }
 }
 
-impl<C: GeoConfig<Distinct>> Count<Distinct, C::BuildHasher> for GeoDistinctCount<'_, C> {
+impl<C: GeoConfig<Distinct>> Count<Distinct> for GeoDistinctCount<'_, C> {
+    type BuildHasher = C::BuildHasher;
+
     fn push_hash(&mut self, hash: u64) {
         self.set_bit(self.config.hash_to_bucket(hash));
     }

crates/geo_filters/src/evaluation/hll.rs

@@ -91,7 +91,9 @@ impl Hasher for NoopHasher {
 
     #[inline]
     fn write(&mut self, _: &[u8]) {
-        unimplemented!("NoopHasher only supports writing u64 values");
+        unimplemented!(
+            "NoopHasher does not support arbitrary byte sequences. Use write_u64 instead"
+        );
     }
 
     #[inline]
@@ -110,7 +112,9 @@ impl BuildHasher for BuildNoopHasher {
 
 impl ReproducibleBuildHasher for BuildNoopHasher {}
 
-impl<C: HllConfig> Count<Distinct, DefaultBuildHasher> for Hll<C> {
+impl<C: HllConfig> Count<Distinct> for Hll<C> {
+    type BuildHasher = DefaultBuildHasher;
+
     fn push_hash(&mut self, hash: u64) {
         self.inner.borrow_mut().insert(&hash)
     }

crates/geo_filters/src/evaluation/simulation.rs

@@ -22,35 +22,35 @@ pub trait SimulationCount {
 }
 impl<C: GeoConfig<Diff> + Clone> SimulationCount for GeoDiffCount<'_, C> {
     fn push_hash(&mut self, hash: u64) {
-        <Self as Count<_, _>>::push_hash(self, hash)
+        <Self as Count<_>>::push_hash(self, hash)
     }
     fn size(&self) -> f32 {
-        <Self as Count<_, _>>::size(self)
+        <Self as Count<_>>::size(self)
     }
     fn bytes_in_memory(&self) -> usize {
-        <Self as Count<_, _>>::bytes_in_memory(self)
+        <Self as Count<_>>::bytes_in_memory(self)
     }
 }
 impl<C: GeoConfig<Distinct>> SimulationCount for GeoDistinctCount<'_, C> {
     fn push_hash(&mut self, hash: u64) {
-        <Self as Count<_, _>>::push_hash(self, hash)
+        <Self as Count<_>>::push_hash(self, hash)
     }
     fn size(&self) -> f32 {
-        <Self as Count<_, _>>::size(self)
+        <Self as Count<_>>::size(self)
     }
     fn bytes_in_memory(&self) -> usize {
-        <Self as Count<_, _>>::bytes_in_memory(self)
+        <Self as Count<_>>::bytes_in_memory(self)
     }
 }
 impl<C: HllConfig> SimulationCount for Hll<C> {
     fn push_hash(&mut self, hash: u64) {
-        <Self as Count<_, _>>::push_hash(self, hash)
+        <Self as Count<_>>::push_hash(self, hash)
     }
     fn size(&self) -> f32 {
-        <Self as Count<_, _>>::size(self)
+        <Self as Count<_>>::size(self)
     }
     fn bytes_in_memory(&self) -> usize {
-        <Self as Count<_, _>>::bytes_in_memory(self)
+        <Self as Count<_>>::bytes_in_memory(self)
     }
 }
 

crates/geo_filters/src/lib.rs

@@ -14,7 +14,7 @@ pub mod distinct_count;
 #[cfg(feature = "evaluation")]
 pub mod evaluation;
 
-use std::hash::Hash;
+use std::hash::{BuildHasher as _, Hash};
 
 use crate::build_hasher::ReproducibleBuildHasher;
 
@@ -32,13 +32,15 @@ pub struct Distinct {}
 impl Method for Distinct {}
 
 /// Trait for types solving the set cardinality estimation problem.
-pub trait Count<M: Method, H: ReproducibleBuildHasher> {
+pub trait Count<M: Method> {
+    type BuildHasher: ReproducibleBuildHasher;
+
     /// Add the given hash to the set.
     fn push_hash(&mut self, hash: u64);
 
     /// Add the hash of the given item, computed with the configured hasher, to the set.
     fn push<I: Hash>(&mut self, item: I) {
-        let build_hasher = H::default();
+        let build_hasher = Self::BuildHasher::default();
         self.push_hash(build_hasher.hash_one(item));
     }