diff --git a/crates/environ/src/cache.rs b/crates/environ/src/cache.rs index cf9aed7883..e1186ce283 100644 --- a/crates/environ/src/cache.rs +++ b/crates/environ/src/cache.rs @@ -14,6 +14,7 @@ mod worker; pub use config::{create_new_config, CacheConfig}; use worker::Worker; +/// Module level cache entry. pub struct ModuleCacheEntry<'config>(Option>); struct ModuleCacheEntryInner<'config> { @@ -24,6 +25,7 @@ struct ModuleCacheEntryInner<'config> { struct Sha256Hasher(Sha256); impl<'config> ModuleCacheEntry<'config> { + /// Create the cache entry. pub fn new<'data>(compiler_name: &str, cache_config: &'config CacheConfig) -> Self { if cache_config.enabled() { Self(Some(ModuleCacheEntryInner::new( @@ -40,6 +42,7 @@ impl<'config> ModuleCacheEntry<'config> { Self(Some(inner)) } + /// Gets cached data if state matches, otherwise calls the `compute`. pub fn get_data(&self, state: T, compute: fn(T) -> Result) -> Result where T: Hash, diff --git a/crates/environ/src/compilation.rs b/crates/environ/src/compilation.rs index 6e872f28a3..11d987a576 100644 --- a/crates/environ/src/compilation.rs +++ b/crates/environ/src/compilation.rs @@ -2,7 +2,6 @@ //! module. use crate::address_map::{ModuleAddressMap, ValueLabelsRanges}; -use crate::CacheConfig; use crate::ModuleTranslation; use cranelift_codegen::{binemit, ir, isa, isa::unwind::UnwindInfo}; use cranelift_entity::PrimaryMap; @@ -201,6 +200,5 @@ pub trait Compiler { fn compile_module( translation: &ModuleTranslation, isa: &dyn isa::TargetIsa, - cache_config: &CacheConfig, ) -> Result; } diff --git a/crates/environ/src/cranelift.rs b/crates/environ/src/cranelift.rs index 4696b5b8e6..99ea83f075 100644 --- a/crates/environ/src/cranelift.rs +++ b/crates/environ/src/cranelift.rs @@ -86,14 +86,13 @@ // assume no valid stack pointer will ever be `usize::max_value() - 32k`. use crate::address_map::{FunctionAddressMap, InstructionAddressMap}; -use crate::cache::ModuleCacheEntry; use crate::compilation::{ Compilation, CompileError, CompiledFunction, Relocation, RelocationTarget, StackMapInformation, TrapInformation, }; use crate::compilation::{CompileResult, Compiler}; use crate::func_environ::{get_func_name, FuncEnvironment}; -use crate::{CacheConfig, FunctionBodyData, ModuleLocal, ModuleTranslation, Tunables}; +use crate::{FunctionBodyData, ModuleLocal, ModuleTranslation, Tunables}; use cranelift_codegen::ir::{self, ExternalName}; use cranelift_codegen::machinst::buffer::MachSrcLoc; use cranelift_codegen::print_errors::pretty_error; @@ -103,7 +102,6 @@ use cranelift_wasm::{DefinedFuncIndex, FuncIndex, FuncTranslator, ModuleTranslat #[cfg(feature = "parallel-compilation")] use rayon::prelude::{IntoParallelRefIterator, ParallelIterator}; use std::convert::TryFrom; -use std::hash::{Hash, Hasher}; /// Implementation of a relocation sink that just saves all the information for later pub struct RelocSink { @@ -290,49 +288,45 @@ impl Compiler for Cranelift { fn compile_module( translation: &ModuleTranslation, isa: &dyn isa::TargetIsa, - cache_config: &CacheConfig, ) -> Result { - let cache_entry = ModuleCacheEntry::new("cranelift", cache_config); - - let result = cache_entry.get_data( - CompileEnv { - local: &translation.module.local, - module_translation: HashedModuleTranslationState( - translation.module_translation.as_ref().unwrap(), - ), - function_body_inputs: &translation.function_body_inputs, - isa: Isa(isa), - tunables: &translation.tunables, - }, - compile, - )?; - Ok(result) + compile( + isa, + &translation.module.local, + translation.module_translation.as_ref().unwrap(), + &translation.function_body_inputs, + &translation.tunables, + ) } } -fn compile(env: CompileEnv<'_>) -> Result { - let Isa(isa) = env.isa; - let mut functions = PrimaryMap::with_capacity(env.function_body_inputs.len()); - let mut relocations = PrimaryMap::with_capacity(env.function_body_inputs.len()); - let mut address_transforms = PrimaryMap::with_capacity(env.function_body_inputs.len()); - let mut value_ranges = PrimaryMap::with_capacity(env.function_body_inputs.len()); - let mut stack_slots = PrimaryMap::with_capacity(env.function_body_inputs.len()); - let mut traps = PrimaryMap::with_capacity(env.function_body_inputs.len()); - let mut stack_maps = PrimaryMap::with_capacity(env.function_body_inputs.len()); +fn compile( + isa: &dyn isa::TargetIsa, + local: &ModuleLocal, + module_translation: &ModuleTranslationState, + function_body_inputs: &PrimaryMap>, + tunables: &Tunables, +) -> Result { + let mut functions = PrimaryMap::with_capacity(function_body_inputs.len()); + let mut relocations = PrimaryMap::with_capacity(function_body_inputs.len()); + let mut address_transforms = PrimaryMap::with_capacity(function_body_inputs.len()); + let mut value_ranges = PrimaryMap::with_capacity(function_body_inputs.len()); + let mut stack_slots = PrimaryMap::with_capacity(function_body_inputs.len()); + let mut traps = PrimaryMap::with_capacity(function_body_inputs.len()); + let mut stack_maps = PrimaryMap::with_capacity(function_body_inputs.len()); type FunctionBodyInput<'a> = (DefinedFuncIndex, &'a FunctionBodyData<'a>); let compile_function = |func_translator: &mut FuncTranslator, (i, input): &FunctionBodyInput| { - let func_index = env.local.func_index(*i); + let func_index = local.func_index(*i); let mut context = Context::new(); context.func.name = get_func_name(func_index); - context.func.signature = env.local.native_func_signature(func_index).clone(); - if env.tunables.debug_info { + context.func.signature = local.native_func_signature(func_index).clone(); + if tunables.debug_info { context.func.collect_debug_info(); } - let mut func_env = FuncEnvironment::new(isa.frontend_config(), env.local, env.tunables); + let mut func_env = FuncEnvironment::new(isa.frontend_config(), local, tunables); // We use these as constant offsets below in // `stack_limit_from_arguments`, so assert their values here. This @@ -368,7 +362,7 @@ fn compile(env: CompileEnv<'_>) -> Result { }); context.func.stack_limit = Some(stack_limit); func_translator.translate( - env.module_translation.0, + module_translation, input.data, input.module_offset, &mut context.func, @@ -397,7 +391,7 @@ fn compile(env: CompileEnv<'_>) -> Result { let address_transform = get_function_address_map(&context, input, code_buf.len(), isa); - let ranges = if env.tunables.debug_info { + let ranges = if tunables.debug_info { let ranges = context.build_value_labels_ranges(isa).map_err(|error| { CompileError::Codegen(pretty_error(&context.func, Some(isa), error)) })?; @@ -419,7 +413,7 @@ fn compile(env: CompileEnv<'_>) -> Result { )) }; - let inputs: Vec = env.function_body_inputs.into_iter().collect(); + let inputs: Vec = function_body_inputs.into_iter().collect(); let results: Result, CompileError> = { cfg_if::cfg_if! { @@ -476,49 +470,3 @@ fn compile(env: CompileEnv<'_>) -> Result { stack_maps, )) } - -#[derive(Hash)] -struct CompileEnv<'a> { - local: &'a ModuleLocal, - module_translation: HashedModuleTranslationState<'a>, - function_body_inputs: &'a PrimaryMap>, - isa: Isa<'a, 'a>, - tunables: &'a Tunables, -} - -/// This is a wrapper struct to hash the specific bits of `TargetIsa` that -/// affect the output we care about. The trait itself can't implement `Hash` -/// (it's not object safe) so we have to implement our own hashing. -struct Isa<'a, 'b>(&'a (dyn isa::TargetIsa + 'b)); - -impl Hash for Isa<'_, '_> { - fn hash(&self, hasher: &mut H) { - self.0.triple().hash(hasher); - self.0.frontend_config().hash(hasher); - - // TODO: if this `to_string()` is too expensive then we should upstream - // a native hashing ability of flags into cranelift itself, but - // compilation and/or cache loading is relatively expensive so seems - // unlikely. - self.0.flags().to_string().hash(hasher); - - // TODO: ... and should we hash anything else? There's a lot of stuff in - // `TargetIsa`, like registers/encodings/etc. Should we be hashing that - // too? It seems like wasmtime doesn't configure it too too much, but - // this may become an issue at some point. - } -} - -/// A wrapper struct around cranelift's `ModuleTranslationState` to implement -/// `Hash` since it's not `Hash` upstream yet. -/// -/// TODO: we should upstream a `Hash` implementation, it would be very small! At -/// this moment though based on the definition it should be fine to not hash it -/// since we'll re-hash the signatures later. -struct HashedModuleTranslationState<'a>(&'a ModuleTranslationState); - -impl Hash for HashedModuleTranslationState<'_> { - fn hash(&self, _hasher: &mut H) { - // nothing to hash right now - } -} diff --git a/crates/environ/src/lib.rs b/crates/environ/src/lib.rs index a0ace205ab..276dd539c2 100644 --- a/crates/environ/src/lib.rs +++ b/crates/environ/src/lib.rs @@ -44,7 +44,7 @@ pub use crate::address_map::{ ModuleVmctxInfo, ValueLabelsRanges, }; pub use crate::cache::create_new_config as cache_create_new_config; -pub use crate::cache::CacheConfig; +pub use crate::cache::{CacheConfig, ModuleCacheEntry}; pub use crate::compilation::{ Compilation, CompileError, CompiledFunction, Compiler, Relocation, RelocationTarget, Relocations, StackMapInformation, StackMaps, TrapInformation, Traps, diff --git a/crates/environ/src/lightbeam.rs b/crates/environ/src/lightbeam.rs index 114dee3ecf..057e10b41f 100644 --- a/crates/environ/src/lightbeam.rs +++ b/crates/environ/src/lightbeam.rs @@ -20,7 +20,6 @@ impl Compiler for Lightbeam { fn compile_module( translation: &ModuleTranslation, isa: &dyn isa::TargetIsa, - _cache_config: &CacheConfig, ) -> Result { if translation.tunables.debug_info { return Err(CompileError::DebugInfoNotSupported); diff --git a/crates/jit/src/compiler.rs b/crates/jit/src/compiler.rs index 534f963fd9..2938e5766c 100644 --- a/crates/jit/src/compiler.rs +++ b/crates/jit/src/compiler.rs @@ -4,17 +4,18 @@ use crate::instantiate::SetupError; use crate::object::{build_object, ObjectUnwindInfo}; use cranelift_codegen::ir; use object::write::Object; +use std::hash::{Hash, Hasher}; use wasmtime_debug::{emit_dwarf, DebugInfoData, DwarfSection}; use wasmtime_environ::entity::{EntityRef, PrimaryMap}; use wasmtime_environ::isa::{unwind::UnwindInfo, TargetFrontendConfig, TargetIsa}; use wasmtime_environ::wasm::{DefinedFuncIndex, DefinedMemoryIndex, MemoryIndex}; use wasmtime_environ::{ - CacheConfig, Compiler as _C, Module, ModuleAddressMap, ModuleMemoryOffset, ModuleTranslation, + Compiler as _C, Module, ModuleAddressMap, ModuleMemoryOffset, ModuleTranslation, ModuleVmctxInfo, StackMaps, Traps, Tunables, VMOffsets, ValueLabelsRanges, }; /// Select which kind of compilation to use. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Hash)] pub enum CompilationStrategy { /// Let Wasmtime pick the strategy. Auto, @@ -38,22 +39,15 @@ pub enum CompilationStrategy { pub struct Compiler { isa: Box, strategy: CompilationStrategy, - cache_config: CacheConfig, tunables: Tunables, } impl Compiler { /// Construct a new `Compiler`. - pub fn new( - isa: Box, - strategy: CompilationStrategy, - cache_config: CacheConfig, - tunables: Tunables, - ) -> Self { + pub fn new(isa: Box, strategy: CompilationStrategy, tunables: Tunables) -> Self { Self { isa, strategy, - cache_config, tunables, } } @@ -126,6 +120,11 @@ impl Compiler { &self.tunables } + /// Return the compilation strategy. + pub fn strategy(&self) -> CompilationStrategy { + self.strategy + } + /// Compile the given function bodies. pub(crate) fn compile<'data>( &self, @@ -144,19 +143,11 @@ impl Compiler { // For now, interpret `Auto` as `Cranelift` since that's the most stable // implementation. CompilationStrategy::Auto | CompilationStrategy::Cranelift => { - wasmtime_environ::cranelift::Cranelift::compile_module( - translation, - &*self.isa, - &self.cache_config, - ) + wasmtime_environ::cranelift::Cranelift::compile_module(translation, &*self.isa) } #[cfg(feature = "lightbeam")] CompilationStrategy::Lightbeam => { - wasmtime_environ::lightbeam::Lightbeam::compile_module( - translation, - &*self.isa, - &self.cache_config, - ) + wasmtime_environ::lightbeam::Lightbeam::compile_module(translation, &*self.isa) } } .map_err(SetupError::Compile)?; @@ -193,3 +184,30 @@ impl Compiler { }) } } + +impl Hash for Compiler { + fn hash(&self, hasher: &mut H) { + let Compiler { + strategy, + isa, + tunables, + } = self; + + // Hash compiler's flags: compilation strategy, isa, frontend config, + // misc tunables. + strategy.hash(hasher); + isa.triple().hash(hasher); + // TODO: if this `to_string()` is too expensive then we should upstream + // a native hashing ability of flags into cranelift itself, but + // compilation and/or cache loading is relatively expensive so seems + // unlikely. + isa.flags().to_string().hash(hasher); + isa.frontend_config().hash(hasher); + tunables.hash(hasher); + + // TODO: ... and should we hash anything else? There's a lot of stuff in + // `TargetIsa`, like registers/encodings/etc. Should we be hashing that + // too? It seems like wasmtime doesn't configure it too too much, but + // this may become an issue at some point. + } +} diff --git a/crates/wasmtime/src/module.rs b/crates/wasmtime/src/module.rs index ca3f6571e5..40f346cba5 100644 --- a/crates/wasmtime/src/module.rs +++ b/crates/wasmtime/src/module.rs @@ -4,6 +4,7 @@ use crate::types::{EntityType, ExportType, ExternType, ImportType}; use anyhow::{bail, Context, Result}; use std::path::Path; use std::sync::{Arc, Mutex}; +use wasmtime_environ::ModuleCacheEntry; use wasmtime_jit::{CompilationArtifacts, CompiledModule}; /// A compiled WebAssembly module, ready to be instantiated. @@ -300,7 +301,17 @@ impl Module { } unsafe fn compile(engine: &Engine, binary: &[u8]) -> Result { - let compiled = CompiledModule::new(engine.compiler(), binary, &*engine.config().profiler)?; + let cache_entry = ModuleCacheEntry::new("wasmtime", engine.cache_config()); + let artifacts = cache_entry + .get_data((engine.compiler(), binary), |(compiler, binary)| { + CompilationArtifacts::build(compiler, binary) + })?; + + let compiled = CompiledModule::from_artifacts( + artifacts, + engine.compiler().isa(), + &*engine.config().profiler, + )?; Ok(Module { engine: engine.clone(), diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index f46fb253b3..12f2b9445a 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -628,12 +628,7 @@ impl Config { fn build_compiler(&self) -> Compiler { let isa = self.target_isa(); - Compiler::new( - isa, - self.strategy, - self.cache_config.clone(), - self.tunables.clone(), - ) + Compiler::new(isa, self.strategy, self.tunables.clone()) } /// Hashes/fingerprints compiler setting to ensure that compatible @@ -798,6 +793,10 @@ impl Engine { &self.inner.compiler } + pub(crate) fn cache_config(&self) -> &CacheConfig { + &self.config().cache_config + } + /// Returns whether the engine `a` and `b` refer to the same configuration. pub fn same(a: &Engine, b: &Engine) -> bool { Arc::ptr_eq(&a.inner, &b.inner) diff --git a/src/commands/wasm2obj.rs b/src/commands/wasm2obj.rs index 028ffd5957..4688f985dc 100644 --- a/src/commands/wasm2obj.rs +++ b/src/commands/wasm2obj.rs @@ -11,7 +11,6 @@ use std::{ }; use structopt::{clap::AppSettings, StructOpt}; use target_lexicon::Triple; -use wasmtime_environ::CacheConfig; /// The after help text for the `wasm2obj` command. pub const WASM2OBJ_AFTER_HELP: &str = "The translation is dependent on the environment chosen.\n\ @@ -60,11 +59,6 @@ impl WasmToObjCommand { pretty_env_logger::init(); } - let cache_config = if self.common.disable_cache { - CacheConfig::new_cache_disabled() - } else { - CacheConfig::from_file(self.common.config.as_deref())? - }; let strategy = pick_compilation_strategy(self.common.cranelift, self.common.lightbeam)?; let data = wat::parse_file(&self.module).context("failed to parse module")?; @@ -76,7 +70,6 @@ impl WasmToObjCommand { self.common.enable_simd, self.common.opt_level(), self.common.debug_info, - &cache_config, )?; let mut file = diff --git a/src/obj.rs b/src/obj.rs index 90fef7146e..b61a61650b 100644 --- a/src/obj.rs +++ b/src/obj.rs @@ -7,8 +7,8 @@ use wasmtime_debug::{emit_dwarf, read_debuginfo}; use wasmtime_environ::Lightbeam; use wasmtime_environ::{ entity::EntityRef, settings, settings::Configurable, wasm::DefinedMemoryIndex, - wasm::MemoryIndex, CacheConfig, Compiler, Cranelift, ModuleEnvironment, ModuleMemoryOffset, - ModuleVmctxInfo, Tunables, VMOffsets, + wasm::MemoryIndex, Compiler, Cranelift, ModuleEnvironment, ModuleMemoryOffset, ModuleVmctxInfo, + Tunables, VMOffsets, }; use wasmtime_jit::native; use wasmtime_obj::{emit_module, ObjectBuilderTarget}; @@ -21,7 +21,6 @@ pub fn compile_to_obj( enable_simd: bool, opt_level: wasmtime::OptLevel, debug_info: bool, - cache_config: &CacheConfig, ) -> Result { let isa_builder = match target { Some(target) => native::lookup(target.clone())?, @@ -72,11 +71,9 @@ pub fn compile_to_obj( _traps, _stack_maps, ) = match strategy { - Strategy::Auto | Strategy::Cranelift => { - Cranelift::compile_module(&translation, &*isa, cache_config) - } + Strategy::Auto | Strategy::Cranelift => Cranelift::compile_module(&translation, &*isa), #[cfg(feature = "lightbeam")] - Strategy::Lightbeam => Lightbeam::compile_module(&translation, &*isa, cache_config), + Strategy::Lightbeam => Lightbeam::compile_module(&translation, &*isa), #[cfg(not(feature = "lightbeam"))] Strategy::Lightbeam => bail!("lightbeam support not enabled"), other => bail!("unsupported compilation strategy {:?}", other), diff --git a/tests/all/debug/obj.rs b/tests/all/debug/obj.rs index 5b9b5700d6..f4b3d14073 100644 --- a/tests/all/debug/obj.rs +++ b/tests/all/debug/obj.rs @@ -5,7 +5,6 @@ use std::path::Path; use target_lexicon::Triple; use wasmtime::Strategy; use wasmtime_cli::compile_to_obj; -use wasmtime_environ::CacheConfig; pub fn compile_cranelift( wasm: &[u8], @@ -19,7 +18,6 @@ pub fn compile_cranelift( false, wasmtime::OptLevel::None, true, - &CacheConfig::new_cache_disabled(), )?; let mut file = File::create(output).context("failed to create object file")?;