Skip to content

Commit

Permalink
Rollup merge of rust-lang#52910 - ljedrz:fix_48994, r=sfackler
Browse files Browse the repository at this point in the history
Calculate capacity when collecting into Option and Result

I was browsing the [perf page](http://perf.rust-lang.org) to see the impact of my recent changes (e.g. rust-lang#52697) and I was surprised that some of the results were not as awesome as I expected. I dug some more and found an issue that is the probable culprit: [Collecting into a Result<Vec<_>> doesn't reserve the capacity in advance](rust-lang#48994).

Collecting into `Option` or `Result` might result in an empty collection, but there is no reason why we shouldn't provide a non-zero lower bound when we know the `Iterator` we are collecting from doesn't contain any `None` or `Err`.

We know this, because the `Adapter` iterator used in the `FromIterator` implementations for `Option` and `Result` registers if any `None` or `Err` are present in the `Iterator` in question; we can use this information and return a more accurate lower bound in case we know it won't be equal to zero.

I [have benchmarked](https://gist.github.com/ljedrz/c2fcc19f6260976ae7a46ae47aa71fb5) collecting into `Option` and `Result` using the current implementation and one with the proposed changes; I have also benchmarked a push loop with a known capacity as a reference that should be slower than using `FromIterator` (i.e. `collect()`). The results are quite promising:
```
test bench_collect_to_option_new ... bench:         246 ns/iter (+/- 23)
test bench_collect_to_option_old ... bench:         954 ns/iter (+/- 54)
test bench_collect_to_result_new ... bench:         250 ns/iter (+/- 25)
test bench_collect_to_result_old ... bench:         939 ns/iter (+/- 104)
test bench_push_loop_to_option   ... bench:         294 ns/iter (+/- 21)
test bench_push_loop_to_result   ... bench:         303 ns/iter (+/- 29)
```
Fixes rust-lang#48994.
  • Loading branch information
cramertj authored Aug 2, 2018
2 parents 9a7af03 + 77aa031 commit 5375ce4
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
3 changes: 1 addition & 2 deletions src/libcore/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1277,8 +1277,7 @@ impl<A, V: FromIterator<A>> FromIterator<Option<A>> for Option<V> {
if self.found_none {
(0, Some(0))
} else {
let (_, upper) = self.iter.size_hint();
(0, upper)
self.iter.size_hint()
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/libcore/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1214,9 +1214,13 @@ impl<A, E, V: FromIterator<A>> FromIterator<Result<A, E>> for Result<V, E> {
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
let (_min, max) = self.iter.size_hint();
(0, max)
if self.err.is_some() {
(0, Some(0))
} else {
self.iter.size_hint()
}
}
}

Expand Down

0 comments on commit 5375ce4

Please sign in to comment.