Skip to content

Commit

Permalink
Auto merge of #338 - AngelicosPhosphoros:master, r=Amanieu
Browse files Browse the repository at this point in the history
Add shortcircuit in iteration if we yielded all elements

Current implementation works little slower than `set.iter().take(set.len())`.
See my comment [here](rust-lang/rust#97215 (comment)).

So why not avoid extra integer which added by `Iterator::take` if we can add limiting logic into our iterator itself?

I don't really know how this change affects [reflect_toogle_full](https://github.com/rust-lang/hashbrown/blob/f3a9f211d06f78c5beb81ac22ea08fdc269e068f/src/raw/mod.rs#L2019) and implementation of [FusedIterator](https://github.com/rust-lang/hashbrown/blob/f3a9f211d06f78c5beb81ac22ea08fdc269e068f/src/raw/mod.rs#L2150). Maybe I should make inner iterator "jump" to the end of its memory block?
  • Loading branch information
bors committed Jun 3, 2022
2 parents 3dbcdcc + ea120c7 commit d11a701
Showing 1 changed file with 42 additions and 27 deletions.
69 changes: 42 additions & 27 deletions src/raw/mod.rs
Expand Up @@ -1907,6 +1907,32 @@ impl<T> RawIterRange<T> {
}
}
}

/// # Safety
/// If DO_CHECK_PTR_RANGE is false, caller must ensure that we never try to iterate
/// after yielding all elements.
#[cfg_attr(feature = "inline-more", inline)]
unsafe fn next_impl<const DO_CHECK_PTR_RANGE: bool>(&mut self) -> Option<Bucket<T>> {
loop {
if let Some(index) = self.current_group.lowest_set_bit() {
self.current_group = self.current_group.remove_lowest_bit();
return Some(self.data.next_n(index));
}

if DO_CHECK_PTR_RANGE && self.next_ctrl >= self.end {
return None;
}

// We might read past self.end up to the next group boundary,
// but this is fine because it only occurs on tables smaller
// than the group size where the trailing control bytes are all
// EMPTY. On larger tables self.end is guaranteed to be aligned
// to the group size (since tables are power-of-two sized).
self.current_group = Group::load_aligned(self.next_ctrl).match_full();
self.data = self.data.next_n(Group::WIDTH);
self.next_ctrl = self.next_ctrl.add(Group::WIDTH);
}
}
}

// We make raw iterators unconditionally Send and Sync, and let the PhantomData
Expand All @@ -1932,25 +1958,8 @@ impl<T> Iterator for RawIterRange<T> {
#[cfg_attr(feature = "inline-more", inline)]
fn next(&mut self) -> Option<Bucket<T>> {
unsafe {
loop {
if let Some(index) = self.current_group.lowest_set_bit() {
self.current_group = self.current_group.remove_lowest_bit();
return Some(self.data.next_n(index));
}

if self.next_ctrl >= self.end {
return None;
}

// We might read past self.end up to the next group boundary,
// but this is fine because it only occurs on tables smaller
// than the group size where the trailing control bytes are all
// EMPTY. On larger tables self.end is guaranteed to be aligned
// to the group size (since tables are power-of-two sized).
self.current_group = Group::load_aligned(self.next_ctrl).match_full();
self.data = self.data.next_n(Group::WIDTH);
self.next_ctrl = self.next_ctrl.add(Group::WIDTH);
}
// SAFETY: We set checker flag to true.
self.next_impl::<true>()
}
}

Expand Down Expand Up @@ -2128,16 +2137,22 @@ impl<T> Iterator for RawIter<T> {

#[cfg_attr(feature = "inline-more", inline)]
fn next(&mut self) -> Option<Bucket<T>> {
if let Some(b) = self.iter.next() {
// Inner iterator iterates over buckets
// so it can do unnecessary work if we already yielded all items.
if self.items == 0 {
return None;
}

let nxt = unsafe {
// SAFETY: We check number of items to yield using `items` field.
self.iter.next_impl::<false>()
};

if nxt.is_some() {
self.items -= 1;
Some(b)
} else {
// We don't check against items == 0 here to allow the
// compiler to optimize away the item count entirely if the
// iterator length is never queried.
debug_assert_eq!(self.items, 0);
None
}

nxt
}

#[inline]
Expand Down

0 comments on commit d11a701

Please sign in to comment.