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.

Sign up for GitHub

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

Avoid sorting in hash map stable hashing #91837

Merged
merged 4 commits into from
Dec 19, 2021
+43 −23
Merged

Avoid sorting in hash map stable hashing #91837

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5f5f6b
Avoid sorting in hash map stable hashing
Kobzol Dec 12, 2021
6e33d3e
Use modular arithmetic
Kobzol Dec 12, 2021
ac08f13
Remove sort from hashing hashset, treeset and treemap
Kobzol Dec 13, 2021
1f284b0
Add special case for length 1
Kobzol Dec 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 43 additions & 23 deletions compiler/rustc_data_structures/src/stable_hasher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ impl StableHasher {
}

impl StableHasherResult for u128 {
#[inline]
fn finish(hasher: StableHasher) -> Self {
let (_0, _1) = hasher.finalize();
u128::from(_0) | (u128::from(_1) << 64)
}
}

impl StableHasherResult for u64 {
#[inline]
fn finish(hasher: StableHasher) -> Self {
hasher.finalize().0
}
Expand Down Expand Up @@ -507,7 +509,11 @@ where
{
#[inline]
fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) {
hash_stable_hashmap(hcx, hasher, self, ToStableHashKey::to_stable_hash_key);
stable_hash_reduce(hcx, hasher, self.iter(), self.len(), |hasher, hcx, (key, value)| {
let key = key.to_stable_hash_key(hcx);
key.hash_stable(hcx, hasher);
value.hash_stable(hcx, hasher);
});
}
}

Expand All @@ -517,9 +523,10 @@ where
R: BuildHasher,
{
fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) {
let mut keys: Vec<_> = self.iter().map(|k| k.to_stable_hash_key(hcx)).collect();
keys.sort_unstable();
keys.hash_stable(hcx, hasher);
stable_hash_reduce(hcx, hasher, self.iter(), self.len(), |hasher, hcx, key| {
let key = key.to_stable_hash_key(hcx);
key.hash_stable(hcx, hasher);
});
}
}

Expand All @@ -529,10 +536,11 @@ where
V: HashStable<HCX>,
{
fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) {
let mut entries: Vec<_> =
self.iter().map(|(k, v)| (k.to_stable_hash_key(hcx), v)).collect();
entries.sort_unstable_by(|&(ref sk1, _), &(ref sk2, _)| sk1.cmp(sk2));
entries.hash_stable(hcx, hasher);
stable_hash_reduce(hcx, hasher, self.iter(), self.len(), |hasher, hcx, (key, value)| {
let key = key.to_stable_hash_key(hcx);
key.hash_stable(hcx, hasher);
value.hash_stable(hcx, hasher);
});
}
}

Expand All @@ -541,26 +549,38 @@ where
K: ToStableHashKey<HCX>,
{
fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) {
let mut keys: Vec<_> = self.iter().map(|k| k.to_stable_hash_key(hcx)).collect();
keys.sort_unstable();
keys.hash_stable(hcx, hasher);
stable_hash_reduce(hcx, hasher, self.iter(), self.len(), |hasher, hcx, key| {
let key = key.to_stable_hash_key(hcx);
key.hash_stable(hcx, hasher);
});
}
}

pub fn hash_stable_hashmap<HCX, K, V, R, SK, F>(
fn stable_hash_reduce<HCX, I, C, F>(
hcx: &mut HCX,
hasher: &mut StableHasher,
map: &::std::collections::HashMap<K, V, R>,
to_stable_hash_key: F,
mut collection: C,
length: usize,
hash_function: F,
) where
K: Eq,
V: HashStable<HCX>,
R: BuildHasher,
SK: HashStable<HCX> + Ord,
F: Fn(&K, &HCX) -> SK,
C: Iterator<Item = I>,
F: Fn(&mut StableHasher, &mut HCX, I),
{
let mut entries: SmallVec<[_; 3]> =
map.iter().map(|(k, v)| (to_stable_hash_key(k, hcx), v)).collect();
entries.sort_unstable_by(|&(ref sk1, _), &(ref sk2, _)| sk1.cmp(sk2));
entries.hash_stable(hcx, hasher);
length.hash_stable(hcx, hasher);

match length {
1 => {
hash_function(hasher, hcx, collection.next().unwrap());
}
_ => {
let hash = collection
.map(|value| {
let mut hasher = StableHasher::new();
hash_function(&mut hasher, hcx, value);
hasher.finish::<u128>()
})
.reduce(|accum, value| accum.wrapping_add(value));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduce produces an option. So we end up hashing an Option::None for zero-length collections. That seems like superfluous work considering we're already hashing length = 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, but it's probably negligible and at least we avoid another conditional check for length == 0. But it's probably worth a try to special case also length zero.

Copy link
Member

@the8472 the8472 Dec 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branch is not really avoided since the StableHash implementation for Option also contains a branch and an extra call to hash_stable.

We could either special-case length = 0 too or use fold(0, ...) instead of reduce.

But yeah, that can happen in a followup PR.

hash.hash_stable(hcx, hasher);
}
}
}