Refactor the representation of `Func` #10897

alexcrichton · 2025-06-02T18:34:01Z

This commit rewrites the internals of wasmtime::Func. The Stored type is no longer used meaning that it's now free to create a wasmtime::Func at any time. It is effectively a store-tagged NonNull<VMFuncRef>. This required a few internal changes to how functions are passed around:

Previously the insertion of a wasm_call-less VMFuncRef was deferred until the raw pointer was loaded. Now the insertion happens immediately as soon as the function is placed within a store. This is done to ensure that a Func corresponds to one exact VMFuncRef and that's it, and lazily filled in versions within the store are still lazily filled in but they're more eagerly allocated. This isn't expected to have much of an impact perf-wise since all these lazily allocated functions were already almost guaranteed to get lazily allocated anyway.
Filling in "holes" in VMFuncRef, notably the wasm_call field, no longer happens lazily when the VMFuncRef is demanded. Instead during instantiation a pass is made to fill in holes with the new module being instantiated (after registration). Additionally when a lazily-allocated VMFuncRef is created the module registry is checked immediately. This means that all store-local VMFuncRef values are either filled in immediately or filled in during instantiation. This notably means that the previous logic in Func::vmimport is now "just" an .unwrap() with a lot of comments saying why the unwrap shouldn't panic.
To implement this commit a previous optimization for the Func API was removed as well, namely Func::call will become slower after this commit. The Func::call API is a dynamically-typed API which requires run-time type-checking of arguments. Previously a FuncType was loaded into a cache once-per-Func which helped amortize the cost of using Func::call repeatedly. Now, though, there's no natural place to put such a cache since Func no longer has dedicated storage within a Store. Historically this optimization was added for the C API before the *_call_unchecked APIs existed, but nowadays the *_call_unchecked APIs should suffice for performance-critical applications where needed. In the future it might also be possible to have a hash map in the Store of a VMSharedTypeIndex to FuncType which is lazily populated based on calls to Func::call, but that feels a bit overkill nowadays for a possibly rarely-used map.
The no-longer-necessary RootedHostFunc type is now gone as its unsafety and various contracts are subsumed by other preexisting unsafe blocks.
The specific drop order between StoreOpaque::store_data and StoreOpaque::rooted_host_funcs is removed. The rooted_host_funcs field now lives in the func_refs field and the FuncKind type, where the destructors came from before, is no more.
The func_refs field has grown storage locations for a variety of "keep this thing alive as long as the store" related to functions and such.

Closes #10868

This commit rewrites the internals of `wasmtime::Func`. The `Stored` type is no longer used meaning that it's now free to create a `wasmtime::Func` at any time. It is effectively a store-tagged `NonNull<VMFuncRef>`. This required a few internal changes to how functions are passed around: * Previously the insertion of a `wasm_call`-less `VMFuncRef` was deferred until the raw pointer was loaded. Now the insertion happens immediately as soon as the function is placed within a store. This is done to ensure that a `Func` corresponds to one exact `VMFuncRef` and that's it, and lazily filled in versions within the store are still lazily filled in but they're more eagerly allocated. This isn't expected to have much of an impact perf-wise since all these lazily allocated functions were already almost guaranteed to get lazily allocated anyway. * Filling in "holes" in `VMFuncRef`, notably the `wasm_call` field, no longer happens lazily when the `VMFuncRef` is demanded. Instead during instantiation a pass is made to fill in holes with the new module being instantiated (after registration). Additionally when a lazily-allocated `VMFuncRef` is created the module registry is checked immediately. This means that all store-local `VMFuncRef` values are either filled in immediately or filled in during instantiation. This notably means that the previous logic in `Func::vmimport` is now "just" an `.unwrap()` with a lot of comments saying why the unwrap shouldn't panic. * To implement this commit a previous optimization for the `Func` API was removed as well, namely `Func::call` will become slower after this commit. The `Func::call` API is a dynamically-typed API which requires run-time type-checking of arguments. Previously a `FuncType` was loaded into a cache once-per-`Func` which helped amortize the cost of using `Func::call` repeatedly. Now, though, there's no natural place to put such a cache since `Func` no longer has dedicated storage within a `Store`. Historically this optimization was added for the C API before the `*_call_unchecked` APIs existed, but nowadays the `*_call_unchecked` APIs should suffice for performance-critical applications where needed. In the future it might also be possible to have a hash map in the `Store` of a `VMSharedTypeIndex` to `FuncType` which is lazily populated based on calls to `Func::call`, but that feels a bit overkill nowadays for a possibly rarely-used map. * The no-longer-necessary `RootedHostFunc` type is now gone as its unsafety and various contracts are subsumed by other preexisting `unsafe` blocks. * The specific drop order between `StoreOpaque::store_data` and `StoreOpaque::rooted_host_funcs` is removed. The `rooted_host_funcs` field now lives in the `func_refs` field and the `FuncKind` type, where the destructors came from before, is no more. * The `func_refs` field has grown storage locations for a variety of "keep this thing alive as long as the store" related to functions and such. Closes bytecodealliance#10868

alexcrichton · 2025-06-02T18:34:14Z

I'll also note that this is split out of #10870

fitzgen

Very nice!!

crates/wasmtime/src/runtime/store/func_refs.rs

fitzgen · 2025-06-02T21:14:22Z

crates/wasmtime/src/runtime/func.rs

+// Double-check that the C representation in `extern.h` matches our in-Rust
+// representation here in terms of size/alignment/etc.
+const _: () = {
+    #[repr(C)]
+    struct C(u64, *mut u8);
+    assert!(core::mem::size_of::<C>() == core::mem::size_of::<Func>());
+    assert!(core::mem::align_of::<C>() == core::mem::align_of::<Func>());
+    assert!(core::mem::offset_of!(Func, store) == 0);
+};


Kind of an aside but should all of these assertions actually live in the wasmtime-c-api crate so that we don't need to have a dummy struct which itself needs to be kept in sync with the struct that we actually care about? We've removed one syncing problem but added another, so we haven't really changed anything. I guess the only assertion that would remain in the wasmtime crate would be the one for the offset of the store field.

Oh I think you might be assuming there's a struct wasmtime_func_t { ... } in Rust in crates/c-api but that doesn't exist. In that sense this struct C is the only C-version-written-in-Rust anywhere in the codebase, and I figured here was a good as place as any to test things out.

Ah, I was indeed

fitzgen · 2025-06-02T21:14:51Z

crates/wasmtime/src/runtime/func.rs

-/// The ways that a function can be created and referenced from within a store.
-enum FuncKind {
-    /// A function already owned by the store via some other means. This is
-    /// used, for example, when creating a `Func` from an instance's exported
-    /// function. The instance's `InstanceHandle` is already owned by the store
-    /// and we just have some pointers into that which represent how to call the
-    /// function.
-    StoreOwned { export: ExportFunction },
-
-    /// A function is shared across possibly other stores, hence the `Arc`. This
-    /// variant happens when a `Linker`-defined function is instantiated within
-    /// a `Store` (e.g. via `Linker::get` or similar APIs). The `Arc` here
-    /// indicates that there's some number of other stores holding this function
-    /// too, so dropping this may not deallocate the underlying
-    /// `InstanceHandle`.
-    SharedHost(Arc<HostFunc>),
-
-    /// A uniquely-owned host function within a `Store`. This comes about with
-    /// `Func::new` or similar APIs. The `HostFunc` internally owns the
-    /// `InstanceHandle` and that will get dropped when this `HostFunc` itself
-    /// is dropped.
-    ///
-    /// Note that this is intentionally placed behind a `Box` to minimize the
-    /// size of this enum since the most common variant for high-performance
-    /// situations is `SharedHost` and `StoreOwned`, so this ideally isn't
-    /// larger than those two.
-    Host(Box<HostFunc>),
-
-    /// A reference to a `HostFunc`, but one that's "rooted" in the `Store`
-    /// itself.
-    ///
-    /// This variant is created when an `InstancePre<T>` is instantiated in to a
-    /// `Store<T>`. In that situation the `InstancePre<T>` already has a list of
-    /// host functions that are packaged up in an `Arc`, so the `Arc<[T]>` is
-    /// cloned once into the `Store` to avoid each individual function requiring
-    /// an `Arc::clone`.
-    ///
-    /// The lifetime management of this type is `unsafe` because
-    /// `RootedHostFunc` is a small wrapper around `NonNull<HostFunc>`. To be
-    /// safe this is required that the memory of the host function is pinned
-    /// elsewhere (e.g. the `Arc` in the `Store`).
-    RootedHost(RootedHostFunc),
-}


Sooooooo nice to get rid of FuncKind

crates/wasmtime/src/runtime/func.rs

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>

alexcrichton requested a review from fitzgen June 2, 2025 18:34

alexcrichton requested a review from a team as a code owner June 2, 2025 18:34

Remove mutability from locations that no longer need it

125a874

github-actions bot added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Jun 2, 2025

fitzgen approved these changes Jun 2, 2025

View reviewed changes

alexcrichton and others added 2 commits June 3, 2025 01:03

Update crates/wasmtime/src/runtime/func.rs

e6002b7

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>

Update crates/wasmtime/src/runtime/store/func_refs.rs

2c74449

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>

alexcrichton enabled auto-merge June 2, 2025 23:04

alexcrichton added this pull request to the merge queue Jun 2, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 2, 2025

alexcrichton added this pull request to the merge queue Jun 2, 2025

alexcrichton removed this pull request from the merge queue due to a manual request Jun 2, 2025

alexcrichton added this pull request to the merge queue Jun 2, 2025

Merged via the queue into bytecodealliance:main with commit 4fcfe17 Jun 3, 2025
43 checks passed

alexcrichton deleted the refactor-func-representation branch June 3, 2025 00:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor the representation of `Func` #10897

Refactor the representation of `Func` #10897

alexcrichton commented Jun 2, 2025

Uh oh!

alexcrichton commented Jun 2, 2025 •

edited

Loading

Uh oh!

fitzgen left a comment

Uh oh!

Uh oh!

fitzgen Jun 2, 2025

Uh oh!

alexcrichton Jun 2, 2025

Uh oh!

fitzgen Jun 2, 2025

Uh oh!

fitzgen Jun 2, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Refactor the representation of Func #10897

Refactor the representation of Func #10897

Conversation

alexcrichton commented Jun 2, 2025

Uh oh!

alexcrichton commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fitzgen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fitzgen Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

alexcrichton Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

fitzgen Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

fitzgen Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Refactor the representation of `Func` #10897

Refactor the representation of `Func` #10897

alexcrichton commented Jun 2, 2025 •

edited

Loading