* Remove a writer task in wasi-http
This commit updates the wasi-http `BodyWriteStream` implementation to
avoid an extra task spawned to manage the outgoing body. Some local
profiling shows that a "hello world" component currently maxes out at
500rps locally which is suprisingly low.
Some investigation shows that one of the primary reasons for this is
concurrency-related issues. For example `taskset -c 1 wasmtime serve ...`
has a significantly better rps than without `taskset`. My hypothesis is
that the extra tasks for both reading the incoming body and writing the
outgoing body are introducing opportunities for the guest to block and
get work-stolen to another thread in the "happy path" where reads/writes
are immediately ready and don't need blocking.
For example writing to the body currently sends the chunk of bytes to a
task which then sends it to another task. This means that if a guest
writes some bytes and then waits for readiness it'll synchronize on
these bytes by doing a round-trip with another task. This opportunity
for blocking apparently lets something, be it Linux or the Tokio
scheduler, to work-steal the wasm task to another thread.
Once a wasm task is work-stolen to another thread it can significantly
increase the cost of VM management syscalls because any happy path the
kernel might have for a single thread is no longer applicable, meaning
that many costs start shooting through the roof.
The fix here in this PR is to remove a helper task when writing HTTP
bodies. This extra task didn't end up doing much and was largely just a
proxy into a channel, so the channel is now directly manipulated
instead. Namely the `flush` implementation in the old worker task didn't
actually do anything and so the only thing that needs to be managed in
the stream is sending chunks along through the channel. This proved
relatively easy and enables the "happy path" where if a body is written
and then flushed it's immediately "ready" so no opportunity for a
context switch happens and wasm stays planted on its thread.
The end result of this PR is that `wasmtime serve` goes from 500rps to
5000rps (10x increase) for the simple "hello world" example that I'm
working with.
* Don't buffer up a pending message
* PCC: refactor heap memory-type creation to allow for use of GVs in facts.
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* PCC: WIP: add some facts for dynamic memories.
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* PCC: WIP: add fact to loaded length on dynamic memories.
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* PCC: add facts for dynamic bounds-checks and extend PCC verification as needed.
This commit also adds an integration test to Wasmtime that checks all
dynamic and static memory cases on x86-64 and aarch64.
Test expected-output changes are due to a change from `iadd_imm(x, -k)`
to `isub(x, iconst(k))` in bounds-check code to facilitate facts on the
computation.
* turn off trace-log in cranelift-codegen.
* rustfmt
* Review feedback.
* Avoid unused-item warnings on s390x/riscv64 in pcc test.
* ignore PCC test in miri run.
---------
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* Rename header-error to validation-error
* Make outbound-request setters fallible
* Add additional tests for setters
* Revert changes to header-error, and use a plain `result` for validation
* Doc fix for fallible setters
* Import fussing
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 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 #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.
* Print the error from the wasi-http incoming handler to stderr
Prior to this change, the error was never printed out, making it a challenge to figure out what caused the error
* use log::error instead of eprintln
Co-authored-by: Trevor Elliott <awesomelyawesome@gmail.com>
* Update src/commands/serve.rs
---------
Co-authored-by: Trevor Elliott <awesomelyawesome@gmail.com>
* Enable copy-on-write memories for components
This commit fixes a mistake in component compilation where copy-on-write
and lazy initialization of function tables was accidentally not
performed. I believe this is an accidental regression from a previous
refactor, and this commit now ensures that the shared infrastructure
between components and core modules accounts for copy-on-write and lazy
table initialization.
* Add some tests
This change is a follow up to https://github.com/bytecodealliance/wasmtime/pull/7443;
after it landed I realized that Winch doesn't include spec tests for
local.get and loca.set.
Those tests uncovered a bug on the handling of the constant pool: given
Winch's singlepass nature, there's very little room know all the
constants ahead of time and to register them all at once at emission
time; instead they are emitted when they are needed by an instruction.
Even though Cranelift's machinery is capable of deuplicated constants in
the pool, `register_constant` assumes and checks that each constat
should only be pushed once. In Winch's case, since we emit as we go, we
need to carefully check if the constant is one was not emitted before,
and if that's the case, register it. Else we break the invariant that
each constant should only be registered once.
This allows clangd to not import the headers in the wasmtime directory,
as not all of them include their dependencies correctly. This means that
clangd can import a header directly and it breaks the build, as the
order `wasmtime.h` #includes is important.
See: https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
* Enable `wasmtime/wat` when the `wat` feature is enabled on the CLI
* Additionally always enable the `wasmtime/wat` feature for the
`wasmtime-wast` crate since the text format is used by some tests.
Closes#7460
* aarch64: Add support for `tbz` and `tbnz`
I noticed these instructions when glancing at some disassembly for other
code and also noticed that Cranelift didn't have support for them. This
adds a few new lowerings for conditional branches in the aarch64 backend
which are for testing a bit and branching if it's zero or not zero.
* Review comments on naming
This is useful as the last step of component pre-initialization with
e.g. [component-init](https://github.com/dicej/component-init), in which case we
want the adapter to forget about any open handles it has (and force it to
re-request the stdio handles next time they're needed) since it will be talking
to a brand new host at runtime.
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
* egraphs: Remove extends and reduces to the shift input in shift instructions
We support variations of this instruction with different inputs, so lets take advantage of this where possible.
* egraphs: Remove `iconcat` as a shift input
* Fix writes being flushed to stdio
This commit fixes an accidental issue with writing to stdout with the
`wasi-common` implementation where writes were line-buffered by the Rust
standard library rather than being flushed out immediately as WASI
semantics required.
Closes#7437
* Fix warning in test
GitHub CI runners are showing some strange behavior: on certain runners
(unknown which ones), the CPUID bits claim that MPK is supported, but
running any MPK code (e.g., `RDPKRU`) causes a `SIGILL` crash. This
change disables MPK until #7445 is resolved.
In #7239 we added a `tracing-log` subscriber that prints logs to stderr
if enabled with an environment variable. It included logic to add ANSI
color sequences when stderr is a terminal, for legibility. Unfortunately
it seems that while this logic *enabled* colors on a terminal, it did
not *disable* colors on a non-terminal; so redirects of stderr to a file
would result in ANSI color sequences being captured in that file.
Specifically, the builder seems not to default to no-color; so rather
than enable-or-nothing, we should explicitly enable or disable always.
Fixes#7435.
* Fix flakiness in tcp_bind test
This commit adds another case to handle in the TCP bind test which binds
a specific port to handle concurrent invocations of the test.
Closes#7429
* Add comments
* update wasi:io/poll wit to https://github.com/WebAssembly/wasi-io/pull/54
* put version in wit package name
* implement changes to the wits
* move contents of Host::poll_one to HostPollable::block
* rename Host::poll_list to Host::poll,
* implement HostPollable::ready, using futures::future::poll_immediate
* wit: fix reference to poll-list
* wasi-http wit: fix reference to poll-list
* clocks implementation: ready returns immediately if deadline has past
this is an optimization, but what it really allows us to do is assert
pollable.ready() for a subscribe_duration(0) is ready immediately.
* component adapter: rename poll-list to poll
* test-programs: renames to poll functions
test-programs/src/bin/preview2_sleep.rs in particular now asserts
ready() on a subscribe_duration(0) and a subscribe_instant(now() - 1),
so we have test coverage for ready as well now
* code review
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
---------
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
This commit fixes logging under `-D log-to-files` mode to not panic when
logging happens in WASI-spawned thread which won't have initialization
for the logger invoked already.
Closes#7418
* Turn `request-options` into a resource
* Fallible setters
* Store timeouts as std::time::Duration
* Trap when conversion from millis to u32 would fail
* Switch to clocks/monotonic-clock/duration for timeouts
In Wasmtime 13 and prior the `--dir` argument was unconditionally used
to open a host dir as the same name and in the guest. In Wasmtime 14+
though this argument is being repurposed with an optional trailing
`::GUEST` to configure the guest directory. This means that
`--dir`-with-remapping behavior is actually unusable without the
environment variable configuration from #7385 due to it parsing
differently in the old and the new.
This commit updates the situation by adding `::`-parsing to the old CLI
meaning that both the old and the new parse this the same way. This will
break any scripts that open host directories with two colons in their
path, but that seems niche enough we can handle that later.
* monotonic clock: introduce `duration` type, split `subscribe`
We are introducing a `duration` type because it has a distinct meaning
from `instant`: an `instant` can only be compared to other `instant`s
from the exact same `monotonic-clock`, whereas a `duration` represents
a duration of time which can be compared to any other duration of time.
The `duration` type is motivated, in part, by a desire to reuse it to
specify durations such as timeouts in other WASI proposals.
Instead of taking a boolean specifying whether the u64 is an absolute or
relative time, `subscribe-instant` takes an `instant` type and
`subscribe-duration` takes a `duration` type.
* wasmtime-wasi: implement changes to monotonic clock interface
* adapter: fixes for subscribe duration and instant
* factor subscribe_to_duration out into a helper function.
* sync wits into wasi-http
* component adapter: fix for interpreting flag for monotonic instant vs duration
* subscribe_instant impl: fix logic for when time is in the past
caught by subtract overflow panic, thankfully
* test-programs: compile against new subscribe instant, duration methods
* Update wasm-tools crates
This commit updates the wasm-tools family of crate for a number of
notable updates:
* bytecodealliance/wasm-tools#1257 - wasmparser's ID-based
infrastructure has been refactored to have more precise types for each
ID rather than one all-purpose `TypeId`.
* bytecodealliance/wasm-tools#1262 - the implementation of
"implementation imports" for the component model which both updates
the binary format in addition to adding more syntactic forms of
imports.
* bytecodealliance/wasm-tools#1260 - a new encoding scheme for component
information for `wit-component` in objects (not used by Wasmtime but
used by bindings generators).
Translation for components needed to be updated to account for the first
change, but otherwise this was a straightforward update.
* Remove a TODO
* Attempt to automatically configure release notes
This commit is an attempt to tackle #7068 by configuring the release
notes in Github Releases with the handwritten release notes from
`RELEASES.md`. The basic idea here is to split the markdown file on
`-----` delimiters and then find the one which matches the version being
released. Once one is found the `body` field of the API call to create
the release is configured.
* Update .github/actions/github-release/main.js
Co-authored-by: Andrew Brown <andrew.brown@intel.com>
---------
Co-authored-by: Andrew Brown <andrew.brown@intel.com>
Alex Crichton
Roman Volosatovs
Afonso Bordado
Chris Fallin
Trevor Elliott
Jeffrey Charles
Jake Champion
Saúl Cabrera
Tyler Rockwood
Nick Fitzgerald
Joel Dice
Andrew Brown
Ryan Levick
Pat Hickey
Simonas Kazlauskas
Dave Bakker