Wasmtime GitHub notifications bot (Nov 03 2023 at 16:14): Wasmtime GitHub notifications bot (Nov 03 2023 at 16:14):alexcrichton requested pchickey for a review on PR #7475.
Wasmtime GitHub notifications bot (Nov 03 2023 at 16:14):alexcrichton requested elliottt for a review on PR #7475.
Wasmtime GitHub notifications bot (Nov 03 2023 at 16:14):alexcrichton requested wasmtime-core-reviewers for a review on PR #7475.
Wasmtime GitHub notifications bot (Nov 03 2023 at 16:14):alexcrichton opened PR #7475 from alexcrichton:http-direct-read to bytecodealliance:main:
This commit is in the same spirit as #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 serveto 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 #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
pollisn't being actively watched unlesssubscribe's result is being actively watched via a wasm call towasi: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.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Nov 03 2023 at 16:28):alexcrichton created PR review comment:
I'll note that this, and the above, are somewhat unrelated but also tangential changes since I was touching these types. The above task previously dropped the error and this task tried to forward it along. In both cases though the result of this task was never actually looked at as the handle was exclusively used to keep the task alive and that's it.
I've updated this so both tasks consistently drop the error. I don't know how to communicate this error through the WASI APIs myself.
Wasmtime GitHub notifications bot (Nov 03 2023 at 16:28):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Nov 03 2023 at 22:03):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Nov 03 2023 at 22:50):pchickey merged PR #7475.
Last updated: Dec 23 2024 at 13:07 UTC