Merge #536

536: Update cache key generation to use backend compiler name too r=MarkMcCaskey a=MarkMcCaskey



Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>

CHANGELOG.md

@@ -6,6 +6,7 @@ Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
 ## 0.5.4
+- [#536](https://github.com/wasmerio/wasmer/pull/536) Update cache to use compiler backend name in cache key
 - [#529](https://github.com/wasmerio/wasmer/pull/529) Updates the Wasm Interface library, which is used by wapm, with bug fixes and error message improvements
 
 ## 0.5.3

lib/runtime-core/src/backend.rs

@@ -29,6 +29,62 @@ pub enum Backend {
     LLVM,
 }
 
+impl Backend {
+    pub fn variants() -> &'static [&'static str] {
+        &[
+            "cranelift",
+            #[cfg(feature = "backend:singlepass")]
+            "singlepass",
+            #[cfg(feature = "backend:llvm")]
+            "llvm",
+        ]
+    }
+
+    /// stable string representation of the backend
+    /// can be used as part of a cache key, for example
+    pub fn to_string(&self) -> &'static str {
+        match self {
+            Backend::Cranelift => "cranelift",
+            Backend::Singlepass => "singlepass",
+            Backend::LLVM => "llvm",
+        }
+    }
+}
+
+impl Default for Backend {
+    fn default() -> Self {
+        Backend::Cranelift
+    }
+}
+
+impl std::str::FromStr for Backend {
+    type Err = String;
+    fn from_str(s: &str) -> Result<Backend, String> {
+        match s.to_lowercase().as_str() {
+            "singlepass" => Ok(Backend::Singlepass),
+            "cranelift" => Ok(Backend::Cranelift),
+            "llvm" => Ok(Backend::LLVM),
+            _ => Err(format!("The backend {} doesn't exist", s)),
+        }
+    }
+}
+
+#[cfg(test)]
+mod backend_test {
+    use super::*;
+    use std::str::FromStr;
+
+    #[test]
+    fn str_repr_matches() {
+        // if this test breaks, think hard about why it's breaking
+        // can we avoid having these be different?
+
+        for &backend in &[Backend::Cranelift, Backend::LLVM, Backend::Singlepass] {
+            assert_eq!(backend, Backend::from_str(backend.to_string()).unwrap());
+        }
+    }
+}
+
 /// This type cannot be constructed from
 /// outside the runtime crate.
 pub struct Token {

lib/runtime-core/src/cache.rs

@@ -1,4 +1,5 @@
 use crate::{
+    backend::Backend,
     module::{Module, ModuleInfo},
     sys::Memory,
 };
@@ -36,17 +37,31 @@ impl From<io::Error> for Error {
 pub struct WasmHash([u8; 32], [u8; 32]);
 
 impl WasmHash {
-    /// Hash a wasm module.
+    /// Hash a wasm module for the default compiler backend.
+    ///
+    /// See also: `WasmHash::generate_for_backend`.
     ///
     /// # Note:
     /// This does no verification that the supplied data
     /// is, in fact, a wasm module.
     pub fn generate(wasm: &[u8]) -> Self {
+        WasmHash::generate_for_backend(wasm, Backend::default())
+    }
+
+    /// Hash a wasm module for a specific compiler backend.
+    /// This allows multiple cache entries containing the same compiled
+    /// module with different compiler backends.
+    ///
+    /// # Note:
+    /// This function also does no verification that the supplied data
+    /// is a wasm module
+    pub fn generate_for_backend(wasm: &[u8], backend: Backend) -> Self {
         let mut first_part = [0u8; 32];
         let mut second_part = [0u8; 32];
 
         let mut state = blake2bp::State::new();
         state.update(wasm);
+        state.update(backend.to_string().as_bytes());
 
         let hasher = state.finalize();
         let generic_array = hasher.as_bytes();

lib/runtime/src/cache.rs

@@ -20,6 +20,7 @@ pub use wasmer_runtime_core::cache::{Artifact, Cache, WasmHash, WASMER_VERSION_H
 ///
 /// ```rust
 /// use wasmer_runtime::cache::{Cache, FileSystemCache, WasmHash};
+/// use wasmer_runtime_core::backend::Backend;
 ///
 /// # use wasmer_runtime::{Module, error::CacheError};
 /// fn store_module(module: Module) -> Result<Module, CacheError> {
@@ -28,7 +29,7 @@ pub use wasmer_runtime_core::cache::{Artifact, Cache, WasmHash, WASMER_VERSION_H
 ///     // corrupted or tampered with.
 ///     let mut fs_cache = unsafe { FileSystemCache::new("some/directory/goes/here")? };
 ///     // Compute a key for a given WebAssembly binary
-///     let key = WasmHash::generate(&[]);
+///     let key = WasmHash::generate_for_backend(&[], Backend::Cranelift);
 ///     // Store a module into the cache given a key
 ///     fs_cache.store(key, module.clone())?;
 ///     Ok(module)

src/bin/wasmer.rs

@@ -23,7 +23,7 @@ use wasmer_runtime::{
 };
 use wasmer_runtime_core::{
     self,
-    backend::{Compiler, CompilerConfig, MemoryBoundCheckMode},
+    backend::{Backend, Compiler, CompilerConfig, MemoryBoundCheckMode},
     loader::{Instance as LoadedInstance, LocalLoader},
 };
 #[cfg(feature = "backend:singlepass")]
@@ -153,42 +153,6 @@ impl FromStr for LoaderName {
     }
 }
 
-#[allow(dead_code)]
-#[derive(Debug, Eq, PartialEq)]
-enum Backend {
-    Cranelift,
-    Singlepass,
-    LLVM,
-}
-
-impl Backend {
-    pub fn variants() -> &'static [&'static str] {
-        &[
-            "cranelift",
-            #[cfg(feature = "backend:singlepass")]
-            "singlepass",
-            #[cfg(feature = "backend:llvm")]
-            "llvm",
-        ]
-    }
-}
-
-impl FromStr for Backend {
-    type Err = String;
-    fn from_str(s: &str) -> Result<Backend, String> {
-        match s.to_lowercase().as_str() {
-            "singlepass" => Ok(Backend::Singlepass),
-            "cranelift" => Ok(Backend::Cranelift),
-            "llvm" => Ok(Backend::LLVM),
-            // "llvm" => Err(
-            //     "The LLVM backend option is not enabled by default due to binary size constraints"
-            //         .to_string(),
-            // ),
-            _ => Err(format!("The backend {} doesn't exist", s)),
-        }
-    }
-}
-
 #[derive(Debug, StructOpt)]
 enum Cache {
     /// Clear the cache
@@ -388,7 +352,7 @@ fn execute_wasm(options: &Run) -> Result<(), String> {
 
         // We generate a hash for the given binary, so we can use it as key
         // for the Filesystem cache
-        let hash = WasmHash::generate(&wasm_binary);
+        let hash = WasmHash::generate_for_backend(&wasm_binary, options.backend);
 
         let wasmer_cache_dir = get_cache_dir();