Skip to content

Commit

Permalink
feat: Remove caching in wasmer_wamr mode (#127)
Browse files Browse the repository at this point in the history
* 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 cc5e49a.

* 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 <[email protected]>

* chore: doc comment clarification

* revert: split error handling line

* chore: typo

---------

Co-authored-by: ThetaSinner <[email protected]>
  • Loading branch information
mattyg and ThetaSinner authored Dec 18, 2024
1 parent acecaca commit c6d0e71
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 29 deletions.
9 changes: 5 additions & 4 deletions crates/common/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ pub enum WasmErrorInner {
/// AND wasm execution MUST immediately halt.
/// The `Vec<u8>` holds the encoded data as though the guest had returned.
HostShortCircuit(Vec<u8>),
/// 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.
Expand All @@ -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.
Expand Down
30 changes: 20 additions & 10 deletions crates/host/src/module.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -184,7 +194,7 @@ impl SerializedModuleCache {
// We do this the long way to get `Bytes` instead of `Vec<u8>` 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()
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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))
}
Expand Down Expand Up @@ -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() {
Expand Down
5 changes: 5 additions & 0 deletions crates/host/src/module/wasmer_sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}

/// Take WASM binary and prepare a wasmer Module suitable for iOS
pub fn build_ios_module(wasm: &[u8]) -> Result<Module, CompileError> {
info!(
Expand Down
32 changes: 32 additions & 0 deletions crates/host/src/module/wasmer_wamr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::prelude::*;
use std::path::Path;
use std::sync::Arc;
use wasmer::CompileError;
use wasmer::DeserializeError;
use wasmer::Engine;
Expand All @@ -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<Arc<Module>, 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<Module, CompileError> {
unimplemented!("The feature flag 'wasmer_sys' must be enabled to support compiling wasm");
Expand All @@ -24,3 +34,25 @@ pub fn build_ios_module(_wasm: &[u8]) -> Result<Module, CompileError> {
pub fn get_ios_module_from_file(_path: &Path) -> Result<Module, DeserializeError> {
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<u8> = 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())
}
}
20 changes: 5 additions & 15 deletions test/src/wasms.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand All @@ -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;
Expand Down Expand Up @@ -131,21 +134,8 @@ impl TestWasm {
}

#[cfg(feature = "wasmer_wamr")]
pub fn module(&self, metered: bool) -> Arc<Module> {
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<Module> {
build_module(self.bytes()).unwrap()
}

pub fn _instance(&self, metered: bool) -> InstanceWithStore {
Expand Down

0 comments on commit c6d0e71

Please sign in to comment.