Add benchmark and fast path for BufReader::read_exact #80201
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @withoutboats (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
crlf0710
added
S-waiting-on-review
|
r? @KodrAus |
| return Ok(()); | ||
| } | ||
|
|
||
| while !buf.is_empty() { |
There was a problem hiding this comment.
We should be able to introduce a read_exact function with this default implementation and call it here and in Read::read_exact so that it’s clear this is our fallback.
|
Thanks @saethlin! Sorry, it looks like the convention here would be to call the function That’s just a tiny nit though. This looks good to me. |
|
Once you’re ready would you be able to squash the commits down and we can merge it in? |
saethlin
force-pushed
the
bufreader-read-exact
branch
from
0b7d599
to
861edce
Compare
|
Oh no I think I did this wrong 🤦 |
|
I tried to rebase then squash away my commits. Maybe that was the wrong thing to do, since GitHub thinks this is now 30 commits? I really have no expertise in this, since at work we squash+merge all PRs. If I did something wrong please assist? |
KodrAus
force-pushed
the
bufreader-read-exact
branch
from
861edce
to
4e27ed3
Compare
|
No problem! Git trips me up all the time. I hope you don't mind I force pushed your branch to fix it all up. I ended going with the nuclear option in git and cherry pick your commit back onto a clean git checkout master
git cherry-pick 861edce52e4e118265d9fa366777546012829798I then pushed that to your |
|
@bors r+ |
|
📌 Commit 4e27ed3 has been approved by |
bors
added
S-waiting-on-bors
|
Thanks for fixing the commit history, I really appreciate it. |
…drAus Add benchmark and fast path for BufReader::read_exact At work, we have a wrapper type that implements this optimization. It would be nice if the standard library were faster. Before: ``` test io::buffered::tests::bench_buffered_reader_small_reads ... bench: 7,670 ns/iter (+/- 45) ``` After: ``` test io::buffered::tests::bench_buffered_reader_small_reads ... bench: 4,457 ns/iter (+/- 41) ```
Rollup of 13 pull requests Successful merges: - rust-lang#79298 (correctly deal with late-bound lifetimes in anon consts) - rust-lang#80031 (resolve: Reject ambiguity built-in attr vs different built-in attr) - rust-lang#80201 (Add benchmark and fast path for BufReader::read_exact) - rust-lang#80635 (Improve diagnostics when closure doesn't meet trait bound) - rust-lang#80765 (resolve: Simplify collection of traits in scope) - rust-lang#80932 (Allow downloading LLVM on Windows and MacOS) - rust-lang#80983 (Remove is_dllimport_foreign_item definition from cg_ssa) - rust-lang#81064 (Support non-stage0 check) - rust-lang#81080 (Force vec![] to expression position only) - rust-lang#81082 (BTreeMap: clean up a few more comments) - rust-lang#81084 (Use Option::map instead of open-coding it) - rust-lang#81095 (Use Option::unwrap_or instead of open-coding it) - rust-lang#81107 (Add NonZeroUn::is_power_of_two) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
At work, we have a wrapper type that implements this optimization. It would be nice if the standard library were faster.
Before:
After: