Tracking issue for Write::write_all_vectored

@cramertj

View all comments

This is a tracking issue for io::Write::write_all_vectored.

Feature gate: #![feature(write_all_vectored)].

Steps:

Implement the RFC (Add io::Write::write_all_vectored #70612).
Stabilization PR.

Unresolved questions:

Can we improve the API? Currently the method takes the IoSlices as mutable slice and modifies them. This is a pretty unusual and potentially error-prone API. We could either find a way to not mutate the argument or to enforce (via the type system) the argument can't be used by the user afterwards. Or we can decide that such an unusual API is fine for this rather low level method.

Original issue:

Details

In the io::Write trait we've got the helpful write_all method, which calls write in a loop to write all bytes. However there is no such function for write_vectored. I would suggest adding a function called write_all_vectored to performance such a task.

A possible implementation. Note that bufs is a mutable slice, also see the discussion in https://github.com/rust-lang/futures-rs/pull/1741/files. On the playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=872e9d973bd8101e7724292f87a82869.

pub trait Write {
    // ...

    fn write_all_vectored(&mut self, mut bufs: &mut [IoSlice<'_>]) -> io::Result<()> {
        while !bufs.is_empty() {
            match self.write_vectored(bufs) {
                Ok(0) => {
                    return Err(Error::new(
                        ErrorKind::WriteZero,
                        "failed to write whole buffer",
                    ));
                }
                Ok(n) => bufs = IoSlice::advance(mem::replace(&mut bufs, &mut []), n),
                Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
                Err(e) => return Err(e),
            }
        }
        Ok(())
    }
}

Related: rust-lang/futures-rs#1741
/cc @cramertj

@sfackler

cc @sfackler, @withoutboats

The main API question I have (which is more about IoSlice::advance than this I guess) is if we're okay saying that the content of the bufs slice is unspecified after this call returns. It's probably fine, but just one "weird" affect that you may not naively expect.

@sfackler

@sfackler I agree, but I don't know how an alternative that takes &[IoSlice<'_>], like you would expect, would work.

It could for example perform vectored writes when the current cursor position is at a slice boundry and switch to a normal write_all if it's in the middle of one of the slices.

But then you're missing out on the efficiency of vectored writes, if that were the case I wouldn't consider using it.

You are only missing out on the efficienty of vectored writes some of the time.

True, but with the implementation I provided (which I still admit isn't great) you don't miss out. I've made the point already in rust-lang/futures-rs#1741 (comment), but personally I don't its really a big deal that the provided slice is modified. In most cases I imagine the bufs slice being build just before this call and only used in this call.

One possibility would be to keep a fixed-size buffer of, say, [IoSlice<'_>; 128] and copy sections of the caller's slice into that, pairing it with an index for the last-most unwritten IoSlice . It's more complicated in that you have to re-load that IoSlice at some clever point in order to both minimize shifting but also maximize the number of IoSlices being offered to the underlying reader, but it prevents mutation of the original caller's slice while also allowing some fixed amount of vectored writes.

I'm not necessarily advocating for this idea, as I think it might be fine to mutate the caller's slice (they can always make a clone if they don't want to deal with that), but it's an option.

Rollup merge of rust-lang#70612 - Thomasdezeeuw:issue_70436, r=LukasK…

@joshtriplett

How about an owned type that wraps T: AsRef<[IoSlice<'_>]> (or Deref) to indicate that the write call "owns" the buffers and can modify them, but allow the buffers wrapper type to be reused (i.e. the Vec in Vec<IoSlice<'_>>). It makes the API a bit more conflicted, but perhaps that will resolve @joshtriplett concerns.

@Thomasdezeeuw

@Thomasdezeeuw wrote:

How about an owned type that wraps T: AsRef<[IoSlice<'_>]> (or Deref) to indicate that the write call "owns" the buffers and can modify them, but allow the buffer_s_ wrapper type to be reused (i.e. the Vec in Vec<IoSlice<'_>>). It makes the API a bit more conflicted, but perhaps that will resolve @joshtriplett concerns.

That doesn't address this concern I wrote above:

The issue isn't just that callers will forget this (&mut is a big hint that you can't count on it being unmodified), though that's also a problem. The primary issue seems like having to deal with it being overwritten by the function in the first place.

What if we used a Cow'd slice? If you never want your slice copied even in the partial write case, pass in a Cow::Owned, and give up your own ownership; if you don't want to give up ownership of your slice and allow mutating it, pass in a Cow::Borrowed.

Also, we could avoid an extra syscall even in the borrowed case, if we're willing to allocate in that case, and Cow will take care of that for us.

@joshtriplett

What if we used a Cow'd slice? If you never want your slice copied even in the partial write case, pass in a Cow::Owned, and give up your own ownership; if you don't want to give up ownership of your slice and allow mutating it, pass in a Cow::Borrowed.

Also, we could avoid an extra syscall even in the borrowed case, if we're willing to allocate in that case, and Cow will take care of that for us.

@joshtriplett I'm sorry, but I don't really have the energy for this issue any more... You're comment completely misses the point of vectored I/O. If people are looking at using vectored I/O it's likely they're looking at it for performance reasons, suggesting to just allocate a new buffer so it can be used once in a system call is really quite far off this target.

It's been more than two years, the API works and has zero overhead as-is, but yes it has a gotcha. At this point either stabilise it or remove it, I'm no longer interested, this has just taken too long.

So, I do care very much about vectored I/O as well, and I care about the use case of being able to reuse the iovecs repeatedly, which is also something people do in C for reasons of both performance and simplicity. I'd like to be able to do that somehow in Rust, and not have to throw away and recreate the array of iovecs every time. I'm trying to brainstorm some data structure that will allow for that reuse. I was trying to favor using existing constructs and types (at the expense of incurring overhead in what I saw as the less common case of partial write), but I acknowledge that that's not ideal. I agree that Cow wasn't a great idea.

As an alternative closer to what you already proposed, perhaps it makes more sense to make a new IoSlices structure that owns the iovecs as you suggested, but whose API guarantees that when the write method is done those iovecs are un-mutated? I would be happy to do that design work.

Rough sketch of a solution that shouldn't need to make any copies at all:

impl IoSlices {
    fn borrow_for_write(&self, offset: usize, impl FnOnce(&[IoSlice]) -> io::Result<usize>) -> io::Result<usize>;
}

If you have an &IoSlices (which write_all_vectored would get) you can call borrow_for_write which will mutate the slices using interior mutability, give you those slices so you can writev them or equivalent, and then modify them back (which hopefully we can optimize away if the IoSlices is going away). If you have a &mut IoSlices or an IoSlices, we can provide ways to get a &mut [IoSlice], but write_all_vectored would take an &IoSlices so it can't arbitrarily mutate the slices. (We can provide analogous types corresponding to IoSliceMut, and additionally a method that transmutes to give an &IoSlices so that you can use the same buffer for both read and write, and also modify through it when not currently reading or writing.)

That should avoid any copying, avoid any more syscalls than absolutely necessary, and overall make this exactly as efficient and capable as writev/readv can allow.

Does something like that sound reasonable?

I wrote up a quick implementation of something similar to the proposed design. It's a pretty straightforward elaboration of advance_slices that takes care to preserve the original version of the current IoSlice and restores it each time the &[IoSlice] is advanced one step. Because only 1 buffer is ever in a mutated state at a time, it's easy to keep a copy of the original version and restore it when we're done with it. It also restores on drop to help keep io::Error returns clean.

struct BufferManager<'a> {
    buffers: &'a mut [io::IoSlice<'a>],
    saved: Option<io::IoSlice<'a>>,
}

impl BufferManager<'a> {
    pub fn new(buffers: &'a mut [io::IoSlice<'a>]) -> Self {
        // strip empty buffers
        let first_non_empty = buffers
            .iter()
            .position(|buf| buf.len() > 0)
            .unwrap_or(buffers.len());

        let buffers = &mut buffers[first_non_empty..];

        Self {
            buffers,
            saved: None,
        }
    }

    pub fn get(&self) -> &[io::IoSlice<'a>] {
        self.buffers
    }

    pub fn count(&self) -> usize {
        self.buffers.len()
    }

    pub fn advance(&mut self, amount: usize) {
        while let Some((head, tail)) = self.buffers.split_first_mut() {
            // The head is smaller than the overall write, so pop it off the
            // the front of the buffers. Be sure to restore the original state,
            // if necessary.
            if let Some(remaining) = amount.checked_sub(head.len()) {
                amount = remaining;

                if let Some(saved) = self.saved.take() {
                    *head = saved;
                }

                *buffers = tail;
            }
            // The head is larger than the overall write, so it needs to be
            // modified in place. Be sure to save the current state of the
            // buffer, if necessary.
            else {
                if amount > 0 {
                    self.saved = Some(self.saved.unwrap_or(*head));
                    *head = io::IoSlice::new(&head[amount..]);
                }

                return;
            }
        }

        assert!(amount == 0, "advanced too far")
    }
}

impl Drop for BufferManager<'_> {
    fn drop(&mut self) {
        // When the buffer manager is dropped, restore the state of the
        // current buffers head, if necessary. It shouldn't be possible for
        // there to be a saved value while the buffer list is empty.
        if let Some(head) = self.buffers.first_mut() {
            if let Some(saved) = self.saved {
                *head = saved
            }
        }
    }
}

Then, write_all_vectored is basically the same as it is now, but using BufferManager to handle slice advancing

fn write_all_vectored(
    &mut self,
    buffers: &mut [io::IoSlice<'_>],
) -> io::Result<()> {
    let mut buffers = BufferManager::new(buffers);

    while buffers.count() > 0 {
        match dest.write_vectored(buffers.get()) {
            Ok(0) => return Err(io::ErrorKind::WriteZero.into()),
            Ok(n) => buffers.advance(n),
            Err(err) if err.kind() == io::ErrorKind::Interrupted => {}
            Err(err) => return Err(err),
        }
    }

    Ok(())
}

The only downside here is that it does require the user to provide an &mut [IoSlice], but we can still promise at the API level (though not the type level) that the slices will be unmodified when this function returns.

Can an extra method, or even option or something, be added to allow retrying the operation with data that wasn't successfully written? I'd like to tolerate some inner write errors like EAGAIN/ErrorKind::WouldBlock without having to reimplement write_all_vectored manually.

Can an extra method, or even option or something, be added to allow retrying the operation with data that wasn't successfully written? I'd like to tolerate some inner write errors like EAGAIN/ErrorKind::WouldBlock without having to reimplement write_all_vectored manually.

We don't do this for the write_all function, so I don't think write_all_vectored should do anything different.

Would you consider an alternate signature that returns the total number of bytes written?

fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> Result<usize>

I am aware that it is not consistent with the signature of write_all, but callers can implement the behavior trivially there:

dest.write_all(&buf)?;
Ok(buf.len)

Unless I am missing something crucial, callers of write_all_vectored cannot ergonomically or efficiently do the same. To implement the behavior they would need to iterate over bufs and call .len() on each IoSlice to sum up the total length. That's at least one extraneous pointer dereference per buffer.

In contrast, write_all_vectored can accumulate the total much more efficiently. The necessary information is already being returned by the underlying calls to writev and/or write. I would expect most of the time this will compile down to be one extra ADD <reg>,<reg> instruction per underlying write call.

In my experience (mostly embedded Linux) the returned count of bytes written is occasionally useful at runtime and extremely useful in unit tests. Since users cannot implement this efficiently without re-implementing write_all_vectored, and the underlying write calls are already returning the information, it seems reasonable to have write_all_vectored pass it along instead of discarding.

Example:

fn send_entercnd1(password: &str, dest: &mut impl std::io::Write) -> std::io::Result<usize> {
    Ok(dest.write_all_vectored(&[
        IoSlice::new(b"AT!ENTERCND=\""),
        IoSlice::new(password.as_bytes()),
        IoSlice::new(b"\"\r"),
    ])?)
}

vs

fn send_entercnd2(password: &str, dest: &mut impl std::io::Write) -> std::io::Result<usize> {
    // I know two of these have constant length and it could just return Ok(15+password.len())
    // In general this won't be the case. I didn't see the need to complicate the example.
    let mut bufs = &[
        IoSlice::new(b"AT!ENTERCND=\""),
        IoSlice::new(password.as_bytes()),
        IoSlice::new(b"\"\r"),
    ];
    // get length before `write_all_vectored`
    let length = {
        let mut acc = 0;
        for buf in bufs {
            acc += buf.len();
        }
        acc
    };
    dest.write_all_vectored(&mut bufs)?;
    Ok(length);
}

Would you consider an alternate signature that returns the total number of bytes written?

This can trivially be done via bufs.iter().map(|s| s.len()).sum(). If your bufs here is the top-level array itself and there's only a few entries, the compiler will likely even be able to avoid the loop.

Can an extra method, or even option or something, be added to allow retrying the operation with data that wasn't successfully written? I'd like to tolerate some inner write errors like EAGAIN/ErrorKind::WouldBlock without having to reimplement write_all_vectored manually.

We don't do this for the write_all function, so I don't think write_all_vectored should do anything different.

We got bitten in our codebase by a bug caused by us handling ErrorKind::WouldBlock by retrying the write_all_vectored() call with the same bufs input, despite the documentation advising against it to be fair.

Still it seems that this scenario could either be 1) more explicitly disallowed through type-checking by e.g. making bufs immutable or 2) supported by making bufs mutable "all the way" by making its type match IoSlice::advance_slices()'s bufs argument (&mut &mut [IoSlice]). We went with 2) in our codebase.

We got bitten in our codebase by a bug caused by us handling ErrorKind::WouldBlock by retrying the write_all_vectored() call with the same bufs input, despite the documentation advising against it to be fair.

IoSlice::advance_slices is stabilized since 1.81. It should be relatively easy to manually implement write_all_vectored now, with special handling of EAGAIN (and EINTR).

I think it's fair to keep the current signature &mut [IoSlice] just to stay consistent with write_all (#70436 (comment)): both methods are shortcut methods to fully write all data at once, and treating any partial write as a non-recoverable failure (unknown length of data is sent to the underlying stream). I don't think returning usize or taking &mut &mut [IoSlice] is useful at all for the common cases.

Is it time to stabilize this?

Hi, is there any progress on this?

treating any partial write as a non-recoverable failure (unknown length of data is sent to the underlying stream)

I think this is a weakness in write_all that prevents many use cases (or alternatively, in case of being used in ignorance, produces buggy networking code). I'm trying to get this misfeature of write_all at least getting properly documented, since fixing it seems night impossible because of backwards compatibility.

However, we shouldn't repeat mistakes just for the sake of consistency.

Navigation Menu

Repository navigation

Tracking issue for `Write::write_all_vectored` #70436

29 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tracking issue for Write::write_all_vectored #70436

Description

Activity

cramertj commented on Mar 26, 2020

sfackler commented on Mar 26, 2020

Thomasdezeeuw commented on Mar 27, 2020

sfackler commented on Mar 27, 2020

Thomasdezeeuw commented on Mar 27, 2020

sfackler commented on Mar 27, 2020

Thomasdezeeuw commented on Mar 27, 2020

cramertj commented on Mar 27, 2020

29 remaining items

Thomasdezeeuw commented on May 13, 2022

joshtriplett commented on May 17, 2022

Thomasdezeeuw commented on May 17, 2022

joshtriplett commented on May 18, 2022

Lucretiel commented on Aug 19, 2022

dead-claudia commented on Dec 19, 2022

Thomasdezeeuw commented on Dec 19, 2022

tbottomparallel commented on Jan 19, 2024

dead-claudia commented on Jan 20, 2024

pliardb commented on May 7, 2024

oxalica commented on May 22, 2025

PureWhiteWu commented on Jun 30, 2025

golddranks commented on Apr 6, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Tracking issue for `Write::write_all_vectored` #70436