about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-01-23 09:25:11 +0000
committerbors <bors@rust-lang.org>2021-01-23 09:25:11 +0000
commit4153fa82ff8403bde877b52d35bf1ef99e54a4a2 (patch)
tree3aa8af47a3752a6945cea30ac28876a1257f172a
parent1986b58c646a9523d0a8a0fa8a0bd20492e7795d (diff)
parente5094a28514f25654f9096601bd1df75657f16ba (diff)
downloadrust-4153fa82ff8403bde877b52d35bf1ef99e54a4a2.tar.gz
rust-4153fa82ff8403bde877b52d35bf1ef99e54a4a2.zip
Auto merge of #80715 - JulianKnodt:skip_opt, r=nagisa
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.
-rw-r--r--library/core/benches/iter.rs24
-rw-r--r--library/core/src/iter/adapters/skip.rs10
2 files changed, 27 insertions, 7 deletions
diff --git a/library/core/benches/iter.rs b/library/core/benches/iter.rs
index fb6b4b78379..f2169914ac9 100644
--- a/library/core/benches/iter.rs
+++ b/library/core/benches/iter.rs
@@ -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
diff --git a/library/core/src/iter/adapters/skip.rs b/library/core/src/iter/adapters/skip.rs
index dd5325660c3..e55c7a6bf5d 100644
--- a/library/core/src/iter/adapters/skip.rs
+++ b/library/core/src/iter/adapters/skip.rs
@@ -1,3 +1,4 @@
+use crate::intrinsics::unlikely;
 use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable};
 use crate::ops::{ControlFlow, Try};
 
@@ -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]