Skip to content

Commit a4322a8

Browse files
authored
Rollup merge of #40739 - cuviper:hash-rev-drop, r=arthurprs
Simplify hash table drops 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.
2 parents 9d659c4 + a033f1a commit a4322a8

File tree

1 file changed

+18
-47
lines changed

1 file changed

+18
-47
lines changed

src/libstd/collections/hash/table.rs

+18-47
Original file line numberDiff line numberDiff line change
@@ -896,15 +896,23 @@ impl<K, V> RawTable<K, V> {
896896
}
897897
}
898898

899-
/// Returns an iterator that copies out each entry. Used while the table
900-
/// is being dropped.
901-
unsafe fn rev_move_buckets(&mut self) -> RevMoveBuckets<K, V> {
902-
let raw_bucket = self.first_bucket_raw();
903-
RevMoveBuckets {
904-
raw: raw_bucket.offset(self.capacity as isize),
905-
hashes_end: raw_bucket.hash,
906-
elems_left: self.size,
907-
marker: marker::PhantomData,
899+
/// Drops buckets in reverse order. It leaves the table in an inconsistent
900+
/// state and should only be used for dropping the table's remaining
901+
/// entries. It's used in the implementation of Drop.
902+
unsafe fn rev_drop_buckets(&mut self) {
903+
let first_raw = self.first_bucket_raw();
904+
let mut raw = first_raw.offset(self.capacity as isize);
905+
let mut elems_left = self.size;
906+
907+
while elems_left != 0 {
908+
debug_assert!(raw.hash != first_raw.hash);
909+
910+
raw = raw.offset(-1);
911+
912+
if *raw.hash != EMPTY_BUCKET {
913+
elems_left -= 1;
914+
ptr::drop_in_place(raw.pair as *mut (K, V));
915+
}
908916
}
909917
}
910918

@@ -964,43 +972,6 @@ impl<'a, K, V> Iterator for RawBuckets<'a, K, V> {
964972
}
965973
}
966974

967-
/// An iterator that moves out buckets in reverse order. It leaves the table
968-
/// in an inconsistent state and should only be used for dropping
969-
/// the table's remaining entries. It's used in the implementation of Drop.
970-
struct RevMoveBuckets<'a, K, V> {
971-
raw: RawBucket<K, V>,
972-
hashes_end: *mut HashUint,
973-
elems_left: usize,
974-
975-
// As above, `&'a (K,V)` would seem better, but we often use
976-
// 'static for the lifetime, and this is not a publicly exposed
977-
// type.
978-
marker: marker::PhantomData<&'a ()>,
979-
}
980-
981-
impl<'a, K, V> Iterator for RevMoveBuckets<'a, K, V> {
982-
type Item = (K, V);
983-
984-
fn next(&mut self) -> Option<(K, V)> {
985-
if self.elems_left == 0 {
986-
return None;
987-
}
988-
989-
loop {
990-
debug_assert!(self.raw.hash != self.hashes_end);
991-
992-
unsafe {
993-
self.raw = self.raw.offset(-1);
994-
995-
if *self.raw.hash != EMPTY_BUCKET {
996-
self.elems_left -= 1;
997-
return Some(ptr::read(self.raw.pair));
998-
}
999-
}
1000-
}
1001-
}
1002-
}
1003-
1004975
/// Iterator over shared references to entries in a table.
1005976
pub struct Iter<'a, K: 'a, V: 'a> {
1006977
iter: RawBuckets<'a, K, V>,
@@ -1227,7 +1198,7 @@ unsafe impl<#[may_dangle] K, #[may_dangle] V> Drop for RawTable<K, V> {
12271198
unsafe {
12281199
if needs_drop::<(K, V)>() {
12291200
// avoid linear runtime for types that don't need drop
1230-
for _ in self.rev_move_buckets() {}
1201+
self.rev_drop_buckets();
12311202
}
12321203
}
12331204

0 commit comments

Comments
 (0)