From 55c2ffce1ccfea7dce385a6996c997d444e4e58b Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Wed, 17 Jul 2024 16:49:50 -0400 Subject: [PATCH 1/2] factors: Allow instance state to be nested in store data Signed-off-by: Lann Martin --- crates/factor-key-value/src/lib.rs | 5 +---- crates/factor-llm/src/lib.rs | 2 +- crates/factor-outbound-http/src/lib.rs | 2 +- crates/factor-outbound-http/src/wasi.rs | 6 ++---- crates/factor-sqlite/src/lib.rs | 4 ++-- crates/factor-variables/src/lib.rs | 5 +---- crates/factor-wasi/src/lib.rs | 4 ++-- crates/factors-derive/src/lib.rs | 22 +++++++++++++++------- crates/factors-test/src/lib.rs | 10 +++++----- crates/factors/src/factor.rs | 21 +++++++++------------ crates/factors/src/lib.rs | 3 --- crates/factors/src/runtime_factors.rs | 17 +++++++++++------ crates/factors/tests/smoke.rs | 25 +++++++++++++++++++++---- 13 files changed, 71 insertions(+), 55 deletions(-) diff --git a/crates/factor-key-value/src/lib.rs b/crates/factor-key-value/src/lib.rs index f066429d8..dc77e713b 100644 --- a/crates/factor-key-value/src/lib.rs +++ b/crates/factor-key-value/src/lib.rs @@ -54,10 +54,7 @@ impl Factor for KeyValueFactor { type AppState = AppState; type InstanceBuilder = InstanceBuilder; - fn init( - &mut self, - mut ctx: InitContext, - ) -> anyhow::Result<()> { + fn init(&mut self, mut ctx: InitContext) -> anyhow::Result<()> { ctx.link_bindings(spin_world::v1::key_value::add_to_linker)?; ctx.link_bindings(spin_world::v2::key_value::add_to_linker)?; Ok(()) diff --git a/crates/factor-llm/src/lib.rs b/crates/factor-llm/src/lib.rs index 13899ec3b..3e40f36a2 100644 --- a/crates/factor-llm/src/lib.rs +++ b/crates/factor-llm/src/lib.rs @@ -34,7 +34,7 @@ impl Factor for LlmFactor { type AppState = AppState; type InstanceBuilder = InstanceState; - fn init( + fn init( &mut self, mut ctx: spin_factors::InitContext, ) -> anyhow::Result<()> { diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs index 5cc845c6f..e17babf2e 100644 --- a/crates/factor-outbound-http/src/lib.rs +++ b/crates/factor-outbound-http/src/lib.rs @@ -17,7 +17,7 @@ impl Factor for OutboundHttpFactor { type AppState = (); type InstanceBuilder = InstanceState; - fn init( + fn init( &mut self, mut ctx: spin_factors::InitContext, ) -> anyhow::Result<()> { diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index 42cf59953..f5b99497e 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -1,14 +1,12 @@ use http::Request; -use spin_factors::{ - wasmtime::component::ResourceTable, RuntimeFactors, RuntimeFactorsInstanceState, -}; +use spin_factors::{wasmtime::component::ResourceTable, RuntimeFactorsInstanceState}; use wasmtime_wasi_http::{ bindings::http::types::ErrorCode, WasiHttpCtx, WasiHttpImpl, WasiHttpView, }; use crate::{wasi_2023_10_18, wasi_2023_11_10, OutboundHttpFactor}; -pub(crate) fn add_to_linker( +pub(crate) fn add_to_linker( ctx: &mut spin_factors::InitContext, ) -> anyhow::Result<()> { fn type_annotate(f: F) -> F diff --git a/crates/factor-sqlite/src/lib.rs b/crates/factor-sqlite/src/lib.rs index 68e95e992..a2673a44b 100644 --- a/crates/factor-sqlite/src/lib.rs +++ b/crates/factor-sqlite/src/lib.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use host::InstanceState; use async_trait::async_trait; -use spin_factors::{anyhow, Factor, RuntimeFactors}; +use spin_factors::{anyhow, Factor}; use spin_locked_app::MetadataKey; use spin_world::v1::sqlite as v1; use spin_world::v2::sqlite as v2; @@ -32,7 +32,7 @@ impl Factor for SqliteFactor { type AppState = AppState; type InstanceBuilder = InstanceState; - fn init( + fn init( &mut self, mut ctx: spin_factors::InitContext, ) -> anyhow::Result<()> { diff --git a/crates/factor-variables/src/lib.rs b/crates/factor-variables/src/lib.rs index 498e96c89..596dc6e74 100644 --- a/crates/factor-variables/src/lib.rs +++ b/crates/factor-variables/src/lib.rs @@ -40,10 +40,7 @@ impl Factor for VariablesFactor { type AppState = AppState; type InstanceBuilder = InstanceState; - fn init( - &mut self, - mut ctx: InitContext, - ) -> anyhow::Result<()> { + fn init(&mut self, mut ctx: InitContext) -> anyhow::Result<()> { ctx.link_bindings(spin_world::v1::config::add_to_linker)?; ctx.link_bindings(spin_world::v2::variables::add_to_linker)?; Ok(()) diff --git a/crates/factor-wasi/src/lib.rs b/crates/factor-wasi/src/lib.rs index 47479236c..62f98c146 100644 --- a/crates/factor-wasi/src/lib.rs +++ b/crates/factor-wasi/src/lib.rs @@ -43,9 +43,9 @@ impl Factor for WasiFactor { type AppState = (); type InstanceBuilder = InstanceBuilder; - fn init( + fn init( &mut self, - mut ctx: InitContext, + mut ctx: InitContext, ) -> anyhow::Result<()> { fn type_annotate(f: F) -> F where diff --git a/crates/factors-derive/src/lib.rs b/crates/factors-derive/src/lib.rs index 0731c7a07..c65f47485 100644 --- a/crates/factors-derive/src/lib.rs +++ b/crates/factors-derive/src/lib.rs @@ -77,18 +77,20 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { type InstanceBuilders = #builders_name; type InstanceState = #state_name; - #[allow(clippy::needless_option_as_deref)] - fn init( + fn init + Send + 'static>( &mut self, - linker: &mut #wasmtime::component::Linker<#state_name>, + linker: &mut #wasmtime::component::Linker, ) -> #Result<()> { #( - #Factor::init::( + #Factor::init::( &mut self.#factor_names, - #factors_path::InitContext::::new( + #factors_path::InitContext::::new( linker, - |state| &mut state.#factor_names, - |state| (&mut state.#factor_names, &mut state.__table), + |data| &mut data.as_mut().#factor_names, + |data| { + let state = data.as_mut(); + (&mut state.#factor_names, &mut state.__table) + }, ) ).map_err(#Error::factor_init_error::<#factor_types>)?; )* @@ -226,5 +228,11 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { &mut self.__table } } + + impl AsMut<#state_name> for #state_name { + fn as_mut(&mut self) -> &mut Self { + self + } + } }) } diff --git a/crates/factors-test/src/lib.rs b/crates/factors-test/src/lib.rs index 0308bd746..103671dfe 100644 --- a/crates/factors-test/src/lib.rs +++ b/crates/factors-test/src/lib.rs @@ -2,8 +2,8 @@ use spin_app::locked::LockedApp; use spin_factors::{ anyhow::{self, Context}, serde::de::DeserializeOwned, - wasmtime::{Config, Engine}, - App, Linker, RuntimeConfigSource, RuntimeFactors, + wasmtime::{component::Linker, Config, Engine}, + App, RuntimeConfigSource, RuntimeFactors, }; use spin_loader::FilesMountStrategy; @@ -57,8 +57,8 @@ impl TestEnvironment { &self, mut factors: T, ) -> anyhow::Result { - let mut linker = Self::new_linker::(); - factors.init(&mut linker)?; + let mut linker = Self::new_linker::(); + factors.init::(&mut linker)?; let locked_app = self .build_locked_app() @@ -77,7 +77,7 @@ impl TestEnvironment { Ok(factors.build_instance_state(builders)?) } - pub fn new_linker() -> Linker { + pub fn new_linker() -> Linker { let engine = Engine::new(Config::new().async_support(true)) .expect("wasmtime engine failed to initialize"); Linker::::new(&engine) diff --git a/crates/factors/src/factor.rs b/crates/factors/src/factor.rs index 8ec6ee8a2..14da60951 100644 --- a/crates/factors/src/factor.rs +++ b/crates/factors/src/factor.rs @@ -1,11 +1,10 @@ use std::any::Any; -use wasmtime::component::ResourceTable; +use wasmtime::component::{Linker, ResourceTable}; use crate::{ prepare::FactorInstanceBuilder, runtime_config::RuntimeConfigTracker, App, Error, - FactorRuntimeConfig, InstanceBuilders, Linker, PrepareContext, RuntimeConfigSource, - RuntimeFactors, + FactorRuntimeConfig, InstanceBuilders, PrepareContext, RuntimeConfigSource, RuntimeFactors, }; /// A contained (i.e., "factored") piece of runtime functionality. @@ -29,7 +28,7 @@ pub trait Factor: Any + Sized { /// This will be called at most once, before any call to [`FactorInstanceBuilder::new`]. /// `InitContext` provides access to a wasmtime `Linker`, so this is where any bindgen /// `add_to_linker` calls go. - fn init(&mut self, mut ctx: InitContext) -> anyhow::Result<()> { + fn init(&mut self, mut ctx: InitContext) -> anyhow::Result<()> { _ = &mut ctx; Ok(()) } @@ -72,22 +71,20 @@ pub trait Factor: Any + Sized { pub type FactorInstanceState = <::InstanceBuilder as FactorInstanceBuilder>::InstanceState; -pub(crate) type GetDataFn = - fn(&mut ::InstanceState) -> &mut FactorInstanceState; +pub(crate) type GetDataFn = fn(&mut T) -> &mut FactorInstanceState; -pub(crate) type GetDataWithTableFn = fn( - &mut ::InstanceState, -) -> (&mut FactorInstanceState, &mut ResourceTable); +pub(crate) type GetDataWithTableFn = + fn(&mut T) -> (&mut FactorInstanceState, &mut ResourceTable); /// An InitContext is passed to [`Factor::init`], giving access to the global /// common [`wasmtime::component::Linker`]. -pub struct InitContext<'a, T: RuntimeFactors, U: Factor> { +pub struct InitContext<'a, T, U: Factor> { pub(crate) linker: &'a mut Linker, pub(crate) get_data: GetDataFn, pub(crate) get_data_with_table: GetDataWithTableFn, } -impl<'a, T: RuntimeFactors, U: Factor> InitContext<'a, T, U> { +impl<'a, T, U: Factor> InitContext<'a, T, U> { #[doc(hidden)] pub fn new( linker: &'a mut Linker, @@ -122,7 +119,7 @@ impl<'a, T: RuntimeFactors, U: Factor> InitContext<'a, T, U> { &mut self, add_to_linker: impl Fn( &mut Linker, - fn(&mut T::InstanceState) -> &mut FactorInstanceState, + fn(&mut T) -> &mut FactorInstanceState, ) -> anyhow::Result<()>, ) -> anyhow::Result<()> { add_to_linker(self.linker, self.get_data) diff --git a/crates/factors/src/lib.rs b/crates/factors/src/lib.rs index 6a9185a20..569b76e9b 100644 --- a/crates/factors/src/lib.rs +++ b/crates/factors/src/lib.rs @@ -16,9 +16,6 @@ pub use crate::{ runtime_factors::{RuntimeFactors, RuntimeFactorsInstanceState}, }; -/// A [`wasmtime::component::Linker`] used for a [`RuntimeFactors`] collection. -pub type Linker = wasmtime::component::Linker<::InstanceState>; - // Temporary wrappers while refactoring pub type App = spin_app::App<'static, spin_app::InertLoader>; pub type AppComponent<'a> = spin_app::AppComponent<'a, spin_app::InertLoader>; diff --git a/crates/factors/src/runtime_factors.rs b/crates/factors/src/runtime_factors.rs index 1293dc13f..1c7e8e1e8 100644 --- a/crates/factors/src/runtime_factors.rs +++ b/crates/factors/src/runtime_factors.rs @@ -1,6 +1,6 @@ -use wasmtime::component::ResourceTable; +use wasmtime::component::{Linker, ResourceTable}; -use crate::{factor::FactorInstanceState, App, ConfiguredApp, Factor, Linker, RuntimeConfigSource}; +use crate::{factor::FactorInstanceState, App, ConfiguredApp, Factor, RuntimeConfigSource}; /// A collection of `Factor`s that are initialized and configured together. /// @@ -10,7 +10,7 @@ use crate::{factor::FactorInstanceState, App, ConfiguredApp, Factor, Linker, Run /// /// A typical usage of `RuntimeFactors` would look something like the following pseudo-code: /// -/// ```no_run +/// ```ignore /// #[derive(RuntimeFactors)] /// struct MyFactors { /// // ... @@ -21,8 +21,10 @@ use crate::{factor::FactorInstanceState, App, ConfiguredApp, Factor, Linker, Run /// factors.init(&mut linker)?; /// // Configure the factors with an app and runtime config /// let configured_app = factors.configure_app(app, runtime_config)?; +/// // Prepare instance state builders +/// let builders = factors.prepare(&configured_app, "component-id")?; /// // Build the instance state for the factors -/// let data factors.build_instance_state(&configured_app, component.id()) +/// let data = factors.build_instance_state(builders)?; /// // Initialize a `wasmtime` store with the instance state /// let mut store = wasmtime::Store::new(&engine, data); /// // Instantiate the component @@ -39,7 +41,10 @@ pub trait RuntimeFactors: Sized + 'static { /// Initialize the factors with a linker. /// /// Each factor's `init` is called in turn. - fn init(&mut self, linker: &mut Linker) -> crate::Result<()>; + fn init + Send + 'static>( + &mut self, + linker: &mut Linker, + ) -> crate::Result<()>; /// Configure the factors with an app and runtime config. fn configure_app( @@ -76,7 +81,7 @@ pub trait RuntimeFactors: Sized + 'static { /// Get the state of a particular Factor from the overall InstanceState /// /// Implemented by `#[derive(RuntimeFactors)]` -pub trait RuntimeFactorsInstanceState: Send + 'static { +pub trait RuntimeFactorsInstanceState: AsMut + Send + 'static { fn get_with_table( &mut self, ) -> Option<(&mut FactorInstanceState, &mut ResourceTable)>; diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs index 342a36034..ed6cdd72f 100644 --- a/crates/factors/tests/smoke.rs +++ b/crates/factors/tests/smoke.rs @@ -23,6 +23,17 @@ struct Factors { key_value: KeyValueFactor, } +struct Data { + factors_instance_state: FactorsInstanceState, + _other_data: usize, +} + +impl AsMut for Data { + fn as_mut(&mut self) -> &mut FactorsInstanceState { + &mut self.factors_instance_state + } +} + #[tokio::test(flavor = "multi_thread")] async fn smoke_test_works() -> anyhow::Result<()> { let mut factors = Factors { @@ -50,14 +61,15 @@ async fn smoke_test_works() -> anyhow::Result<()> { let engine = wasmtime::Engine::new(wasmtime::Config::new().async_support(true))?; let mut linker = wasmtime::component::Linker::new(&engine); - factors.init(&mut linker).unwrap(); + factors.init::(&mut linker).unwrap(); let configured_app = factors.configure_app(app, TestSource)?; let builders = factors.prepare(&configured_app, "smoke-app")?; - let data = factors.build_instance_state(builders)?; + let state = factors.build_instance_state(builders)?; assert_eq!( - data.variables + state + .variables .resolver() .resolve("smoke-app", "other".try_into().unwrap()) .await @@ -65,6 +77,11 @@ async fn smoke_test_works() -> anyhow::Result<()> { "" ); + let data = Data { + factors_instance_state: state, + _other_data: 1, + }; + let mut store = wasmtime::Store::new(&engine, data); let component = configured_app.app().components().next().unwrap(); @@ -84,7 +101,7 @@ async fn smoke_test_works() -> anyhow::Result<()> { // Invoke handler let req = http::Request::get("/").body(Default::default()).unwrap(); - let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(store.data_mut()).unwrap(); + let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(store.data_mut().as_mut()).unwrap(); let request = wasi_http.new_incoming_request(req)?; let (response_tx, response_rx) = tokio::sync::oneshot::channel(); let response = wasi_http.new_response_outparam(response_tx)?; From c7f7ba415e89a0f6e6122c0b85c1b81514cddbd7 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Thu, 18 Jul 2024 10:09:35 -0400 Subject: [PATCH 2/2] factors: Enhance Factor doc comments Signed-off-by: Lann Martin --- crates/factors/src/factor.rs | 9 ++++++--- crates/factors/src/runtime_factors.rs | 7 ++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/factors/src/factor.rs b/crates/factors/src/factor.rs index 14da60951..72c57371f 100644 --- a/crates/factors/src/factor.rs +++ b/crates/factors/src/factor.rs @@ -25,9 +25,12 @@ pub trait Factor: Any + Sized { /// Initializes this `Factor` for a runtime once at runtime startup. /// - /// This will be called at most once, before any call to [`FactorInstanceBuilder::new`]. - /// `InitContext` provides access to a wasmtime `Linker`, so this is where any bindgen - /// `add_to_linker` calls go. + /// This will be called at most once, before any call to + /// [`Factor::prepare`]. `InitContext` provides access to a wasmtime + /// `Linker`, so this is where any bindgen `add_to_linker` calls go. + /// + /// The type parameter `T` here is the same as the [`wasmtime::Store`] type + /// parameter `T`, which will contain the [`RuntimeFactors::InstanceState`]. fn init(&mut self, mut ctx: InitContext) -> anyhow::Result<()> { _ = &mut ctx; Ok(()) diff --git a/crates/factors/src/runtime_factors.rs b/crates/factors/src/runtime_factors.rs index 1c7e8e1e8..78154b7eb 100644 --- a/crates/factors/src/runtime_factors.rs +++ b/crates/factors/src/runtime_factors.rs @@ -38,15 +38,16 @@ pub trait RuntimeFactors: Sized + 'static { /// The collection of all the `InstanceBuilder`s of the factors. type InstanceBuilders; - /// Initialize the factors with a linker. + /// Initialize the factors with the given linker. /// - /// Each factor's `init` is called in turn. + /// Each factor's `init` is called in turn. Must be called once before + /// [`RuntimeFactors::prepare`]. fn init + Send + 'static>( &mut self, linker: &mut Linker, ) -> crate::Result<()>; - /// Configure the factors with an app and runtime config. + /// Configure the factors with the given app and runtime config. fn configure_app( &self, app: App,