RFC: describe new approach to runtime

This commit is contained in:
Niko Matsakis 2020-07-02 10:00:31 +00:00
parent c6663f3dcb
commit 4bf46f5f24

View file

@ -138,6 +138,33 @@ but you must now use `dyn MyDatabase`:
fn my_query(db: &dyn MyDatabase, ...) { .. }
```
### Databases embed a `Storage<DB>` with a fixed field name
The "Hello World" database becomes the following:
```rust,ignore
#[salsa::database(QueryGroup1, ..., QueryGroupN)]
struct MyDatabase {
storage: salsa::Storage<Self>
}
impl salsa::Database for MyDatabase {}
```
In particular:
* You now embed a `salsa::Storage<Self>` instead of a `salsa::Runtime<Self>`
* 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<DB::Storage>`. 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<DB: ?Sized + salsa::Database> 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<DB>` type
The Salsa runtime is currently `Runtime<DB>` 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<DB>` type which is defined like so:
```rust,ignore
pub struct Storage<DB: DatabaseImpl> {
query_store: Arc<DB::DatabaseStorage>,
runtime: Runtime,
}
impl<DB> Storage<DB> {
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<DB>` 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<DatabaseKeyIndex>;
...
}
```
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<DB: DatabaseImpl> {
...
}
```
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