* Optimize `Func::call` and its C API
This commit is an alternative to #3298 which achieves effectively the
same goal of optimizing the `Func::call` API as well as its C API
sibling of `wasmtime_func_call`. The strategy taken here is different
than #3298 though where a new API isn't created, rather a small tweak to
an existing API is done. Specifically this commit handles the major
sources of slowness with `Func::call` with:
* Looking up the type of a function, to typecheck the arguments with and
use to guide how the results should be loaded, no longer hits the
rwlock in the `Engine` but instead each `Func` contains its own
`FuncType`. This can be an unnecessary allocation for funcs not used
with `Func::call`, so this is a downside of this implementation
relative to #3298. A mitigating factor, though, is that instance
exports are loaded lazily into the `Store` and in theory not too many
funcs are active in the store as `Func` objects.
* Temporary storage is amortized with a long-lived `Vec` in the `Store`
rather than allocating a new vector on each call. This is basically
the same strategy as #3294 only applied to different types in
different places. Specifically `wasmtime::Store` now retains a
`Vec<u128>` for `Func::call`, and the C API retains a `Vec<Val>` for
calling `Func::call`.
* Finally, an API breaking change is made to `Func::call` and its type
signature (as well as `Func::call_async`). Instead of returning
`Box<[Val]>` as it did before this function now takes a
`results: &mut [Val]` parameter. This allows the caller to manage the
allocation and we can amortize-remove it in `wasmtime_func_call` by
using space after the parameters in the `Vec<Val>` we're passing in.
This change is naturally a breaking change and we'll want to consider
it carefully, but mitigating factors are that most embeddings are
likely using `TypedFunc::call` instead and this signature taking a
mutable slice better aligns with `Func::new` which receives a mutable
slice for the results.
Overall this change, in the benchmark of "call a nop function from the C
API" is not quite as good as #3298. It's still a bit slower, on the
order of 15ns, because there's lots of capacity checks around vectors
and the type checks are slightly less optimized than before. Overall
though this is still significantly better than today because allocations
and the rwlock to acquire the type information are both avoided. I
personally feel that this change is the best to do because it has less
of an API impact than #3298.
* Rebase issues
* Load generated trampolines into jitdump when profiling
This commit updates the jitdump profiler to generate JIT profiling
records for generated trampolines in a wasm module in addition to the
functions already in a module. It's also updated to learn about
trampolines generated via `Func::new` and friends. These trampolines
were all not previously registered meaning that stack traces with these
pc values would be confusing to see in the profile output. While the
names aren't the best it should at least be more clear than before if a
function is hot!
* Fix more builds
We've got a large crop of fuzz-bugs from fuzzing with enabled-with-SIMD
on oss-fuzz but at this point the fuzz stats from oss-fuzz say that the
fuzzers like v8 are spending less than 50% of its time actually fuzzing
and presumably mostly hitting crashes and such. While we fix the other
issues this disables simd for fuzzing with v8 so we can try to see if we
can weed out other issues.
* Exports symbols to be shared with external GDB/JIT debugging interface tools.
Windows O/S specific requirement.
* Moved comments into platform specific compiler directive sections.
* Use `__builtin_setjmp` instead of `sigsetjmp`.
Use [`__builtin_setjmp`] instead of `sigsetjmp`, as it is implemented in
the compiler, performed inline, and saves much less state. This speeds up
calls into wasm by about 8% on my machine.
[`__builtin_setjmp`]: https://gcc.gnu.org/onlinedocs/gcc/Nonlocal-Gotos.html
* Add a comment confirming that 5 really is the documented size.
* Add a comment about callee-saved state and __builtin_setjmp.
* On clang on aarch64, use sigsetjmp.
* Fix a stray `#endif`.
- Add relocation handling needed after PR #3275
- Fix incorrect handling of signed constants detected by PR #3056 test
- Fix LabelUse max pos/neg ranges; fix overflow in buffers.rs
- Disable fuzzing tests that require pre-built v8 binaries
- Disable cranelift test that depends on i128
- Temporarily disable memory64 tests
* Use rsix to make system calls in Wasmtime.
`rsix` is a system call wrapper crate that we use in `wasi-common`,
which can provide the following advantages in the rest of Wasmtime:
- It eliminates some `unsafe` blocks in Wasmtime's code. There's
still an `unsafe` block in the library, but this way, the `unsafe`
is factored out and clearly scoped.
- And, it makes error handling more consistent, factoring out code for
checking return values and `io::Error::last_os_error()`, and code that
does `errno::set_errno(0)`.
This doesn't cover *all* system calls; `rsix` doesn't implement
signal-handling APIs, and this doesn't cover calls made through `std` or
crates like `userfaultfd`, `rand`, and `region`.
* allow the ResourceLimiter to reject a memory grow before the
memory's own maximum.
* add a hook so a ResourceLimiter can detect any reason that
a memory grow fails, including if the OS denies additional memory
* add tests for this new functionality. I only took the time to
test the OS denial on Linux, it should be possible on Mac OS
as well but I don't have a test setup. I have no idea how to
do this on windows.
This can be useful for host functions that want to consume fuel to
reflect their relative cost. Additionally it's a relatively easy
addition to have and someone's asking for it!
Closes#3315
We _must not_ trigger a GC when moving refs from host code into
Wasm (e.g. returned from a host function or passed as arguments to a Wasm
function). After insertion into the table, this reference is no longer
rooted. If multiple references are being sent from the host into Wasm and we
allowed GCs during insertion, then the following events could happen:
* Reference A is inserted into the activations table. This does not trigger a
GC, but does fill the table to capacity.
* The caller's reference to A is removed. Now the only reference to A is from
the activations table.
* Reference B is inserted into the activations table. Because the table is at
capacity, a GC is triggered.
* A is reclaimed because the only reference keeping it alive was the activation
table's reference (it isn't inside any Wasm frames on the stack yet, so stack
scanning and stack maps don't increment its reference count).
* We transfer control to Wasm, giving it A and B. Wasm uses A. That's a use
after free.
To prevent uses after free, we cannot GC when moving refs into the
`VMExternRefActivationsTable` because we are passing them from the host to Wasm.
On the other hand, when we are *cloning* -- as opposed to moving -- refs from
the host to Wasm, then it is fine to GC while inserting into the activations
table, because the original referent that we are cloning from is still alive and
rooting the ref.
I'm not sure why when run repeatedly v8 has different limits on
call-stack-size but it's not particularly interesting to assert exact
matches here, so this should fix a fuzz-bug-failure found on oss-fuzz.
Modify the `poll_oneoff_files` test to avoid assuming that `poll_oneoff`
returns all pending events, as it may sometimes return only a subset of
events. When multiple events are expected, use a loop, and loop until
all events have been recorded.
The new backends will not emit a stack map for a safepoint if there are zero
live references. Our fuzzy search for stack maps, which was necessary for the
old backend, caused us to use the wrong stack map for some PCs which would in
turn cause us to treat arbitrary stack slots as reference types pointers.
When debug info was enabled, we would put the debug info sections in between the
text section and the unwind info section. But the unwind info is encoded in a
position-independent manner (so that we don't need relocs for it) that relies on
it directly following the text section. The result of the misplacement was some
crashes inside the unwinder.
We were previously using `_wasmtime_eh_frame` but there is no good reason to
add the prefix Wasmtime-specific prefix. Using the standard name allows for
better inspection with standard tools like `dwarfdump`.
This commit adds an assertion which was previously forgotten when
inserting a `HostFunc` into a `Store`. This can happen when a `Linker`
is defined with one engine but it's used to interoperate with a store
defined within a different engine.
A function contains type information that's only valid relative to the
engine that it was defined within. This means that if a function is used
within a different engine then type information may look valid when in
fact it is not. For example it's otherwise possible to insert a function
into an engine with one type and call it in a different engine with a
different type.
Similar to how `Store` misuse is a panic throughout `wasmtime`'s API
this commit also turns this behavior into panic, so there's no API
impact. Documentation has been updated accordingly to indicate that
various functions on `Linker` will panic if a `store` is provided that's
connected to a different `Engine`.
This commit improves the runtime support for wasm-to-host invocations
for functions created with `Func::new` or `wasmtime_func_new` in the C
API. Previously a `Vec` (sometimes a `SmallVec`) would be dynamically
allocated on each host call to store the arguments that are coming from
wasm and going to the host. In the case of the `wasmtime` crate we need
to decode the `u128`-stored values, and in the case of the C API we need
to decode the `Val` into the C API's `wasmtime_val_t`.
The technique used in this commit is to store a singular `Vec<T>` inside
the "store", be it the literal `Store<T>` or within the `T` in the case
of the C API, which can be reused across wasm->host calls. This means
that we're unlikely to actually perform dynamic memory allocation and
instead we should hit a faster path where the `Vec` always has enough
capacity.
Note that this is just a mild improvement for `Func::new`-based
functions. It's still the case that `Func::wrap` is much faster, but
unfortunately the C API doesn't have access to `Func::wrap`, so the main
motivation here is accelerating the C API.
* Refactor the internals of `Store<T>`
This commit is an overdue refactoring and renaming of some internals of
the `Store` type in Wasmtime. The actual implementation of `Store<T>`
has evolved from the original implementation to the point where some of
the aspects of how things are structured no longer makes sense. There's
also always been a lot of unnecessary gymnastics when trying to get
access to various store pieces depending on where you are in `wasmtime`.
This refactoring aims to simplify all this and make the internals much
easier to read/write. The following changes were made:
* The `StoreOpaque<'_>` type is deleted, along with the `opaque()`
method.
* The `StoreInnermost` type was renamed to `StoreOpaque`.
`StoreOpaque<'_>` is dead. Long live `StoreOpaque`. This renaming
and a few small tweaks means that this type now suffices for all
consumers.
* The `AsContextMut` and `AsContext` traits are now implemented for
`StoreInner<T>`.
These changes, while subtly small, help clean up a lot of the internals
of `wasmtime`. There's a lot less verbose `&mut
store.as_context_mut().opaque()` now. Additionally many methods can
simply start with `let store = store.as_context_mut().0;` and use things
internally. One of the nicer aspects of using references directly is
that the compiler automatically reborrows references as necessary
meaning there's lots of less manual reborrowing.
The main motivation for this change was actually somewhat roundabout
where I found that when `StoreOpaque<'_>` was being captured in closures
and iterators it's 3 pointers wide which is a lot of data to move
around. Now things capture over `&mut StoreOpaque` which is just one
nice and small pointer to move around. In any case though I've long
wanted to revisit the design of these internals to improve the
ergonomics. It's not expected that this change alone will really have
all that much impact on the performance of `wasmtime`.
Finally a doc comment was added to `store.rs` to try to explain all the
`Store`-related types since there are a nontrivial amount.
* Rustfmt
This commit optimizes the runtime execution of `Func::new` by removing
an indirect function call that happens whenever a host function is
called. This indirection was generally done to prevent monomoprhizing a
lot into consumer code but the few extra functions this makes
monomorphic are fairly small, and in general wasm->host call performance
is pretty important.
While not a massive win this is expected to improve codegen, especially
because with the indirect call removed the compiler should now be able
to prove more often when a `Func::new` closure doesn't panic or return
an error.
Wasmtime has a few representations of `Val` and `ValType` across the
internal crates, the `wasmtime` crate, and the C API. These were
previously sometimes mentioned in different orders which means that
converting between the two took a little extra code than before. This
commit is a micro-optimization to align the types across the various
places we define these to help reduce the codegen burden when converting
between these types.
This is not expected to have a major impact on performance, rather it's
a small cleanup which should be easy-ish to preserve I've noticed while
staring at assembly.
The `()` type accidentally wasn't getting its trivial type conversions
inlined because it doesn't actually have any type parameters. This
commit adds `#[inline]` to the relevant functions to ensure that these
get inlined across crates.
Currently wasm-calls work with `Result<T, Trap>` internally but `Trap`
is an enum defined in `wasmtime-runtime` which is actually quite large.
Since traps are supposed to be rare this commit changes these functions
to return a `Box<Trap>` which is un-boxed later up in the `wasmtime`
crate within a `#[cold]` function.
which is more expressive than the former.
Instead of just configuring Module::deserialize to ignore version
information, we can configure Module::serialize to emit a custom version
string, and Module::deserialize to check for that string. A new enum
ModuleVersionStrategy is declared, and
Config::deserialize_check_wasmtime_version:bool is replaced with
Config::module_version:ModuleVersionStrategy.
* Use relative `call` instructions between wasm functions
This commit is a relatively major change to the way that Wasmtime
generates code for Wasm modules and how functions call each other.
Prior to this commit all function calls between functions, even if they
were defined in the same module, were done indirectly through a
register. To implement this the backend would emit an absolute 8-byte
relocation near all function calls, load that address into a register,
and then call it. While this technique is simple to implement and easy
to get right, it has two primary downsides associated with it:
* Function calls are always indirect which means they are more difficult
to predict, resulting in worse performance.
* Generating a relocation-per-function call requires expensive
relocation resolution at module-load time, which can be a large
contributing factor to how long it takes to load a precompiled module.
To fix these issues, while also somewhat compromising on the previously
simple implementation technique, this commit switches wasm calls within
a module to using the `colocated` flag enabled in Cranelift-speak, which
basically means that a relative call instruction is used with a
relocation that's resolved relative to the pc of the call instruction
itself.
When switching the `colocated` flag to `true` this commit is also then
able to move much of the relocation resolution from `wasmtime_jit::link`
into `wasmtime_cranelift::obj` during object-construction time. This
frontloads all relocation work which means that there's actually no
relocations related to function calls in the final image, solving both
of our points above.
The main gotcha in implementing this technique is that there are
hardware limitations to relative function calls which mean we can't
simply blindly use them. AArch64, for example, can only go +/- 64 MB
from the `bl` instruction to the target, which means that if the
function we're calling is a greater distance away then we would fail to
resolve that relocation. On x86_64 the limits are +/- 2GB which are much
larger, but theoretically still feasible to hit. Consequently the main
increase in implementation complexity is fixing this issue.
This issue is actually already present in Cranelift itself, and is
internally one of the invariants handled by the `MachBuffer` type. When
generating a function relative jumps between basic blocks have similar
restrictions. This commit adds new methods for the `MachBackend` trait
and updates the implementation of `MachBuffer` to account for all these
new branches. Specifically the changes to `MachBuffer` are:
* For AAarch64 the `LabelUse::Branch26` value now supports veneers, and
AArch64 calls use this to resolve relocations.
* The `emit_island` function has been rewritten internally to handle
some cases which previously didn't come up before, such as:
* When emitting an island the deadline is now recalculated, where
previously it was always set to infinitely in the future. This was ok
prior since only a `Branch19` supported veneers and once it was
promoted no veneers were supported, so without multiple layers of
promotion the lack of a new deadline was ok.
* When emitting an island all pending fixups had veneers forced if
their branch target wasn't known yet. This was generally ok for
19-bit fixups since the only kind getting a veneer was a 19-bit
fixup, but with mixed kinds it's a bit odd to force veneers for a
26-bit fixup just because a nearby 19-bit fixup needed a veneer.
Instead fixups are now re-enqueued unless they're known to be
out-of-bounds. This may run the risk of generating more islands for
19-bit branches but it should also reduce the number of islands for
between-function calls.
* Otherwise the internal logic was tweaked to ideally be a bit more
simple, but that's a pretty subjective criteria in compilers...
I've added some simple testing of this for now. A synthetic compiler
option was create to simply add padded 0s between functions and test
cases implement various forms of calls that at least need veneers. A
test is also included for x86_64, but it is unfortunately pretty slow
because it requires generating 2GB of output. I'm hoping for now it's
not too bad, but we can disable the test if it's prohibitive and
otherwise just comment the necessary portions to be sure to run the
ignored test if these parts of the code have changed.
The final end-result of this commit is that for a large module I'm
working with the number of relocations dropped to zero, meaning that
nothing actually needs to be done to the text section when it's loaded
into memory (yay!). I haven't run final benchmarks yet but this is the
last remaining source of significant slowdown when loading modules,
after I land a number of other PRs both active and ones that I only have
locally for now.
* Fix arm32
* Review comments
This makes Cranelift use the Rust `alloc` API its allocations,
rather than directly calling into `libc`, which makes it respect
the `#[global_allocator]` configuration.
Also, use `region::page::ceil` instead of having our own copies of
that logic.
- Fixes for compiling on OpenBSD
- io-lifetimes 0.3.0 has an option (io_lifetimes_use_std, which is off
by default) for testing the `io_safety` feature in Rust nightly.
