Move std::io::copy to alloc::io

[bushrat011899]

View all comments

ACP: rust-lang/libs-team#755
Tracking issue: #154046
Split From: #156527
Blocked On: #158547

Description

Moves std::io::copy into alloc::io. Blocked on #158547.

This relies on specialization to allow std to provide optimised copy implementations for its types where appropriate. The exact technique involves defining a new trait, alloc::io::SpecCopy:

#[doc(hidden)]
#[unstable(feature = "core_io_internals", reason = "exposed only for libstd", issue = "none")]
#[rustc_specialization_trait]
pub trait SpecCopy: Read {
    /// Attempt to copy from this reader to the provided writer using a specialized
    /// process.
    fn copy<R: Read + ?Sized, W: Write + ?Sized>(
        _reader: &mut R,
        _writer: &mut W,
    ) -> Result<CopyState>;
}

Since optimised copying requires both the reader and writer to support the operation between each other, we can choose one of them to be the implementer of the copy algorithm, and delegate specialization to it. In this case, I've chosen the reader to be the provider of the specialized copy implementation arbitrarily. Note that the SpecCopy::copy function is generic over the reader specifically to allow wrappers like Take<R> to be visible to the implementation of copy.

Because this introduces a new layer of specialization to io::copy, I think this PR should be benchmarked to make sure performance characteristics aren't too different. I am expecting compilation time to be slightly worse, since there's just more specialization happening, but the actual code run should be the same.

---

Notes

• No AI tooling of any kind was used during the creation of this PR.
• Please see Tracking Issue for alloc::io and core::io #154046 (comment) for a review order and broader context for this PR.

[rustbot]

r? @clarfonthey

rustbot has assigned @clarfonthey.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @ChrisDenton, libs
• @ChrisDenton, libs expanded to 13 candidates
• Random selection from 6 candidates

[bushrat011899]

@rustbot blocked

[bushrat011899]

@rustbot label -T-compiler -T-rustdoc -T-rustdoc-frontend

[clarfonthey]

Is there a reason why this has to go through a layer of indirection via specialized_copy instead of just being used directly?

View changes since the review

[bushrat011899]

Yeah we can't directly call SpecCopy::copy since that would require a default blanket implementation of SpecCopy for all Read types, which doesn't work without another intermediate type for marking that specialization too. I also prefer to have a simple function like specialized_copy exported instead of SpecCopyInner::copy, since that trait purely exists as a specialization hack.

[clarfonthey]

Right, I guess what I meant was whether specialized_copy should exist instead of just calling SpecCopyInner::copy.

[clarfonthey]

Left some comments as I try to wrap my brain around how these things work. Also going to want a perf run on the final versions since my guess is that these are going to be high-impact due to cross-crate inlining changes, but, we'll see if that's moot like it was for previous runs.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[bushrat011899]

Left some comments as I try to wrap my brain around how these things work. Also going to want a perf run on the final versions since my guess is that these are going to be high-impact due to cross-crate inlining changes, but, we'll see if that's moot like it was for previous runs.

I'm hopeful there wont be any performance loss with this move, but this is definitely the one move I would expect it to happen with, since specialization is very not ready for general use. I'm not even sure if cross-crate specialization is currently used at all. So yes please on a perf run!

[clarfonthey]

(Mostly just wanted to clarify I was going to wait until the doc/etc. comments are fixed before running perf, but yes, we'll be doing that.)