From b4f5dfbdfd476f2aa64481519f46ab0ad713f30f Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 6 Jan 2023 12:14:14 -0800 Subject: [PATCH 1/4] Wasmtime: Add `Config::disable_cache` --- crates/wasmtime/src/config.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index e8321baf7049..dbd5f47a7145 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -946,6 +946,23 @@ impl Config { Ok(self) } + /// Disable caching. + /// + /// Every call to [`Module::new(my_wasm)`][crate::Module::new] will + /// recompile `my_wasm`, even when it is unchanged. + /// + /// By default, new configs do not have caching enabled. This method is only + /// useful for disabling a previous cache configuration. + /// + /// This method is only available when the `cache` feature of this crate is + /// enabled. + #[cfg(feature = "cache")] + #[cfg_attr(nightlydoc, doc(cfg(feature = "cache")))] + pub fn disable_cache(&mut self) -> &mut Self { + self.cache_config = CacheConfig::new_cache_disabled(); + self + } + /// Loads cache configuration from the system default path. /// /// This commit is the same as [`Config::cache_config_load`] except that it From 51ede70c32d91b876c1d67fa93224222951a2ebb Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 6 Jan 2023 12:15:15 -0800 Subject: [PATCH 2/4] bench-api: Always disable the cache --- crates/bench-api/src/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/bench-api/src/lib.rs b/crates/bench-api/src/lib.rs index 7d894fe710af..44f192d8fc42 100644 --- a/crates/bench-api/src/lib.rs +++ b/crates/bench-api/src/lib.rs @@ -432,12 +432,15 @@ impl BenchState { execution_end: extern "C" fn(*mut u8), make_wasi_cx: impl FnMut() -> Result + 'static, ) -> Result { - let config = if let Some(o) = &options { + let mut config = if let Some(o) = &options { o.config(Some(&Triple::host().to_string()))? } else { Config::new() }; - // NB: do not configure a code cache. + + // NB: always disable the compilation cache. + config.disable_cache(); + let engine = Engine::new(&config)?; let mut linker = Linker::::new(&engine); From 4ebc6ec36bb25e62341640a80676d89a4900377e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 6 Jan 2023 12:08:42 -0800 Subject: [PATCH 3/4] bench-api: Always get a `Config` from CLI flags This commit fixes an issue that I ran into just now where benchmarking one `*.so` with `--engine-flags` was giving wildly unexpected results comparing to something without `--engine-flags`. The root cause here appears to that when specifying `--engine-flags` the CLI parsing code is used to create a `Config` and when omitted a `Config::new` instance is created. The main difference between these is that for the CLI caching is enabled by default and for `Config::new` it is not. Coupled with the fact that caching doesn't really work for the `main` branch this ended up giving wild results. The fix here is to first always use the CLI parsing code to create a `Config` to ensure that a config is consistently created. Next the `--disable-cache` flag is unconditionally passed to the CLI parsing to ensure that compilation actually happens. Once applied this enables comparing an engine without flags and an engine with flags which provides consistent results. --- Cargo.lock | 1 + crates/bench-api/Cargo.toml | 1 + crates/bench-api/src/lib.rs | 54 ++++++++++++++++--------------------- crates/cli-flags/src/lib.rs | 33 +---------------------- 4 files changed, 26 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7861f8ecfc53..e63f27df44d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3405,6 +3405,7 @@ version = "6.0.0" dependencies = [ "anyhow", "cap-std", + "clap 3.2.8", "shuffling-allocator", "target-lexicon", "wasi-cap-std-sync", diff --git a/crates/bench-api/Cargo.toml b/crates/bench-api/Cargo.toml index 36c830c7971b..150886297134 100644 --- a/crates/bench-api/Cargo.toml +++ b/crates/bench-api/Cargo.toml @@ -26,6 +26,7 @@ wasmtime-wasi-crypto = { workspace = true, optional = true } wasmtime-wasi-nn = { workspace = true, optional = true } wasi-cap-std-sync = { workspace = true } cap-std = { workspace = true } +clap = { workspace = true } [dev-dependencies] wat = { workspace = true } diff --git a/crates/bench-api/src/lib.rs b/crates/bench-api/src/lib.rs index 44f192d8fc42..662bbbe6c4ec 100644 --- a/crates/bench-api/src/lib.rs +++ b/crates/bench-api/src/lib.rs @@ -137,11 +137,12 @@ mod unsafe_send_sync; use crate::unsafe_send_sync::UnsafeSendSync; use anyhow::{Context, Result}; +use clap::Parser; use std::os::raw::{c_int, c_void}; use std::slice; use std::{env, path::PathBuf}; use target_lexicon::Triple; -use wasmtime::{Config, Engine, Instance, Linker, Module, Store}; +use wasmtime::{Engine, Instance, Linker, Module, Store}; use wasmtime_cli_flags::{CommonOptions, WasiModules}; use wasmtime_wasi::{sync::WasiCtxBuilder, I32Exit, WasiCtx}; @@ -238,19 +239,23 @@ impl WasmBenchConfig { Ok(Some(stdin_path.into())) } - fn execution_flags(&self) -> Result> { - if self.execution_flags_ptr.is_null() { - return Ok(None); - } - - let execution_flags = unsafe { - std::slice::from_raw_parts(self.execution_flags_ptr, self.execution_flags_len) + fn execution_flags(&self) -> Result { + let flags = if self.execution_flags_ptr.is_null() { + "" + } else { + let execution_flags = unsafe { + std::slice::from_raw_parts(self.execution_flags_ptr, self.execution_flags_len) + }; + std::str::from_utf8(execution_flags) + .context("given execution flags string is not valid UTF-8")? }; - let execution_flags = std::str::from_utf8(execution_flags) - .context("given execution flags string is not valid UTF-8")?; - - let options = CommonOptions::parse_from_str(execution_flags)?; - Ok(Some(options)) + let options = CommonOptions::try_parse_from( + ["wasmtime"] + .into_iter() + .chain(flags.split(' ').filter(|s| !s.is_empty())), + ) + .context("failed to parse options")?; + Ok(options) } } @@ -420,7 +425,7 @@ struct HostState { impl BenchState { fn new( - options: Option, + options: CommonOptions, compilation_timer: *mut u8, compilation_start: extern "C" fn(*mut u8), compilation_end: extern "C" fn(*mut u8), @@ -432,15 +437,9 @@ impl BenchState { execution_end: extern "C" fn(*mut u8), make_wasi_cx: impl FnMut() -> Result + 'static, ) -> Result { - let mut config = if let Some(o) = &options { - o.config(Some(&Triple::host().to_string()))? - } else { - Config::new() - }; - + let config = options.config(Some(&Triple::host().to_string()))?; // NB: always disable the compilation cache. config.disable_cache(); - let engine = Engine::new(&config)?; let mut linker = Linker::::new(&engine); @@ -459,17 +458,10 @@ impl BenchState { Ok(()) })?; - let mut epoch_interruption = false; - let mut fuel = None; - if let Some(opts) = &options { - epoch_interruption = opts.epoch_interruption; - fuel = opts.fuel; - } + let epoch_interruption = options.epoch_interruption; + let fuel = options.fuel; - let wasi_modules = options - .map(|o| o.wasi_modules) - .flatten() - .unwrap_or(WasiModules::default()); + let wasi_modules = options.wasi_modules.unwrap_or(WasiModules::default()); if wasi_modules.wasi_common { wasmtime_wasi::add_to_linker(&mut linker, |cx| &mut cx.wasi)?; diff --git a/crates/cli-flags/src/lib.rs b/crates/cli-flags/src/lib.rs index 279712cbc9f7..fb778af2dea5 100644 --- a/crates/cli-flags/src/lib.rs +++ b/crates/cli-flags/src/lib.rs @@ -16,7 +16,7 @@ ) )] -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; use clap::Parser; use std::collections::HashMap; use std::path::PathBuf; @@ -234,17 +234,6 @@ pub struct CommonOptions { } impl CommonOptions { - pub fn parse_from_str(s: &str) -> Result { - let parts = s.split(" ").filter(|s| !s.is_empty()); - // The first argument is the name of the executable, which we don't use - // here, but have to provide because `clap` skips over it, and otherwise - // our first CLI flag will be ignored. - let parts = Some("wasmtime").into_iter().chain(parts); - let options = - Self::try_parse_from(parts).context("unable to parse options from passed flags")?; - Ok(options) - } - pub fn init_logging(&self) { if self.disable_logging { return; @@ -723,24 +712,4 @@ mod test { } ); } - - #[test] - fn test_parse_from_str() { - fn use_func(flags: &str) -> CommonOptions { - CommonOptions::parse_from_str(flags).unwrap() - } - fn use_clap_parser(flags: &[&str]) -> CommonOptions { - CommonOptions::try_parse_from(flags).unwrap() - } - - assert_eq!(use_func(""), use_clap_parser(&[])); - assert_eq!( - use_func("--wasm-features=threads"), - use_clap_parser(&["foo", "--wasm-features=threads"]) - ); - assert_eq!( - use_func("--cranelift-set enable_simd=true"), - use_clap_parser(&["foo", "--cranelift-set", "enable_simd=true"]) - ); - } } From 0e0cb505700c3f8c7fe168e63cf186d9fe518044 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 6 Jan 2023 12:36:53 -0800 Subject: [PATCH 4/4] Fix compile error --- crates/bench-api/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bench-api/src/lib.rs b/crates/bench-api/src/lib.rs index 662bbbe6c4ec..3d605a75f987 100644 --- a/crates/bench-api/src/lib.rs +++ b/crates/bench-api/src/lib.rs @@ -437,7 +437,7 @@ impl BenchState { execution_end: extern "C" fn(*mut u8), make_wasi_cx: impl FnMut() -> Result + 'static, ) -> Result { - let config = options.config(Some(&Triple::host().to_string()))?; + let mut config = options.config(Some(&Triple::host().to_string()))?; // NB: always disable the compilation cache. config.disable_cache(); let engine = Engine::new(&config)?;