383: Have users of `SalsaStruct` specify allowed options r=XFFXFF a=DropDemBits
This allows adding new options exclusively to `tracked`, `input`, `interned`, or any combination of them, without having to add support code to `SalsaStruct`.
The motivating example is #337, where `input` needs to accept a new `singleton` option, but `tracked` and `interned` should reject it.
Co-authored-by: DropDemBits <r3usrlnd@gmail.com>
376: add `synthetic_write` r=nikomatsakis a=XFFXFF
fixes#364
add `synthetic_write` and use it in test `lru_keeps_dependency_info`, the test will now be broken. We can use `lru_keeps_dependency_info` as a test for https://github.com/salsa-rs/salsa/issues/365, which already has a pr https://github.com/salsa-rs/salsa/pull/371.
379: Update test.yml r=nikomatsakis a=agluszak
- Add names to steps
- No need to run clippy AND check (https://stackoverflow.com/questions/57449356/is-cargo-clippy-a-superset-of-cargo-check)
- Run clippy on all targets with all features
- Fix issues reported by clippy
Co-authored-by: XFFXFF <1247714429@qq.com>
Co-authored-by: Andrzej Głuszak <gluszak.andrzej@gmail.com>
Co-authored-by: andrzej.gluszak <andrzej.gluszak@mpi-sp.org>
371: Ported LRU improvements from old salsa to salsa 2022 r=nikomatsakis a=OLUWAMUYIWA
## What?
In handling evictions from the LRU, I replaced the `delete_memo` with `evict` in the while keeping the `delete_memo` intact.
## How?
This mechanism was already implemented in old `salsa `. I only had to port it.
Fixes: #365
Co-authored-by: OLUWAMUYIWA <onigbindemy@gmail.com>
Co-authored-by: Onigbinde Oluwamuyiwa Elijah <onigbindemy@gmail.com>
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
369: Try to fix issue#340 r=XFFXFF a=zjp-CN
(WIP)
Follow the instructions from https://github.com/salsa-rs/salsa/issues/340#issuecomment-1210153085.
Still not sure what to do with `fmt_index`...
Fix#340
Co-authored-by: zjp <jiping_zhou@foxmail.com>
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
360: Permit renaming constructors, getters and setters r=nikomatsakis a=MihailMihov
The goal is to add an option `constructor_name` to `#[salsa::input]`, `#[salsa::interned]` and `#[salsa::tracked]` that allows changing the name of the generated constructor. After that add attributes `get` and `set` to the fields which allow overriding the names of the getters and setters. In the end the following snippet should compile and work:
```rust
#[salsa::interned(constructor = from_string)]
struct MyInterned {
#[get(text)] #[set(set_text)] #[return_ref]
field: String,
}
impl MyInterned {
pub fn new(db: &dyn Db, s: impl Display) -> MyInterned {
MyInterned::from_string(db, s.to_string())
}
pub fn field(self, db: &dyn Db) -> &str {
self.text(db)
}
pub fn set_field(self, db: &mut dyn Db, id: String) {
self.set_text(&mut db, id)
}
}
```
resolves#332
Co-authored-by: Mihail Mihov <mmihov.personal@gmail.com>
353: start documenting plumbing r=nikomatsakis a=nikomatsakis
Feedback desired! I am trying to document an overview of the new salsa 2022 plumbing. I'd love for folks to [read these docs and tell me if they make sense](https://deploy-preview-353--salsa-rs.netlify.app/plumbing.html).
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
356: make Id::from_u32 public r=nikomatsakis a=nikomatsakis
I made it private 'just because', but it turns out that dada uses it, and it seems reasonable.
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
352: Add options to tracked funcitons for lru capacity r=nikomatsakis a=XFFXFF
fixes#344
Now we can write something like the following to set the lru capacity of tracked functions
```rust
#[salsa::tracked(lru=32)]
fn my_tracked_fn(db: &dyn crate::Db, ...) { }
```
some details:
* lru should not be combined with specify. We will report an error if people do #[salsa::tracked(lru = 32, specify)]
* set 0 as default capacity to disable LRU (Because I think doing this would make the code simpler when implementing `create_ingredients` of tracked functions).
* old salsa support to change lru capacity at runtime, [as noted here](https://salsa-rs.github.io/salsa/rfcs/RFC0004-LRU.html?highlight=change#reference-guide), but we do not support this now
Co-authored-by: XFFXFF <1247714429@qq.com>
Accumulators don't currently work across revisions
due to a few bugs. This commit adds 2 tests to show
the problems and reworks the implementation strategy.
We keep track of when the values in an accumulator were pushed
and reset the vector to empty when the push occurs in a new
revision.
We also ignore stale values from old revisions
(but update the revision when it is marked as validated).
Finally, we treat an accumulator as an untracked read,
which is quite conservative but correct. To get better
reuse, we would need to (a) somehow determine when different
values were pushed, e.g. by hashing or tracked the old values;
and (b) have some `DatabaseKeyIndex` we can use to identify
"the values pushed by this query".
Both of these would add overhead to accumulators and I didn'τ
feel like doing it, particularly since the main use case for
them is communicating errors and things which are not typically
used from within queries.
It turns out that we have some outputs (accumulators) for which
it only makes sense to have a `DependencyIndex` (they don't have
individual keys to identify).
We also track whether reset is required at the ingredient level.
For tracked struct fields, we were not using `push_mut`,
and I think that was an oversight.
The plan is to do a "dependent ingredient" interlinking pass
once the database is constructed.
I realized I can do this better:
* require that outputs are DatabaseKeyIndex, fewer unwraps,
more clearly justified
* when we validate result of tracked fn, also validate its outputs
(this is incompletely implemented, would ideally be separated
into its own commit, but I'm short for time)
The last step will allow us to keep the memoized results for
assigned values and means we don't have to eagerly clear them.
If we see an "assigned value" that is not verified in the current
revision, it can simply be considered dirty.
We can still delete them when entities are deleted, but they're
less special.
We don't do anything with this info right now besides log it,
but the logs show we are reporting it at the right times
in the `specify_tracked_fn_in_rev_1_but_not_2` test
(also fix an oversight in the test where it was creating a new input
each time).
Rename QueryInputs to QueryEdges.
Modify its fields to track both inputs and outputs.
The size of the struct doesn't actually change,
as the separator comes out of padding.
We will record each thing that gets *output* by the query.
We use a btree-set so that we can get a sorted list.
That will allow us to easily compare what is output between revisions.
We will use that to clear stale values.
We don't do anything with this info right now besides log it,
but you can see that we are reporting it at the right times
in the `specify_tracked_fn_in_rev_1_but_not_2` test
(also fix an oversight in the test where it was creating a new input
each time).
Rename QueryInputs to QueryEdges and modify its fields
to track both inputs and outputs. The size of the struct
doesn't actually change, the separator comes out of padding.
We will record each thing that gets *output* by the query.
Use a btree-set so that we can get a sorted list.
That will allow us to easily compare what is output between revisions.
We will use that to clear stale values.
296: Slot no more: overhauled internal algorithm r=nikomatsakis a=nikomatsakis
This is the overhauled implementation that avoids slots, is more parallel friendly, and paves the way to fixed point and more expressive cycle handling.
We just spent 90 minutes going over it. [Some rough notes are available here,](https://hackmd.io/6x9f6mavTRS2imfG96tP5A) and a video will be posted soon.
You may find the [flowgraph useful](https://raw.githubusercontent.com/nikomatsakis/salsa/slot-no-more/book/src/derived-query-read.drawio.svg).
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
This change resolves a fixme that referenced #120.
This change breaks no tests, and, if I understand
correctly, does not affect user-facing API.
Here is the difference for the `HelloWorldGroupStorage__` struct
generated from macros in the `hello_world` example:
**Before:**
```rs
struct HelloWorldGroupStorage__ {
pub input_string:std::sync::Arc<<InputStringQuery as salsa::Query> ::Storage> ,pub length:std::sync::Arc<<LengthQuery as salsa::Query> ::Storage> ,
}
```
**After:**
```rs
struct HelloWorldGroupStorage__ {
input_string:std::sync::Arc<<InputStringQuery as salsa::Query> ::Storage> ,length:std::sync::Arc<<LengthQuery as salsa::Query> ::Storage> ,
}
```
Instead of grabbing the arc, just pass back an `&mut Runtime`.
The eventual goal is to get rid of the lock on the `set` pathway
altogether, but one step at a time.
290: Update doc in macro about query.in_db for dyn db r=nikomatsakis a=mheiber
Update the macro for `query_group` so the comment
on `fn in_db` no longer says that it is more common
to use the trait method on `db`.
Afaict, the trait methods referred to were removed
when dyn database were introduced in RFC0006:
./book/src/rfcs/RFC0006-Dynamic-Databases.md, as
described in the section
"Instead of `db.query(Q)`, you write `Q.in_db(&db)`"
Co-authored-by: Maxwell Elliot Heiber <mheiber@fb.com>
Update the macro for `query_group` so the comment
on `fn in_db` no longer says that it is more common
to use the trait method on `db`.
Afaict, the trait methods referred to were removed
when dyn database were introduced in RFC0006:
./book/src/rfcs/RFC0006-Dynamic-Databases.md, as
described in the section
"Instead of `db.query(Q)`, you write `Q.in_db(&db)`"
Rather than checking return value of from `Q::cycle_fallback`, we
now consult the computed recovery strategy to decide whether to
panic or to recover. We can thus assume that we will successfully
recover and don't need to check for `None` results anymore.
This had two unexpected consequences, one unfortunate, one "medium":
* All `salsa::Database` must be `'static`. This falls out from
`Q::DynDb` not having access to any lifetimes, but also the defaulting
rules for `dyn QueryGroup` that make it `dyn QueryGroup + 'static`. We
don't really support generic databases anyway yet so this isn't a big
deal, and we can add workarounds later (ideally via GATs).
* It is now statically impossible to invoke `snapshot` from a query,
and so we don't need to test that it panics. This is because the
signature of `snapshot` returns a `Snapshot<Self>` and that is not
accessible to a `dyn QueryGroup` type. Similarly, invoking
`Runtime::snapshot` directly is not possible becaues it is
crate-private. So I removed the test. This seems ok, but eventually I
would like to expose ways for queries to do parallel
execution (matklad and I had talked about a "speculation" primitive
for enabling that).
* This commit is 99% boilerplate I did with search-and-replace. I also
rolled in a few other changes I might have preferred to factor out,
most notably removing the `GetQueryTable` plumbing trait in favor of
free-methods, but it was awkward to factor them out and get all the
generics right (so much simpler in this version).
Quickest POC I could create to get some potentially cyclic queries to
not panic and instead return a result I could act on. (gluon's module
importing need to error on cycles).
```
// Causes `db.query()` to actually return `Result<V, CycleError>`
fn query(&self, key: K, key2: K2) -> V;
```
A proper implementation of this would likely return
`Result<V, CycleError<(K, K2)>>` or maybe larger changes are needed.
cc #6
The unsafe impl now asserts that the `DatabaseSlot` implementor type
is indeed `Send+Sync` if `DB::DatabaseData` is `Send+Sync`. Since our
query keys/values are a part of database-data, this means that `Slot`
must be `Send+Sync` if the key/value are `Send+Sync`. We test this
with a function that will cause compliation to fail if we accidentally
introduce an `Rc<T>` etc.
Transparent queries are not really queries: they are just plain
uncached functions without any backing storage.
Making a query transparent can be useful to figure out if caching it
at all is a win
Changes:
- Add interned keys to salsa (#150) -- see salsa-rs/salsa-rfcs#2
for more details.
- Fix bugs re: GC and volatile queries
- Optimization for GC (#144), though I think this later got undone
as part of the bug fixes =)
- GC API now more orthogonal + flexible (#138)
- Removed `set_unchecked` testing mechanism (#141)
- Generated enums now squelch `non_camel_case_types` lint (#135)
- Tests now using `set_foo` (#139)
- `Query::group_storage` now called `Query::query_storage` (#142)
Contributors to this release:
- @matklad
- @memoryruins
- @nikomatsakis
All the example code uses Default to create the db structs, but it turns
out the *GroupStorage's `#[derive(Default)]` adds a trait bound of
(e.g.) `DB__ + Default + HelloWorldDatabase` even though it never
actually needs to call HelloWorldDatabase::default(). So if you didn't
implement Default, then you couldn't be a salsa database struct.
