Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions crates/iddqd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ proptest = { workspace = true, optional = true }

[dev-dependencies]
expectorate.workspace = true
foldhash.workspace = true
iddqd-test-utils.workspace = true
proptest.workspace = true
serde.workspace = true
Expand Down
46 changes: 13 additions & 33 deletions crates/iddqd/src/bi_hash_map/imp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ use crate::{
internal::{ValidateCompact, ValidationError},
support::{
ItemIndex,
alloc::{AllocWrapper, Allocator, Global, global_alloc},
alloc::{Allocator, Global, global_alloc},
borrow::DormantMutRef,
fmt_utils::StrDisplayAsDebug,
hash_table,
item_set::ItemSet,
map_hash::MapHash,
},
Expand All @@ -24,7 +25,6 @@ use core::{
hash::{BuildHasher, Hash},
};
use equivalent::Equivalent;
use hashbrown::hash_table;

/// A 1:1 (bijective) map for two keys and a value.
///
Expand Down Expand Up @@ -759,14 +759,8 @@ impl<T: BiHashItem, S: Clone + BuildHasher, A: Allocator> BiHashMap<T, S, A> {
/// ```
pub fn reserve(&mut self, additional: usize) {
self.items.reserve(additional);
let items = &self.items;
let state = &self.tables.state;
self.tables
.k1_to_item
.reserve(additional, |ix| state.hash_one(items[*ix].key1()));
self.tables
.k2_to_item
.reserve(additional, |ix| state.hash_one(items[*ix].key2()));
self.tables.k1_to_item.reserve(additional);
self.tables.k2_to_item.reserve(additional);
}

/// Tries to reserve capacity for at least `additional` more elements to be
Expand Down Expand Up @@ -820,15 +814,13 @@ impl<T: BiHashItem, S: Clone + BuildHasher, A: Allocator> BiHashMap<T, S, A> {
additional: usize,
) -> Result<(), crate::errors::TryReserveError> {
self.items.try_reserve(additional)?;
let items = &self.items;
let state = &self.tables.state;
self.tables
.k1_to_item
.try_reserve(additional, |ix| state.hash_one(items[*ix].key1()))
.try_reserve(additional)
.map_err(crate::errors::TryReserveError::from_hashbrown)?;
self.tables
.k2_to_item
.try_reserve(additional, |ix| state.hash_one(items[*ix].key2()))
.try_reserve(additional)
.map_err(crate::errors::TryReserveError::from_hashbrown)?;
Ok(())
}
Expand Down Expand Up @@ -875,14 +867,8 @@ impl<T: BiHashItem, S: Clone + BuildHasher, A: Allocator> BiHashMap<T, S, A> {
self.tables.k1_to_item.remap_indexes(&remap);
self.tables.k2_to_item.remap_indexes(&remap);
}
let items = &self.items;
let state = &self.tables.state;
self.tables
.k1_to_item
.shrink_to_fit(|ix| state.hash_one(items[*ix].key1()));
self.tables
.k2_to_item
.shrink_to_fit(|ix| state.hash_one(items[*ix].key2()));
self.tables.k1_to_item.shrink_to_fit();
self.tables.k2_to_item.shrink_to_fit();
}

/// Shrinks the capacity of the map with a lower limit. It will drop
Expand Down Expand Up @@ -932,14 +918,8 @@ impl<T: BiHashItem, S: Clone + BuildHasher, A: Allocator> BiHashMap<T, S, A> {
self.tables.k1_to_item.remap_indexes(&remap);
self.tables.k2_to_item.remap_indexes(&remap);
}
let items = &self.items;
let state = &self.tables.state;
self.tables
.k1_to_item
.shrink_to(min_capacity, |ix| state.hash_one(items[*ix].key1()));
self.tables
.k2_to_item
.shrink_to(min_capacity, |ix| state.hash_one(items[*ix].key2()));
self.tables.k1_to_item.shrink_to(min_capacity);
self.tables.k2_to_item.shrink_to(min_capacity);
}

/// Returns an iterator over all items in the map.
Expand Down Expand Up @@ -2436,13 +2416,13 @@ impl<T: BiHashItem + Eq, S: Clone + BuildHasher, A: Allocator> Eq
}

fn detect_dup_or_insert<'a, A: Allocator>(
item: hash_table::Entry<'a, ItemIndex, AllocWrapper<A>>,
item: hash_table::Entry<'a, A>,
duplicates: &mut BTreeSet<ItemIndex>,
) -> Option<hash_table::VacantEntry<'a, ItemIndex, AllocWrapper<A>>> {
) -> Option<hash_table::VacantEntry<'a, A>> {
match item {
hash_table::Entry::Vacant(slot) => Some(slot),
hash_table::Entry::Occupied(slot) => {
duplicates.insert(*slot.get());
duplicates.insert(slot.get());
None
}
}
Expand Down
106 changes: 86 additions & 20 deletions crates/iddqd/src/id_hash_map/imp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::{
ItemIndex,
alloc::{Allocator, Global, global_alloc},
borrow::DormantMutRef,
hash_table,
item_set::ItemSet,
map_hash::MapHash,
},
Expand All @@ -20,7 +21,6 @@ use core::{
hash::{BuildHasher, Hash},
};
use equivalent::Equivalent;
use hashbrown::hash_table;

/// A hash map where the key is part of the value.
///
Expand Down Expand Up @@ -642,11 +642,7 @@ impl<T: IdHashItem, S: Clone + BuildHasher, A: Allocator> IdHashMap<T, S, A> {
/// ```
pub fn reserve(&mut self, additional: usize) {
self.items.reserve(additional);
let items = &self.items;
let state = &self.tables.state;
self.tables
.key_to_item
.reserve(additional, |ix| state.hash_one(items[*ix].key()));
self.tables.key_to_item.reserve(additional);
}

/// Tries to reserve capacity for at least `additional` more elements to be
Expand Down Expand Up @@ -696,11 +692,9 @@ impl<T: IdHashItem, S: Clone + BuildHasher, A: Allocator> IdHashMap<T, S, A> {
additional: usize,
) -> Result<(), crate::errors::TryReserveError> {
self.items.try_reserve(additional)?;
let items = &self.items;
let state = &self.tables.state;
self.tables
.key_to_item
.try_reserve(additional, |ix| state.hash_one(items[*ix].key()))
.try_reserve(additional)
.map_err(crate::errors::TryReserveError::from_hashbrown)?;
Ok(())
}
Expand Down Expand Up @@ -742,11 +736,7 @@ impl<T: IdHashItem, S: Clone + BuildHasher, A: Allocator> IdHashMap<T, S, A> {
if !remap.is_identity() {
self.tables.key_to_item.remap_indexes(&remap);
}
let items = &self.items;
let state = &self.tables.state;
self.tables
.key_to_item
.shrink_to_fit(|ix| state.hash_one(items[*ix].key()));
self.tables.key_to_item.shrink_to_fit();
}

/// Shrinks the capacity of the map with a lower limit. It will drop
Expand Down Expand Up @@ -791,11 +781,7 @@ impl<T: IdHashItem, S: Clone + BuildHasher, A: Allocator> IdHashMap<T, S, A> {
if !remap.is_identity() {
self.tables.key_to_item.remap_indexes(&remap);
}
let items = &self.items;
let state = &self.tables.state;
self.tables
.key_to_item
.shrink_to(min_capacity, |ix| state.hash_one(items[*ix].key()));
self.tables.key_to_item.shrink_to(min_capacity);
}

/// Iterates over the items in the map.
Expand Down Expand Up @@ -1403,7 +1389,7 @@ impl<T: IdHashItem, S: Clone + BuildHasher, A: Allocator> IdHashMap<T, S, A> {
.entry(state, key, |index| self.items[index].key())
{
hash_table::Entry::Occupied(slot) => {
duplicates.insert(*slot.get());
duplicates.insert(slot.get());
None
}
hash_table::Entry::Vacant(slot) => Some(slot),
Expand Down Expand Up @@ -1789,3 +1775,83 @@ impl<T: IdHashItem, S: Default + Clone + BuildHasher, A: Allocator + Default>
map
}
}

#[cfg(all(test, feature = "std"))]
mod tests {
use super::*;
use core::{cell::Cell, hash::Hasher};

std::thread_local! {
static USER_HASH_CALLS: Cell<u32> = const { Cell::new(0) };
}

#[derive(Debug)]
struct CountedKey(u32);

impl Hash for CountedKey {
fn hash<H: Hasher>(&self, state: &mut H) {
USER_HASH_CALLS.with(|c| c.set(c.get() + 1));
self.0.hash(state);
}
}

impl PartialEq for CountedKey {
fn eq(&self, other: &Self) -> bool {
self.0 == other.0
}
}
impl Eq for CountedKey {}

#[derive(Debug)]
struct CountedItem {
id: u32,
}

impl IdHashItem for CountedItem {
type Key<'a> = CountedKey;
fn key(&self) -> Self::Key<'_> {
CountedKey(self.id)
}
id_upcast!();
}

// This is a unit test and not an integration test to ensure rehashing
// actually happens. (Rehashing is not externally observable in integration
// tests.)
#[test]
fn reserve_rehash_uses_cached_hash() {
let mut map = IdHashMap::<CountedItem, _>::with_hasher(
foldhash::fast::FixedState::with_seed(0),
);
// Insert items (legitimately calls user Hash).
for id in [
0u32, 17, 1, 12, 5, 21, 8, 10, 18, 4, 16, 22, 9, 24, 23, 13, 7, 25,
26, 20, 31, 11, 14, 2, 6,
] {
let _ = map.insert_overwrite(CountedItem { id });
}
// Drop most entries, leaving the table heavy with tombstones so the
// next reserve favors `rehash_in_place` over a fresh-allocation grow.
map.retain(|item| item.id % 2 == 1 && item.id % 3 != 0);

USER_HASH_CALLS.with(|c| c.set(0));
let rehash_calls = map.tables.key_to_item.reserve_counting_rehash(10);
let user_calls = USER_HASH_CALLS.with(Cell::get);

assert!(
rehash_calls > 0,
"expected reserve to invoke the rehash callback at least once \
(setup did not actually trigger a rehash; if hashbrown's \
growth/rehash heuristic changed, retune the constants)",
);
assert_eq!(
user_calls, 0,
"reserve must not invoke user `Hash` during rehash; got \
{user_calls} call(s) for {rehash_calls} rehash-callback \
invocation(s)",
);

map.validate(ValidateCompact::NonCompact)
.expect("map remains valid after reserve");
}
}
Loading
Loading