Make table/memory creation async functions by alexcrichton · Pull Request #11470 · bytecodealliance/wasmtime

alexcrichton · 2025-08-19T23:52:17Z

This commit is a large-ish refactor which is made possible by the many
previous refactorings to internals w.r.t. async-in-Wasmtime. The end
goal of this change is that table and memory allocation are both async
functions. Achieving this, however, required some refactoring to enable
it to work:

To work with Send neither function can close over dyn VMStore.
This required changing their Option<&mut dyn VMStore> arugment to
Option<&mut StoreResourceLimiter<'_>>
Somehow a StoreResourceLimiter needed to be acquired from an
InstanceAllocationRequest. Previously the store was stored here as
an unsafe raw pointer, but I've refactored this now so
InstanceAllocationRequest directly stores &StoreOpaque and
Option<&mut StoreResourceLimiter> meaning it's trivial to acquire
them. This additionally means no more unsafe access of the store
during instance allocation (yay!).
Now-redundant fields of InstanceAllocationRequest were removed since
they can be safely inferred from &StoreOpaque. For example passing
around &Tunables is now all gone.
Methods upwards from table/memory allocation to the
InstanceAllocator trait needed to be made async. This includes new
#[async_trait] methods for example.
StoreOpaque::ensure_gc_store is now an async function. This
internally carries a new unsafe block carried over from before with
the raw point passed around in InstanceAllocationRequest. A future
PR will delete this unsafe block, it's just temporary.

I attempted a few times to split this PR up into separate commits but
everything is relatively intertwined here so this is the smallest
"atomic" unit I could manage to land these changes and refactorings.

cc #11430

alexcrichton · 2025-08-19T23:53:35Z

I'll note that I've split this into two commits, the first of which is #11456 resurrected here. That commit cannot be split out to a second PR due to all the various constraints in play unfortunately, so this is a bit larger than I would have otherwise anticipated.

alexcrichton · 2025-08-19T23:55:23Z

Oh, I should also note, this is a 5% performance penalty overhead to instance instantiation. Benchmarking makes me think it's primarily related to the Box-cost of #[async_trait]. We can burn down that cost I think if we really need from allocation-per-table-and-memory to once-per-module (aka two for most modules to one), but I don't know how to get away from it entirely. I subjectively concluded a few extra allocations is fine, but others can reasonably differ.

alexcrichton · 2025-08-20T00:53:08Z

I was a bit afraid of this, but tests here will fail until I rebase this on top of #11468. So ready for review, but won't pass tests until #11468 lands first.

This commit is a step in preparation for bytecodealliance#11430, notably core instance allocation, or `StoreOpaque::allocate_instance` is now an `async fn`. This function does not actually use the `async`-ness just yet so it's a noop from that point of view, but this propagates outwards to enough locations that I wanted to split this off to make future changes more digestable. Notably some creation functions here such as making an `Instance`, `Table`, or `Memory` are refactored internally to use this new `async` function. Annotations of `assert_ready` or `one_poll` are used as appropriate as well. For reference this commit was benchmarked with our `instantiation.rs` benchmark in the pooling allocator and shows no changes relative to the original baseline from before-`async`-PRs.

This commit is a large-ish refactor which is made possible by the many previous refactorings to internals w.r.t. async-in-Wasmtime. The end goal of this change is that table and memory allocation are both `async` functions. Achieving this, however, required some refactoring to enable it to work: * To work with `Send` neither function can close over `dyn VMStore`. This required changing their `Option<&mut dyn VMStore>` arugment to `Option<&mut StoreResourceLimiter<'_>>` * Somehow a `StoreResourceLimiter` needed to be acquired from an `InstanceAllocationRequest`. Previously the store was stored here as an unsafe raw pointer, but I've refactored this now so `InstanceAllocationRequest` directly stores `&StoreOpaque` and `Option<&mut StoreResourceLimiter>` meaning it's trivial to acquire them. This additionally means no more `unsafe` access of the store during instance allocation (yay!). * Now-redundant fields of `InstanceAllocationRequest` were removed since they can be safely inferred from `&StoreOpaque`. For example passing around `&Tunables` is now all gone. * Methods upwards from table/memory allocation to the `InstanceAllocator` trait needed to be made `async`. This includes new `#[async_trait]` methods for example. * `StoreOpaque::ensure_gc_store` is now an `async` function. This internally carries a new `unsafe` block carried over from before with the raw point passed around in `InstanceAllocationRequest`. A future PR will delete this `unsafe` block, it's just temporary. I attempted a few times to split this PR up into separate commits but everything is relatively intertwined here so this is the smallest "atomic" unit I could manage to land these changes and refactorings.

fitzgen · 2025-08-20T18:31:09Z

crates/wasmtime/src/runtime/externals/table.rs

+        vm::one_poll(Table::_new(store.as_context_mut().0, ty, init))
+            .expect("must use `new_async` when async resource limiters are in use")


Should we not assert that this is a non-async config before the .expect(..) to catch mismatches a little earlier?

Oh this is actually replicating the documented behavior where this works with async_support so long as an async resource limiter isn't used. In that sense to avoid breaking the semantics here one_poll is what's required as opposed to an assert!(!async_support) plus vm::assert_ready.

fitzgen · 2025-08-20T18:31:42Z

crates/wasmtime/src/runtime/component/instance.rs

-        );
-        store.on_fiber(|store| self.instantiate_impl(store)).await?
+    pub async fn instantiate_async(&self, store: impl AsContextMut<Data = T>) -> Result<Instance> {
+        self._instantiate(store).await


And similarly assert that the config is async here?

As I've been making these changes I've actually been undoing a lot of assert!(async_support)-style assertions. Previously that was required because on_fiber was immediately used which required async_support to be turned on, but now it's just normal Rust async functions so there's no reason to prevent usage when async_support is disabled. In that sense it's intentional that the assert here is lost, but how's that sound to you?

fitzgen · 2025-08-20T18:33:54Z

crates/wasmtime/src/runtime/vm/instance/allocator/pooling.rs

+        // FIXME(rust-lang/rust#145127) this should ideally use a version of
+        // `with_flush_and_retry` but adapted for async closures instead of only
+        // sync closures. Right now that won't compile though so this is the
+        // manually expanded version of the method.


Add a note for this to our items to discuss with the lang team, if we don't have one already?

Oh this one's technically already fixed and just waiting on stabilization, and browsing rust-lang/rust#110338 shows this is pretty well-known, so not a major issue.

fitzgen · 2025-08-20T18:34:45Z

crates/wasmtime/src/runtime/vm/instance/allocator.rs

+    /// The store that this instance is being allocated into.
+    pub store: &'a StoreOpaque,


This is so nice to clean up

alexcrichton · 2025-08-21T00:02:29Z

I'm going to go ahead and land this, but @fitzgen if you have follow up comments I'm happy to address them.

fitzgen · 2025-08-21T16:50:58Z

I'm going to go ahead and land this, but @fitzgen if you have follow up comments I'm happy to address them.

Nope, looks good

* Make core instance allocation an `async` function This commit is a step in preparation for bytecodealliance#11430, notably core instance allocation, or `StoreOpaque::allocate_instance` is now an `async fn`. This function does not actually use the `async`-ness just yet so it's a noop from that point of view, but this propagates outwards to enough locations that I wanted to split this off to make future changes more digestable. Notably some creation functions here such as making an `Instance`, `Table`, or `Memory` are refactored internally to use this new `async` function. Annotations of `assert_ready` or `one_poll` are used as appropriate as well. For reference this commit was benchmarked with our `instantiation.rs` benchmark in the pooling allocator and shows no changes relative to the original baseline from before-`async`-PRs. * Make table/memory creation `async` functions This commit is a large-ish refactor which is made possible by the many previous refactorings to internals w.r.t. async-in-Wasmtime. The end goal of this change is that table and memory allocation are both `async` functions. Achieving this, however, required some refactoring to enable it to work: * To work with `Send` neither function can close over `dyn VMStore`. This required changing their `Option<&mut dyn VMStore>` arugment to `Option<&mut StoreResourceLimiter<'_>>` * Somehow a `StoreResourceLimiter` needed to be acquired from an `InstanceAllocationRequest`. Previously the store was stored here as an unsafe raw pointer, but I've refactored this now so `InstanceAllocationRequest` directly stores `&StoreOpaque` and `Option<&mut StoreResourceLimiter>` meaning it's trivial to acquire them. This additionally means no more `unsafe` access of the store during instance allocation (yay!). * Now-redundant fields of `InstanceAllocationRequest` were removed since they can be safely inferred from `&StoreOpaque`. For example passing around `&Tunables` is now all gone. * Methods upwards from table/memory allocation to the `InstanceAllocator` trait needed to be made `async`. This includes new `#[async_trait]` methods for example. * `StoreOpaque::ensure_gc_store` is now an `async` function. This internally carries a new `unsafe` block carried over from before with the raw point passed around in `InstanceAllocationRequest`. A future PR will delete this `unsafe` block, it's just temporary. I attempted a few times to split this PR up into separate commits but everything is relatively intertwined here so this is the smallest "atomic" unit I could manage to land these changes and refactorings. * Shuffle `async-trait` dep * Fix configured build

alexcrichton requested a review from a team as a code owner August 19, 2025 23:52

alexcrichton requested review from dicej and removed request for a team August 19, 2025 23:52

github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Aug 20, 2025

alexcrichton mentioned this pull request Aug 20, 2025

Make const-expr evaluation async #11468

Merged

alexcrichton added 2 commits August 20, 2025 10:54

alexcrichton force-pushed the table-memory-creation-async branch from 02b763f to cfc2cea Compare August 20, 2025 17:54

alexcrichton requested review from fitzgen and removed request for dicej August 20, 2025 17:55

Shuffle async-trait dep

0575316

fitzgen approved these changes Aug 20, 2025

View reviewed changes

Fix configured build

568c123

This was referenced Aug 20, 2025

Deduplicate some more GC functions #11480

Merged

Remove unsoundness of widening store borrows #11481

Merged

alexcrichton added this pull request to the merge queue Aug 21, 2025

Merged via the queue into bytecodealliance:main with commit e1f50aa Aug 21, 2025
44 checks passed

alexcrichton deleted the table-memory-creation-async branch August 21, 2025 00:25

lann mentioned this pull request Aug 21, 2025

Add PoolingAllocatorMetrics #11490

Merged

alexcrichton mentioned this pull request Aug 21, 2025

Use async fn internally within Wasmtime #11430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make table/memory creation async functions #11470

Make table/memory creation async functions #11470
alexcrichton merged 4 commits intobytecodealliance:mainfrom
alexcrichton:table-memory-creation-async

alexcrichton commented Aug 19, 2025 •

edited

Loading

Uh oh!

alexcrichton commented Aug 19, 2025

Uh oh!

alexcrichton commented Aug 19, 2025

Uh oh!

alexcrichton commented Aug 20, 2025

Uh oh!

fitzgen Aug 20, 2025

Uh oh!

alexcrichton Aug 20, 2025

Uh oh!

fitzgen Aug 20, 2025

Uh oh!

alexcrichton Aug 20, 2025

Uh oh!

fitzgen Aug 20, 2025

Uh oh!

alexcrichton Aug 20, 2025

Uh oh!

fitzgen Aug 20, 2025

Uh oh!

alexcrichton commented Aug 21, 2025

Uh oh!

Uh oh!

fitzgen commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		vm::one_poll(Table::_new(store.as_context_mut().0, ty, init))
		.expect("must use `new_async` when async resource limiters are in use")

		/// The store that this instance is being allocated into.
		pub store: &'a StoreOpaque,

Conversation

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

Uh oh!

Uh oh!

alexcrichton commented Aug 19, 2025

Uh oh!

alexcrichton commented Aug 19, 2025

Uh oh!

alexcrichton commented Aug 20, 2025

Uh oh!

fitzgen Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

alexcrichton Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

fitzgen Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

alexcrichton Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

fitzgen Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

alexcrichton Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

fitzgen Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Aug 21, 2025

Uh oh!

Uh oh!

fitzgen commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alexcrichton commented Aug 19, 2025 •

edited

Loading