diff --git a/book/src/rfcs/RFC0006-Dynamic-Databases.md b/book/src/rfcs/RFC0006-Dynamic-Databases.md index 82ec9c9b..42edbdf0 100644 --- a/book/src/rfcs/RFC0006-Dynamic-Databases.md +++ b/book/src/rfcs/RFC0006-Dynamic-Databases.md @@ -138,6 +138,33 @@ but you must now use `dyn MyDatabase`: fn my_query(db: &dyn MyDatabase, ...) { .. } ``` +### Databases embed a `Storage` with a fixed field name + +The "Hello World" database becomes the following: + +```rust,ignore +#[salsa::database(QueryGroup1, ..., QueryGroupN)] +struct MyDatabase { + storage: salsa::Storage +} + +impl salsa::Database for MyDatabase {} +``` + +In particular: + +* You now embed a `salsa::Storage` instead of a `salsa::Runtime` +* The field **must** be named `storage` by default; we can include a `#[salsa::storge_field(xxx)]` annotation to change that default if desired. + * Or we could scrape the struct declaration and infer it, I suppose. +* You no longer have to define the `salsa_runtime` and `salsa_runtime_mut` methods, they move to the `DatabaseOps` trait and are manually implemented by doing `self.storage.runtime()` and so forth. + +Why these changes, and what is this `Storage` struct? This is because the actual +storage for queries is moving outside of the runtime. The Storage struct just +combines the `Runtime` (whose type no longer references `DB` directly) with an +`Arc`. The full type of `Storage`, since it includes the database +type, cannot appear in any public interface, it is just used by the various +implementations that are created by `salsa::database`. + ### The query method on `salsa::Database` moves to a helper method As a consequence of the previous point, the existing `query` and `query_mut` @@ -166,10 +193,14 @@ impl DatabaseExt for DB { ### The salsa-event mechanism will move to dynamic dispatch A further consequence is that the existing `salsa_event` method will be -converted to a signature suitable for dynamic dispatch: +simplified and made suitable for dynamic dispatch. It used to take a closure +that would produce the event if necessary; it now simply takes the event itself. +This is partly because events themselves no longer contain complex information: +they used to have database-keys, which could require expensive cloning, but they +now have simple indices. ```rust -fn salsa_event(&self, event_fn: &dyn Fn() -> Event) { +fn salsa_event(&self, event: Event) { #![allow(unused_variables)] } ``` @@ -543,6 +574,72 @@ graph LR; Ellipsis1["..."]; ``` +### DatabaseImpl + +We introduce a `plumbing::DatabaseImpl` trait extends `salsa::Database`. The +impl for this is generated by the `salsa::database` macro. It just defines the +`DatabaseStorage` type. + +```rust,ignore +pub trait DatabaseImpl: Database { + type DatabaseStorage; +} +``` + +### Wrap runtime in a `Storage` type + +The Salsa runtime is currently `Runtime` but it will change to just +`Runtime` and thus not be generic over the database. This means it can be +referenced directly by query storage implementations. This is very useful +because it allows that type to have a number of `pub(crate)` details that query +storage implementations make use of but which are not exposed as part of our +public API. + +However, the `Runtime` crate used to contain a `DB::Storage`, and without the +`DB` in its type, it no longer can. Therefore, we will introduce a new type +`Storage` type which is defined like so: + +```rust,ignore +pub struct Storage { + query_store: Arc, + runtime: Runtime, +} + +impl Storage { + pub fn query_store(&self) -> &DB::DatabaseStorage { + &self.query_store + } + + pub fn salsa_runtime(&self) -> &Runtime { + &self.runtime + } + + pub fn salsa_runtime_mut(&mut self) -> &mut Runtime { + &self.runtime + } + + /// Used for parallel queries + pub fn snapshot(&self) -> Self { + Storage { + query_store: query_store.clone(), + runtime: runtime.snapshot(), + } + } +} +``` + +The user is expected to include a field `storage: Storage` in their database +definition. The `salsa::database` procedural macro, when it generates impls of +traits like `HasQueryGroup`, will embed code like `self.storage` that looks for +that field. + +### `salsa_runtime` methods move to `DatabaseOps` trait + +The `salsa_runtime` methods used to be manually implemented by users to define +the field that contains the salsa runtime. This was always boilerplate. The +`salsa::database` macro now handles that job by defining them to invoke the +corresponding methods on `Storage`. + ### Salsa database trait becomes dyn safe Under this proposal, the Salsa database must be dyn safe. This implies that @@ -550,46 +647,7 @@ we have to make a few changes: * The `query` and `query_mut` methods move to an extension trait. * The `DatabaseStorageTypes` supertrait is removed (that trait is renamed and altered, see next section). -* The `runtime` and `runtime_mut` methods need to return a `&dyn Runtime` and `&mut dyn Runtime`. - -### Runtime trait - -We introduce a `Runtime` trait with the publicly available methods from a `Runtime`: - -```rust,ignore -pub trait Runtime { - fn report_untracked_read(&self); - fn report_synthetic_read(&self, durability: Durability); - fn is_current_revision_canceled(&self) -> bool; - fn active_query(&self) -> Option; - ... -} -``` - -XXX I guess this may need to have some of the "plumbing" methods that are -presently `pub(crate)`. Oh well. It's hard to come up with a compelling -alternative, so long as the type of the runtime embeds the true `DB` type. - -### RuntimeImpl and DatabaseImpl - -We introduce a `DatabaseImpl` trait that replaces `DatabaseStorageTypes`: - -```rust,ignore -pub trait DatabaseImpl: Database { - type DatabaseStorage; - type DatabaseKey; -} -``` - -The runtime struct is renamed to `RuntimeImpl`: - -```rust,ignore -pub struct RuntimeImpl { - ... -} -``` - -and it will implement the `Runtime` trait. +* The `salsa_event` method changes, as described in the User's guide. ### Salsa query group traits are extended with `HasQueryGroup` supertrait