Skip to content

feat: HashTable::try_insert_unique_within_capacity #621

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
46 changes: 38 additions & 8 deletions src/raw/mod.rs
Original file line number Diff line number Diff line change
@@ -1045,18 +1045,15 @@ impl<T, A: Allocator> RawTable<T, A> {
)
}

/// Inserts a new element into the table, and returns its raw bucket.
///
/// This does not check if the given element already exists in the table.
#[cfg_attr(feature = "inline-more", inline)]
pub fn insert(&mut self, hash: u64, value: T, hasher: impl Fn(&T) -> u64) -> Bucket<T> {
#[inline(always)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used #[inline(always)] unconditionally because this is a helper method which was previously written in line, so I didn't want to worry about abstraction overhead.

Copy link
Member

Choose a reason for hiding this comment

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

A simple #[inline] would be sufficient here. We don't want to over-inline in debug builds since this can hurt build times.

fn find_insert_slot(&self, hash: u64) -> Option<InsertSlot> {
unsafe {
// SAFETY:
// 1. The [`RawTableInner`] must already have properly initialized control bytes since
// we will never expose `RawTable::new_uninitialized` in a public API.
//
// 2. We reserve additional space (if necessary) right after calling this function.
let mut slot = self.table.find_insert_slot(hash);
let slot = self.table.find_insert_slot(hash);

// We can avoid growing the table once we have reached our load factor if we are replacing
// a tombstone. This works since the number of EMPTY slots does not change in this case.
@@ -1065,13 +1062,46 @@ impl<T, A: Allocator> RawTable<T, A> {
// in the range `0..=self.buckets()`.
let old_ctrl = *self.table.ctrl(slot.index);
if unlikely(self.table.growth_left == 0 && old_ctrl.special_is_empty()) {
None
} else {
Some(slot)
}
}
}

/// Inserts a new element into the table, and returns its raw bucket.
///
/// This does not check if the given element already exists in the table.
#[cfg_attr(feature = "inline-more", inline)]
pub fn insert(&mut self, hash: u64, value: T, hasher: impl Fn(&T) -> u64) -> Bucket<T> {
let slot = match self.find_insert_slot(hash) {
None => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose someone should check if the unlikely from the new helper find_insert_slot carries over here.

Copy link
Member

Choose a reason for hiding this comment

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

You can use core::hint::cold_path for this (with a wrapper so it works on stable).

self.reserve(1, hasher);
// SAFETY: We know for sure that `RawTableInner` has control bytes
// initialized and that there is extra space in the table.
slot = self.table.find_insert_slot(hash);
unsafe { self.table.find_insert_slot(hash) }
}
Some(slot) => slot,
};
// SAFETY: todo
unsafe { self.insert_in_slot(hash, slot, value) }
}

self.insert_in_slot(hash, slot, value)
/// Tries to insert a new element into the table if there is capacity.
/// Returns its raw bucket if successful, and otherwise returns `value`
/// to the caller on error.
///
/// This does not check if the given element already exists in the table.
#[inline]
pub(crate) fn try_insert_within_capacity(
&mut self,
hash: u64,
value: T,
) -> Result<Bucket<T>, T> {
match self.find_insert_slot(hash) {
// SAFETY: todo
Some(slot) => Ok(unsafe { self.insert_in_slot(hash, slot, value) }),
None => Err(value),
}
}

42 changes: 42 additions & 0 deletions src/table.rs
Original file line number Diff line number Diff line change
@@ -415,6 +415,48 @@ where
}
}

/// Inserts an element into the `HashTable` with the given hash value, but
/// without checking whether an equivalent element already exists within
/// the table. If there is insufficient capacity, then this returns an
/// error holding the provided value.
///
/// # Examples
///
/// ```
/// # #[cfg(feature = "nightly")]
/// # fn test() {
/// use hashbrown::{HashTable, DefaultHashBuilder};
/// use std::hash::BuildHasher;
///
/// let mut v = HashTable::new();
/// let hasher = DefaultHashBuilder::default();
/// let hasher = |val: &_| hasher.hash_one(val);
/// assert!(v.try_insert_unique_within_capacity(hasher(&1), 1).is_err());
/// assert!(v.is_empty());
/// v.reserve(1, hasher);
/// assert!(v.try_insert_unique_within_capacity(hasher(&1), 1).is_ok());
/// assert!(!v.is_empty());
/// # }
/// # fn main() {
/// # #[cfg(feature = "nightly")]
/// # test()
/// # }
/// ```
pub fn try_insert_unique_within_capacity(
&mut self,
hash: u64,
value: T,
) -> Result<OccupiedEntry<'_, T, A>, T> {
match self.raw.try_insert_within_capacity(hash, value) {
Ok(bucket) => Ok(OccupiedEntry {
hash,
bucket,
table: self,
}),
Err(err) => Err(err),
}
}

/// Clears the table, removing all values.
///
/// # Examples