Implement vectored write functionality for files #5958
Conversation
tokio/tests/fs_file.rs
Outdated
| .write_vectored(&[IoSlice::new(HELLO), IoSlice::new(HELLO)]) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(ret, HELLO.bytes().count() * 2); |
There was a problem hiding this comment.
This is in the test twice.
| assert_eq!(ret, HELLO.bytes().count() * 2); | |
| assert_eq!(ret, HELLO.len() * 2); |
tokio/src/io/blocking.rs
Outdated
| pub(crate) fn copy_from_bufs(&mut self, bufs: &[io::IoSlice<'_>]) -> usize { | ||
| assert!(self.is_empty()); | ||
|
|
||
| let n = bufs.iter().map(|b| b.len()).sum::<usize>().min(MAX_BUF); |
There was a problem hiding this comment.
This iterates all of the buffers every time, even if we only write a few of them. If the buffers are very long and this is called in a loop, that gives quadratic performance.
We should be able to embed this logic inside the for loop instead to avoid that.
There was a problem hiding this comment.
You mean b.len() is O(n) and causes the bufs.iter() to be O(n^2)? I thought since it's a Deref to &[u8], it's O(1).
I've provided an alternate implementation that doesn't use bufs.iter().
There was a problem hiding this comment.
No, b.len() is constant time. Instead, it's O(n) in the length of bufs, which you iterate over.
There was a problem hiding this comment.
Ah, I almost forgot. You have to implement https://docs.rs/tokio/latest/tokio/io/trait.AsyncWrite.html#method.is_write_vectored too.
The
poll_write_vectoredfunction forFilecurrently uses the default implementation inAsyncWrite, which only writes the first non-empty buffer inbufs. This PR adds proper vectored write support for files.Motivation
Solution
This implementation copies the maximum permissible data (as determined by the
MAX_BUFconstant) from the buffers to the internal buffer ofFile. All other behavior aligns with the existingpoll_writemethod.Resolves Issue #5949.