wasi-http: Remove extra task when reading bodies #1
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
alexcrichton
force-pushed
the
no-read-task
branch
from
1cc77c1 to
68a2d45
Compare
lann
approved these changes
Nov 8, 2023
|
UGH #2 conflicted with this; sorry... |
|
No worries! I'll work on rebasing |
This commit is in the same spirit as bytecodealliance#7461 and is aimed at removing an extra task when interoperating between WebAssembly and Hyper. The intention here is to ensure that when data is ready immediately a context switch between Tokio tasks isn't necessary. I'm still not sure why this context switch would tank performance so much. The goal of this commit is to remove the Tokio task associated with reading bodies. Currently a task is spawned with two channels coming out: one for frames and one for trailers. Instead the body is now read directly when data frames are read and trailers are managed similarly. The interactions here are somewhat nontrivial due to the management of resources and how the body needs to jump from the body stream object to the trailers object, but it's overall not the worst thing in the world. Locally on Linux this takes a "hello world" component which starts out by reading its HTTP body and then responds with a string from 6.7k rps locally via `wasmtime serve` to 16.8k rps. This restores the performance of wasi-http to be in-line with the historical SDK that Fermyon offered, for example, that's based on a custom HTTP interface. Note that these performance numbers use bytecodealliance#7461 as a baseline. This commit has a downside, however, that if the incoming body is not being subscribed to actively by the wasm then no activity will happen on the host. This is a function of `poll` isn't being actively watched unless `subscribe`'s result is being actively watched via a wasm call to `wasi:io/poll`. Manual investigation of Hyper shows that this is probably ok for now since bodies are primarily channel-based which seems like the tasks doing the actual I/O are managed elsewhere. In that sense this shouldn't cause any immediate impact, but Hyper's situation in the future could change or my analysis could additionally be wrong. Given the performance benefits here though I'd be tempted to push in favor of this commit and handle "poll in the background" as necessary in the future if needed. Signed-off-by: Alex Crichton <alex@alexcrichton.com>
alexcrichton
force-pushed
the
no-read-task
branch
from
68a2d45 to
43d67e6
Compare
alexcrichton
merged commit 69aa452
into
fermyon:backport-14-0-4/wasi-http-tasks
1 check passed
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
This is a backport of bytecodealliance#7475 to the 14.0.x branch. Note that this can't go upstream because it contains a large number of API-breaking changes, but for our use case that should be fine.