io: reintroduce vectored writes #3149

seanmonstar · 2020-11-17T16:38:51Z

This adds AsyncWrite::poll_write_vectored, and implements it for
TcpStream and UnixStream. Based on the proposal in #3135.

This adds `AsyncWrite::poll_write_vectored`, and implements it for `TcpStream` and `UnixStream`. Based on the proposal in #3135.

tokio/src/io/async_write.rs

hawkw · 2020-11-17T22:40:15Z

tokio/src/io/async_write.rs

+    /// Like [`poll_write`], except that it writes from a slice of buffers.
+    ///


it might be worth saying something about how this is generally expected to use platform vectored write APIs?

I took the documentation for these 2 new methods directly from the standard library. I kind of like that they don't mention platform gather IO, and just leave that up to the implementor. 🤷

hmm, that's fair!

I think it is worth saying that, if any data is written, the function returns Ok(n). I.e. an error / pending means no data was written.

hawkw · 2020-11-17T22:41:16Z

tokio/src/io/async_write.rs

+    /// The default implementation calls [`poll_write`] with either the first nonempty
+    /// buffer provided, or an empty one if none exists.


should there be a note that the default implementation is less efficient than just using poll_write and that poll_write_vectored should only be called when is_write_vectored returns true? maybe including an example of checking for vectored write capability before using it?

hawkw · 2020-11-17T22:43:19Z

tokio/src/io/async_write.rs

+    /// and coalesce writes into a single buffer for higher performance.
+    ///
+    /// The default implementation returns `false`.
+    ///


it may be worth documenting a contract around whether or not the return value from a given instance of a type implementing AsyncWrite is ever expected to change over its lifetime? e.g. can a library reasonably expect to be able to call is_write_vectored once for a given AsyncWrite instance and then always do vectored writes, or should it always check before writing?

tokio/src/io/poll_evented.rs

Tokio's `AsyncWrite` trait once again has support for vectored writes in Tokio 0.3.4 (see tokio-rs/tokio#3149. This branch re-enables vectored writes in Hyper for HTTP/1. Using vectored writes in HTTP/2 will require an upstream change in the `h2` crate as well. I've removed the adaptive write buffer implementation that attempts to detect whether vectored IO is or is not available, since the Tokio 0.3.4 `AsyncWrite` trait exposes this directly via the `is_write_vectored` method. Now, we just ask the IO whether or not it supports vectored writes, and configure the buffer accordingly. This makes the implementation somewhat simpler. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Tokio's AsyncWrite trait once again has support for vectored writes in Tokio 0.3.4 (see tokio-rs/tokio#3149. This branch re-enables vectored writes in h2. This change doesn't make all that big of a performance improvement in Hyper's HTTP/2 benchmarks, but they use a BytesMut as the buffer. With a buffer that turns into more IO vectors in bytes_vectored, there might be a more noticeable performance improvement. I spent a bit trying to refactor the flush logic to coalesce into fewer writev calls with more buffers, but the current implementation seems like about the best we're going to get without a bigger refactor. It's basically the same as what h2 did previously, so it's probably fine.

Tokio's `AsyncWrite` trait once again has support for vectored writes in Tokio 0.3.4 (see tokio-rs/tokio#3149). This branch re-enables vectored writes in Hyper for HTTP/1. Using vectored writes in HTTP/2 will require an upstream change in the `h2` crate as well. I've removed the adaptive write buffer implementation that attempts to detect whether vectored IO is or is not available, since the Tokio 0.3.4 `AsyncWrite` trait exposes this directly via the `is_write_vectored` method. Now, we just ask the IO whether or not it supports vectored writes, and configure the buffer accordingly. This makes the implementation somewhat simpler. This also removes `http1_writev()` methods from the builders. These are no longer necessary, as Hyper can now determine whether or not to use vectored writes based on `is_write_vectored`, rather than trying to auto-detect it. Closes #2320 BREAKING CHANGE: Removed `http1_writev` methods from `client::Builder`, `client::conn::Builder`, `server::Builder`, and `server::conn::Builder`. Vectored writes are now enabled based on whether the `AsyncWrite` implementation in use supports them, rather than though adaptive detection. To explicitly disable vectored writes, users may wrap the IO in a newtype that implements `AsyncRead` and `AsyncWrite` and returns `false` from its `AsyncWrite::is_write_vectored` method.

## Motivation In Tokio 0.2, `AsyncRead` and `AsyncWrite` had `poll_write_buf` and `poll_read_buf` methods for reading and writing to implementers of `bytes` `Buf` and `BufMut` traits. In 0.3, these were removed, but `poll_read_buf` was added as a free function in `tokio-util`. However, there is currently no `poll_write_buf`. Now that `AsyncWrite` has regained support for vectored writes in #3149, there's a lot of potential benefit in having a `poll_write_buf` that uses vectored writes when supported and non-vectored writes when not supported, so that users don't have to reimplement this. ## Solution This PR adds a `poll_write_buf` function to `tokio_util::io`, analogous to the existing `poll_read_buf` function. This function writes from a `Buf` to an `AsyncWrite`, advancing the `Buf`'s internal cursor. In addition, when the `AsyncWrite` supports vectored writes (i.e. its `is_write_vectored` method returns `true`), it will use vectored IO. I copied the documentation for this functions from the docs from Tokio 0.2's `AsyncWrite::poll_write_buf` , with some minor modifications as appropriate. Finally, I fixed a minor issue in the existing docs for `poll_read_buf` and `read_buf`, and updated `tokio_util::codec` to use `poll_write_buf`. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Tokio's AsyncWrite trait once again has support for vectored writes in Tokio 0.3.4 (see tokio-rs/tokio#3149. This branch re-enables vectored writes in h2. This change doesn't make all that big of a performance improvement in Hyper's HTTP/2 benchmarks, but they use a BytesMut as the buffer. With a buffer that turns into more IO vectors in bytes_vectored, there might be a more noticeable performance improvement. I spent a bit trying to refactor the flush logic to coalesce into fewer writev calls with more buffers, but the current implementation seems like about the best we're going to get without a bigger refactor. It's basically the same as what h2 did previously, so it's probably fine.

Tokio's `AsyncWrite` trait once again has support for vectored writes in Tokio 0.3.4 (see tokio-rs/tokio#3149). This branch re-enables vectored writes in Hyper for HTTP/1. Using vectored writes in HTTP/2 will require an upstream change in the `h2` crate as well. I've removed the adaptive write buffer implementation that attempts to detect whether vectored IO is or is not available, since the Tokio 0.3.4 `AsyncWrite` trait exposes this directly via the `is_write_vectored` method. Now, we just ask the IO whether or not it supports vectored writes, and configure the buffer accordingly. This makes the implementation somewhat simpler. This also removes `http1_writev()` methods from the builders. These are no longer necessary, as Hyper can now determine whether or not to use vectored writes based on `is_write_vectored`, rather than trying to auto-detect it. Closes hyperium#2320 BREAKING CHANGE: Removed `http1_writev` methods from `client::Builder`, `client::conn::Builder`, `server::Builder`, and `server::conn::Builder`. Vectored writes are now enabled based on whether the `AsyncWrite` implementation in use supports them, rather than though adaptive detection. To explicitly disable vectored writes, users may wrap the IO in a newtype that implements `AsyncRead` and `AsyncWrite` and returns `false` from its `AsyncWrite::is_write_vectored` method.

io: reintroduce vectored writes

92a4dea

This adds `AsyncWrite::poll_write_vectored`, and implements it for `TcpStream` and `UnixStream`. Based on the proposal in #3135.

seanmonstar requested a review from carllerche November 17, 2020 16:38

seanmonstar added 2 commits November 17, 2020 08:51

fixup is_write_vectored intradoc link

582797d

fixup net feature on poll_write_vectored

6f473ed

seanmonstar mentioned this pull request Nov 17, 2020

RFC: Vectored Writes #3135

Closed

hawkw reviewed Nov 17, 2020

View reviewed changes

taiki-e added A-tokio Area: The main tokio crate M-io Module: tokio/io labels Nov 17, 2020

Matthias247 approved these changes Nov 17, 2020

View reviewed changes

tokio/src/io/poll_evented.rs Show resolved Hide resolved

document that it should be an atomic write

a6238cc

carllerche mentioned this pull request Nov 18, 2020

chore: prepare v0.3.4 release #3152

Merged

carllerche approved these changes Nov 18, 2020

View reviewed changes

carllerche merged commit 34fcef2 into master Nov 18, 2020

carllerche deleted the sean/io-writev branch November 18, 2020 18:41

hawkw mentioned this pull request Nov 19, 2020

util: add writev-aware poll_write_buf #3156

Merged

hawkw mentioned this pull request Nov 19, 2020

feat(lib): re-enable writev support hyperium/hyper#2338

Merged

hawkw mentioned this pull request Nov 20, 2020

Trait AsyncVectoredWrite and related utilities #3092

Closed

bikeshedder mentioned this pull request Nov 20, 2020

Re-enable vectored writes hyperium/hyper#2320

Closed

hawkw mentioned this pull request Nov 20, 2020

re-enable vectored writes hyperium/h2#500

Merged

mzabaluev mentioned this pull request Nov 21, 2020

Efficient implementation of vectored writes for BufWriter #3163

Merged

This was referenced Mar 10, 2021

Bump tokio-util from 0.3.1 to 0.6.4 huitseeker/diem#98

Closed

Bump tokio-util from 0.3.1 to 0.6.4 gdanezis/libra#76

Closed

chore(deps): bump tokio-test from 0.2.1 to 0.4.1 shumin1027/vector#21

Closed

lilyball mentioned this pull request Sep 20, 2021

Support for non-contiguous buffers in tokio_util::codec::Encoder #4122

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

io: reintroduce vectored writes #3149

io: reintroduce vectored writes #3149

seanmonstar commented Nov 17, 2020

hawkw Nov 17, 2020

seanmonstar Nov 17, 2020

hawkw Nov 17, 2020

carllerche Nov 17, 2020

hawkw Nov 17, 2020

hawkw Nov 17, 2020

		/// Like [`poll_write`], except that it writes from a slice of buffers.
		///

		/// The default implementation calls [`poll_write`] with either the first nonempty
		/// buffer provided, or an empty one if none exists.

io: reintroduce vectored writes #3149

io: reintroduce vectored writes #3149

Conversation

seanmonstar commented Nov 17, 2020

hawkw Nov 17, 2020

Choose a reason for hiding this comment

seanmonstar Nov 17, 2020

Choose a reason for hiding this comment

hawkw Nov 17, 2020

Choose a reason for hiding this comment

carllerche Nov 17, 2020

Choose a reason for hiding this comment

hawkw Nov 17, 2020

Choose a reason for hiding this comment

hawkw Nov 17, 2020

Choose a reason for hiding this comment