add write_all_vectored to AsyncWriteExt
#7768
Conversation
leshow
changed the title
write_all_vectored to AsyncWriteExt (#3679)write_all_vectored to AsyncWriteExt
leshow
force-pushed
the
write_all_vectored
branch
from
16eed02 to
34aa2a3
Compare
|
We could implement |
|
I thought that as well but I saw |
martin-g
left a comment
martin-g
left a comment
There was a problem hiding this comment.
If this PR will be finished then it also needs an IT test.
tokio/src/io/util/async_write_ext.rs
Outdated
| /// &mut self, | ||
| /// bufs: &mut [IoSlice<'_>], | ||
| /// ) -> io::Result<()> { | ||
| /// let mut pos = 0; |
There was a problem hiding this comment.
The pos variable is never used. Maybe return it as a result to make the example more complete ?!
There was a problem hiding this comment.
My bad, it was left over from a previous iteration. If you have a look at this playground link (ignore the generic param, just for testing) I think this is a valid representation if you were translating to async https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=f25ce7e316683307b91a474f6c7f9496
|
|
||
| pub(crate) fn write_all_vectored<'a, 'b, W>( | ||
| writer: &'a mut W, | ||
| bufs: &'a mut [IoSlice<'b>], |
There was a problem hiding this comment.
The signature in std is a bit different:
| bufs: &'a mut [IoSlice<'b>], | |
| bufs: &'a mut &'a mut [IoSlice<'b>], |
There was a problem hiding this comment.
That's for advance_slices though, std signature for write_all_vectored has single &mut. https://doc.rust-lang.org/std/io/trait.Write.html#method.write_all_vectored
| let me = self.project(); | ||
| while !me.bufs.is_empty() { | ||
| let n = ready!(Pin::new(&mut *me.writer).poll_write_vectored(cx, me.bufs))?; | ||
| if n == 0 { |
There was a problem hiding this comment.
I think this needs extra care for the case when all bufs are empty. In this case 0 is not really an error.
Same improvement should be done in the (ignored) sample code in async_write_ext.rs
There was a problem hiding this comment.
This is what write_all_buf does. It calls poll_write_vectored and checks n == 0. It works in practice even with empty buffers when I've tried it with the async version (see the playground link)
I didn't see the tests folder, I will add some tests there and update the branch
edit:
I added some tests, including some with empty buffers to show this should work as expected
leshow
force-pushed
the
write_all_vectored
branch
from
34aa2a3 to
c2ff951
Compare
leshow
force-pushed
the
write_all_vectored
branch
from
c2ff951 to
a6c9bef
Compare
3 participants
Motivation
Closes #3679
Was recently looking for
write_all_vectoredand found it missing, went to add it before realizing the MSRV is not there yet.I thought I'd submit this anyway, when the MSRV version moves ahead you can accept, or we could copy in the
IoSlice::advance_slicesfn until that happens?Solution
Tried to follow existing pattern for
write_xmethods