Skip to content

Commit

Permalink
core: optimize {option,result}::collect
Browse files Browse the repository at this point in the history
The bug #11084 causes these collect functions to run about
twice as slow as they should because llvm is having trouble
optimizing away the closure for some reason. This patch works
around that performance bug by using a simple adapter iterator
explicitly for capturing if the outer iterator returns an
error.
  • Loading branch information
erickt committed Jun 29, 2014
1 parent 1ea9991 commit ab1bd3a
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 20 deletions.
32 changes: 22 additions & 10 deletions src/libcore/option.rs
Expand Up @@ -587,20 +587,32 @@ impl<A> ExactSize<A> for Item<A> {}
/// ```
#[inline]
pub fn collect<T, Iter: Iterator<Option<T>>, V: FromIterator<T>>(iter: Iter) -> Option<V> {
// FIXME(#11084): This should be twice as fast once this bug is closed.
let mut iter = iter.scan(false, |state, x| {
match x {
Some(x) => Some(x),
None => {
*state = true;
None
// FIXME(#11084): This could be replaced with Iterator::scan when this
// performance bug is closed.

struct Adapter<Iter> {
iter: Iter,
found_none: bool,
}

impl<T, Iter: Iterator<Option<T>>> Iterator<T> for Adapter<Iter> {
#[inline]
fn next(&mut self) -> Option<T> {
match self.iter.next() {
Some(Some(value)) => Some(value),
Some(None) => {
self.found_none = true;
None
}
None => None,
}
}
});
}

let v: V = FromIterator::from_iter(iter.by_ref());
let mut adapter = Adapter { iter: iter, found_none: false };
let v: V = FromIterator::from_iter(adapter.by_ref());

if iter.state {
if adapter.found_none {
None
} else {
Some(v)
Expand Down
32 changes: 22 additions & 10 deletions src/libcore/result.rs
Expand Up @@ -585,20 +585,32 @@ impl<T: Show, E> Result<T, E> {
/// ```
#[inline]
pub fn collect<T, E, Iter: Iterator<Result<T, E>>, V: FromIterator<T>>(iter: Iter) -> Result<V, E> {
// FIXME(#11084): This should be twice as fast once this bug is closed.
let mut iter = iter.scan(None, |state, x| {
match x {
Ok(x) => Some(x),
Err(err) => {
*state = Some(err);
None
// FIXME(#11084): This could be replaced with Iterator::scan when this
// performance bug is closed.

struct Adapter<Iter, E> {
iter: Iter,
err: Option<E>,
}

impl<T, E, Iter: Iterator<Result<T, E>>> Iterator<T> for Adapter<Iter, E> {
#[inline]
fn next(&mut self) -> Option<T> {
match self.iter.next() {
Some(Ok(value)) => Some(value),
Some(Err(err)) => {
self.err = Some(err);
None
}
None => None,
}
}
});
}

let v: V = FromIterator::from_iter(iter.by_ref());
let mut adapter = Adapter { iter: iter, err: None };
let v: V = FromIterator::from_iter(adapter.by_ref());

match iter.state {
match adapter.err {
Some(err) => Err(err),
None => Ok(v),
}
Expand Down

5 comments on commit ab1bd3a

@bors
Copy link
Contributor

@bors bors commented on ab1bd3a Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at erickt@ab1bd3a

@bors
Copy link
Contributor

@bors bors commented on ab1bd3a Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging erickt/rust/optimizations = ab1bd3a into auto

@bors
Copy link
Contributor

@bors bors commented on ab1bd3a Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erickt/rust/optimizations = ab1bd3a merged ok, testing candidate = e25eb6b

@bors
Copy link
Contributor

@bors bors commented on ab1bd3a Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = e25eb6b

Please sign in to comment.