From 71e76a054d20221e9782cebb12091a81a48fcf4c Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 21 Jun 2017 16:56:59 -0700 Subject: [PATCH] Be smarter when clearing the bloom filter. --- components/selectors/bloom.rs | 12 ++++++++++++ components/style/bloom.rs | 21 ++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/components/selectors/bloom.rs b/components/selectors/bloom.rs index 083907d37e6..8caab9935cb 100644 --- a/components/selectors/bloom.rs +++ b/components/selectors/bloom.rs @@ -108,6 +108,18 @@ impl BloomFilter { self.counters = [0; ARRAY_SIZE] } + // Slow linear accessor to make sure the bloom filter is zeroed. This should + // never be used in release builds. + #[cfg(debug_assertions)] + pub fn is_zeroed(&self) -> bool { + self.counters.iter().all(|x| *x == 0) + } + + #[cfg(not(debug_assertions))] + pub fn is_zeroed(&self) -> bool { + unreachable!() + } + #[inline] pub fn insert_hash(&mut self, hash: u32) { { diff --git a/components/style/bloom.rs b/components/style/bloom.rs index 5033ff6ce83..34b6b608962 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -54,6 +54,16 @@ pub struct StyleBloom { pushed_hashes: SmallVec<[u32; 64]>, } +/// The very rough benchmarks in the selectors crate show clear() +/// costing about 25 times more than remove_hash(). We use this to implement +/// clear() more efficiently when only a small number of hashes have been +/// pushed. +/// +/// One subtly to note is that remove_hash() will not touch the value +/// if the filter overflowed. However, overflow can only occur if we +/// get 255 collisions on the same hash value, and 25 < 255. +const MEMSET_CLEAR_THRESHOLD: usize = 25; + struct PushedElement { /// The element that was pushed. element: SendElement, @@ -166,8 +176,17 @@ impl StyleBloom { /// Clears the bloom filter. pub fn clear(&mut self) { - self.filter.clear(); self.elements.clear(); + + if self.pushed_hashes.len() > MEMSET_CLEAR_THRESHOLD { + self.filter.clear(); + self.pushed_hashes.clear(); + } else { + for hash in self.pushed_hashes.drain() { + self.filter.remove_hash(hash); + } + debug_assert!(self.filter.is_zeroed()); + } } /// Rebuilds the bloom filter up to the parent of the given element.