Skip to content

Commit 683d4c1

Browse files
committed
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.
1 parent d139713 commit 683d4c1

File tree

9 files changed

+97
-130
lines changed

9 files changed

+97
-130
lines changed

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

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ impl<'a> Instantiator<'a> {
634634
}
635635
}
636636

637-
fn run<T>(&mut self, store: &mut StoreContextMut<'_, T>) -> Result<()> {
637+
async fn run<T>(&mut self, store: &mut StoreContextMut<'_, T>) -> Result<()> {
638638
let env_component = self.component.env_component();
639639

640640
// Before all initializers are processed configure all destructors for
@@ -714,7 +714,7 @@ impl<'a> Instantiator<'a> {
714714
// if required.
715715

716716
let i = unsafe {
717-
crate::Instance::new_started_impl(store, module, imports.as_ref())?
717+
crate::Instance::new_started(store, module, imports.as_ref()).await?
718718
};
719719
self.instance_mut(store.0).push_instance_id(i.id());
720720
}
@@ -991,37 +991,26 @@ impl<T: 'static> InstancePre<T> {
991991
!store.as_context().async_support(),
992992
"must use async instantiation when async support is enabled"
993993
);
994-
self.instantiate_impl(store)
994+
vm::assert_ready(self._instantiate(store))
995995
}
996996
/// Performs the instantiation process into the store specified.
997997
///
998998
/// Exactly like [`Self::instantiate`] except for use on async stores.
999999
//
10001000
// TODO: needs more docs
10011001
#[cfg(feature = "async")]
1002-
pub async fn instantiate_async(
1003-
&self,
1004-
mut store: impl AsContextMut<Data = T>,
1005-
) -> Result<Instance>
1006-
where
1007-
T: Send,
1008-
{
1009-
let mut store = store.as_context_mut();
1010-
assert!(
1011-
store.0.async_support(),
1012-
"must use sync instantiation when async support is disabled"
1013-
);
1014-
store.on_fiber(|store| self.instantiate_impl(store)).await?
1002+
pub async fn instantiate_async(&self, store: impl AsContextMut<Data = T>) -> Result<Instance> {
1003+
self._instantiate(store).await
10151004
}
10161005

1017-
fn instantiate_impl(&self, mut store: impl AsContextMut<Data = T>) -> Result<Instance> {
1006+
async fn _instantiate(&self, mut store: impl AsContextMut<Data = T>) -> Result<Instance> {
10181007
let mut store = store.as_context_mut();
10191008
store
10201009
.engine()
10211010
.allocator()
10221011
.increment_component_instance_count()?;
10231012
let mut instantiator = Instantiator::new(&self.component, store.0, &self.imports);
1024-
instantiator.run(&mut store).map_err(|e| {
1013+
instantiator.run(&mut store).await.map_err(|e| {
10251014
store
10261015
.engine()
10271016
.allocator()

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ impl Table {
9494
/// # }
9595
/// ```
9696
pub fn new(mut store: impl AsContextMut, ty: TableType, init: Ref) -> Result<Table> {
97-
Table::_new(store.as_context_mut().0, ty, init)
97+
vm::one_poll(Table::_new(store.as_context_mut().0, ty, init))
98+
.expect("must use `new_async` when async resource limiters are in use")
9899
}
99100

100101
/// Async variant of [`Table::new`]. You must use this variant with
@@ -111,18 +112,11 @@ impl Table {
111112
ty: TableType,
112113
init: Ref,
113114
) -> Result<Table> {
114-
let mut store = store.as_context_mut();
115-
assert!(
116-
store.0.async_support(),
117-
"cannot use `new_async` without enabling async support on the config"
118-
);
119-
store
120-
.on_fiber(|store| Table::_new(store.0, ty, init))
121-
.await?
115+
Table::_new(store.as_context_mut().0, ty, init).await
122116
}
123117

124-
fn _new(store: &mut StoreOpaque, ty: TableType, init: Ref) -> Result<Table> {
125-
let table = generate_table_export(store, &ty)?;
118+
async fn _new(store: &mut StoreOpaque, ty: TableType, init: Ref) -> Result<Table> {
119+
let table = generate_table_export(store, &ty).await?;
126120
table._fill(store, 0, init, ty.minimum())?;
127121
Ok(table)
128122
}

crates/wasmtime/src/runtime/instance.rs

Lines changed: 30 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ impl Instance {
117117
// Note that the unsafety here should be satisfied by the call to
118118
// `typecheck_externs` above which satisfies the condition that all
119119
// the imports are valid for this module.
120-
unsafe { Instance::new_started(&mut store, module, imports.as_ref()) }
120+
assert!(!store.0.async_support());
121+
vm::assert_ready(unsafe { Instance::new_started(&mut store, module, imports.as_ref()) })
121122
}
122123

123124
/// Same as [`Instance::new`], except for usage in [asynchronous stores].
@@ -200,7 +201,7 @@ impl Instance {
200201
let mut store = store.as_context_mut();
201202
let imports = Instance::typecheck_externs(store.0, module, imports)?;
202203
// See `new` for notes on this unsafety
203-
unsafe { Instance::new_started_async(&mut store, module, imports.as_ref()).await }
204+
unsafe { Instance::new_started(&mut store, module, imports.as_ref()).await }
204205
}
205206

206207
fn typecheck_externs(
@@ -242,62 +243,31 @@ impl Instance {
242243
/// Internal function to create an instance and run the start function.
243244
///
244245
/// This function's unsafety is the same as `Instance::new_raw`.
245-
pub(crate) unsafe fn new_started<T>(
246-
store: &mut StoreContextMut<'_, T>,
247-
module: &Module,
248-
imports: Imports<'_>,
249-
) -> Result<Instance> {
250-
assert!(
251-
!store.0.async_support(),
252-
"must use async instantiation when async support is enabled",
253-
);
254-
255-
// SAFETY: the safety contract of `new_started_impl` is the same as this
256-
// function.
257-
unsafe { Self::new_started_impl(store, module, imports) }
258-
}
259-
260-
/// Internal function to create an instance and run the start function.
261-
///
262-
/// ONLY CALL THIS IF YOU HAVE ALREADY CHECKED FOR ASYNCNESS AND HANDLED
263-
/// THE FIBER NONSENSE
264-
pub(crate) unsafe fn new_started_impl<T>(
246+
pub(crate) async unsafe fn new_started<T>(
265247
store: &mut StoreContextMut<'_, T>,
266248
module: &Module,
267249
imports: Imports<'_>,
268250
) -> Result<Instance> {
269251
// SAFETY: the safety contract of `new_raw` is the same as this
270252
// function.
271-
let (instance, start) = unsafe { Instance::new_raw(store.0, module, imports)? };
253+
let (instance, start) = unsafe { Instance::new_raw(store.0, module, imports).await? };
272254
if let Some(start) = start {
273-
instance.start_raw(store, start)?;
255+
if store.0.async_support() {
256+
#[cfg(feature = "async")]
257+
{
258+
store
259+
.on_fiber(|store| instance.start_raw(store, start))
260+
.await??;
261+
}
262+
#[cfg(not(feature = "async"))]
263+
unreachable!();
264+
} else {
265+
instance.start_raw(store, start)?;
266+
}
274267
}
275268
Ok(instance)
276269
}
277270

278-
/// Internal function to create an instance and run the start function.
279-
///
280-
/// This function's unsafety is the same as `Instance::new_raw`.
281-
#[cfg(feature = "async")]
282-
async unsafe fn new_started_async<T>(
283-
store: &mut StoreContextMut<'_, T>,
284-
module: &Module,
285-
imports: Imports<'_>,
286-
) -> Result<Instance> {
287-
assert!(
288-
store.0.async_support(),
289-
"must use sync instantiation when async support is disabled",
290-
);
291-
292-
store
293-
.on_fiber(|store| {
294-
// SAFETY: the unsafe contract of `new_started_impl` is the same
295-
// as this function.
296-
unsafe { Self::new_started_impl(store, module, imports) }
297-
})
298-
.await?
299-
}
300-
301271
/// Internal function to create an instance which doesn't have its `start`
302272
/// function run yet.
303273
///
@@ -313,7 +283,7 @@ impl Instance {
313283
/// This method is unsafe because it does not type-check the `imports`
314284
/// provided. The `imports` provided must be suitable for the module
315285
/// provided as well.
316-
unsafe fn new_raw(
286+
async unsafe fn new_raw(
317287
store: &mut StoreOpaque,
318288
module: &Module,
319289
imports: Imports<'_>,
@@ -341,11 +311,13 @@ impl Instance {
341311
// SAFETY: this module, by construction, was already validated within
342312
// the store.
343313
let id = unsafe {
344-
store.allocate_instance(
345-
AllocateInstanceKind::Module(module_id),
346-
&ModuleRuntimeInfo::Module(module.clone()),
347-
imports,
348-
)?
314+
store
315+
.allocate_instance(
316+
AllocateInstanceKind::Module(module_id),
317+
&ModuleRuntimeInfo::Module(module.clone()),
318+
imports,
319+
)
320+
.await?
349321
};
350322

351323
// Additionally, before we start doing fallible instantiation, we
@@ -905,7 +877,10 @@ impl<T: 'static> InstancePre<T> {
905877
// This unsafety should be handled by the type-checking performed by the
906878
// constructor of `InstancePre` to assert that all the imports we're passing
907879
// in match the module we're instantiating.
908-
unsafe { Instance::new_started(&mut store, &self.module, imports.as_ref()) }
880+
assert!(!store.0.async_support());
881+
vm::assert_ready(unsafe {
882+
Instance::new_started(&mut store, &self.module, imports.as_ref())
883+
})
909884
}
910885

911886
/// Creates a new instance, running the start function asynchronously
@@ -935,7 +910,7 @@ impl<T: 'static> InstancePre<T> {
935910
// This unsafety should be handled by the type-checking performed by the
936911
// constructor of `InstancePre` to assert that all the imports we're passing
937912
// in match the module we're instantiating.
938-
unsafe { Instance::new_started_async(&mut store, &self.module, imports.as_ref()).await }
913+
unsafe { Instance::new_started(&mut store, &self.module, imports.as_ref()).await }
939914
}
940915
}
941916

crates/wasmtime/src/runtime/memory.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ impl Memory {
260260
/// # }
261261
/// ```
262262
pub fn new(mut store: impl AsContextMut, ty: MemoryType) -> Result<Memory> {
263-
Self::_new(store.as_context_mut().0, ty)
263+
vm::one_poll(Self::_new(store.as_context_mut().0, ty))
264+
.expect("must use `new_async` when async resource limiters are in use")
264265
}
265266

266267
/// Async variant of [`Memory::new`]. You must use this variant with
@@ -273,17 +274,12 @@ impl Memory {
273274
/// [`Store`](`crate::Store`).
274275
#[cfg(feature = "async")]
275276
pub async fn new_async(mut store: impl AsContextMut, ty: MemoryType) -> Result<Memory> {
276-
let mut store = store.as_context_mut();
277-
assert!(
278-
store.0.async_support(),
279-
"cannot use `new_async` without enabling async support on the config"
280-
);
281-
store.on_fiber(|store| Self::_new(store.0, ty)).await?
277+
Self::_new(store.as_context_mut().0, ty).await
282278
}
283279

284280
/// Helper function for attaching the memory to a "frankenstein" instance
285-
fn _new(store: &mut StoreOpaque, ty: MemoryType) -> Result<Memory> {
286-
generate_memory_export(store, &ty, None)
281+
async fn _new(store: &mut StoreOpaque, ty: MemoryType) -> Result<Memory> {
282+
generate_memory_export(store, &ty, None).await
287283
}
288284

289285
/// Returns the underlying type of this memory.
@@ -1005,7 +1001,10 @@ impl SharedMemory {
10051001
/// Construct a single-memory instance to provide a way to import
10061002
/// [`SharedMemory`] into other modules.
10071003
pub(crate) fn vmimport(&self, store: &mut StoreOpaque) -> crate::runtime::vm::VMMemoryImport {
1008-
generate_memory_export(store, &self.ty(), Some(&self.vm))
1004+
// Note `vm::assert_ready` shouldn't panic here because this isn't
1005+
// actually allocating any new memory so resource limiting shouldn't
1006+
// kick in.
1007+
vm::assert_ready(generate_memory_export(store, &self.ty(), Some(&self.vm)))
10091008
.unwrap()
10101009
.vmimport(store)
10111010
}

crates/wasmtime/src/runtime/store.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -688,15 +688,17 @@ impl<T> Store<T> {
688688
.unwrap();
689689

690690
unsafe {
691-
let id = inner
692-
.allocate_instance(
693-
AllocateInstanceKind::Dummy {
694-
allocator: &allocator,
695-
},
696-
&shim,
697-
Default::default(),
698-
)
699-
.expect("failed to allocate default callee");
691+
// Note that this dummy instance doesn't allocate tables or memories
692+
// so it won't have an async await point meaning that it should be
693+
// ok to assert the future is always ready.
694+
let id = vm::assert_ready(inner.allocate_instance(
695+
AllocateInstanceKind::Dummy {
696+
allocator: &allocator,
697+
},
698+
&shim,
699+
Default::default(),
700+
))
701+
.expect("failed to allocate default callee");
700702
let default_caller_vmctx = inner.instance(id).vmctx();
701703
inner.default_caller_vmctx = default_caller_vmctx.into();
702704
}
@@ -2208,7 +2210,7 @@ at https://bytecodealliance.org/security.
22082210
///
22092211
/// The `imports` provided must be correctly sized/typed for the module
22102212
/// being allocated.
2211-
pub(crate) unsafe fn allocate_instance(
2213+
pub(crate) async unsafe fn allocate_instance(
22122214
&mut self,
22132215
kind: AllocateInstanceKind<'_>,
22142216
runtime_info: &ModuleRuntimeInfo,

crates/wasmtime/src/runtime/trampoline.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,21 @@ use crate::store::StoreOpaque;
1919
use crate::{MemoryType, TableType, TagType};
2020
use wasmtime_environ::{MemoryIndex, TableIndex, TagIndex};
2121

22-
pub fn generate_memory_export(
22+
pub async fn generate_memory_export(
2323
store: &mut StoreOpaque,
2424
m: &MemoryType,
2525
preallocation: Option<&SharedMemory>,
2626
) -> Result<crate::Memory> {
2727
let id = store.id();
28-
let instance = create_memory(store, m, preallocation)?;
28+
let instance = create_memory(store, m, preallocation).await?;
2929
Ok(store
3030
.instance_mut(instance)
3131
.get_exported_memory(id, MemoryIndex::from_u32(0)))
3232
}
3333

34-
pub fn generate_table_export(store: &mut StoreOpaque, t: &TableType) -> Result<crate::Table> {
34+
pub async fn generate_table_export(store: &mut StoreOpaque, t: &TableType) -> Result<crate::Table> {
3535
let id = store.id();
36-
let instance = create_table(store, t)?;
36+
let instance = create_table(store, t).await?;
3737
Ok(store
3838
.instance_mut(instance)
3939
.get_exported_table(id, TableIndex::from_u32(0)))

crates/wasmtime/src/runtime/trampoline/memory.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use wasmtime_environ::{
2424
/// This separate instance is necessary because Wasm objects in Wasmtime must be
2525
/// attached to instances (versus the store, e.g.) and some objects exist
2626
/// outside: a host-provided memory import, shared memory.
27-
pub fn create_memory(
27+
pub async fn create_memory(
2828
store: &mut StoreOpaque,
2929
memory_ty: &MemoryType,
3030
preallocation: Option<&SharedMemory>,
@@ -52,13 +52,15 @@ pub fn create_memory(
5252
ondemand: OnDemandInstanceAllocator::default(),
5353
};
5454
unsafe {
55-
store.allocate_instance(
56-
AllocateInstanceKind::Dummy {
57-
allocator: &allocator,
58-
},
59-
&ModuleRuntimeInfo::bare(Arc::new(module)),
60-
Default::default(),
61-
)
55+
store
56+
.allocate_instance(
57+
AllocateInstanceKind::Dummy {
58+
allocator: &allocator,
59+
},
60+
&ModuleRuntimeInfo::bare(Arc::new(module)),
61+
Default::default(),
62+
)
63+
.await
6264
}
6365
}
6466

0 commit comments

Comments
 (0)