From bc7adb97edb8505f011d0718fc567d3b662a0718 Mon Sep 17 00:00:00 2001
From: behzad nouri <behzadnouri@gmail.com>
Date: Thu, 3 Sep 2020 20:32:23 +0000
Subject: [PATCH] builds crds filters without looping over filters (#11998)

---
 core/src/crds_gossip_pull.rs | 86 +++++++++++++++++++++++++++++++++---
 1 file changed, 81 insertions(+), 5 deletions(-)

diff --git a/core/src/crds_gossip_pull.rs b/core/src/crds_gossip_pull.rs
index 334f6c5fd1..82ba4a973a 100644
--- a/core/src/crds_gossip_pull.rs
+++ b/core/src/crds_gossip_pull.rs
@@ -57,8 +57,32 @@ impl CrdsFilter {
             mask_bits,
         }
     }
+
+    // CrdsFilter::new_complete_set returns a vector of filters. Given a hash
+    // value, this function returns the filter within that vector which
+    // corresponds to the item.
+    // TODO: Consider making Vec<CrdsFilter> a separate type, and implementing
+    // this (and new_complete_set) function as methods on that type.
+    pub fn get_crds_filter<'a>(
+        filters: &'a mut Vec<CrdsFilter>,
+        item: &Hash,
+    ) -> Option<&'a mut CrdsFilter> {
+        if let Some(filter) = filters.first() {
+            let shift = 64 - filter.mask_bits.min(64);
+            let index = CrdsFilter::hash_as_u64(item)
+                .checked_shr(shift)
+                .unwrap_or(0u64);
+            filters.get_mut(index as usize)
+        } else {
+            None
+        }
+    }
+
     // generates a vec of filters that together hold a complete set of Hashes
     pub fn new_complete_set(num_items: usize, max_bytes: usize) -> Vec<Self> {
+        // Note: get_crds_filter above relies on the order of filters and
+        // bit-mask pattern here. If changed, update get_crds_filter
+        // accordingly.
         let max_bits = (max_bytes * 8) as f64;
         let max_items = Self::max_items(max_bits, FALSE_RATE, KEYS);
         let mask_bits = Self::mask_bits(num_items as f64, max_items as f64);
@@ -362,13 +386,17 @@ impl CrdsGossipPull {
             crds.table.values().count() + self.purged_values.len(),
         );
         let mut filters = CrdsFilter::new_complete_set(num, bloom_size);
+        let mut add_value_hash = |value_hash| {
+            if let Some(filter) = CrdsFilter::get_crds_filter(&mut filters, value_hash) {
+                debug_assert!(filter.test_mask(value_hash));
+                filter.filter.add(value_hash);
+            }
+        };
         for v in crds.table.values() {
-            filters
-                .iter_mut()
-                .for_each(|filter| filter.add(&v.value_hash));
+            add_value_hash(&v.value_hash);
         }
         for (value_hash, _insert_timestamp) in &self.purged_values {
-            filters.iter_mut().for_each(|filter| filter.add(value_hash));
+            add_value_hash(&value_hash);
         }
 
         filters
@@ -519,8 +547,9 @@ mod test {
     use crate::contact_info::ContactInfo;
     use crate::crds_value::{CrdsData, Vote};
     use itertools::Itertools;
+    use rand::{thread_rng, RngCore};
     use solana_perf::test_tx::test_tx;
-    use solana_sdk::hash::hash;
+    use solana_sdk::hash::{hash, HASH_BYTES};
     use solana_sdk::packet::PACKET_DATA_SIZE;
 
     #[test]
@@ -613,6 +642,53 @@ mod test {
         assert!(options.contains(&node_456.pubkey()));
     }
 
+    #[test]
+    fn test_crds_filter_get_crds_filter() {
+        let mut filters =
+            CrdsFilter::new_complete_set(/*num_items=*/ 9672788, /*max_bytes=*/ 8196);
+        assert_eq!(filters.len(), 1024);
+        let mut bytes = [0u8; HASH_BYTES];
+        let mut rng = thread_rng();
+        for _ in 0..100 {
+            rng.fill_bytes(&mut bytes);
+            let hash_value = Hash::new(&bytes);
+            let filter = CrdsFilter::get_crds_filter(&mut filters, &hash_value)
+                .unwrap()
+                .clone();
+            assert!(filter.test_mask(&hash_value));
+            // Validate that the returned filter is the *unique* filter which
+            // corresponds to the hash value (i.e. test_mask returns true).
+            let mut num_hits = 0;
+            for f in &filters {
+                if *f == filter {
+                    num_hits += 1;
+                    assert!(f.test_mask(&hash_value));
+                } else {
+                    assert!(!f.test_mask(&hash_value));
+                }
+            }
+            assert_eq!(num_hits, 1);
+        }
+    }
+
+    #[test]
+    fn test_crds_filter_new_complete_set() {
+        // Validates invariances required by CrdsFilter::get_crds_filter in the
+        // vector of filters generated by CrdsFilter::new_complete_set.
+        let filters =
+            CrdsFilter::new_complete_set(/*num_items=*/ 55345017, /*max_bytes=*/ 4098);
+        assert_eq!(filters.len(), 16384);
+        let mask_bits = filters[0].mask_bits;
+        let right_shift = 64 - mask_bits;
+        let ones = !0u64 >> mask_bits;
+        for (i, filter) in filters.iter().enumerate() {
+            // Check that all mask_bits are equal.
+            assert_eq!(mask_bits, filter.mask_bits);
+            assert_eq!(i as u64, filter.mask >> right_shift);
+            assert_eq!(ones, ones & filter.mask);
+        }
+    }
+
     #[test]
     fn test_new_pull_request() {
         let mut crds = Crds::default();