Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pack bloom filter hashes better and save a word on Rule #17281

Merged
merged 6 commits into from Jun 12, 2017
Next

Shift by KEY_SIZE instead of something larger.

Currently all Gecko and Servo do the KEY_SHIFT thing, but there's no reason
we need to follow that here.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1371949#c10

MozReview-Commit-ID: CqNi7r9e5s0
  • Loading branch information
bholley committed Jun 12, 2017
commit cf982d17b9ef0966c77d1161ed1346dde543136e
@@ -7,10 +7,13 @@
use fnv::FnvHasher;
use std::hash::{Hash, Hasher};

// The top 12 bits of the 32-bit hash value are not used by the bloom filter.

This comment has been minimized.

@MattWis

MattWis Jun 23, 2017

Contributor

This comment seems incorrect to me. Based on a cursory reading of this PR, it seems that the hashes are 24 bits, leaving 8 bits unused in a 32 bit word.

This comment has been minimized.

@emilio

emilio Jun 23, 2017

Member

Yes, that's right.

// Consumers may rely on this to pack hashes more efficiently.
pub const BLOOM_HASH_MASK: u32 = 0x00ffffff;
const KEY_SIZE: usize = 12;

const ARRAY_SIZE: usize = 1 << KEY_SIZE;
const KEY_MASK: u32 = (1 << KEY_SIZE) - 1;
const KEY_SHIFT: usize = 16;

/// A counting Bloom filter with 8-bit counters. For now we assume
/// that having two hash functions is enough, but we may revisit that
@@ -183,7 +186,7 @@ fn hash1(hash: u32) -> u32 {

#[inline]
fn hash2(hash: u32) -> u32 {
(hash >> KEY_SHIFT) & KEY_MASK
(hash >> KEY_SIZE) & KEY_MASK
}

#[test]
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.