Skip to content

Commit

Permalink
Auto merge of #80715 - JulianKnodt:skip_opt, r=nagisa
Browse files Browse the repository at this point in the history
Change branching in `iter.skip()`

Optimize branching in `Skip`, which was brought up in #80416.
This assumes that if `next` is called, it's likely that there will be more calls to `next`, and the branch for skip will only be hit once thus it's unlikely to take that path. Even w/o the `unlikely` intrinsic, it compiles more efficiently, I believe because the path where `next` is called is always taken.

It should be noted there are very few places in the compiler where `Skip` is used, so probably won't have a noticeable perf impact.

[New impl](https://godbolt.org/z/85rdj4)
[Old impl](https://godbolt.org/z/Wc74rh)

[Some additional asm examples](https://godbolt.org/z/feKzoz) although they really don't have a ton of difference between them.
  • Loading branch information
bors committed Jan 23, 2021
2 parents 1986b58 + e5094a2 commit 4153fa8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
24 changes: 23 additions & 1 deletion library/core/benches/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,29 @@ bench_sums! {
bench_sums! {
bench_cycle_take_sum,
bench_cycle_take_ref_sum,
(0i64..10000).cycle().take(1000000)
(0..10000).cycle().take(1000000)
}

bench_sums! {
bench_cycle_skip_take_sum,
bench_cycle_skip_take_ref_sum,
(0..100000).cycle().skip(1000000).take(1000000)
}

bench_sums! {
bench_cycle_take_skip_sum,
bench_cycle_take_skip_ref_sum,
(0..100000).cycle().take(1000000).skip(100000)
}

bench_sums! {
bench_skip_cycle_skip_zip_add_sum,
bench_skip_cycle_skip_zip_add_ref_sum,
(0..100000).skip(100).cycle().skip(100)
.zip((0..100000).cycle().skip(10))
.map(|(a,b)| a+b)
.skip(100000)
.take(1000000)
}

// Checks whether Skip<Zip<A,B>> is as fast as Zip<Skip<A>, Skip<B>>, from
Expand Down
10 changes: 4 additions & 6 deletions library/core/src/iter/adapters/skip.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::intrinsics::unlikely;
use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable};
use crate::ops::{ControlFlow, Try};

Expand Down Expand Up @@ -31,13 +32,10 @@ where

#[inline]
fn next(&mut self) -> Option<I::Item> {
if self.n == 0 {
self.iter.next()
} else {
let old_n = self.n;
self.n = 0;
self.iter.nth(old_n)
if unlikely(self.n > 0) {
self.iter.nth(crate::mem::take(&mut self.n) - 1);
}
self.iter.next()
}

#[inline]
Expand Down

0 comments on commit 4153fa8

Please sign in to comment.