From c6d0e71eb4a9a55a8f3a7d3a600385d1f4f84d4c Mon Sep 17 00:00:00 2001 From: mattyg Date: Wed, 18 Dec 2024 10:50:31 -0700 Subject: [PATCH] feat: Remove caching in wasmer_wamr mode (#127) * feat: expose fn to enable building a wasmer module outside of the ModuleCache trait * revert: accidentally removed comment * build: bump wasmer to 5.0.3 * Revert "build: bump wasmer to 5.0.3" This reverts commit cc5e49a5b22903b157d1d6687b272a1dbf8b049d. * chore: fmt + clippy * fix: scripts are executable * feat: ModuleCache is only exposed in wasmer_sys mode, build_module() is only exposed in wasmer_wamr mode, rename error variant for accuracy * test: smoke test for building module in wasmer_wamr mode * doc: cargo doc warning * chore: fmt * refactor: always export caching functionality -- avoid passing up feature flag conditionals that make the consuming code more complex and harder to understand * chore: empty list of features is unnecessary * chore: reduce git diff * chore: reduce git diff 2 * chore: reduce git diff 3 * chore: clippy * chore: meaningless empty features * chore: make internal-only types & crates private * chore: make scripts executable * chore: fmt * fix: high-level scripts run from root directory * chore: rename file * chore: reduce diff * chore: make internal fns private * chore: diff * feat: ensure consistent public api to avoid conditional imports * chore: tests mod is private Co-authored-by: ThetaSinner * chore: doc comment clarification * revert: split error handling line * chore: typo --------- Co-authored-by: ThetaSinner --- crates/common/src/result.rs | 9 ++++---- crates/host/src/module.rs | 30 ++++++++++++++++--------- crates/host/src/module/wasmer_sys.rs | 5 +++++ crates/host/src/module/wasmer_wamr.rs | 32 +++++++++++++++++++++++++++ test/src/wasms.rs | 20 +++++------------ 5 files changed, 67 insertions(+), 29 deletions(-) diff --git a/crates/common/src/result.rs b/crates/common/src/result.rs index b81f938f..a013cd43 100644 --- a/crates/common/src/result.rs +++ b/crates/common/src/result.rs @@ -30,8 +30,9 @@ pub enum WasmErrorInner { /// AND wasm execution MUST immediately halt. /// The `Vec` holds the encoded data as though the guest had returned. HostShortCircuit(Vec), - /// Wasmer failed to compile machine code from wasm byte code. - 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), /// The host failed to call a function in the guest. CallError(String), /// Host attempted to interact with the module cache before it was initialized. @@ -50,8 +51,8 @@ impl WasmErrorInner { | Self::ErrorWhileError // Bad memory is bad memory. | Self::Memory - // Failing to compile means we cannot reuse. - | Self::Compile(_) + // Failing to build the wasmer Module means we cannot use it. + | Self::ModuleBuild(_) // This is ambiguous so best to treat as potentially corrupt. | Self::CallError(_) // We have no cache so cannot cache. diff --git a/crates/host/src/module.rs b/crates/host/src/module.rs index 0d6fc181..34277e0f 100644 --- a/crates/host/src/module.rs +++ b/crates/host/src/module.rs @@ -1,3 +1,13 @@ +//! Wasmer Host Module Manager +//! +//! This provides two ways to build & access wasm modules: +//! +//! 1. When using the feature flag `wasmer_sys`, modules should be accessed only via the [`ModuleCache`]. +//! This ensures that wasm modules are compiled once, then cached and stored efficiently. +//! +//! 2. When using the feature flag `wasmer_wamr`, modules should be built via the exported build_module function. +//! There is no need for caching, as the wasm module is interpreted. + use crate::plru::MicroCache; use crate::prelude::*; use bimap::BiMap; @@ -184,7 +194,7 @@ impl SerializedModuleCache { // We do this the long way to get `Bytes` instead of `Vec` so // that the clone when we both deserialize and cache is cheap. let mut file = File::open(module_path).map_err(|e| { - wasm_error!(WasmErrorInner::Compile(format!( + wasm_error!(WasmErrorInner::ModuleBuild(format!( "{} Path: {}", e, module_path.display() @@ -193,7 +203,7 @@ impl SerializedModuleCache { let mut bytes_mut = BytesMut::new().writer(); std::io::copy(&mut file, &mut bytes_mut).map_err(|e| { - wasm_error!(WasmErrorInner::Compile(format!( + wasm_error!(WasmErrorInner::ModuleBuild(format!( "{} Path: {}", e, module_path.display() @@ -205,7 +215,7 @@ impl SerializedModuleCache { Some(Ok(serialized_module)) => { let deserialized_module = unsafe { Module::deserialize(&self.runtime_engine, serialized_module.clone()) } - .map_err(|e| wasm_error!(WasmErrorInner::Compile(e.to_string())))?; + .map_err(|e| wasm_error!(WasmErrorInner::ModuleBuild(e.to_string())))?; (deserialized_module, serialized_module) } // no serialized module found on the file system, so serialize the @@ -216,10 +226,10 @@ impl SerializedModuleCache { // module once and available in all instances created from it. let compiler_engine = (self.make_engine)(); let module = Module::from_binary(&compiler_engine, wasm) - .map_err(|e| wasm_error!(WasmErrorInner::Compile(e.to_string())))?; + .map_err(|e| wasm_error!(WasmErrorInner::ModuleBuild(e.to_string())))?; let serialized_module = module .serialize() - .map_err(|e| wasm_error!(WasmErrorInner::Compile(e.to_string())))?; + .map_err(|e| wasm_error!(WasmErrorInner::ModuleBuild(e.to_string())))?; if let Some(module_path) = maybe_module_path { match OpenOptions::new() @@ -253,12 +263,12 @@ impl SerializedModuleCache { // prevent memory access out of bounds errors. // // This procedure facilitates caching of modules that can be - // instatiated with fresh stores free from state. Instance + // instantiated with fresh stores free from state. Instance // creation is highly performant which makes caching of instances // and stores unnecessary. let module = unsafe { Module::deserialize(&self.runtime_engine, serialized_module.clone()) - .map_err(|e| wasm_error!(WasmErrorInner::Compile(e.to_string())))? + .map_err(|e| wasm_error!(WasmErrorInner::ModuleBuild(e.to_string())))? }; (module, serialized_module) @@ -277,7 +287,7 @@ impl SerializedModuleCache { let module = unsafe { Module::deserialize(&self.runtime_engine, (**serialized_module).clone()) } - .map_err(|e| wasm_error!(WasmErrorInner::Compile(e.to_string())))?; + .map_err(|e| wasm_error!(WasmErrorInner::ModuleBuild(e.to_string())))?; self.touch(&key); Ok(Arc::new(module)) } @@ -364,8 +374,8 @@ impl ModuleCache { } #[cfg(test)] -pub mod tests { - use crate::module::{CacheKey, ModuleCache, PlruCache}; +mod tests { + use super::{CacheKey, ModuleCache, PlruCache}; #[test] fn cache_test() { diff --git a/crates/host/src/module/wasmer_sys.rs b/crates/host/src/module/wasmer_sys.rs index dbba3d35..82b010d3 100644 --- a/crates/host/src/module/wasmer_sys.rs +++ b/crates/host/src/module/wasmer_sys.rs @@ -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, wasmer::RuntimeError> { + unimplemented!("The feature flag 'wasmer_wamr' must be enabled to support building a Module directly. Please use the ModuleCache instead."); +} + /// Take WASM binary and prepare a wasmer Module suitable for iOS pub fn build_ios_module(wasm: &[u8]) -> Result { info!( diff --git a/crates/host/src/module/wasmer_wamr.rs b/crates/host/src/module/wasmer_wamr.rs index 07cf51b3..8d620fe4 100644 --- a/crates/host/src/module/wasmer_wamr.rs +++ b/crates/host/src/module/wasmer_wamr.rs @@ -1,4 +1,6 @@ +use crate::prelude::*; use std::path::Path; +use std::sync::Arc; use wasmer::CompileError; use wasmer::DeserializeError; use wasmer::Engine; @@ -15,6 +17,14 @@ pub(crate) fn make_runtime_engine() -> Engine { Engine::default() } +/// Build an interpreter module from wasm bytes. +pub fn build_module(wasm: &[u8]) -> Result, wasmer::RuntimeError> { + let compiler_engine = make_engine(); + let res = Module::from_binary(&compiler_engine, wasm); + let module = res.map_err(|e| wasm_error!(WasmErrorInner::ModuleBuild(e.to_string())))?; + Ok(Arc::new(module)) +} + /// Take WASM binary and prepare a wasmer Module suitable for iOS pub fn build_ios_module(_wasm: &[u8]) -> Result { unimplemented!("The feature flag 'wasmer_sys' must be enabled to support compiling wasm"); @@ -24,3 +34,25 @@ pub fn build_ios_module(_wasm: &[u8]) -> Result { pub fn get_ios_module_from_file(_path: &Path) -> Result { unimplemented!("The feature flag 'wasmer_sys' must be enabled to support compiling wasm"); } + +#[cfg(test)] +pub mod tests { + use super::build_module; + + #[test] + fn build_module_test() { + // simple example wasm taken from wasmer docs + // https://docs.rs/wasmer/latest/wasmer/struct.Module.html#example + let wasm: Vec = vec![ + 0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x01, 0x06, 0x01, 0x60, 0x01, 0x7f, + 0x01, 0x7f, 0x03, 0x02, 0x01, 0x00, 0x07, 0x0b, 0x01, 0x07, 0x61, 0x64, 0x64, 0x5f, + 0x6f, 0x6e, 0x65, 0x00, 0x00, 0x0a, 0x09, 0x01, 0x07, 0x00, 0x20, 0x00, 0x41, 0x01, + 0x6a, 0x0b, 0x00, 0x1a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x01, 0x0a, 0x01, 0x00, 0x07, + 0x61, 0x64, 0x64, 0x5f, 0x6f, 0x6e, 0x65, 0x02, 0x07, 0x01, 0x00, 0x01, 0x00, 0x02, + 0x70, 0x30, + ]; + + let res = build_module(wasm.as_slice()); + assert!(res.is_ok()) + } +} diff --git a/test/src/wasms.rs b/test/src/wasms.rs index 9f9146fc..a9ca8fef 100644 --- a/test/src/wasms.rs +++ b/test/src/wasms.rs @@ -1,4 +1,6 @@ use crate::import::imports; +#[cfg(feature = "wasmer_wamr")] +use holochain_wasmer_host::module::build_module; use holochain_wasmer_host::module::InstanceWithStore; use holochain_wasmer_host::module::SerializedModuleCache; use holochain_wasmer_host::prelude::*; @@ -10,6 +12,7 @@ use wasmer::wasmparser::Operator; use wasmer::AsStoreMut; #[cfg(feature = "wasmer_sys")] use wasmer::CompilerConfig; +#[cfg(feature = "wasmer_sys")] use wasmer::Engine; use wasmer::FunctionEnv; use wasmer::Imports; @@ -131,21 +134,8 @@ impl TestWasm { } #[cfg(feature = "wasmer_wamr")] - pub fn module(&self, metered: bool) -> Arc { - match self.module_cache(false).get() { - Some(cache) => cache.write().get(self.key(false), self.bytes()).unwrap(), - None => { - // This will error if the cache is already initialized - // which could happen if two tests are running in parallel. - // It doesn't matter which one wins, so we just ignore the error. - let _did_init_ok = self.module_cache(metered).set(parking_lot::RwLock::new( - SerializedModuleCache::default_with_engine(Engine::default, None), - )); - - // Just recurse now that the cache is initialized. - self.module(metered) - } - } + pub fn module(&self, _metered: bool) -> Arc { + build_module(self.bytes()).unwrap() } pub fn _instance(&self, metered: bool) -> InstanceWithStore {