RFC (Wasmtime): add host functions at the Engine-level.

This RFC proposes to extend the Wasmtime API to allow users to define host
functions at the `Engine`-level.

Today, `Func` is used to define host functions, but it is tied to a `Store`.
If users desire short-lived stores to ensure short-lived instances, host
functions need to be redefined upon every module instantiation.

By defining host functions at the `Engine`-level, instances can be created
without having to redefine the host functions; this will make for faster module
instantiations in scenarios where a module is repeatedly instantiated.

accepted/engine-host-functions.md

@@ -0,0 +1,254 @@
+# Summary
+
+[summary]: #summary
+
+This RFC proposes adding support in the Wasmtime API for defining host functions in an `Engine`.
+
+# Motivation
+[motivation]: #motivation
+
+Wasmtime is currently well-suited for long-lived module instantiations, where a singular `Instance` is created for a 
+WebAssembly program and the program is run to completion.
+
+In this model, the time spent defining host functions for the instance is negligible because the setup is only performed once 
+and the instance is, potentially, expected to run for a long time.
+
+However, this model is not ideal for services that create short-lived instantiations for each service request, where the time 
+spent creating host functions to import at instantiation-time can be considerable for the repeated creation of many instances.
+
+As an instance is expected to live only for the duration of the request, each instance would be associated with its own `Store` 
+that is dropped when the request completes.
+
+Given the nature of `Store` isolation in Wasmtime, this means host function definitions must be recreated for each and every 
+request handled by the service.
+
+If instead host functions could be defined for an `Engine` (in addition to `Store`), then the repeated effort to define the 
+host functions would be eliminated and the time it takes to create an instance to handle a request would therefore be 
+noticeably reduced.
+
+# Proposal
+[proposal]: #proposal
+
+As `Func` is inherently tied to a `Store`, it cannot be used to represent a function associated with an `Engine`.
+
+This RFC proposes not having an analogous type to represent these functions; instead, users will define the function via 
+methods on `Engine`, but use `Store` (or `Linker`) to get the `Func` representing the function.
+
+## Overview example
+
+A simple example that demonstrates defining a host function in an `Engine`:
+
+```rust
+let engine = Engine::default();
+
+engine.wrap_func("", "hello", |caller: Caller, ptr: i32, len: i32| {
+    let mem = caller.get_export("memory").unwrap().into_memory().unwrap();
+    println!(
+        "Hello {}!",
+        std::str::from_utf8(unsafe{ &mem.data_unchecked()[ptr..ptr + len] }).unwrap()
+    );
+});
+
+let module = Module::new(
+    &engine,
+    r#"
+    (module
+        (import "" "hello" (func $hello (param i32 i32)))
+        (memory (export "memory") 1)
+        (func (export "run")
+            i32.const 0
+            i32.const 5
+            call $hello
+        )
+        (data (i32.const 0) "world")
+    )
+    "#,
+)?;
+
+let store = Store::new(module.engine());
+let linker = Linker::new(&store);
+
+let instance = linker.instantiate(&module)?;
+let run = instance
+    .get_export("run")
+    .unwrap()
+    .into_func()
+    .unwrap()
+    .get0::<()>();
+
+run()?;
+```
+
+## Changes to `wasmtime::Engine`
+
+This RFC proposes that the following methods be added to `Engine`:
+
+```rust
+/// Defines a function for the [`Engine`] for the given callback.
+///
+/// Use [`Store::get_engine_func`] to get a [`Func`] representing the function.
+///
+/// Note that the implementation of `func` must adhere to the `ty`
+/// signature given, error or traps may occur if it does not respect the
+/// `ty` signature.
+///
+/// Additionally note that this is quite a dynamic function since signatures
+/// are not statically known. For performance reasons, it's recommended
+/// to use [`Engine::wrap_func`] if you can because with statically known
+/// signatures the engine can optimize the implementation much more.
+pub fn define_func(
+    &self,
+    module: &str,
+    name: &str,
+    ty: FuncType,
+    func: impl Fn(Caller<'_>, &[Val], &mut [Val]) -> Result<(), Trap> + Send + Sync + 'static,
+) -> Result<()>;
+
+/// Defines a function for the [`Engine`] from the given Rust closure.
+///
+/// Use [`Store::get_engine_func`] to get a [`Func`] representing the function.
+///
+/// See [`Func::wrap`] for information about accepted parameter and result types for the closure.
+pub fn wrap_func<Params, Results>(
+    &self,
+    module: &str,
+    name: &str,
+    func: impl IntoFunc<Params, Results> + Send + Sync,
+);
+```
+
+Similar to `Func::new`, `define_func` will define an engine function that can be used for any Wasm function type.
+
+Similar to `Func::wrap`, `wrap_func` will generically accept different `Fn` signatures to determine the WebAssembly type of 
+the function.
+
+These methods will internally create an `InstanceHandle` to represent the host function.
+
+However, the instance handle will not owned by a store; instead, the engine will own the associated instance handles and deallocate them when the engine is dropped.
+
+Note: the `IntoFunc` trait is documented as internal to Wasmtime and will need to be extended to implement this feature;
+therefore that will not be considered a breaking change.
+
+## Changes to `wasmtime::Store`
+
+To use `Engine` defined functions as imports for module instantiation or as `funcref` values, a `Func` representation is 
+required.
+
+This proposal calls for adding the following methods to `Store`:
+
+```rust
+/// Gets a function from the [`Engine`] associated with this [`Store`].
+///
+/// Returns `None` if the given function is not defined.
+pub fn get_engine_func(&self, module: &str, name: &str) -> Option<Func>;
+
+/// Calls the given closure with the current context value of the given type.
+///
+/// If a context value of the given type is not present, the closure is passed `None`.
+///
+/// Returns the value returned by the closure.
+pub fn with_context<T: Any, F, R>(&self, func: F) -> R
+where
+    F: Fn(Option<&mut T>) -> R;
+
+/// Inserts a context value into the store.
+///
+/// Returns the previous context value of the same type if present.
+pub fn insert_context<T: Any>(&self, value: T) -> Option<T>;
+
+/// Removes a context value from the store.
+///
+/// Returns the previous context value of the same type if present.
+pub fn remove_context<T: Any>(&self) -> Option<T>;
+```
+
+`get_engine_func` will register the function's instance handle with the `Store`, but as a *borrowed* handle which it is 
+not responsible for deallocation. As `Store` keeps a reference on its associated `Engine`, this ownership model should be 
+sufficient to prevent `funcref` values or imports of engine functions from outliving the function instance itself.
+
+For host functions that require contextual data (e.g. WASI), the `insert_context` method will be used for associating context with 
+a `Store` and it can later be retrieved via `caller.store().with_context()`.
+
+## Changes to `wasmtime::Linker`
+
+There will be no changes to the API surface of `Linker` to support the changes proposed in this RFC.
+
+However, the `Linker` implementation will be changed such that it will fallback to calling `Store::get_engine_func` when 
+resolving imports for `Linker::instantiate` and `Linker::module`.
+
+This should allow for overriding engine function imports in the `Linker` if an import of the same name has already been defined 
+in the `Engine`.
+
+## Changes to `wasmtime_wasi::Wasi`
+
+`Wasi` is a type generated from `wasmtime-wiggle`.
+
+This proposal adds the following methods to the generated `Wasi` type:
+
+```rust
+/// Adds WASI functions to the given `Engine`.
+pub fn add_to_engine(engine: &Engine);
+```
+
+`add_to_engine` will add the various WASI functions to the given engine.
+
+The function implementations will expect that `insert_context` has been called on the `Store` prior to any invocations.
+
+If the context is not set, the WASI function implementations will trap.
+
+### WASI engine function example
+
+```rust
+let engine = Engine::default();
+Wasi::add_to_engine(&engine);
+
+let module = Module::new(
+    &engine,
+    r#"
+    (module
+        (import "wasi_snapshot_preview1" "fd_write" (func $fd_write (param i32 i32 i32 i32) (result i32)))
+        (memory (export "memory") 1)
+        (func (export "run")
+            i32.const 1
+            i32.const 0
+            i32.const 1
+            i32.const 12
+            call $fd_write
+            drop
+        )
+        (data (i32.const 0) "\0C\00\00\00\0D\00\00\00\00\00\00\00Hello world!\n")
+    )
+    "#,
+)?;
+
+let store = Store::new(module.engine());
+
+// Set the WasiCtx in the store
+// Without this, the call to `fd_write` will trap
+store::insert_context(WasiCtxBuilder::new().build()?);
+
+let linker = Linker::new(&store);
+let instance = linker.instantiate(&module)?;
+
+let run = instance
+   .get_export("run")
+   .unwrap()
+   .into_func()
+   .unwrap()
+   .get0::<()>();
+
+run()?;
+```
+
+# Rationale and alternatives
+[rationale-and-alternatives]: #rationale-and-alternatives
+
+No other designs have been considered.
+
+# Open questions
+[open-questions]: #open-questions
+
+* Should this be exposed via Wasmtime-specific C API functions?
+
+* Is `Store` the right place for storing context?
+  For WASI, this means all instances that use the same store will share the `WasiCtx`.