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.

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

Merged
merged 35 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7302ad0
feat: expose fn to enable building a wasmer module outside of the Mod…
mattyg Nov 11, 2024
85336ee
revert: accidentally removed comment
mattyg Nov 11, 2024
052dca6
Merge branch 'main' into feat/support-build-module-outside-cache
mattyg Nov 27, 2024
cc5e49a
build: bump wasmer to 5.0.3
mattyg Dec 13, 2024
ce10788
Revert "build: bump wasmer to 5.0.3"
mattyg Dec 13, 2024
fc15501
chore: fmt + clippy
mattyg Dec 14, 2024
e40c9f3
fix: scripts are executable
mattyg Dec 14, 2024
0fe1843
feat: ModuleCache is only exposed in wasmer_sys mode, build_module() …
mattyg Dec 14, 2024
c2ba5b7
test: smoke test for building module in wasmer_wamr mode
mattyg Dec 14, 2024
3b745ca
doc: cargo doc warning
mattyg Dec 14, 2024
d866ebd
chore: fmt
mattyg Dec 14, 2024
a299e31
refactor: always export caching functionality -- avoid passing up fea…
mattyg Dec 14, 2024
fd98bee
chore: empty list of features is unnecessary
mattyg Dec 14, 2024
d092438
chore: reduce git diff
mattyg Dec 14, 2024
c0ad0fe
chore: reduce git diff 2
mattyg Dec 14, 2024
6dd0b41
chore: reduce git diff 3
mattyg Dec 14, 2024
1a5b46b
chore: clippy
mattyg Dec 14, 2024
5f5813f
chore: meaningless empty features
mattyg Dec 14, 2024
e79ec8a
chore: make internal-only types & crates private
mattyg Dec 14, 2024
2253824
chore: make scripts executable
mattyg Dec 14, 2024
773d114
chore: fmt
mattyg Dec 14, 2024
c3354aa
fix: high-level scripts run from root directory
mattyg Dec 14, 2024
41ac607
chore: rename file
mattyg Dec 14, 2024
5207718
Merge branch 'chore/cleanup-make-private' into feat/support-build-mod…
mattyg Dec 14, 2024
a04b6a2
chore: reduce diff
mattyg Dec 14, 2024
c366edf
chore: make internal fns private
mattyg Dec 14, 2024
17da637
Merge branch 'chore/cleanup-make-private' into feat/support-build-mod…
mattyg Dec 14, 2024
b55a420
chore: diff
mattyg Dec 14, 2024
594a8c1
feat: ensure consistent public api to avoid conditional imports
mattyg Dec 14, 2024
d9614af
Merge branch 'main' into feat/support-build-module-outside-cache
mattyg Dec 16, 2024
99e3980
Merge branch 'main' into feat/support-build-module-outside-cache
mattyg Dec 16, 2024
a5c86ac
chore: tests mod is private
mattyg Dec 18, 2024
c409299
chore: doc comment clarification
mattyg Dec 18, 2024
acfb72f
revert: split error handling line
mattyg Dec 18, 2024
cd40252
chore: typo
mattyg Dec 18, 2024
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
7 changes: 4 additions & 3 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),
Copy link
Member Author

@mattyg mattyg Dec 14, 2024

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.

/// 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 @@ -51,7 +52,7 @@ impl WasmErrorInner {
// Bad memory is bad memory.
| Self::Memory
// Failing to compile means we cannot reuse.
mattyg marked this conversation as resolved.
Show resolved Hide resolved
| Self::Compile(_)
| Self::ModuleBuild(_)
// This is ambiguous so best to treat as potentially corrupt.
| Self::CallError(_)
// We have no cache so cannot cache.
Expand Down
2 changes: 1 addition & 1 deletion crates/guest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ crate-type = ["cdylib", "rlib"]
path = "src/guest.rs"

[dependencies]
holochain_serialized_bytes = { version = "=0.0.55", features = [] }
holochain_serialized_bytes = "=0.0.55"
holochain_wasmer_common = { version = "=0.0.96", path = "../common" }
serde = "1"
tracing = "0.1"
Expand Down
53 changes: 26 additions & 27 deletions crates/host/src/module/mod.rs → 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 All @@ -11,15 +21,11 @@ use std::fs::File;
use std::fs::OpenOptions;
use std::io::Write;
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::Arc;
use wasmer::CpuFeature;
use wasmer::Engine;
use wasmer::Instance;
use wasmer::Module;
use wasmer::Store;
use wasmer::Target;
use wasmer::Triple;

#[cfg(feature = "wasmer_sys")]
mod wasmer_sys;
Expand All @@ -36,9 +42,9 @@ pub use wasmer_wamr::*;
pub type CacheKey = [u8; 32];
/// Plru uses a usize to track "recently used" so we need a map between 32 byte cache
/// keys and the bits used to evict things from the cache.
pub type PlruKeyMap = BiMap<usize, CacheKey>;
type PlruKeyMap = BiMap<usize, CacheKey>;
/// Modules serialize to a vec of bytes as per wasmer.
pub type SerializedModule = Bytes;
type SerializedModule = Bytes;

#[derive(Clone, Debug)]
pub struct ModuleWithStore {
Expand All @@ -56,7 +62,7 @@ pub struct InstanceWithStore {
/// with consistently. Default implementations for key functions are provided.
/// Notably handles keeping the mapping between cache keys and items, and the
/// plru tracking including touching and evicting.
pub trait PlruCache {
trait PlruCache {
/// The type of items in the cache.
type Item;
/// Accessor for mutable reference to internal plru cache.
Expand Down Expand Up @@ -104,12 +110,14 @@ pub trait PlruCache {

/// Delete the plru for a given cache key. Care must be taken to ensure this
/// is not called before a subsequent call to `plru_key` or it will panic.
#[allow(dead_code)]
fn trash(&mut self, key: &CacheKey) {
let plru_key = self.plru_key(key);
self.plru_mut().trash(plru_key);
}

/// Remove an item from the cache and the associated plru entry.
#[allow(dead_code)]
fn remove_item(&mut self, key: &CacheKey) -> Option<Arc<Self::Item>> {
let maybe_item = self.cache_mut().remove(key);
if maybe_item.is_some() {
Expand Down Expand Up @@ -206,7 +214,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 @@ -215,7 +223,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 @@ -227,7 +235,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 @@ -237,11 +245,12 @@ impl SerializedModuleCache {
// of middleware like metering. Middleware is compiled into the
// 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())))?;
let res = Module::from_binary(&compiler_engine, wasm);
let module =
mattyg marked this conversation as resolved.
Show resolved Hide resolved
res.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 @@ -280,7 +289,7 @@ impl SerializedModuleCache {
// 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 @@ -299,7 +308,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 All @@ -312,7 +321,7 @@ impl SerializedModuleCache {
/// the cache to create callable instances is slow. Therefore modules are
/// cached in memory after deserialization.
#[derive(Default, Debug)]
pub struct DeserializedModuleCache {
struct DeserializedModuleCache {
plru: MicroCache,
key_map: PlruKeyMap,
cache: BTreeMap<CacheKey, Arc<Module>>,
Expand Down Expand Up @@ -385,19 +394,9 @@ impl ModuleCache {
}
}

/// Configuration of a Target for wasmer for iOS
pub fn wasmer_ios_target() -> Target {
// use what I see in
// platform ios headless example
// https://github.com/wasmerio/wasmer/blob/447c2e3a152438db67be9ef649327fabcad6f5b8/examples/platform_ios_headless.rs#L38-L53
let triple = Triple::from_str("aarch64-apple-ios").unwrap();
let cpu_feature = CpuFeature::set();
Target::new(triple, cpu_feature)
}

#[cfg(test)]
pub mod tests {
mattyg marked this conversation as resolved.
Show resolved Hide resolved
use crate::module::{CacheKey, ModuleCache, PlruCache};
use super::{CacheKey, ModuleCache, PlruCache};

#[test]
fn cache_test() {
Expand Down
9 changes: 7 additions & 2 deletions crates/host/src/module/wasmer_sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub const WASM_METERING_LIMIT: u64 = 10_000_000;

/// Generate an engine with a wasm compiler
/// and Metering (use limits) in place.
pub fn make_engine() -> Engine {
pub(crate) fn make_engine() -> Engine {
let cost_function = |_operator: &wasmparser::Operator| -> u64 { 1 };
// @todo 100 giga-ops is totally arbitrary cutoff so we probably
// want to make the limit configurable somehow.
Expand All @@ -50,10 +50,15 @@ pub fn make_engine() -> Engine {
engine
}

pub fn make_runtime_engine() -> Engine {
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.");
Copy link
Member Author

@mattyg mattyg Dec 14, 2024

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.

}

/// Take WASM binary and prepare a wasmer Module suitable for iOS
pub fn build_ios_module(wasm: &[u8]) -> Result<Module, CompileError> {
info!(
Expand Down
36 changes: 34 additions & 2 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 @@ -7,14 +9,22 @@ use wasmer::Module;
/// Generate an engine with a wasm interpreter
/// The interpreter used (wasm micro runtime) does not support metering
/// See tracking issue: https://github.com/bytecodealliance/wasm-micro-runtime/issues/2163
pub fn make_engine() -> Engine {
pub(crate) fn make_engine() -> Engine {
Engine::default()
}

pub fn make_runtime_engine() -> Engine {
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() {
Copy link
Member Author

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.

// 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())
}
}
6 changes: 3 additions & 3 deletions scripts/bench.sh
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#! /usr/bin/env bash

./bench-wasmer_sys_dev.sh
./bench-wasmer_sys_prod.sh
./bench-wasmer_wamr.sh
./scripts/bench-wasmer_sys_dev.sh
./scripts/bench-wasmer_sys_prod.sh
./scripts/bench-wasmer_wamr.sh
6 changes: 3 additions & 3 deletions scripts/test.sh
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#! /usr/bin/env bash

./test-wasmer_sys_dev.sh
./test-wasmer_sys_prod.sh
./test-wasmer_wamr.sh
./scripts/test-wasmer_sys_dev.sh
./scripts/test-wasmer_sys_prod.sh
./scripts/test-wasmer_wamr.sh
10 changes: 5 additions & 5 deletions test/benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use wasmer::Store;
pub fn wasm_module_compile(c: &mut Criterion) {
let mut group = c.benchmark_group("wasm_module_compile");

for wasm in vec![
for wasm in [
TestWasm::Empty,
TestWasm::Io,
TestWasm::Test,
Expand All @@ -31,7 +31,7 @@ pub fn wasm_module_compile(c: &mut Criterion) {
pub fn wasm_module_deserialize_from_file(c: &mut Criterion) {
let mut group = c.benchmark_group("wasm_module_deserialize_from_file");

for wasm in vec![
for wasm in [
TestWasm::Empty,
TestWasm::Io,
TestWasm::Test,
Expand All @@ -56,7 +56,7 @@ pub fn wasm_module_deserialize_from_file(c: &mut Criterion) {
pub fn wasm_module(c: &mut Criterion) {
let mut group = c.benchmark_group("wasm_module");

for wasm in vec![
for wasm in [
TestWasm::Empty,
TestWasm::Io,
TestWasm::Test,
Expand All @@ -76,7 +76,7 @@ pub fn wasm_module(c: &mut Criterion) {
pub fn wasm_instance(c: &mut Criterion) {
let mut group = c.benchmark_group("wasm_instance");

for wasm in vec![
for wasm in [
TestWasm::Empty,
TestWasm::Io,
TestWasm::Test,
Expand Down Expand Up @@ -213,7 +213,7 @@ pub fn test_process_string(c: &mut Criterion) {

let instance_with_store = TestWasm::Test.unmetered_instance();

for n in vec![0, 1, 1_000, 1_000_000] {
for n in [0, 1, 1_000, 1_000_000] {
group.throughput(Throughput::Bytes(n));
group.sample_size(10);
let input = test_common::StringType::from(".".repeat(n.try_into().unwrap()));
Expand Down
14 changes: 7 additions & 7 deletions test/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ pub mod tests {
&mut store.lock().as_store_mut(),
instance,
"ignore_args_process_string",
&StringType::from(String::new()),
StringType::from(String::new()),
)
.expect("ignore_args_process_string call");
assert_eq!(String::new(), String::from(result));
Expand All @@ -254,7 +254,7 @@ pub mod tests {
&mut store.lock().as_store_mut(),
instance,
"process_string",
&StringType::from(s.clone()),
StringType::from(s.clone()),
)
.expect("process string call");

Expand All @@ -267,15 +267,15 @@ pub mod tests {
fn process_string_test() {
// use a "crazy" string that is much longer than a single wasm page to show that pagination
// and utf-8 are both working OK
let starter_string = "╰▐ ✖ 〜 ✖ ▐╯"
.repeat(usize::try_from(10_u32 * u32::try_from(std::u16::MAX).unwrap()).unwrap());
let starter_string =
"╰▐ ✖ 〜 ✖ ▐╯".repeat(usize::try_from(10_u32 * u32::from(u16::MAX)).unwrap());
let InstanceWithStore { store, instance } = TestWasm::Test.instance();
let result: StringType = guest::call(
&mut store.lock().as_store_mut(),
instance,
"process_string",
// This is by reference just to show that it can be done as borrowed or owned.
&StringType::from(starter_string.clone()),
StringType::from(starter_string.clone()),
)
.expect("process string call");

Expand Down Expand Up @@ -320,8 +320,8 @@ pub mod tests {
)
}
});
assert!(matches!(call_1.join(), Ok(_)));
assert!(matches!(call_2.join(), Ok(_)));
assert!(call_1.join().is_ok());
assert!(call_2.join().is_ok());
}

#[test]
Expand Down
Loading
Loading