Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazily load types into Func #3727

Merged
merged 2 commits into from
Jan 26, 2022
Merged

Lazily load types into Func #3727

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81a983e
Lazily load types into `Func`
alexcrichton Jan 26, 2022
4ea3413
Review comments
alexcrichton Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 37 additions & 16 deletions crates/wasmtime/src/func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,12 @@ pub struct Func(Stored<FuncData>);

pub(crate) struct FuncData {
kind: FuncKind,
ty: FuncType,

// This is somewhat expensive to load from the `Engine` and in most
// optimized use cases (e.g. `TypedFunc`) it's not actually needed or it's
// only needed rarely. To handle that this is an optionally-contained field
// which is lazily loaded into as part of `Func::call`.
ty: Option<FuncType>,
}

/// The three ways that a function can be created and referenced from within a
Expand Down Expand Up @@ -677,7 +682,33 @@ impl Func {
///
/// Panics if `store` does not own this function.
pub fn ty(&self, store: impl AsContext) -> FuncType {
store.as_context()[self.0].ty.clone()
self.load_ty(&store.as_context().0)
}

/// Forcibly loads the type of this function from the `Engine`.
///
/// Note that this is a somewhat expensive method since it requires taking a
/// lock as well as cloning a type.
fn load_ty(&self, store: &StoreOpaque) -> FuncType {
FuncType::from_wasm_func_type(
store
.engine()
.signatures()
.lookup_type(self.sig_index(store.store_data()))
.expect("signature should be registered"),
)
}

/// Gets a reference to the `FuncType` for this function.
fn ty_ref<'a>(&self, store: &'a mut StoreOpaque) -> (&'a FuncType, &'a StoreOpaque) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's probably a mundane borrow-checker-subtlety reason for returning the downgraded immutable-borrow of StoreOpaque here, but could we add a comment describing why?

// If we haven't loaded our type into the store yet then do so lazily at
// this time.
if store.store_data()[self.0].ty.is_none() {
let ty = self.load_ty(store);
store.store_data_mut()[self.0].ty = Some(ty);
}

(store.store_data()[self.0].ty.as_ref().unwrap(), store)
}

pub(crate) fn sig_index(&self, data: &StoreData) -> VMSharedSignatureIndex {
Expand Down Expand Up @@ -843,7 +874,7 @@ impl Func {
// this function. This involves checking to make sure we have the right
// number and types of arguments as well as making sure everything is
// from the same `Store`.
let ty = &store[self.0].ty;
let (ty, opaque) = self.ty_ref(store.0);
if ty.params().len() != params.len() {
bail!(
"expected {} arguments, got {}",
Expand All @@ -866,7 +897,7 @@ impl Func {
ty
);
}
if !arg.comes_from_same_store(store.0) {
if !arg.comes_from_same_store(opaque) {
bail!("cross-`Store` values are not currently supported");
}
}
Expand Down Expand Up @@ -904,7 +935,7 @@ impl Func {
}

for ((i, slot), val) in results.iter_mut().enumerate().zip(&values_vec) {
let ty = store[self.0].ty.results().nth(i).unwrap();
let ty = self.ty_ref(store.0).0.results().nth(i).unwrap();
*slot = unsafe { Val::from_raw(&mut *store, *val, ty) };
}
values_vec.truncate(0);
Expand All @@ -930,17 +961,7 @@ impl Func {
}

fn from_func_kind(kind: FuncKind, store: &mut StoreOpaque) -> Self {
// Signatures should always be registered in the engine's registry of
// shared signatures, so we should be able to unwrap safely here.
let ty = unsafe { kind.export().anyfunc.as_ref().type_index };
let ty = FuncType::from_wasm_func_type(
store
.engine()
.signatures()
.lookup_type(ty)
.expect("signature should be registered"),
);
Func(store.store_data_mut().insert(FuncData { kind, ty }))
Func(store.store_data_mut().insert(FuncData { kind, ty: None }))
}

pub(crate) fn vmimport(&self, store: &mut StoreOpaque) -> VMFunctionImport {
Expand Down