bench-api: Clean up the benchmarking API

Mostly just tweaks to docs/naming/readability/tidying up.

The biggest thing is that the wasm bytes are passed in during compilation now,
rather than on initialization, which lets us remove the lifetime from our state
struct and makes wrangling unsafe conversions that much easier.

crates/bench-api/src/lib.rs

@@ -1,145 +1,189 @@
-//! Expose a C-compatible API for controlling the Wasmtime engine during benchmarking. The API expects very sequential
+//! A C API for benchmarking Wasmtime's WebAssembly compilation, instantiation,
-//! use:
+//! and execution.
-//!  - `engine_create`
-//!  - `engine_compile_module`
-//!  - `engine_instantiate_module`
-//!  - `engine_execute_module`
-//!  - `engine_free`
 //!
-//! An example of this C-style usage, without error checking, is shown below:
+//! The API expects sequential calls to:
+//!
+//!  - `wasm_bench_create`
+//!  - `wasm_bench_compile`
+//!  - `wasm_bench_instantiate`
+//!  - `wasm_bench_execute`
+//!  - `wasm_bench_free`
+//!
+//! You may repeat this sequence of calls multiple times to take multiple
+//! measurements of compilation, instantiation, and execution time within a
+//! single process.
+//!
+//! All API calls must happen on the same thread.
+//!
+//! Functions which return pointers use null as an error value. Function which
+//! return `int` use `0` as OK and non-zero as an error value.
+//!
+//! # Example
 //!
 //! ```
 //! use wasmtime_bench_api::*;
-//! let module = wat::parse_bytes(br#"(module
+//!
+//! let engine = unsafe { wasm_bench_create() };
+//! assert!(!engine.is_null());
+//!
+//! let wasm = wat::parse_bytes(br#"
+//!     (module
 //!         (func $bench_start (import "bench" "start"))
 //!         (func $bench_end (import "bench" "end"))
 //!         (func $start (export "_start")
-//!      (call $bench_start) (i32.const 2) (i32.const 2) (i32.add) (drop) (call $bench_end))
+//!             call $bench_start
-//! )"#).unwrap();
+//!             i32.const 1
-//! let engine = unsafe { engine_create(module.as_ptr(), module.len()) };
+//!             i32.const 2
+//!             i32.add
+//!             drop
+//!             call $bench_end
+//!         )
+//!     )
+//! "#).unwrap();
 //!
-//! // Start compilation timer.
+//! // Start your compilation timer here.
-//! unsafe { engine_compile_module(engine) };
+//! let code = unsafe { wasm_bench_compile(engine, wasm.as_ptr(), wasm.len()) };
-//! // End compilation timer.
+//! // End your compilation timer here.
+//! assert_eq!(code, OK);
 //!
-//! // The Wasm benchmark will expect us to provide functions to start ("bench" "start") and stop ("bench" "stop") the
+//! // The Wasm benchmark will expect us to provide functions to start ("bench"
-//! // measurement counters/timers during execution; here we provide a no-op implementation.
+//! // "start") and stop ("bench" "stop") the measurement counters/timers during
-//! extern "C" fn noop() {}
+//! // execution.
+//! extern "C" fn bench_start() {
+//!     // Start your execution timer here.
+//! }
+//! extern "C" fn bench_stop() {
+//!     // End your execution timer here.
+//! }
 //!
-//! // Start instantiation timer.
+//! // Start your instantiation timer here.
-//! unsafe { engine_instantiate_module(engine, noop, noop) };
+//! let code = unsafe { wasm_bench_instantiate(engine, bench_start, bench_stop) };
-//! // End instantiation timer.
+//! // End your instantiation timer here.
+//! assert_eq!(code, OK);
 //!
-//! // No need to start timers for the execution since, by convention, the timer functions we passed during
+//! // No need to start timers for the execution since, by convention, the timer
-//! // instantiation will be called by the benchmark at the appropriate time (before and after the benchmarked section).
+//! // functions we passed during instantiation will be called by the benchmark
-//! unsafe { engine_execute_module(engine) };
+//! // at the appropriate time (before and after the benchmarked section).
+//! let code = unsafe { wasm_bench_execute(engine) };
+//! assert_eq!(code, OK);
 //!
-//! unsafe { engine_free(engine) }
+//! unsafe {
+//!     wasm_bench_free(engine);
+//! }
 //! ```
-use anyhow::{anyhow, Result};
+
-use core::slice;
+use anyhow::{anyhow, Context, Result};
-use std::os::raw::c_int;
+use std::os::raw::{c_int, c_void};
+use std::slice;
 use wasi_common::WasiCtxBuilder;
 use wasmtime::{Config, Engine, Instance, Linker, Module, Store};
 use wasmtime_wasi::Wasi;
 
-/// Exposes a C-compatible way of creating the engine from the bytes of a single Wasm module. This function returns a
+pub type ExitCode = c_int;
-/// pointer to an opaque structure that contains the engine's initialized state.
+pub const OK: ExitCode = 0;
+pub const ERR: ExitCode = -1;
+
+/// Exposes a C-compatible way of creating the engine from the bytes of a single
+/// Wasm module.
+///
+/// This function returns a pointer to a structure that contains the engine's
+/// initialized state.
 #[no_mangle]
-pub extern "C" fn engine_create(
+pub extern "C" fn wasm_bench_create() -> *mut c_void {
-    wasm_bytes: *const u8,
+    let state = Box::new(BenchState::new());
-    wasm_bytes_length: usize,
+    Box::into_raw(state) as _
-) -> *mut OpaqueEngineState {
-    let wasm_bytes = unsafe { slice::from_raw_parts(wasm_bytes, wasm_bytes_length) };
-    let state = Box::new(EngineState::new(wasm_bytes));
-    Box::into_raw(state) as *mut _
 }
 
 /// Free the engine state allocated by this library.
 #[no_mangle]
-pub extern "C" fn engine_free(state: *mut OpaqueEngineState) {
+pub extern "C" fn wasm_bench_free(state: *mut c_void) {
+    assert!(!state.is_null());
     unsafe {
-        Box::from_raw(state);
+        Box::from_raw(state as *mut BenchState);
     }
 }
 
 /// Compile the Wasm benchmark module.
 #[no_mangle]
-pub extern "C" fn engine_compile_module(state: *mut OpaqueEngineState) -> c_int {
+pub extern "C" fn wasm_bench_compile(
-    let result = unsafe { OpaqueEngineState::convert(state) }.compile();
+    state: *mut c_void,
-    to_c_error(result, "failed to compile")
+    wasm_bytes: *const u8,
+    wasm_bytes_length: usize,
+) -> ExitCode {
+    let state = unsafe { (state as *mut BenchState).as_mut().unwrap() };
+    let wasm_bytes = unsafe { slice::from_raw_parts(wasm_bytes, wasm_bytes_length) };
+    let result = state.compile(wasm_bytes).context("failed to compile");
+    to_exit_code(result)
 }
 
 /// Instantiate the Wasm benchmark module.
 #[no_mangle]
-pub extern "C" fn engine_instantiate_module(
+pub extern "C" fn wasm_bench_instantiate(
-    state: *mut OpaqueEngineState,
+    state: *mut c_void,
     bench_start: extern "C" fn(),
     bench_end: extern "C" fn(),
-) -> c_int {
+) -> ExitCode {
-    let result = unsafe { OpaqueEngineState::convert(state) }.instantiate(bench_start, bench_end);
+    let state = unsafe { (state as *mut BenchState).as_mut().unwrap() };
-    to_c_error(result, "failed to instantiate")
+    let result = state
+        .instantiate(bench_start, bench_end)
+        .context("failed to instantiate");
+    to_exit_code(result)
 }
 
 /// Execute the Wasm benchmark module.
 #[no_mangle]
-pub extern "C" fn engine_execute_module(state: *mut OpaqueEngineState) -> c_int {
+pub extern "C" fn wasm_bench_execute(state: *mut c_void) -> ExitCode {
-    let result = unsafe { OpaqueEngineState::convert(state) }.execute();
+    let state = unsafe { (state as *mut BenchState).as_mut().unwrap() };
-    to_c_error(result, "failed to execute")
+    let result = state.execute().context("failed to execute");
+    to_exit_code(result)
 }
 
-/// Helper function for converting a Rust result to a C error code (0 == success). Additionally, this will print an
+/// Helper function for converting a Rust result to a C error code.
-/// error indicating some information regarding the failure.
+///
-fn to_c_error<T>(result: Result<T>, message: &str) -> c_int {
+/// This will print an error indicating some information regarding the failure.
-    match result {
+fn to_exit_code<T>(result: impl Into<Result<T>>) -> ExitCode {
-        Ok(_) => 0,
+    match result.into() {
+        Ok(_) => OK,
         Err(error) => {
-            println!("{}: {:?}", message, error);
+            eprintln!("{:?}", error);
-            1
+            ERR
         }
     }
 }
 
-/// Opaque pointer type for hiding the engine state details.
+/// This structure contains the actual Rust implementation of the state required
-#[repr(C)]
+/// to manage the Wasmtime engine between calls.
-pub struct OpaqueEngineState {
+struct BenchState {
-    _private: [u8; 0],
-}
-impl OpaqueEngineState {
-    unsafe fn convert(ptr: *mut OpaqueEngineState) -> &'static mut EngineState<'static> {
-        assert!(!ptr.is_null());
-        &mut *(ptr as *mut EngineState)
-    }
-}
-
-/// This structure contains the actual Rust implementation of the state required to manage the Wasmtime engine between
-/// calls.
-struct EngineState<'a> {
-    bytes: &'a [u8],
     engine: Engine,
     store: Store,
     module: Option<Module>,
     instance: Option<Instance>,
+    did_execute: bool,
 }
 
-impl<'a> EngineState<'a> {
+impl BenchState {
-    fn new(bytes: &'a [u8]) -> Self {
+    fn new() -> Self {
-        // TODO turn off caching?
         let mut config = Config::new();
         config.wasm_simd(true);
+        // NB: do not configure a code cache.
+
         let engine = Engine::new(&config);
         let store = Store::new(&engine);
         Self {
-            bytes,
             engine,
             store,
             module: None,
             instance: None,
+            did_execute: false,
         }
     }
 
-    fn compile(&mut self) -> Result<()> {
+    fn compile(&mut self, bytes: &[u8]) -> Result<()> {
-        self.module = Some(Module::from_binary(&self.engine, self.bytes)?);
+        assert!(
+            self.module.is_none(),
+            "create a new engine to repeat compilation"
+        );
+        self.module = Some(Module::from_binary(&self.engine, bytes)?);
         Ok(())
     }
 
@@ -148,9 +192,15 @@ impl<'a> EngineState<'a> {
         bench_start: extern "C" fn(),
         bench_end: extern "C" fn(),
     ) -> Result<()> {
-        // TODO instantiate WASI modules?
+        assert!(
-        match &self.module {
+            self.instance.is_none(),
-            Some(module) => {
+            "create a new engine to repeat instantiation"
+        );
+        let module = self
+            .module
+            .as_mut()
+            .expect("compile the module before instantiating it");
+
         let mut linker = Linker::new(&self.store);
 
         // Import a very restricted WASI environment.
@@ -164,35 +214,35 @@ impl<'a> EngineState<'a> {
         linker.func("bench", "start", move || bench_start())?;
         linker.func("bench", "end", move || bench_end())?;
 
-                self.instance = Some(linker.instantiate(module)?);
+        self.instance = Some(linker.instantiate(&module)?);
-            }
-            None => panic!("compile the module before instantiating it"),
-        }
         Ok(())
     }
 
-    fn execute(&self) -> Result<()> {
+    fn execute(&mut self) -> Result<()> {
-        match &self.instance {
+        assert!(!self.did_execute, "create a new engine to repeat execution");
-            Some(instance) => {
+        self.did_execute = true;
+
+        let instance = self
+            .instance
+            .as_ref()
+            .expect("instantiate the module before executing it");
+
         let start_func = instance.get_func("_start").expect("a _start function");
         let runnable_func = start_func.get0::<()>()?;
         match runnable_func() {
-                    Ok(_) => {}
+            Ok(_) => Ok(()),
             Err(trap) => {
                 // Since _start will likely return by using the system `exit` call, we must
                 // check the trap code to see if it actually represents a successful exit.
-                        let status = trap.i32_exit_status();
+                match trap.i32_exit_status() {
-                        if status != Some(0) {
+                    Some(0) => Ok(()),
-                            return Err(anyhow!(
+                    Some(n) => Err(anyhow!("_start exited with a non-zero code: {}", n)),
-                                "_start exited with a non-zero code: {}",
+                    None => Err(anyhow!(
-                                status.unwrap()
+                        "executing the benchmark resulted in a trap: {}",
-                            ));
+                        trap
+                    )),
                 }
             }
-                };
         }
-            None => panic!("instantiate the module before executing it"),
-        }
-        Ok(())
     }
 }