-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: Remove caching in wasmer_wamr mode #127
Conversation
…is only exposed in wasmer_wamr mode, rename error variant for accuracy
…ture flag conditionals that make the consuming code more complex and harder to understand
Compile(String), | ||
/// Wasmer failed to build a Module from wasm byte code. | ||
/// With the feature `wasmer_sys` enabled, building a Module includes compiling the wasm. | ||
ModuleBuild(String), |
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.
Rename error type to clarify that it is an error arising when creating a new wasmer Module. Only in wasmer_sys
mode does compilation occur here.
use super::build_module; | ||
|
||
#[test] | ||
fn build_module_test() { |
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.
smoke test for building a wasm module in wasmer_wamr
mode.
…ule-outside-cache
…ule-outside-cache
@@ -54,6 +54,11 @@ pub(crate) fn make_runtime_engine() -> Engine { | |||
Engine::headless() | |||
} | |||
|
|||
/// Build an interpreter module from wasm bytes. | |||
pub fn build_module(_wasm: &[u8]) -> Result<Arc<Module>, wasmer::RuntimeError> { | |||
unimplemented!("The feature flag 'wasmer_wamr' must be enabled to support building a Module directly. Please use the ModuleCache instead."); |
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 is keep a consistent api so we don't need to use conditional feature flags in the consumer.
I recognize this is silly, since the function below build_module_for_ios
does the same thing. But for clarity I'd like to wait to remove that til we remove the "prebuilt ios modules" functionality in its entirety.
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.
Not sure how much value I'm providing with a review here but the reason for the change makes sense and all the code changes seem consistent with the goal and each other :)
Co-authored-by: ThetaSinner <[email protected]>
In
wasmer_wamr
mode, expose a public functionbuild_module
to build a wasm module directly, bypassing the cache.This is a prerequisite for holochain/holochain#4441
This PR is merging into
chore/cleanup-make-private
for a more legible diff.