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<u8>`.
* Move the remaining serialization-related code to `serialization.rs`.

RELEASES.md

@@ -6,14 +6,13 @@
 
 ### Added
 
-* The `wasmtime compile` command was added to support AOT compilation of Wasm
+* Added the `wasmtime compile` command to support AOT compilation of Wasm modules.
-  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
+  configuration. This can be used in conjunction with `Engine::precompile_module`
-  a different host triple than the current one.
+  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.

crates/c-api/src/module.rs

@@ -185,13 +185,10 @@ pub extern "C" fn wasmtime_module_deserialize(
     binary: &wasm_byte_vec_t,
     ret: &mut *mut wasm_module_t,
 ) -> Option<Box<wasmtime_error_t>> {
-    handle_result(
+    handle_result(Module::new(&engine.engine, binary.as_slice()), |module| {
-        Module::deserialize(&engine.engine, binary.as_slice()),
-        |module| {
         let module = Box::new(wasm_module_t::new(module));
         *ret = Box::into_raw(module);
-        },
+    })
-    )
 }
 
 #[no_mangle]

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<Vec<u8>> {
+        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 {

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;
+pub use serialization::SerializedModule;
-
-const COMPILED_MODULE_HEADER: &[u8] = b"\0wasmtime-aot";
 
 /// 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<Module> {
         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<Module> {
-        if binary.starts_with(COMPILED_MODULE_HEADER) {
+        if let Some(module) = SerializedModule::from_bytes(binary)? {
-            return Self::deserialize(engine, &binary[COMPILED_MODULE_HEADER.len()..]);
+            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.
-    pub fn serialize(&self) -> Result<Vec<u8>> {
-        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
+    /// Use `Module::new` or `Module::from_binary` to create the module
-    /// one are different. The method does not verify the serialized artifacts
+    /// from the bytes.
-    /// for modifications or corruptions. All responsibly of signing and its
+    pub fn serialize(&self) -> Result<Vec<u8>> {
-    /// verification falls on the embedder.
+        SerializedModule::new(self).to_bytes()
-    pub fn deserialize(engine: &Engine, serialized: &[u8]) -> Result<Module> {
-        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::<SerializedModule<'_>>(&serialized[1 + version_len..])
-            .context("Deserialize compilation artifacts")?
-            .into_module(engine)
     }
 
     /// 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<T: Send + Sync>() {}
     _assert::<Module>();

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<Vec<u8>> {
+        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<Option<Self>> {
+        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::<SerializedModule<'_>>(&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))?;
 

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.

src/commands/compile.rs

@@ -2,15 +2,14 @@
 
 use crate::CommonOptions;
 use anyhow::{bail, Context, Result};
-use std::fs::{self, File};
+use std::fs;
-use std::io::BufWriter;
 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)?);
+        fs::write(output, engine.precompile_module(&input)?)?;
-        Module::compile(&engine, &input, &mut writer)?;
 
         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<()> {

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());
+    let bytes = engine.precompile_module(
-    Module::compile(
-        &engine,
         "(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);

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(())

tests/all/module_serialize.rs

@@ -7,7 +7,7 @@ fn serialize(engine: &Engine, wat: &'static str) -> Result<Vec<u8>> {
 }
 
 fn deserialize_and_instantiate(store: &Store, buffer: &[u8]) -> Result<Instance> {
-    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<Instance>
 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(),