Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

as_slice/into_slice for IoSlice/IoSliceMut #93

Closed
mpdn opened this issue Aug 29, 2022 · 2 comments
Closed

as_slice/into_slice for IoSlice/IoSliceMut #93

mpdn opened this issue Aug 29, 2022 · 2 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@mpdn
Copy link

mpdn commented Aug 29, 2022

Proposal

Add an as_slice on IoSlice to go from &IoSlice<'a> to &'a [u8].
Add an into_slice on IoSliceMut to go from IoSliceMut<'a> to &'a mut [u8].

Problem statement

Using vectored IO (eg. write_vectored) is close to impossible to do correctly due to the difficulty of slicing/advancing IoSlice

Consider something like this:

fn test(mut out: impl Write) -> std::io::Result<()> {
    let data1 = [1; 8];
    let data2 = [15; 8];
    let io_slice1 = IoSlice::new(&data1);
    let io_slice2 = IoSlice::new(&data2);
    out.write_vectored(&[io_slice1, io_slice2])?;
    Ok(())
}

The issue here is that write_vectored may only do a partial write of a slices. To ensure we write all data we need to advance the slices. Dealing with this is a little bit more annoying than ordinary writes, but we might try something like this:

fn test(mut out: impl Write) -> std::io::Result<()> {
    let data1 = [1; 8];
    let data2 = [15; 8];
    let io_slice1 = IoSlice::new(&data1);
    let io_slice2 = IoSlice::new(&data2);

    let mut buf = [io_slice1, io_slice2];
    let mut buf = &mut buf[..];
    let mut written = 0;
    
    while !buf.is_empty() {
        if buf[0].len() < written {
            written -= buf[0].len();
            buf = &mut buf[1..];
        } else {
            buf[0] = IoSlice::new(&buf[0][written..]);
            written = out.write_vectored(buf)?;
        }
    }

    Ok(())
}

But this fails to compile! The problem is that in IoSlice::new(&buf[0][written..]) we try to slice the IoSlice. This uses the Deref trait implemented on the IoSlice, but this ends up borrowing the IO slice. Thus we cannot modify buf as we are also borrowing data stored in it. Instead, if we simply had an as_slice as above, we could simply write that as IoSlice::new(&buf[0].as_slice()[written..]) instead.

There has been a few similar solutions to this problem, such as rust-lang/rust#62726 and rust-lang/rust#70436. These have sort of stalled, in part due to the resulting signatures ending up being sort of odd in cases where we want to advance a slice of IoSlices. I think the above proposal is simpler and more basic as it allows the user to construct functions like write_all_vectored, advance, or advance_slices in ordinary safe Rust.

Motivation, use-cases

None found - I haven't actually been able to find any applications doing vectored IO in Rust. Only vectored IO I have found has been wrapping write_vectored and the like: https://github.com/search?l=Rust&p=2&q=%22write_vectored%22&type=Code

Solution sketches

Already have it implemented here: https://github.com/rust-lang/rust/compare/master...mpdn:rust:master?expand=1

Links and related work

@mpdn mpdn added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Aug 29, 2022
@nathaniel-bennett
Copy link

I've been trying to use vectored I/O for some performant networking use cases, and having a feature like this would really help--there's just no good way to update slice indices right now.

@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label May 18, 2023
@m-ou-se
Copy link
Member

m-ou-se commented May 18, 2023

We discussed this in the libs meetup. This looks fine, except that IoSlice::as_slice() should just take self by value, since it is Copy anyway.

Feel free to open a tracking issue and open a PR to rust-lang/rust to add this as an unstable feature.

@m-ou-se m-ou-se closed this as completed May 18, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 8, 2024
…again, r=<try>

Add as_slice/into_slice for IoSlice/IoSliceMut.

ACP: rust-lang/libs-team#93

Based on a623c52 (CC `@mpdn)` and rust-lang#111277 (CC `@Lucretiel).`

Closes: rust-lang#124659

Tracking Issue: TODO

try-job: test-various
try-job: dist-various-1
try-job: dist-various-2

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 9, 2024
…again, r=<try>

Add as_slice/into_slice for IoSlice/IoSliceMut.

ACP: rust-lang/libs-team#93

Based on a623c52 (CC `@mpdn)` and rust-lang#111277 (CC `@Lucretiel).`

Closes: rust-lang#124659

Tracking Issue: TODO

try-job: test-various
try-job: dist-various-1
try-job: dist-various-2

r? `@ghost`
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 15, 2024
…s-again, r=cuviper

Add as_slice/into_slice for IoSlice/IoSliceMut.

ACP: rust-lang/libs-team#93

Tracking issue: rust-lang#132818

Based on a623c52 (CC `@mpdn)` and rust-lang#111277 (CC `@Lucretiel).`

Closes: rust-lang#124659

Tracking Issue: TODO

try-job: test-various
try-job: dist-various-1
try-job: dist-various-2

r? libs
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 15, 2024
Rollup merge of rust-lang#132790 - aDotInTheVoid:ioslice-asslice-rides-again, r=cuviper

Add as_slice/into_slice for IoSlice/IoSliceMut.

ACP: rust-lang/libs-team#93

Tracking issue: rust-lang#132818

Based on a623c52 (CC `@mpdn)` and rust-lang#111277 (CC `@Lucretiel).`

Closes: rust-lang#124659

Tracking Issue: TODO

try-job: test-various
try-job: dist-various-1
try-job: dist-various-2

r? libs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

4 participants