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

Refactor/clarify dylib ios functionality #118

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

- **BREAKING CHANGE:** Remove unused function `wasmer_ios_target`
- **BREAKING CHANGE:** Rename functions `build_ios_module` to `write_precompiled_serialized_module_to_file` and `get_ios_module_from_file` to `read_precompiled_serialized_module_from_file` to clarify that they are not an ios-specific.
mattyg marked this conversation as resolved.
Show resolved Hide resolved

## [0.0.93] - 2024-04-24

### Changed
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ bytes = "1"
hex = "0.4"
thiserror = "1"

[dev-dependencies]
tempfile = "3.12.0"

[lib]
name = "holochain_wasmer_host"
crate-type = ["cdylib", "rlib"]
Expand Down
10 changes: 10 additions & 0 deletions crates/host/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use holochain_wasmer_common::WasmError;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use wasmer::{CompileError, SerializeError};

/// Wraps a WasmErrorInner with a file and line number.
/// The easiest way to generate this is with the `wasm_error!` macro that will
Expand Down Expand Up @@ -41,3 +42,12 @@ macro_rules! wasm_host_error {
$crate::wasm_host_error!(std::format!($($arg)*))
}};
}

#[derive(Debug, Error)]
pub enum PreCompiledSerilializedModuleError {
#[error(transparent)]
CompileError(#[from] CompileError),

#[error(transparent)]
SerializeError(#[from] SerializeError),
}
37 changes: 22 additions & 15 deletions crates/host/src/module/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,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 Down Expand Up @@ -385,19 +381,9 @@ impl ModuleCache {
}
}

/// Configuration of a Target for wasmer for iOS
pub fn wasmer_ios_target() -> Target {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function does not appear to be used anywhere in consuming crates

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added by @jost-s, was it something that used to be in Holochain core and got moved here? That's what I vaguely remember. If so, was it something that Guillem was using?

Copy link
Member Author

@mattyg mattyg Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that's right -- I don't see it used anywhere https://github.com/search?q=wasmer_ios_target&type=code

But actually now I realized that parts of this functionality aren't working so I'm not sure it was ever really tested or used. I'm going to wait to merge this PR and see if more changes needed to this crate first.

// 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 {
use crate::module::{CacheKey, ModuleCache, PlruCache};
use crate::module::{CacheKey, ModuleCache, PlruCache, write_precompiled_serialized_module_to_file, read_precompiled_serialized_module_from_file};

#[test]
fn cache_test() {
Expand Down Expand Up @@ -438,4 +424,25 @@ pub mod tests {
assert_eq!(*deserialized_cached_module, *module);
}
}

#[test]
#[cfg(feature = "wasmer_sys")]
fn precompiled_serialized_module_roundtrip_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 tmp = tempfile::tempdir().unwrap();
let path = tmp.path().join("module.dylib");
write_precompiled_serialized_module_to_file(wasm.as_slice(), path.clone()).unwrap();
let _module = read_precompiled_serialized_module_from_file(path.as_path()).unwrap();
mattyg marked this conversation as resolved.
Show resolved Hide resolved
}
}
24 changes: 14 additions & 10 deletions crates/host/src/module/wasmer_sys.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::error::PreCompiledSerilializedModuleError;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use tracing::info;
use wasmer::sys::BaseTunables;
use wasmer::sys::CompilerConfig;
use wasmer::wasmparser;
use wasmer::CompileError;
use wasmer::Cranelift;
use wasmer::DeserializeError;
use wasmer::Engine;
Expand Down Expand Up @@ -49,18 +49,22 @@ pub fn make_runtime_engine() -> Engine {
Engine::headless()
}

/// Take WASM binary and prepare a wasmer Module suitable for iOS
pub fn build_ios_module(wasm: &[u8]) -> Result<Module, CompileError> {
info!(
"Found wasm and was instructed to serialize it for ios in wasmer format, doing so now..."
);
/// Compile a wasm binary, serialize it with wasmer's serializtion format, and write to a file.
mattyg marked this conversation as resolved.
Show resolved Hide resolved
/// This file can later be used for contexts where JIT compilation is not possible (iOS for example).
pub fn write_precompiled_serialized_module_to_file(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove references to iOS in function name and comments, as this functionality can be used in other platforms as well

wasm: &[u8],
path: PathBuf,
) -> Result<(), PreCompiledSerilializedModuleError> {
let compiler_engine = make_engine();
let store = Store::new(compiler_engine);
Module::from_binary(&store, wasm)
Module::from_binary(&store, wasm)?.serialize_to_file(path.clone())?;
Ok(())
}

/// Deserialize a previously compiled module for iOS from a file.
pub fn get_ios_module_from_file(path: &Path) -> Result<Module, DeserializeError> {
/// Deserialize a previously precompiled and serialized module
pub fn read_precompiled_serialized_module_from_file(
path: &Path,
) -> Result<Module, DeserializeError> {
let engine = Engine::headless();
unsafe { Module::deserialize_from_file(&engine, path) }
}
19 changes: 14 additions & 5 deletions crates/host/src/module/wasmer_wamr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::error::PreCompiledSerilializedModuleError;
use std::path::Path;
use wasmer::CompileError;
use std::path::PathBuf;
use wasmer::DeserializeError;
use wasmer::Engine;
use wasmer::Module;
Expand All @@ -15,12 +16,20 @@ pub fn make_runtime_engine() -> Engine {
Engine::default()
}

/// Take WASM binary and prepare a wasmer Module suitable for iOS
pub fn build_ios_module(_wasm: &[u8]) -> Result<Module, CompileError> {
/// Compile a wasm binary, serialize it with wasmer's serializtion format, and write to a file.
mattyg marked this conversation as resolved.
Show resolved Hide resolved
/// This file can later be used for contexts where JIT compilation is not possible (iOS for example).
pub fn write_precompiled_serialized_module_to_file(
_wasm: &[u8],
_path: PathBuf,
) -> Result<(), PreCompiledSerilializedModuleError> {
unimplemented!("The feature flag 'wasmer_sys' must be enabled to support compiling wasm");
}

/// Deserialize a previously compiled module for iOS from a file.
pub fn get_ios_module_from_file(_path: &Path) -> Result<Module, DeserializeError> {
/// Deserialize a previously precompiled and serialized module.
/// Even though the `wasmer_wamr` feature flag supports deserializing a pre-compiled and serialized module,
/// it doesn't make sense to use a pre-compiled module as it would be executed by the interpreter engine anyway.
pub fn read_precompiled_serialized_module_from_file(
mattyg marked this conversation as resolved.
Show resolved Hide resolved
_path: &Path,
) -> Result<Module, DeserializeError> {
unimplemented!("The feature flag 'wasmer_sys' must be enabled to support compiling wasm");
}
Loading