Skip to content

Commit

Permalink
Simplify hash table drops
Browse files Browse the repository at this point in the history
This replaces the `std::collections::hash::table::RevMoveBuckets`
iterator with a simpler `while` loop.  This iterator was only used for
dropping the remaining elements of a `RawTable`, so instead we can just
loop through directly and drop them in place.

This should be functionally equivalent to the former code, but a little
easier to read.  I was hoping it might have some performance benefit
too, but it seems the optimizer was already good enough to see through
the iterator -- the generated code is nearly the same.  Maybe it will
still help if an element type has more complicated drop code.
  • Loading branch information
cuviper committed Mar 22, 2017
1 parent cab4bff commit a033f1a
Showing 1 changed file with 18 additions and 47 deletions.
65 changes: 18 additions & 47 deletions src/libstd/collections/hash/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,15 +896,23 @@ impl<K, V> RawTable<K, V> {
}
}

/// Returns an iterator that copies out each entry. Used while the table
/// is being dropped.
unsafe fn rev_move_buckets(&mut self) -> RevMoveBuckets<K, V> {
let raw_bucket = self.first_bucket_raw();
RevMoveBuckets {
raw: raw_bucket.offset(self.capacity as isize),
hashes_end: raw_bucket.hash,
elems_left: self.size,
marker: marker::PhantomData,
/// Drops buckets in reverse order. It leaves the table in an inconsistent
/// state and should only be used for dropping the table's remaining
/// entries. It's used in the implementation of Drop.
unsafe fn rev_drop_buckets(&mut self) {
let first_raw = self.first_bucket_raw();
let mut raw = first_raw.offset(self.capacity as isize);
let mut elems_left = self.size;

while elems_left != 0 {
debug_assert!(raw.hash != first_raw.hash);

raw = raw.offset(-1);

if *raw.hash != EMPTY_BUCKET {
elems_left -= 1;
ptr::drop_in_place(raw.pair as *mut (K, V));
}
}
}

Expand Down Expand Up @@ -964,43 +972,6 @@ impl<'a, K, V> Iterator for RawBuckets<'a, K, V> {
}
}

/// An iterator that moves out buckets in reverse order. It leaves the table
/// in an inconsistent state and should only be used for dropping
/// the table's remaining entries. It's used in the implementation of Drop.
struct RevMoveBuckets<'a, K, V> {
raw: RawBucket<K, V>,
hashes_end: *mut HashUint,
elems_left: usize,

// As above, `&'a (K,V)` would seem better, but we often use
// 'static for the lifetime, and this is not a publicly exposed
// type.
marker: marker::PhantomData<&'a ()>,
}

impl<'a, K, V> Iterator for RevMoveBuckets<'a, K, V> {
type Item = (K, V);

fn next(&mut self) -> Option<(K, V)> {
if self.elems_left == 0 {
return None;
}

loop {
debug_assert!(self.raw.hash != self.hashes_end);

unsafe {
self.raw = self.raw.offset(-1);

if *self.raw.hash != EMPTY_BUCKET {
self.elems_left -= 1;
return Some(ptr::read(self.raw.pair));
}
}
}
}
}

/// Iterator over shared references to entries in a table.
pub struct Iter<'a, K: 'a, V: 'a> {
iter: RawBuckets<'a, K, V>,
Expand Down Expand Up @@ -1227,7 +1198,7 @@ unsafe impl<#[may_dangle] K, #[may_dangle] V> Drop for RawTable<K, V> {
unsafe {
if needs_drop::<(K, V)>() {
// avoid linear runtime for types that don't need drop
for _ in self.rev_move_buckets() {}
self.rev_drop_buckets();
}
}

Expand Down

0 comments on commit a033f1a

Please sign in to comment.