feat(lib): re-enable writev support #2338
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I have no idea why CI won't start... (also, super minor conflict in |
seanmonstar
reviewed
Nov 20, 2020
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>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Apparently, we actually hit this when the user explicitly enables vectored writes but the IO doesn't return `true` from `is_write_vectored`. This kind of begs the question, do those APIs still make sense? Is there ever a reason to enable writev on an IO that doesn't return `true` from `is_write_vectored`? I think that *disabling* vectored writes explicitly when the IO supports them makes sense, but I'm unconvinced about *enabling* them when it doesn't. @seanmonstar any thoughts? Signed-off-by: Eliza Weisman <eliza@buoyant.io>
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. 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.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
also, consolidate the magic numbers. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
changed the title
|
|
hawkw
commented
Nov 24, 2020
now that hyperium/h2#500 has merged, we can switch this back to master
seanmonstar
approved these changes
Nov 24, 2020
BenxiangGe
pushed a commit
to BenxiangGe/hyper
that referenced
this pull request
Jul 26, 2021
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tokio's
AsyncWritetrait once again has support for vectored writes inTokio 0.3.4 (see tokio-rs/tokio#3149.
This branch re-enables vectored writes in Hyper. Using vectored writes
in HTTP/2 requires a git dependency on a
h2branch that adds writevsupport in
h2.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
AsyncWritetrait exposes this directly via theis_write_vectoredmethod. Now, we just ask the IO whether or not itsupports vectored writes, and configure the buffer accordingly. This
makes the implementation somewhat simpler.
This makes a pretty noticeable performance improvement in the
HTTP/1 end to end benchmarks.
Before:
...and after:
Signed-off-by: Eliza Weisman eliza@buoyant.io
BREAKING CHANGE:
Removed
http1_writevmethods fromclient::Builder,client::conn::Builder,server::Builder, andserver::conn::Builder.Vectored writes are now enabled based on whether the
AsyncWriteimplementation in use supports them, rather than though adaptive
detection. To explicitly disable vectored writes, users may wrap the IO
in a newtype that implements
AsyncReadandAsyncWriteand returnsfalsefrom itsAsyncWrite::is_write_vectoredmethod.