Skip to content

Commit d139713

Browse files
authored
Make const-expr evaluation async (#11468)
* Make const-expr evaluation `async` This commit is extracted from #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. * Fix configured build * Undo rebase mistake
1 parent 1a88c70 commit d139713

File tree

8 files changed

+321
-368
lines changed

8 files changed

+321
-368
lines changed

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

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,6 @@ impl ArrayRef {
353353
Self::_new_async(store.as_context_mut().0, allocator, elem, len).await
354354
}
355355

356-
#[cfg(feature = "async")]
357356
pub(crate) async fn _new_async(
358357
store: &mut StoreOpaque,
359358
allocator: &ArrayRefPre,
@@ -367,25 +366,6 @@ impl ArrayRef {
367366
.await
368367
}
369368

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-
389369
/// Allocate a new array of the given elements.
390370
///
391371
/// Does not attempt a GC on OOM; leaves that to callers.
@@ -531,7 +511,6 @@ impl ArrayRef {
531511
Self::_new_fixed_async(store.as_context_mut().0, allocator, elems).await
532512
}
533513

534-
#[cfg(feature = "async")]
535514
pub(crate) async fn _new_fixed_async(
536515
store: &mut StoreOpaque,
537516
allocator: &ArrayRefPre,
@@ -544,21 +523,6 @@ impl ArrayRef {
544523
.await
545524
}
546525

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-
562526
#[inline]
563527
pub(crate) fn comes_from_same_store(&self, store: &StoreOpaque) -> bool {
564528
self.inner.comes_from_same_store(store)

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

Lines changed: 0 additions & 24 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")]
@@ -284,16 +280,11 @@ impl StructRef {
284280
Self::_new_async(store.as_context_mut().0, allocator, fields).await
285281
}
286282

287-
#[cfg(feature = "async")]
288283
pub(crate) async fn _new_async(
289284
store: &mut StoreOpaque,
290285
allocator: &StructRefPre,
291286
fields: &[Val],
292287
) -> Result<Rooted<StructRef>> {
293-
assert!(
294-
store.async_support(),
295-
"use `StructRef::new` with synchronous stores"
296-
);
297288
Self::type_check_fields(store, allocator, fields)?;
298289
store
299290
.retry_after_gc_async((), |store, ()| {
@@ -302,21 +293,6 @@ impl StructRef {
302293
.await
303294
}
304295

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-
320296
/// Type check the field values before allocating a new struct.
321297
fn type_check_fields(
322298
store: &mut StoreOpaque,

crates/wasmtime/src/runtime/instance.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,24 @@ 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+
#[cfg(feature = "async")]
382+
store.block_on(|store| {
383+
let module = compiled_module.module().clone();
384+
Box::pin(
385+
async move { vm::initialize_instance(store, id, &module, bulk_memory).await },
386+
)
387+
})??;
388+
#[cfg(not(feature = "async"))]
389+
unreachable!();
390+
} else {
391+
vm::assert_ready(vm::initialize_instance(
392+
store,
393+
id,
394+
compiled_module.module(),
395+
bulk_memory,
396+
))?;
397+
}
381398

382399
Ok((instance, compiled_module.module().start_func))
383400
}

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

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -198,56 +198,6 @@ impl StoreOpaque {
198198
}
199199
}
200200

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-
}
247-
}
248-
249-
#[cfg(feature = "async")]
250-
impl StoreOpaque {
251201
/// Attempt an allocation, if it fails due to GC OOM, then do a GC and
252202
/// retry.
253203
pub(crate) async fn retry_after_gc_async<T, U>(
@@ -258,10 +208,6 @@ impl StoreOpaque {
258208
where
259209
T: Send + Sync + 'static,
260210
{
261-
assert!(
262-
self.async_support(),
263-
"you must configure async to use the `*_async` versions of methods"
264-
);
265211
self.ensure_gc_store()?;
266212
match alloc_func(self, value) {
267213
Ok(x) => Ok(x),

0 commit comments

Comments
 (0)