Skip to content

Commit cfc0495

Browse files
committed
Make const-expr evaluation async
This commit is extracted from bytecodealliance#11430 to accurately reflect how const-expr evaluation is an async operation due to GC pauses that may happen. The changes in this commit are: * Const-expr evaluation is, at its core, now an `async` function. * To leverage this new `async`-ness all internal operations are switched from `*_maybe_async` to `*_async` meaning all the `*_maybe_async` methods can be removed. * Some libcalls using `*_maybe_async` are switch to using `*_async` plus the `block_on!` utility to help jettison more `*_maybe_async` methods. * Instance initialization is now an `async` function. This is temporarily handled with `block_on` during instance initialization to avoid propagating the `async`-ness further upwards. This `block_on` will get deleted in future refactorings. * Const-expr evaluation has been refactored slightly to enable having a fast path in global initialization which skips an `await` point entirely, achieving performance-parity in benchmarks prior to this commit. This ended up fixing a niche issue with GC where if a wasm execution was suspended during `table.init`, for example, during a const-expr evaluation triggering a GC then if the wasm execution was cancelled it would panic the host. This panic was because the GC operation returned `Result` but it was `unwrap`'d as part of the const-expr evaluation which can fail not only to invalid-ness but also due to "computation is cancelled" traps.
1 parent 1a88c70 commit cfc0495

File tree

8 files changed

+336
-400
lines changed

8 files changed

+336
-400
lines changed

crates/wasmtime/src/runtime/gc/enabled/arrayref.rs

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -367,25 +367,6 @@ impl ArrayRef {
367367
.await
368368
}
369369

370-
/// Like `ArrayRef::new` but when async is configured must only ever be
371-
/// called from on a fiber stack.
372-
pub(crate) unsafe fn new_maybe_async(
373-
store: &mut StoreOpaque,
374-
allocator: &ArrayRefPre,
375-
elem: &Val,
376-
len: u32,
377-
) -> Result<Rooted<ArrayRef>> {
378-
// Type check the initial element value against the element type.
379-
elem.ensure_matches_ty(store, allocator.ty.element_type().unpack())
380-
.context("element type mismatch")?;
381-
382-
unsafe {
383-
store.retry_after_gc_maybe_async((), |store, ()| {
384-
Self::new_from_iter(store, allocator, RepeatN(elem, len))
385-
})
386-
}
387-
}
388-
389370
/// Allocate a new array of the given elements.
390371
///
391372
/// Does not attempt a GC on OOM; leaves that to callers.
@@ -544,21 +525,6 @@ impl ArrayRef {
544525
.await
545526
}
546527

547-
/// Like `ArrayRef::new_fixed[_async]` but it is the caller's responsibility
548-
/// to ensure that when async is enabled, this is only called from on a
549-
/// fiber stack.
550-
pub(crate) unsafe fn new_fixed_maybe_async(
551-
store: &mut StoreOpaque,
552-
allocator: &ArrayRefPre,
553-
elems: &[Val],
554-
) -> Result<Rooted<ArrayRef>> {
555-
unsafe {
556-
store.retry_after_gc_maybe_async((), |store, ()| {
557-
Self::new_from_iter(store, allocator, elems.iter())
558-
})
559-
}
560-
}
561-
562528
#[inline]
563529
pub(crate) fn comes_from_same_store(&self, store: &StoreOpaque) -> bool {
564530
self.inner.comes_from_same_store(store)

crates/wasmtime/src/runtime/gc/enabled/structref.rs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -269,10 +269,6 @@ impl StructRef {
269269
///
270270
/// # Panics
271271
///
272-
/// Panics if your engine is not configured for async; use
273-
/// [`StructRef::new`][crate::StructRef::new] to perform synchronous
274-
/// allocation instead.
275-
///
276272
/// Panics if the allocator, or any of the field values, is not associated
277273
/// with the given store.
278274
#[cfg(feature = "async")]
@@ -290,10 +286,6 @@ impl StructRef {
290286
allocator: &StructRefPre,
291287
fields: &[Val],
292288
) -> Result<Rooted<StructRef>> {
293-
assert!(
294-
store.async_support(),
295-
"use `StructRef::new` with synchronous stores"
296-
);
297289
Self::type_check_fields(store, allocator, fields)?;
298290
store
299291
.retry_after_gc_async((), |store, ()| {
@@ -302,21 +294,6 @@ impl StructRef {
302294
.await
303295
}
304296

305-
/// Like `Self::new` but caller's must ensure that if the store is
306-
/// configured for async, this is only ever called from on a fiber stack.
307-
pub(crate) unsafe fn new_maybe_async(
308-
store: &mut StoreOpaque,
309-
allocator: &StructRefPre,
310-
fields: &[Val],
311-
) -> Result<Rooted<StructRef>> {
312-
Self::type_check_fields(store, allocator, fields)?;
313-
unsafe {
314-
store.retry_after_gc_maybe_async((), |store, ()| {
315-
Self::new_unchecked(store, allocator, fields)
316-
})
317-
}
318-
}
319-
320297
/// Type check the field values before allocating a new struct.
321298
fn type_check_fields(
322299
store: &mut StoreOpaque,

crates/wasmtime/src/runtime/instance.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,21 @@ impl Instance {
377377
.features()
378378
.contains(WasmFeatures::BULK_MEMORY);
379379

380-
vm::initialize_instance(store, id, compiled_module.module(), bulk_memory)?;
380+
if store.async_support() {
381+
store.block_on(|store| {
382+
let module = compiled_module.module().clone();
383+
Box::pin(
384+
async move { vm::initialize_instance(store, id, &module, bulk_memory).await },
385+
)
386+
})??;
387+
} else {
388+
vm::assert_ready(vm::initialize_instance(
389+
store,
390+
id,
391+
compiled_module.module(),
392+
bulk_memory,
393+
))?;
394+
}
381395

382396
Ok((instance, compiled_module.module().start_func))
383397
}

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

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -197,53 +197,6 @@ impl StoreOpaque {
197197
},
198198
}
199199
}
200-
201-
/// Like `retry_after_gc` but async yielding (if necessary) is transparent.
202-
///
203-
/// # Safety
204-
///
205-
/// When async is enabled, it is the caller's responsibility to ensure that
206-
/// this is called on a fiber stack.
207-
pub(crate) unsafe fn retry_after_gc_maybe_async<T, U>(
208-
&mut self,
209-
value: T,
210-
alloc_func: impl Fn(&mut Self, T) -> Result<U>,
211-
) -> Result<U>
212-
where
213-
T: Send + Sync + 'static,
214-
{
215-
self.ensure_gc_store()?;
216-
match alloc_func(self, value) {
217-
Ok(x) => Ok(x),
218-
Err(e) => match e.downcast::<crate::GcHeapOutOfMemory<T>>() {
219-
Ok(oom) => {
220-
let (value, oom) = oom.take_inner();
221-
// Note it's the caller's responsibility to ensure that
222-
// this is on a fiber stack if necessary.
223-
//
224-
// SAFETY: FIXME(#11409)
225-
unsafe {
226-
if self.async_support() {
227-
#[cfg(feature = "async")]
228-
self.block_on(|store| {
229-
Box::pin(
230-
store.gc_unsafe_get_limiter(None, Some(oom.bytes_needed())),
231-
)
232-
})?;
233-
#[cfg(not(feature = "async"))]
234-
unreachable!();
235-
} else {
236-
vm::assert_ready(
237-
self.gc_unsafe_get_limiter(None, Some(oom.bytes_needed())),
238-
);
239-
}
240-
}
241-
alloc_func(self, value)
242-
}
243-
Err(e) => Err(e),
244-
},
245-
}
246-
}
247200
}
248201

249202
#[cfg(feature = "async")]
@@ -258,10 +211,6 @@ impl StoreOpaque {
258211
where
259212
T: Send + Sync + 'static,
260213
{
261-
assert!(
262-
self.async_support(),
263-
"you must configure async to use the `*_async` versions of methods"
264-
);
265214
self.ensure_gc_store()?;
266215
match alloc_func(self, value) {
267216
Ok(x) => Ok(x),

0 commit comments

Comments
 (0)