Simplify a few functions in rustc_data_structures #52697
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Xanewok
reviewed
Jul 25, 2018
| @@ -209,11 +209,7 @@ impl<A> Decodable for SmallVec<A> | |||
| 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); | |||
There was a problem hiding this comment.
I have a hunch that this was deliberately done for optimization
cc @nnethercote
There was a problem hiding this comment.
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?
|
@bors r+ rollup |
|
📌 Commit 86d0e9e has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Jul 30, 2018
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
|
☀️ Test successful - status-appveyor, status-travis |
cramertj
added a commit
to cramertj/rust
that referenced
this pull request
Aug 2, 2018
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.
kennytm
added a commit
to kennytm/rust
that referenced
this pull request
Aug 10, 2018
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.
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Aug 12, 2018
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.
bors
added a commit
that referenced
this pull request
Feb 1, 2019
[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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
try!()where it's superfluoustry!()to?pushwithpush_str