From 585169ef370bff54894ebc1513407d15ff8265e5 Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Wed, 31 Mar 2021 23:46:30 -0700 Subject: [PATCH] Code review feedback. * Move `Module::compile` to `Engine::precompile_module`. * Remove `Module::deserialize` method. * Make `Module::serialize` the same format as `Engine::precompile_module`. * Make `Engine::precompile_module` return a `Vec`. * Move the remaining serialization-related code to `serialization.rs`. --- RELEASES.md | 15 +-- crates/c-api/src/module.rs | 2 +- crates/wasmtime/src/engine.rs | 28 +++++ crates/wasmtime/src/module.rs | 123 +++----------------- crates/wasmtime/src/module/serialization.rs | 70 ++++++++++- examples/serialize.rs | 2 +- src/commands/compile.rs | 10 +- tests/all/module.rs | 15 +-- tests/all/module_linking.rs | 2 +- tests/all/module_serialize.rs | 6 +- 10 files changed, 133 insertions(+), 140 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index dc1944877818..59f5297a87b2 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -6,14 +6,13 @@ ### Added -* The `wasmtime compile` command was added to support AOT compilation of Wasm - modules. +* Added the `wasmtime compile` command to support AOT compilation of Wasm modules. -* The `Module::compile` method was added to support AOT compilation of a module. +* Added the `Engine::precompile_module` method to support AOT module compilation. * Added the `Config::target` method to change the compilation target of the - configuration. This can be used in conjunction with `Module::compile` to target - a different host triple than the current one. + configuration. This can be used in conjunction with `Engine::precompile_module` + to target a different host triple than the current one. * Added the `Config::cranelift_flag_enable` method to enable setting Cranelift boolean flags or presets in a config. @@ -26,6 +25,8 @@ singular `--wasm-features` option. The previous options are still supported, but are not displayed in help text. +* Breaking: `Module::deserialize` has been removed in favor of `Module::new`. + * Breaking: `Config::cranelift_clear_cpu_flags` was removed. Use `Config::target` to clear the CPU flags for the host's target. @@ -42,10 +43,6 @@ * Breaking: the CLI option `--enable-bulk-memory=false` has been changed to `--wasm-features=-bulk-memory`. -* Modules serialized with `Module::serialize` can now be deserialized with - `Module::deserialize` on a compatible host that does not have to match the - original environment exactly. - ## 0.25.0 Released 2021-03-16. diff --git a/crates/c-api/src/module.rs b/crates/c-api/src/module.rs index c31f125b70b3..25e596ce82bb 100644 --- a/crates/c-api/src/module.rs +++ b/crates/c-api/src/module.rs @@ -186,7 +186,7 @@ pub extern "C" fn wasmtime_module_deserialize( ret: &mut *mut wasm_module_t, ) -> Option> { handle_result( - Module::deserialize(&engine.engine, binary.as_slice()), + Module::new(&engine.engine, binary.as_slice()), |module| { let module = Box::new(wasm_module_t::new(module)); *ret = Box::into_raw(module); diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index eab429d509a1..28391d298b73 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -78,6 +78,34 @@ impl Engine { pub fn same(a: &Engine, b: &Engine) -> bool { Arc::ptr_eq(&a.inner, &b.inner) } + + /// Ahead-of-time (AOT) compiles a WebAssembly module. + /// + /// The `bytes` provided must be in one of two formats: + /// + /// * A [binary-encoded][binary] WebAssembly module. This is always supported. + /// * A [text-encoded][text] instance of the WebAssembly text format. + /// This is only supported when the `wat` feature of this crate is enabled. + /// If this is supplied then the text format will be parsed before validation. + /// Note that the `wat` feature is enabled by default. + /// + /// [binary]: https://webassembly.github.io/spec/core/binary/index.html + /// [text]: https://webassembly.github.io/spec/core/text/index.html + pub fn precompile_module(&self, bytes: &[u8]) -> Result> { + const USE_PAGED_MEM_INIT: bool = cfg!(all(feature = "uffd", target_os = "linux")); + + #[cfg(feature = "wat")] + let bytes = wat::parse_bytes(&bytes)?; + + let (_, artifacts, types) = wasmtime_jit::CompilationArtifacts::build( + &self.inner.compiler, + &bytes, + USE_PAGED_MEM_INIT, + )?; + + crate::module::SerializedModule::from_artifacts(&self.inner.compiler, &artifacts, &types) + .to_bytes() + } } impl Default for Engine { diff --git a/crates/wasmtime/src/module.rs b/crates/wasmtime/src/module.rs index de0e29c12a6b..cffcc042786c 100644 --- a/crates/wasmtime/src/module.rs +++ b/crates/wasmtime/src/module.rs @@ -1,9 +1,7 @@ use crate::types::{ExportType, ExternType, ImportType}; use crate::{Engine, ModuleType}; use anyhow::{bail, Context, Result}; -use bincode::Options; use std::fs; -use std::io::Write; use std::path::Path; use std::sync::Arc; use wasmparser::Validator; @@ -15,9 +13,7 @@ use wasmtime_jit::{CompilationArtifacts, CompiledModule, TypeTables}; mod serialization; -use serialization::SerializedModule; - -const COMPILED_MODULE_HEADER: &[u8] = b"\0wasmtime-aot"; +pub use serialization::SerializedModule; /// A compiled WebAssembly module, ready to be instantiated. /// @@ -111,14 +107,16 @@ struct ModuleInner { impl Module { /// Creates a new WebAssembly `Module` from the given in-memory `bytes`. /// - /// The `bytes` provided must be in one of three formats: + /// The `bytes` provided must be in one of the following formats: /// /// * A [binary-encoded][binary] WebAssembly module. This is always supported. /// * A [text-encoded][text] instance of the WebAssembly text format. /// This is only supported when the `wat` feature of this crate is enabled. /// If this is supplied then the text format will be parsed before validation. /// Note that the `wat` feature is enabled by default. - /// * A module compiled with [`Module::compile`] or the `wasmtime compile` command. + /// * A module serialized with [`Module::serialize`]. + /// * A module compiled with [`Engine::precompile_module`] or the + /// `wasmtime compile` command. /// /// The data for the wasm module must be loaded in-memory if it's present /// elsewhere, for example on disk. This requires that the entire binary is @@ -177,8 +175,9 @@ impl Module { /// ``` pub fn new(engine: &Engine, bytes: impl AsRef<[u8]>) -> Result { let bytes = bytes.as_ref(); - if bytes.starts_with(COMPILED_MODULE_HEADER) { - return Self::deserialize(engine, &bytes[COMPILED_MODULE_HEADER.len()..]); + + if let Some(module) = SerializedModule::from_bytes(bytes)? { + return module.into_module(engine); } #[cfg(feature = "wat")] @@ -267,8 +266,8 @@ impl Module { /// # } /// ``` pub fn from_binary(engine: &Engine, binary: &[u8]) -> Result { - if binary.starts_with(COMPILED_MODULE_HEADER) { - return Self::deserialize(engine, &binary[COMPILED_MODULE_HEADER.len()..]); + if let Some(module) = SerializedModule::from_bytes(binary)? { + return module.into_module(engine); } // Check to see that the config's target matches the host @@ -344,41 +343,6 @@ impl Module { Ok(()) } - /// Ahead-of-time (AOT) compiles a WebAssembly module. - /// - /// The `bytes` provided must be in one of two formats: - /// - /// * A [binary-encoded][binary] WebAssembly module. This is always supported. - /// * A [text-encoded][text] instance of the WebAssembly text format. - /// This is only supported when the `wat` feature of this crate is enabled. - /// If this is supplied then the text format will be parsed before validation. - /// Note that the `wat` feature is enabled by default. - /// - /// See [`Module::new`] for errors that may be returned by this function. - /// - /// [binary]: https://webassembly.github.io/spec/core/binary/index.html - /// [text]: https://webassembly.github.io/spec/core/text/index.html - pub fn compile(engine: &Engine, bytes: &[u8], mut output: impl Write) -> Result<()> { - const USE_PAGED_MEM_INIT: bool = cfg!(all(feature = "uffd", target_os = "linux")); - - if bytes.starts_with(COMPILED_MODULE_HEADER) { - bail!("input is already a compiled module"); - } - - #[cfg(feature = "wat")] - let bytes = wat::parse_bytes(&bytes)?; - - let (_, artifacts, types) = - CompilationArtifacts::build(engine.compiler(), &bytes, USE_PAGED_MEM_INIT)?; - - // Write a header that marks this as a compiled module - output.write_all(COMPILED_MODULE_HEADER)?; - Self::serialize_module( - &SerializedModule::from_artifacts(engine.compiler(), &artifacts, &types), - output, - ) - } - /// Returns the type signature of this module. pub fn ty(&self) -> ModuleType { let mut sig = ModuleType::new(); @@ -396,58 +360,12 @@ impl Module { sig } - /// Serialize compilation artifacts to the buffer. See also `deserialize`. + /// Serialize the module to a vector of bytes. + /// + /// Use `Module::new` or `Module::from_binary` to create the module + /// from the bytes. pub fn serialize(&self) -> Result> { - let mut buffer = Vec::new(); - Self::serialize_module(&SerializedModule::new(self), &mut buffer)?; - Ok(buffer) - } - - fn serialize_module(module: &SerializedModule, mut output: impl Write) -> Result<()> { - // Preface the data with a version so we can do a version check independent - // of the serialized data. - let version = env!("CARGO_PKG_VERSION"); - assert!( - version.len() < 256, - "package version must be less than 256 bytes" - ); - output.write(&[version.len() as u8])?; - output.write_all(version.as_bytes())?; - bincode_options().serialize_into(output, module)?; - Ok(()) - } - - /// Deserializes and creates a module from the compilation artifacts. - /// The `serialize` saves the compilation artifacts along with the host - /// fingerprint, which consists of target, compiler flags, and wasmtime - /// package version. - /// - /// The method will fail if fingerprints of current host and serialized - /// one are different. The method does not verify the serialized artifacts - /// for modifications or corruptions. All responsibly of signing and its - /// verification falls on the embedder. - pub fn deserialize(engine: &Engine, serialized: &[u8]) -> Result { - if serialized.is_empty() { - bail!("serialized data data is empty"); - } - - let version_len = serialized[0] as usize; - if serialized.len() < version_len + 1 { - bail!("serialized data is malformed"); - } - - let version = std::str::from_utf8(&serialized[1..1 + version_len])?; - if version != env!("CARGO_PKG_VERSION") { - bail!( - "Module was compiled with incompatible Wasmtime version '{}'", - version - ); - } - - bincode_options() - .deserialize::>(&serialized[1 + version_len..]) - .context("Deserialize compilation artifacts")? - .into_module(engine) + SerializedModule::new(self).to_bytes() } /// Creates a submodule `Module` value from the specified parameters. @@ -732,17 +650,6 @@ impl Module { } } -fn bincode_options() -> impl Options { - // Use a variable-length integer encoding instead of fixed length. The - // module shown on #2318 gets compressed from ~160MB to ~110MB simply using - // this, presumably because there's a lot of 8-byte integers which generally - // have small values. Local testing shows that the deserialization - // performance, while higher, is in the few-percent range. For huge size - // savings this seems worthwhile to lose a small percentage of - // deserialization performance. - bincode::DefaultOptions::new().with_varint_encoding() -} - fn _assert_send_sync() { fn _assert() {} _assert::(); diff --git a/crates/wasmtime/src/module/serialization.rs b/crates/wasmtime/src/module/serialization.rs index 3daa7b925992..27936ffe9d84 100644 --- a/crates/wasmtime/src/module/serialization.rs +++ b/crates/wasmtime/src/module/serialization.rs @@ -2,7 +2,8 @@ use super::ModuleInner; use crate::{Engine, Module, OptLevel}; -use anyhow::{anyhow, bail, Result}; +use anyhow::{anyhow, bail, Context, Result}; +use bincode::Options; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::hash::{Hash, Hasher}; @@ -14,6 +15,19 @@ use wasmtime_jit::{ CompilationArtifacts, CompilationStrategy, CompiledModule, Compiler, TypeTables, }; +const HEADER: &[u8] = b"\0wasmtime-aot"; + +fn bincode_options() -> impl Options { + // Use a variable-length integer encoding instead of fixed length. The + // module shown on #2318 gets compressed from ~160MB to ~110MB simply using + // this, presumably because there's a lot of 8-byte integers which generally + // have small values. Local testing shows that the deserialization + // performance, while higher, is in the few-percent range. For huge size + // savings this seems worthwhile to lose a small percentage of + // deserialization performance. + bincode::DefaultOptions::new().with_varint_encoding() +} + // This exists because `wasmparser::WasmFeatures` isn't serializable #[derive(Hash, Debug, Copy, Clone, Serialize, Deserialize)] struct WasmFeatures { @@ -273,6 +287,60 @@ impl<'a> SerializedModule<'a> { } } + pub fn to_bytes(&self) -> Result> { + use std::io::Write; + + let mut bytes = Vec::new(); + + bytes.write_all(HEADER)?; + + // Preface the data with a version so we can do a version check independent + // of the serialized data. + let version = env!("CARGO_PKG_VERSION"); + assert!( + version.len() < 256, + "package version must be less than 256 bytes" + ); + bytes.write(&[version.len() as u8])?; + + bytes.write_all(version.as_bytes())?; + + bincode_options().serialize_into(&mut bytes, self)?; + + Ok(bytes) + } + + pub fn from_bytes(bytes: &[u8]) -> Result> { + if !bytes.starts_with(HEADER) { + return Ok(None); + } + + let bytes = &bytes[HEADER.len()..]; + + if bytes.is_empty() { + bail!("serialized data data is empty"); + } + + let version_len = bytes[0] as usize; + if bytes.len() < version_len + 1 { + bail!("serialized data is malformed"); + } + + let version = std::str::from_utf8(&bytes[1..1 + version_len])?; + if version != env!("CARGO_PKG_VERSION") { + bail!( + "Module was compiled with incompatible Wasmtime version '{}'", + version + ); + } + + Ok(Some( + bincode_options() + .deserialize::>(&bytes[1 + version_len..]) + .context("deserialize compilation artifacts")?, + )) + } + fn check_triple(&self, isa: &dyn TargetIsa) -> Result<()> { let triple = target_lexicon::Triple::from_str(&self.target).map_err(|e| anyhow!(e))?; diff --git a/examples/serialize.rs b/examples/serialize.rs index dd30b47a98a1..3cd709e7489d 100644 --- a/examples/serialize.rs +++ b/examples/serialize.rs @@ -31,7 +31,7 @@ fn deserialize(buffer: &[u8]) -> Result<()> { // Compile the wasm binary into an in-memory instance of a `Module`. println!("Deserialize module..."); - let module = Module::deserialize(store.engine(), buffer)?; + let module = Module::new(store.engine(), buffer)?; // Here we handle the imports of the module, which in this case is our // `HelloCallback` type and its associated implementation of `Callback. diff --git a/src/commands/compile.rs b/src/commands/compile.rs index 8401dd77058e..21f0cedc76f7 100644 --- a/src/commands/compile.rs +++ b/src/commands/compile.rs @@ -2,15 +2,14 @@ use crate::CommonOptions; use anyhow::{bail, Context, Result}; -use std::fs::{self, File}; -use std::io::BufWriter; +use std::fs; use std::path::PathBuf; use structopt::{ clap::{AppSettings, ArgGroup}, StructOpt, }; use target_lexicon::Triple; -use wasmtime::{Engine, Module}; +use wasmtime::Engine; lazy_static::lazy_static! { static ref AFTER_HELP: String = { @@ -100,8 +99,7 @@ impl CompileCommand { output }); - let mut writer = BufWriter::new(File::create(&output)?); - Module::compile(&engine, &input, &mut writer)?; + fs::write(output, engine.precompile_module(&input)?)?; Ok(()) } @@ -112,7 +110,7 @@ mod test { use super::*; use std::io::Write; use tempfile::NamedTempFile; - use wasmtime::{Instance, Store}; + use wasmtime::{Instance, Module, Store}; #[test] fn test_successful_compile() -> Result<()> { diff --git a/tests/all/module.rs b/tests/all/module.rs index 08996debac2c..279c91e829f4 100644 --- a/tests/all/module.rs +++ b/tests/all/module.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use std::io::BufWriter; use wasmtime::*; #[test] @@ -28,18 +27,18 @@ fn caches_across_engines() { .serialize() .unwrap(); - let res = Module::deserialize(&Engine::new(&Config::new()).unwrap(), &bytes); + let res = Module::new(&Engine::new(&Config::new()).unwrap(), &bytes); assert!(res.is_ok()); // differ in shared cranelift flags - let res = Module::deserialize( + let res = Module::new( &Engine::new(Config::new().cranelift_nan_canonicalization(true)).unwrap(), &bytes, ); assert!(res.is_err()); // differ in cranelift settings - let res = Module::deserialize( + let res = Module::new( &Engine::new(Config::new().cranelift_opt_level(OptLevel::None)).unwrap(), &bytes, ); @@ -47,7 +46,7 @@ fn caches_across_engines() { // Missing required cpu flags if cfg!(target_arch = "x86_64") { - let res = Module::deserialize( + let res = Module::new( &Engine::new( Config::new() .target(&target_lexicon::Triple::host().to_string()) @@ -63,14 +62,10 @@ fn caches_across_engines() { #[test] fn aot_compiles() -> Result<()> { let engine = Engine::default(); - let mut writer = BufWriter::new(Vec::new()); - Module::compile( - &engine, + let bytes = engine.precompile_module( "(module (func (export \"f\") (param i32) (result i32) local.get 0))".as_bytes(), - &mut writer, )?; - let bytes = writer.into_inner()?; let module = Module::from_binary(&engine, &bytes)?; let store = Store::new(&engine); diff --git a/tests/all/module_linking.rs b/tests/all/module_linking.rs index b010573c6e7d..101655eff318 100644 --- a/tests/all/module_linking.rs +++ b/tests/all/module_linking.rs @@ -39,7 +39,7 @@ fn compile() -> Result<()> { assert_eq!(m.imports().len(), 0); assert_eq!(m.exports().len(), 0); let bytes = m.serialize()?; - Module::deserialize(&engine, &bytes)?; + Module::new(&engine, &bytes)?; assert_eq!(m.imports().len(), 0); assert_eq!(m.exports().len(), 0); Ok(()) diff --git a/tests/all/module_serialize.rs b/tests/all/module_serialize.rs index dab73f67757f..abb0f7752d43 100644 --- a/tests/all/module_serialize.rs +++ b/tests/all/module_serialize.rs @@ -7,7 +7,7 @@ fn serialize(engine: &Engine, wat: &'static str) -> Result> { } fn deserialize_and_instantiate(store: &Store, buffer: &[u8]) -> Result { - let module = Module::deserialize(store.engine(), buffer)?; + let module = Module::new(store.engine(), buffer)?; Ok(Instance::new(&store, &module, &[])?) } @@ -15,9 +15,9 @@ fn deserialize_and_instantiate(store: &Store, buffer: &[u8]) -> Result fn test_version_mismatch() -> Result<()> { let engine = Engine::default(); let mut buffer = serialize(&engine, "(module)")?; - buffer[1] = 'x' as u8; + buffer[13 /* header length */ + 1 /* version length */] = 'x' as u8; - match Module::deserialize(&engine, &buffer) { + match Module::new(&engine, &buffer) { Ok(_) => bail!("expected deserialization to fail"), Err(e) => assert_eq!( e.to_string(),