-
Notifications
You must be signed in to change notification settings - Fork 819
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.
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
RFC Host yielding API #2219
RFC Host yielding API #2219
Changes from all commits
2a0f7c9
638702d
fe4e38f
666574c
5d2203a
98278af
8c0865f
ae5ffa7
f94beea
5df791e
708269e
a8d8bbb
fd30e36
4b04930
dad39c0
ea8c891
c14a97a
57ba569
729fad3
89b63d0
2cd5250
f70c791
01877eb
21730c3
99a6a0c
5ed888d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,53 @@ pub struct Function { | |
pub(crate) exported: ExportFunction, | ||
} | ||
|
||
#[cfg(feature = "async")] | ||
fn build_export_function_metadata<Env>( | ||
env: Env, | ||
import_init_function_ptr: for<'a> fn( | ||
&'a mut Env, | ||
&'a crate::Instance, | ||
) -> Result<(), crate::HostEnvInitError>, | ||
host_env_set_yielder_fn: fn(*mut std::ffi::c_void, *const std::ffi::c_void), | ||
) -> (*mut std::ffi::c_void, ExportFunctionMetadata) | ||
where | ||
Env: Clone + Sized + 'static + Send + Sync, | ||
{ | ||
let import_init_function_ptr = Some(unsafe { | ||
std::mem::transmute::<fn(_, _) -> Result<(), _>, fn(_, _) -> Result<(), _>>( | ||
import_init_function_ptr, | ||
) | ||
}); | ||
let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { | ||
let env_ref: &Env = unsafe { | ||
ptr.cast::<Env>() | ||
.as_ref() | ||
.expect("`ptr` to the environment is null when cloning it") | ||
}; | ||
Box::into_raw(Box::new(env_ref.clone())) as _ | ||
}; | ||
let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr| { | ||
unsafe { Box::from_raw(ptr.cast::<Env>()) }; | ||
}; | ||
let env = Box::into_raw(Box::new(env)) as _; | ||
|
||
// # Safety | ||
// - All these functions work on all threads | ||
// - The host env is `Send`. | ||
let metadata = unsafe { | ||
ExportFunctionMetadata::new( | ||
env, | ||
import_init_function_ptr, | ||
host_env_clone_fn, | ||
host_env_set_yielder_fn, | ||
host_env_drop_fn, | ||
) | ||
}; | ||
|
||
(env, metadata) | ||
} | ||
|
||
#[cfg(not(feature = "async"))] | ||
fn build_export_function_metadata<Env>( | ||
env: Env, | ||
import_init_function_ptr: for<'a> fn( | ||
|
@@ -170,12 +217,19 @@ impl Function { | |
}; | ||
Box::into_raw(Box::new(duped_env)) as _ | ||
}; | ||
|
||
let host_env_drop_fn: fn(*mut c_void) = |ptr: *mut c_void| { | ||
unsafe { | ||
Box::from_raw(ptr as *mut VMDynamicFunctionContext<DynamicFunctionWithoutEnv>) | ||
}; | ||
}; | ||
|
||
#[cfg(feature = "async")] | ||
let host_env_set_yielder_fn = |_, _| { | ||
//no-op | ||
println!("No-op"); | ||
}; | ||
|
||
Self { | ||
store: store.clone(), | ||
definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: false }), | ||
|
@@ -189,6 +243,8 @@ impl Function { | |
host_env, | ||
None, | ||
host_env_clone_fn, | ||
#[cfg(feature = "async")] | ||
host_env_set_yielder_fn, | ||
host_env_drop_fn, | ||
) | ||
}, | ||
|
@@ -270,10 +326,27 @@ impl Function { | |
Env::init_with_instance(&mut *env.ctx.env, instance) | ||
}; | ||
|
||
#[cfg(feature = "async")] | ||
let (host_env, metadata) = { | ||
let host_env_set_yielder_fn = |env_ptr, yielder_ptr| { | ||
let env: &mut Env = unsafe { std::mem::transmute(env_ptr) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's nice to be super explicit about what exactly is being transmuted from what to what to make bugs and intent more obvious. I'd prefer all |
||
env.set_yielder(yielder_ptr); | ||
}; | ||
|
||
build_export_function_metadata::<VMDynamicFunctionContext<DynamicFunctionWithEnv<Env>>>( | ||
dynamic_ctx, | ||
import_init_function_ptr, | ||
host_env_set_yielder_fn, | ||
) | ||
}; | ||
|
||
#[cfg(not(feature = "async"))] | ||
let (host_env, metadata) = build_export_function_metadata::< | ||
VMDynamicFunctionContext<DynamicFunctionWithEnv<Env>>, | ||
>(dynamic_ctx, import_init_function_ptr); | ||
|
||
// We don't yet have the address with the Wasm ABI signature. | ||
|
||
// We don't yet have the address with the Wasm ABI signature. | ||
// The engine linker will replace the address with one pointing to a | ||
// generated dynamic trampoline. | ||
|
@@ -387,6 +460,21 @@ impl Function { | |
let function = inner::Function::<Args, Rets>::new(func); | ||
let address = function.address(); | ||
|
||
#[cfg(feature = "async")] | ||
let (host_env, metadata) = { | ||
let host_env_set_yielder_fn = |env_ptr, yielder_ptr| { | ||
let env: &mut Env = unsafe { std::mem::transmute(env_ptr) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
env.set_yielder(yielder_ptr); | ||
}; | ||
|
||
build_export_function_metadata::<Env>( | ||
env, | ||
Env::init_with_instance, | ||
host_env_set_yielder_fn, | ||
) | ||
}; | ||
|
||
#[cfg(not(feature = "async"))] | ||
let (host_env, metadata) = | ||
build_export_function_metadata::<Env>(env, Env::init_with_instance); | ||
|
||
|
@@ -484,7 +572,7 @@ impl Function { | |
&self.store | ||
} | ||
|
||
fn call_wasm( | ||
pub(crate) fn call_wasm( | ||
&self, | ||
func: &WasmFunctionDefinition, | ||
params: &[Val], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
use async_wormhole::AsyncYielder; | ||
|
||
use crate::{RuntimeError, Val}; | ||
|
||
/// Wrapper around `async-wormhole`'s yielder | ||
#[derive(Clone)] | ||
pub struct Yielder { | ||
inner: *const std::ffi::c_void, | ||
} | ||
|
||
impl Yielder { | ||
/// Create a new instance from a raw pointer to a yielder | ||
pub fn new(inner: *const std::ffi::c_void) -> Self { | ||
Self { inner } | ||
} | ||
|
||
/// Get the `AsyncYielder` | ||
pub fn get(&self) -> &mut AsyncYielder<Result<Box<[Val]>, RuntimeError>> { | ||
let yielder: &mut AsyncYielder<Result<Box<[Val]>, RuntimeError>> = | ||
unsafe { std::mem::transmute(self.inner) }; | ||
|
||
yielder | ||
} | ||
} | ||
|
||
//FIXME | ||
unsafe impl Send for Yielder {} | ||
unsafe impl Sync for Yielder {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs to be
unsafe
and have a# Safety
section describing how to correctly call and/or implement this function