Skip to content

Commit 8b6a431

Browse files
committed
Auto merge of #111379 - nyurik:intersperse-speed-up, r=cuviper
Boost iterator intersperse(_with) performance I did some benchmark digging into the `intersperse` and `intersperse_with` code as part of [this discussion](https://internals.rust-lang.org/t/add-iterate-with-separators-iterator-function/18781/13), and as a result I optimized them a bit, without relying on the peekable iterator. See also [full benchmark repo](https://github.com/nyurik/intersperse_perf) Benchmarks show near 2x performance improvements with the simple `sum` [benchmarks](https://gist.github.com/nyurik/68b6c9b3d90f0d14746d4186bf8fa1e2): ![image](https://user-images.githubusercontent.com/1641515/237005195-16aebef4-9eed-4514-8b7c-da1d1f5bd9e0.png)
2 parents 04521fd + 77f31ef commit 8b6a431

File tree

1 file changed

+98
-49
lines changed

1 file changed

+98
-49
lines changed

library/core/src/iter/adapters/intersperse.rs

+98-49
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
use super::Peekable;
1+
use crate::fmt;
2+
use crate::iter::{Fuse, FusedIterator};
23

34
/// An iterator adapter that places a separator between all elements.
45
///
@@ -10,17 +11,26 @@ pub struct Intersperse<I: Iterator>
1011
where
1112
I::Item: Clone,
1213
{
14+
started: bool,
1315
separator: I::Item,
14-
iter: Peekable<I>,
15-
needs_sep: bool,
16+
next_item: Option<I::Item>,
17+
iter: Fuse<I>,
18+
}
19+
20+
#[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")]
21+
impl<I> FusedIterator for Intersperse<I>
22+
where
23+
I: FusedIterator,
24+
I::Item: Clone,
25+
{
1626
}
1727

1828
impl<I: Iterator> Intersperse<I>
1929
where
2030
I::Item: Clone,
2131
{
2232
pub(in crate::iter) fn new(iter: I, separator: I::Item) -> Self {
23-
Self { iter: iter.peekable(), separator, needs_sep: false }
33+
Self { started: false, separator, next_item: None, iter: iter.fuse() }
2434
}
2535
}
2636

@@ -33,27 +43,43 @@ where
3343
type Item = I::Item;
3444

3545
#[inline]
36-
fn next(&mut self) -> Option<I::Item> {
37-
if self.needs_sep && self.iter.peek().is_some() {
38-
self.needs_sep = false;
39-
Some(self.separator.clone())
46+
fn next(&mut self) -> Option<Self::Item> {
47+
if self.started {
48+
if let Some(v) = self.next_item.take() {
49+
Some(v)
50+
} else {
51+
let next_item = self.iter.next();
52+
if next_item.is_some() {
53+
self.next_item = next_item;
54+
Some(self.separator.clone())
55+
} else {
56+
None
57+
}
58+
}
4059
} else {
41-
self.needs_sep = true;
60+
self.started = true;
4261
self.iter.next()
4362
}
4463
}
4564

65+
fn size_hint(&self) -> (usize, Option<usize>) {
66+
intersperse_size_hint(&self.iter, self.started, self.next_item.is_some())
67+
}
68+
4669
fn fold<B, F>(self, init: B, f: F) -> B
4770
where
4871
Self: Sized,
4972
F: FnMut(B, Self::Item) -> B,
5073
{
5174
let separator = self.separator;
52-
intersperse_fold(self.iter, init, f, move || separator.clone(), self.needs_sep)
53-
}
54-
55-
fn size_hint(&self) -> (usize, Option<usize>) {
56-
intersperse_size_hint(&self.iter, self.needs_sep)
75+
intersperse_fold(
76+
self.iter,
77+
init,
78+
f,
79+
move || separator.clone(),
80+
self.started,
81+
self.next_item,
82+
)
5783
}
5884
}
5985

@@ -66,39 +92,50 @@ pub struct IntersperseWith<I, G>
6692
where
6793
I: Iterator,
6894
{
95+
started: bool,
6996
separator: G,
70-
iter: Peekable<I>,
71-
needs_sep: bool,
97+
next_item: Option<I::Item>,
98+
iter: Fuse<I>,
99+
}
100+
101+
#[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")]
102+
impl<I, G> FusedIterator for IntersperseWith<I, G>
103+
where
104+
I: FusedIterator,
105+
G: FnMut() -> I::Item,
106+
{
72107
}
73108

74109
#[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")]
75-
impl<I, G> crate::fmt::Debug for IntersperseWith<I, G>
110+
impl<I, G> fmt::Debug for IntersperseWith<I, G>
76111
where
77-
I: Iterator + crate::fmt::Debug,
78-
I::Item: crate::fmt::Debug,
79-
G: crate::fmt::Debug,
112+
I: Iterator + fmt::Debug,
113+
I::Item: fmt::Debug,
114+
G: fmt::Debug,
80115
{
81-
fn fmt(&self, f: &mut crate::fmt::Formatter<'_>) -> crate::fmt::Result {
116+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
82117
f.debug_struct("IntersperseWith")
118+
.field("started", &self.started)
83119
.field("separator", &self.separator)
84120
.field("iter", &self.iter)
85-
.field("needs_sep", &self.needs_sep)
121+
.field("next_item", &self.next_item)
86122
.finish()
87123
}
88124
}
89125

90126
#[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")]
91-
impl<I, G> crate::clone::Clone for IntersperseWith<I, G>
127+
impl<I, G> Clone for IntersperseWith<I, G>
92128
where
93-
I: Iterator + crate::clone::Clone,
94-
I::Item: crate::clone::Clone,
129+
I: Iterator + Clone,
130+
I::Item: Clone,
95131
G: Clone,
96132
{
97133
fn clone(&self) -> Self {
98-
IntersperseWith {
134+
Self {
135+
started: self.started,
99136
separator: self.separator.clone(),
100137
iter: self.iter.clone(),
101-
needs_sep: self.needs_sep.clone(),
138+
next_item: self.next_item.clone(),
102139
}
103140
}
104141
}
@@ -109,7 +146,7 @@ where
109146
G: FnMut() -> I::Item,
110147
{
111148
pub(in crate::iter) fn new(iter: I, separator: G) -> Self {
112-
Self { iter: iter.peekable(), separator, needs_sep: false }
149+
Self { started: false, separator, next_item: None, iter: iter.fuse() }
113150
}
114151
}
115152

@@ -122,38 +159,52 @@ where
122159
type Item = I::Item;
123160

124161
#[inline]
125-
fn next(&mut self) -> Option<I::Item> {
126-
if self.needs_sep && self.iter.peek().is_some() {
127-
self.needs_sep = false;
128-
Some((self.separator)())
162+
fn next(&mut self) -> Option<Self::Item> {
163+
if self.started {
164+
if let Some(v) = self.next_item.take() {
165+
Some(v)
166+
} else {
167+
let next_item = self.iter.next();
168+
if next_item.is_some() {
169+
self.next_item = next_item;
170+
Some((self.separator)())
171+
} else {
172+
None
173+
}
174+
}
129175
} else {
130-
self.needs_sep = true;
176+
self.started = true;
131177
self.iter.next()
132178
}
133179
}
134180

181+
fn size_hint(&self) -> (usize, Option<usize>) {
182+
intersperse_size_hint(&self.iter, self.started, self.next_item.is_some())
183+
}
184+
135185
fn fold<B, F>(self, init: B, f: F) -> B
136186
where
137187
Self: Sized,
138188
F: FnMut(B, Self::Item) -> B,
139189
{
140-
intersperse_fold(self.iter, init, f, self.separator, self.needs_sep)
141-
}
142-
143-
fn size_hint(&self) -> (usize, Option<usize>) {
144-
intersperse_size_hint(&self.iter, self.needs_sep)
190+
intersperse_fold(self.iter, init, f, self.separator, self.started, self.next_item)
145191
}
146192
}
147193

148-
fn intersperse_size_hint<I>(iter: &I, needs_sep: bool) -> (usize, Option<usize>)
194+
fn intersperse_size_hint<I>(iter: &I, started: bool, next_is_some: bool) -> (usize, Option<usize>)
149195
where
150196
I: Iterator,
151197
{
152198
let (lo, hi) = iter.size_hint();
153-
let next_is_elem = !needs_sep;
154199
(
155-
lo.saturating_sub(next_is_elem as usize).saturating_add(lo),
156-
hi.and_then(|hi| hi.saturating_sub(next_is_elem as usize).checked_add(hi)),
200+
lo.saturating_sub(!started as usize)
201+
.saturating_add(next_is_some as usize)
202+
.saturating_add(lo),
203+
hi.and_then(|hi| {
204+
hi.saturating_sub(!started as usize)
205+
.saturating_add(next_is_some as usize)
206+
.checked_add(hi)
207+
}),
157208
)
158209
}
159210

@@ -162,7 +213,8 @@ fn intersperse_fold<I, B, F, G>(
162213
init: B,
163214
mut f: F,
164215
mut separator: G,
165-
needs_sep: bool,
216+
started: bool,
217+
mut next_item: Option<I::Item>,
166218
) -> B
167219
where
168220
I: Iterator,
@@ -171,12 +223,9 @@ where
171223
{
172224
let mut accum = init;
173225

174-
if !needs_sep {
175-
if let Some(x) = iter.next() {
176-
accum = f(accum, x);
177-
} else {
178-
return accum;
179-
}
226+
let first = if started { next_item.take() } else { iter.next() };
227+
if let Some(x) = first {
228+
accum = f(accum, x);
180229
}
181230

182231
iter.fold(accum, |mut accum, x| {

0 commit comments

Comments
 (0)