Simplify a few functions in rustc_data_structures#52697

ljedrz · 2018-07-25T12:12:42Z

drop try!() where it's superfluous
change try!() to ?
squash a push with push_str
refactor a push loop into an iterator

rust-highfive · 2018-07-25T12:12:44Z

(rust_highfive has picked a reviewer for you, use r? to override)

Xanewok · 2018-07-25T14:54:53Z

src/librustc_data_structures/small_vec.rs

          A::Element: Decodable {
    fn decode<D: Decoder>(d: &mut D) -> Result<SmallVec<A>, D::Error> {
        d.read_seq(|d, len| {
-            let mut vec = SmallVec::with_capacity(len);


I have a hunch that this was deliberately done for optimization

cc @nnethercote

I don't have a good sense of how collect() works. Will it use size_hint from the range to pre-allocate an appropriate capacity?

It should, yes.

Mark-Simulacrum · 2018-07-29T15:02:59Z

@bors r+ rollup

bors · 2018-07-29T15:03:00Z

📌 Commit 86d0e9e has been approved by Mark-Simulacrum

bors · 2018-07-30T12:21:05Z

⌛ Testing commit 86d0e9e with merge 54628c8...

Simplify a few functions in rustc_data_structures - drop `try!()` where it's superfluous - change `try!()` to `?` - squash a `push` with `push_str` - refactor a push loop into an iterator

bors · 2018-07-30T14:20:12Z

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing 54628c8 to master...

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.

Don't collect() when size_hint is useless This adjusts PRs rust-lang#52738 and rust-lang#52697 by falling back to calculating capacity and extending or pushing in a loop where `collect()` can't be trusted to calculate the right capacity. It is a performance win.

[WIP] 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. #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](#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 #48994.

Simplify a few functions in rustc_data_structures

86d0e9e

rust-highfive assigned cramertj Jul 25, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2018

Xanewok reviewed Jul 25, 2018

View reviewed changes

kennytm closed this Jul 25, 2018

kennytm reopened this Jul 25, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 29, 2018

Mark-Simulacrum mentioned this pull request Jul 30, 2018

Rollup of 8 pull requests #52844

Closed

bors merged commit 86d0e9e into rust-lang:master Jul 30, 2018

ljedrz deleted the misc_data_structures branch July 30, 2018 14:21

ljedrz mentioned this pull request Jul 31, 2018

[WIP] Calculate capacity when collecting into Option and Result #52910

Closed

ljedrz mentioned this pull request Aug 3, 2018

Don't collect() when size_hint is useless #53019

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Simplify a few functions in rustc_data_structures#52697