Skip to content

Commit

Permalink
Auto merge of #96350 - austinabell:skip_optimization, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
fix(iter::skip): Optimize `next` and `nth` implementations of `Skip`

This avoids calling nth/next or nth/nth to first skip elements and then get the next one (unless necessary due to usize overflow).
  • Loading branch information
bors committed Aug 15, 2022
2 parents 76c427d + 00bc9e8 commit 80ed61f
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 8 deletions.
27 changes: 19 additions & 8 deletions library/core/src/iter/adapters/skip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,32 @@ where
#[inline]
fn next(&mut self) -> Option<I::Item> {
if unlikely(self.n > 0) {
self.iter.nth(crate::mem::take(&mut self.n) - 1)?;
self.iter.nth(crate::mem::take(&mut self.n))
} else {
self.iter.next()
}
self.iter.next()
}

#[inline]
fn nth(&mut self, n: usize) -> Option<I::Item> {
// Can't just add n + self.n due to overflow.
if self.n > 0 {
let to_skip = self.n;
self.n = 0;
// nth(n) skips n+1
self.iter.nth(to_skip - 1)?;
let skip: usize = crate::mem::take(&mut self.n);
// Checked add to handle overflow case.
let n = match skip.checked_add(n) {
Some(nth) => nth,
None => {
// In case of overflow, load skip value, before loading `n`.
// Because the amount of elements to iterate is beyond `usize::MAX`, this
// is split into two `nth` calls where the `skip` `nth` call is discarded.
self.iter.nth(skip - 1)?;
n
}
};
// Load nth element including skip.
self.iter.nth(n)
} else {
self.iter.nth(n)
}
self.iter.nth(n)
}

#[inline]
Expand Down
31 changes: 31 additions & 0 deletions library/core/tests/iter/adapters/skip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,34 @@ fn test_skip_non_fused() {
// advance it further. `Unfuse` tests that this doesn't happen by panicking in that scenario.
let _ = non_fused.skip(20).next();
}

#[test]
fn test_skip_non_fused_nth_overflow() {
let non_fused = Unfuse::new(0..10);

// Ensures that calling skip and `nth` where the sum would overflow does not fail for non-fused
// iterators.
let _ = non_fused.skip(20).nth(usize::MAX);
}

#[test]
fn test_skip_overflow_wrapping() {
// Test to ensure even on overflowing on `skip+nth` the correct amount of elements are yielded.
struct WrappingIterator(usize);

impl Iterator for WrappingIterator {
type Item = usize;

fn next(&mut self) -> core::option::Option<Self::Item> {
<Self as Iterator>::nth(self, 0)
}

fn nth(&mut self, nth: usize) -> core::option::Option<Self::Item> {
self.0 = self.0.wrapping_add(nth.wrapping_add(1));
Some(self.0)
}
}

let wrap = WrappingIterator(0);
assert_eq!(wrap.skip(20).nth(usize::MAX), Some(20));
}

0 comments on commit 80ed61f

Please sign in to comment.