Extend io::copy buffer reuse to BufReader too

[the8472]

previously it was only able to use BufWriter. This was due to a limitation in the BufReader generics that prevented specialization. This change works around the issue by using BufReader where Self: Read instead of BufReader<I> where I: Read. This limits our options, e.g. we can't access the inner reader, but it happens to work out if we rely on some implementation details.

Copying 1MiB from /dev/zero to /dev/null through a 256KiB BufReader yields following improvements

OLD:
    io::copy::tests::bench_copy_buf_reader  51.44µs/iter +/- 703.00ns
NEW:
    io::copy::tests::bench_copy_buf_reader  18.55µs/iter +/- 237.00ns

Previously this would read 256KiB into the reader but then copy 8KiB chunks to the writer through an additional intermediate buffer inside io::copy. Since those devices don't do much work most of the speedup should come from fewer syscalls and avoided memcopies.

The b3sum crate notes that the default buffer size in io::copy is too small. With this optimization they could achieve the desired performance by wrapping the reader in a BufReader instead of handrolling it.

Currently the optimization doesn't apply to things like StdinLock, but this can be addressed with an additional AsMutBufReader specialization trait.

[rustbot]

r? @thomcc

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

[the8472]

Ah, I could maybe get everything I want by adding a bunch of hidden helper functions to the Read trait and only implement them on BufReader (and provide stub default impls for everything else) but imo that would be an even bigger hack than this. And that would require combining associated type defaults with specialization which is uncharted territory.

[thomcc]

I don't think this is worth doing right now.

[bors]

☔ The latest upstream changes (presumably #112716) made this pull request unmergeable. Please resolve the merge conflicts.

[thomcc]

Looks good to me. r=me after the rebase.

[the8472]

@bors r=thomcc

[bors]

📌 Commit 3738785 has been approved by thomcc

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3738785 with merge 7513407...

[bors]

☀️ Test successful - checks-actions
Approved by: thomcc
Pushing 7513407 to master...

[rust-timer]

Finished benchmarking commit (7513407): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.9%	[-1.9%, -1.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.9%	[-1.9%, -1.9%]	1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.1%	[3.1%, 3.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.8%	[2.8%, 2.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.1%, 0.1%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.1%, 0.1%]	5

Bootstrap: 656.303s -> 655.196s (-0.17%)