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

Optimize Clone implementation #146

Merged
merged 3 commits into from
Mar 16, 2020
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
52 changes: 52 additions & 0 deletions benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,55 @@ bench_suite!(
iter_ahash_random,
iter_std_random
);

#[bench]
fn clone_small(b: &mut Bencher) {
let mut m = HashMap::new();
for i in 0..10 {
m.insert(i, i);
}

b.iter(|| {
black_box(m.clone());
})
}

#[bench]
fn clone_from_small(b: &mut Bencher) {
let mut m = HashMap::new();
let mut m2 = HashMap::new();
for i in 0..10 {
m.insert(i, i);
}

b.iter(|| {
m2.clone_from(&m);
black_box(&mut m2);
})
}

#[bench]
fn clone_large(b: &mut Bencher) {
let mut m = HashMap::new();
for i in 0..1000 {
m.insert(i, i);
}

b.iter(|| {
black_box(m.clone());
})
}

#[bench]
fn clone_from_large(b: &mut Bencher) {
let mut m = HashMap::new();
let mut m2 = HashMap::new();
for i in 0..1000 {
m.insert(i, i);
}

b.iter(|| {
m2.clone_from(&m);
black_box(&mut m2);
})
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
test,
core_intrinsics,
dropck_eyepatch,
specialization,
)
)]
#![allow(
Expand Down
61 changes: 60 additions & 1 deletion src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,56 @@ pub enum DefaultHashBuilder {}
/// .iter().cloned().collect();
/// // use the values stored in map
/// ```
#[derive(Clone)]
pub struct HashMap<K, V, S = DefaultHashBuilder> {
pub(crate) hash_builder: S,
pub(crate) table: RawTable<(K, V)>,
}

impl<K: Clone, V: Clone, S: Clone> Clone for HashMap<K, V, S> {
fn clone(&self) -> Self {
HashMap {
hash_builder: self.hash_builder.clone(),
table: self.table.clone(),
}
}

fn clone_from(&mut self, source: &Self) {
// We clone the hash_builder first since this might panic and we don't
// want the table to have elements hashed with the wrong hash_builder.
let hash_builder = source.hash_builder.clone();

#[cfg(not(feature = "nightly"))]
{
self.table.clone_from(&source.table);
}
#[cfg(feature = "nightly")]
{
trait HashClone<S> {
fn clone_from(&mut self, source: &Self, hash_builder: &S);
}
impl<K: Clone, V: Clone, S> HashClone<S> for HashMap<K, V, S> {
default fn clone_from(&mut self, source: &Self, _hash_builder: &S) {
self.table.clone_from(&source.table);
}
}
impl<K: Clone, V: Clone, S> HashClone<S> for HashMap<K, V, S>
where
K: Eq + Hash,
S: BuildHasher,
{
fn clone_from(&mut self, source: &Self, hash_builder: &S) {
self.table
.clone_from_with_hasher(&source.table, |x| make_hash(hash_builder, &x.0));
}
}
HashClone::clone_from(self, source, &hash_builder);
}

// Update hash_builder only if we successfully cloned all elements.
self.hash_builder = hash_builder;
}
}

#[cfg_attr(feature = "inline-more", inline)]
pub(crate) fn make_hash<K: Hash + ?Sized>(hash_builder: &impl BuildHasher, val: &K) -> u64 {
let mut state = hash_builder.build_hasher();
Expand Down Expand Up @@ -2820,6 +2864,21 @@ mod test_map {
assert_eq!(m2.len(), 2);
}

#[test]
fn test_clone_from() {
let mut m = HashMap::new();
let mut m2 = HashMap::new();
assert_eq!(m.len(), 0);
assert!(m.insert(1, 2).is_none());
assert_eq!(m.len(), 1);
assert!(m.insert(2, 4).is_none());
assert_eq!(m.len(), 2);
m2.clone_from(&m);
assert_eq!(*m2.get(&1).unwrap(), 2);
assert_eq!(*m2.get(&2).unwrap(), 4);
assert_eq!(m2.len(), 2);
}

thread_local! { static DROP_VECTOR: RefCell<Vec<i32>> = RefCell::new(Vec::new()) }

#[derive(Hash, PartialEq, Eq)]
Expand Down
186 changes: 156 additions & 30 deletions src/raw/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -987,43 +987,169 @@ impl<T: Clone> Clone for RawTable<T> {
.unwrap_or_else(|_| hint::unreachable_unchecked()),
);

// Copy the control bytes unchanged. We do this in a single pass
self.ctrl(0)
.copy_to_nonoverlapping(new_table.ctrl(0), self.num_ctrl_bytes());

{
// The cloning of elements may panic, in which case we need
// to make sure we drop only the elements that have been
// cloned so far.
let mut guard = guard((0, &mut new_table), |(index, new_table)| {
if mem::needs_drop::<T>() {
for i in 0..=*index {
if is_full(*new_table.ctrl(i)) {
new_table.bucket(i).drop();
}
}
}
new_table.free_buckets();
});
new_table.clone_from_spec(self, |new_table| {
// We need to free the memory allocated for the new table.
new_table.free_buckets();
});

for from in self.iter() {
let index = self.bucket_index(&from);
let to = guard.1.bucket(index);
to.write(from.as_ref().clone());
// Return the newly created table.
ManuallyDrop::into_inner(new_table)
}
}
}

// Update the index in case we need to unwind.
guard.0 = index;
fn clone_from(&mut self, source: &Self) {
if source.is_empty_singleton() {
*self = Self::new();
} else {
unsafe {
// First, drop all our elements without clearing the control bytes.
if mem::needs_drop::<T>() {
for item in self.iter() {
item.drop();
}
}

// Successfully cloned all items, no need to clean up.
mem::forget(guard);
// If necessary, resize our table to match the source.
if self.buckets() != source.buckets() {
// Skip our drop by using ptr::write.
if !self.is_empty_singleton() {
self.free_buckets();
}
(self as *mut Self).write(
Self::new_uninitialized(source.buckets(), Fallibility::Infallible)
.unwrap_or_else(|_| hint::unreachable_unchecked()),
);
}

// Return the newly created table.
new_table.items = self.items;
new_table.growth_left = self.growth_left;
ManuallyDrop::into_inner(new_table)
self.clone_from_spec(source, |self_| {
// We need to leave the table in an empty state.
self_.clear_no_drop()
});
}
}
}
}

/// Specialization of `clone_from` for `Copy` types
trait RawTableClone {
unsafe fn clone_from_spec(&mut self, source: &Self, on_panic: impl FnMut(&mut Self));
}
impl<T: Clone> RawTableClone for RawTable<T> {
#[cfg(feature = "nightly")]
#[cfg_attr(feature = "inline-more", inline)]
default unsafe fn clone_from_spec(&mut self, source: &Self, on_panic: impl FnMut(&mut Self)) {
self.clone_from_impl(source, on_panic);
}

#[cfg(not(feature = "nightly"))]
#[cfg_attr(feature = "inline-more", inline)]
unsafe fn clone_from_spec(&mut self, source: &Self, on_panic: impl FnMut(&mut Self)) {
self.clone_from_impl(source, on_panic);
}
}
#[cfg(feature = "nightly")]
impl<T: Copy> RawTableClone for RawTable<T> {
#[cfg_attr(feature = "inline-more", inline)]
unsafe fn clone_from_spec(&mut self, source: &Self, _on_panic: impl FnMut(&mut Self)) {
source
.ctrl(0)
.copy_to_nonoverlapping(self.ctrl(0), self.num_ctrl_bytes());
source
.data
.as_ptr()
.copy_to_nonoverlapping(self.data.as_ptr(), self.buckets());

self.items = source.items;
self.growth_left = source.growth_left;
}
}

impl<T: Clone> RawTable<T> {
/// Common code for clone and clone_from. Assumes `self.buckets() == source.buckets()`.
#[cfg_attr(feature = "inline-more", inline)]
unsafe fn clone_from_impl(&mut self, source: &Self, mut on_panic: impl FnMut(&mut Self)) {
// Copy the control bytes unchanged. We do this in a single pass
source
.ctrl(0)
.copy_to_nonoverlapping(self.ctrl(0), self.num_ctrl_bytes());

// The cloning of elements may panic, in which case we need
// to make sure we drop only the elements that have been
// cloned so far.
let mut guard = guard((0, &mut *self), |(index, self_)| {
if mem::needs_drop::<T>() {
for i in 0..=*index {
if is_full(*self_.ctrl(i)) {
self_.bucket(i).drop();
}
}
}

// Depending on whether we were called from clone or clone_from, we
// either need to free the memory for the destination table or just
// clear the control bytes.
on_panic(self_);
});

for from in source.iter() {
let index = source.bucket_index(&from);
let to = guard.1.bucket(index);
to.write(from.as_ref().clone());

// Update the index in case we need to unwind.
guard.0 = index;
}

// Successfully cloned all items, no need to clean up.
mem::forget(guard);

self.items = source.items;
self.growth_left = source.growth_left;
}

/// Variant of `clone_from` to use when a hasher is available.
#[cfg(any(feature = "nightly", feature = "raw"))]
pub fn clone_from_with_hasher(&mut self, source: &Self, hasher: impl Fn(&T) -> u64) {
// If we have enough capacity in the table, just clear it and insert
// elements one by one. We don't do this if we have the same number of
// buckets as the source since we can just copy the contents directly
// in that case.
if self.buckets() != source.buckets()
&& bucket_mask_to_capacity(self.bucket_mask) >= source.len()
{
self.clear();

let guard_self = guard(&mut *self, |self_| {
// Clear the partially copied table if a panic occurs, otherwise
// items and growth_left will be out of sync with the contents
// of the table.
self_.clear();
});

unsafe {
for item in source.iter() {
// This may panic.
let item = item.as_ref().clone();
let hash = hasher(&item);

// We can use a simpler version of insert() here since:
// - there are no DELETED entries.
// - we know there is enough space in the table.
// - all elements are unique.
let index = guard_self.find_insert_slot(hash);
guard_self.set_ctrl(index, h2(hash));
guard_self.bucket(index).write(item);
}
}

// Successfully cloned all items, no need to clean up.
mem::forget(guard_self);

self.items = source.items;
self.growth_left -= source.items;
} else {
self.clone_from(source);
}
}
}
Expand Down
13 changes: 12 additions & 1 deletion src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,22 @@ use super::map::{self, DefaultHashBuilder, HashMap, Keys};
/// [`HashMap`]: struct.HashMap.html
/// [`PartialEq`]: https://doc.rust-lang.org/std/cmp/trait.PartialEq.html
/// [`RefCell`]: https://doc.rust-lang.org/std/cell/struct.RefCell.html
#[derive(Clone)]
pub struct HashSet<T, S = DefaultHashBuilder> {
pub(crate) map: HashMap<T, (), S>,
}

impl<T: Clone, S: Clone> Clone for HashSet<T, S> {
fn clone(&self) -> Self {
HashSet {
map: self.map.clone(),
}
}

fn clone_from(&mut self, source: &Self) {
self.map.clone_from(&source.map);
}
}

#[cfg(feature = "ahash")]
impl<T: Hash + Eq> HashSet<T, DefaultHashBuilder> {
/// Creates an empty `HashSet`.
Expand Down