Skip to content

Commit

Permalink
s/maybe_changed_since/maybe_changed_after/
Browse files Browse the repository at this point in the history
  • Loading branch information
nikomatsakis committed Nov 13, 2021
1 parent cd265bf commit fc020de
Show file tree
Hide file tree
Showing 15 changed files with 57 additions and 51 deletions.
2 changes: 1 addition & 1 deletion book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
- [Database](./plumbing/database.md)
- [The `salsa` crate](./plumbing/salsa_crate.md)
- [Query operations](./plumbing/query_ops.md)
- [Maybe changed since](./plumbing/maybe_changed_since.md)
- [maybe changed after](./plumbing/maybe_changed_after.md)
- [Fetch](./plumbing/fetch.md)
- [Derived queries flowchart](./plumbing/derived_flowchart.md)
- [Cycle handling](./plumbing/cycles.md)
Expand Down
4 changes: 2 additions & 2 deletions book/src/plumbing/derived_flowchart.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Derived queries flowchart

Derived queries are by far the most complex. This flowchart documents the flow of the [maybe changed since] and [fetch] operations. This flowchart can be edited on [draw.io]:
Derived queries are by far the most complex. This flowchart documents the flow of the [maybe changed after] and [fetch] operations. This flowchart can be edited on [draw.io]:

[draw.io]: https://draw.io
[fetch]: ./fetch.md
[maybe changed since]: ./maybe_changed_since.md
[maybe changed after]: ./maybe_changed_after.md

<!-- The explicit div is there because, otherwise, the flowchart is unreadable when using "dark mode" -->
<div style="background-color:white;">
Expand Down
2 changes: 1 addition & 1 deletion book/src/plumbing/fetch.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ The logic for derived queries is more complex. We summarize the high-level ideas
* Otherwise, if the memo contains a memoized value, we must check whether [dependencies] have been modified:
* Let R be the revision in which the memo was last verified; we wish to know if any of the dependencies have changed since revision R.
* First, we check the [durability]. For each memo, we track the minimum durability of the memo's dependencies. If the memo has durability D, and there have been no changes to an input with durability D since the last time the memo was verified, then we can consider the memo verified without any further work.
* If the durability check is not sufficient, then we must check the dependencies individually. For this, we iterate over each dependency D and invoke the [maybe changed since](./maybe_changed_since.md) operation to check whether D has changed since the revision R.
* If the durability check is not sufficient, then we must check the dependencies individually. For this, we iterate over each dependency D and invoke the [maybe changed after](./maybe_changed_after.md) operation to check whether D has changed since the revision R.
* If no dependency was modified:
* We can mark the memo as verified and return its memoized value.
* Assuming dependencies have been modified or the memo does not contain a memoized value:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# Maybe changed since
# Maybe changed after

```rust,no_run,noplayground
{{#include ../../../src/plumbing.rs:maybe_changed_since}}
{{#include ../../../src/plumbing.rs:maybe_changed_after}}
```

The `maybe_changed_since` operation computes whether a query's value *may have changed* since the given revision.
The `maybe_changed_after` operation computes whether a query's value *may have changed* **after** the given revision. In other words, `Q.maybe_change_since(R)` is true if the value of the query `Q` may have changed in the revisions `(R+1)..R_now`, where `R_now` is the current revision. Note that it doesn't make sense to ask `maybe_changed_after(R_now)`.

## Input queries

Input queries are set explicitly by the user. `maybe_changed_since` can therefore just check when the value was last set and compare.
Input queries are set explicitly by the user. `maybe_changed_after` can therefore just check when the value was last set and compare.

## Interned queries

Expand All @@ -20,7 +20,7 @@ The logic for derived queries is more complex. We summarize the high-level ideas
* Otherwise, we must check whether [dependencies] have been modified:
* Let R be the revision in which the memo was last verified; we wish to know if any of the dependencies have changed since revision R.
* First, we check the [durability]. For each memo, we track the minimum durability of the memo's dependencies. If the memo has durability D, and there have been no changes to an input with durability D since the last time the memo was verified, then we can consider the memo verified without any further work.
* If the durability check is not sufficient, then we must check the dependencies individually. For this, we iterate over each dependency D and invoke the [maybe changed since](./maybe_changed_since.md) operation to check whether D has changed since the revision R.
* If the durability check is not sufficient, then we must check the dependencies individually. For this, we iterate over each dependency D and invoke the [maybe changed after](./maybe_changed_after.md) operation to check whether D has changed since the revision R.
* If no dependency was modified:
* We can mark the memo as verified and use its [changed at] revision to return true or false.
* Assuming dependencies have been modified:
Expand Down
2 changes: 1 addition & 1 deletion book/src/plumbing/query_ops.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Each of the query storage struct implements the `QueryStorageOps` trait found in

which defines the basic operations that all queries support. The most important are these two:

* [Maybe changed since](./maybe_changed_since.md): Returns true if the value of the query (for the given key) may have changed since the given revision.
* [maybe changed after](./maybe_changed_after.md): Returns true if the value of the query (for the given key) may have changed since the given revision.
* [Fetch](./fetch.md): Returms the up-to-date value for the given K (or an error in the case of an "unrecovered" cycle).

[`plumbing`]: https://github.com/salsa-rs/salsa/blob/master/src/plumbing.rs
4 changes: 2 additions & 2 deletions book/src/plumbing/terminology/verified.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Verified

A [memo] is *verified* in a revision R if we have checked that its value is still up-to-date (i.e., if we were to reexecute the [query function], we are guaranteed to get the same result). Each memo tracks the revision in which it was last verified to avoid repeatedly checking whether dependencies have changed during the [fetch] and [maybe changed since] operations.
A [memo] is *verified* in a revision R if we have checked that its value is still up-to-date (i.e., if we were to reexecute the [query function], we are guaranteed to get the same result). Each memo tracks the revision in which it was last verified to avoid repeatedly checking whether dependencies have changed during the [fetch] and [maybe changed after] operations.

[query function]: ./query_function.md
[fetch]: ../fetch.md
[maybe changed since]: ../maybe_changed_since.md
[maybe changed after]: ../maybe_changed_after.md
[memo]: ./memo.md
8 changes: 4 additions & 4 deletions book/src/rfcs/RFC0001-Query-Group-Traits.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ impl of `QueryGroup`. That involves generating the following things:
but since we cannot, it is entitled `MyGroupGroupKey`
- It is an enum which contains one variant per query with the value being the key:
- `my_query(<MyQueryQuery as salsa::plumbing::Query<DB>>::Key)`
- The group key enum offers a public, inherent method `maybe_changed_since`:
- `fn maybe_changed_since<DB>(db: &DB, db_descriptor: &DB::DatabaseKey, revision: Revision)`
- it is invoked when implementing `maybe_changed_since` for the database key
- The group key enum offers a public, inherent method `maybe_changed_after`:
- `fn maybe_changed_after<DB>(db: &DB, db_descriptor: &DB::DatabaseKey, revision: Revision)`
- it is invoked when implementing `maybe_changed_after` for the database key

### Lowering database storage

Expand Down Expand Up @@ -298,7 +298,7 @@ It generates the following things:
- This contains a `for_each_query` method, which is implemented by invoking, in turn,
the inherent methods defined on each query group storage struct.
- impl of `plumbing::DatabaseKey` for the database key enum
- This contains a method `maybe_changed_since`. We implement this by
- This contains a method `maybe_changed_after`. We implement this by
matching to get a particular group key, and then invoking the
inherent method on the group key struct.

Expand Down
2 changes: 1 addition & 1 deletion book/src/rfcs/RFC0006-Dynamic-Databases.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ some type for dependencies that is independent of the dtabase type `DB`.
There are a number of methods that can be dispatched through the database
interface on a `DatabaseKeyIndex`. For example, we already mentioned
`fmt_debug`, which emits a debug representation of the key, but there is also
`maybe_changed_since`, which checks whether the value for a given key may have
`maybe_changed_after`, which checks whether the value for a given key may have
changed since the given revision. Each of these methods is a member of the
`DatabaseOps` trait and they are dispatched as follows.

Expand Down
4 changes: 2 additions & 2 deletions components/salsa-macros/src/database_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub(crate) fn database(args: TokenStream, input: TokenStream) -> TokenStream {
#group_index => {
let storage: &#group_storage =
<Self as salsa::plumbing::HasQueryGroup<#group_path>>::group_storage(self);
storage.maybe_changed_since(self, input, revision)
storage.maybe_changed_after(self, input, revision)
}
});
cycle_recovery_strategy_ops.extend(quote! {
Expand Down Expand Up @@ -165,7 +165,7 @@ pub(crate) fn database(args: TokenStream, input: TokenStream) -> TokenStream {
}
}

fn maybe_changed_since(
fn maybe_changed_after(
&self,
input: salsa::DatabaseKeyIndex,
revision: salsa::Revision
Expand Down
4 changes: 2 additions & 2 deletions components/salsa-macros/src/query_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
for (Query { fn_name, .. }, query_index) in non_transparent_queries().zip(0_u16..) {
maybe_changed_ops.extend(quote! {
#query_index => {
salsa::plumbing::QueryStorageOps::maybe_changed_since(
salsa::plumbing::QueryStorageOps::maybe_changed_after(
&*self.#fn_name, db, input, revision
)
}
Expand Down Expand Up @@ -590,7 +590,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
}
}

#trait_vis fn maybe_changed_since(
#trait_vis fn maybe_changed_after(
&self,
db: &(#dyn_db + '_),
input: salsa::DatabaseKeyIndex,
Expand Down
5 changes: 3 additions & 2 deletions src/derived.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,22 +141,23 @@ where
write!(fmt, "{}({:?})", Q::QUERY_NAME, key)
}

fn maybe_changed_since(
fn maybe_changed_after(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
input: DatabaseKeyIndex,
revision: Revision,
) -> bool {
assert_eq!(input.group_index, self.group_index);
assert_eq!(input.query_index, Q::QUERY_INDEX);
debug_assert!(revision < db.salsa_runtime().current_revision());
let slot = self
.slot_map
.read()
.get_index(input.key_index as usize)
.unwrap()
.1
.clone();
slot.maybe_changed_since(db, revision)
slot.maybe_changed_after(db, revision)
}

fn fetch(&self, db: &<Q as QueryDb<'_>>::DynDb, key: &Q::Key) -> Q::Value {
Expand Down
24 changes: 12 additions & 12 deletions src/derived/slot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ enum ProbeState<V, G> {
UpToDate(V),
}

/// Return value of `maybe_changed_since_probe` helper.
/// Return value of `maybe_changed_after_probe` helper.
enum MaybeChangedSinceProbeState<G> {
/// Another thread was active but has completed.
/// Try again!
Expand Down Expand Up @@ -453,7 +453,7 @@ where
}
}

pub(super) fn maybe_changed_since(
pub(super) fn maybe_changed_after(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
revision: Revision,
Expand All @@ -464,7 +464,7 @@ where
db.unwind_if_cancelled();

debug!(
"maybe_changed_since({:?}) called with revision={:?}, revision_now={:?}",
"maybe_changed_after({:?}) called with revision={:?}, revision_now={:?}",
self, revision, revision_now,
);

Expand All @@ -474,18 +474,18 @@ where
// but hasn't been verified in this revision, we'll have to
// do more.
loop {
match self.maybe_changed_since_probe(db, self.state.read(), runtime, revision_now) {
match self.maybe_changed_after_probe(db, self.state.read(), runtime, revision_now) {
MaybeChangedSinceProbeState::Retry => continue,
MaybeChangedSinceProbeState::ChangedAt(changed_at) => return changed_at > revision,
MaybeChangedSinceProbeState::Stale(state) => {
drop(state);
return self.maybe_changed_since_upgrade(db, revision);
return self.maybe_changed_after_upgrade(db, revision);
}
}
}
}

fn maybe_changed_since_probe<StateGuard>(
fn maybe_changed_after_probe<StateGuard>(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
state: StateGuard,
Expand Down Expand Up @@ -514,7 +514,7 @@ where
}
}

fn maybe_changed_since_upgrade(
fn maybe_changed_after_upgrade(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
revision: Revision,
Expand All @@ -525,7 +525,7 @@ where
// Get an upgradable read lock, which permits other reads but no writers.
// Probe again. If the value is stale (needs to be verified), then upgrade
// to a write lock and swap it with InProgress while we work.
let mut old_memo = match self.maybe_changed_since_probe(
let mut old_memo = match self.maybe_changed_after_probe(
db,
self.state.upgradable_read(),
runtime,
Expand All @@ -536,7 +536,7 @@ where
// If another thread was active, then the cache line is going to be
// either verified or cleared out. Just recurse to figure out which.
// Note that we don't need an upgradable read.
MaybeChangedSinceProbeState::Retry => return self.maybe_changed_since(db, revision),
MaybeChangedSinceProbeState::Retry => return self.maybe_changed_after(db, revision),

MaybeChangedSinceProbeState::Stale(state) => {
type RwLockUpgradableReadGuard<'a, T> =
Expand Down Expand Up @@ -713,7 +713,7 @@ where
/// valid in the current revision. If so, returns a stamped value.
///
/// If needed, this will walk each dependency and
/// recursively invoke `maybe_changed_since`, which may in turn
/// recursively invoke `maybe_changed_after`, which may in turn
/// re-execute the dependency. This can cause cycles to occur,
/// so the current query must be pushed onto the
/// stack to permit cycle detection and recovery: therefore,
Expand Down Expand Up @@ -742,7 +742,7 @@ where
/// Determines whether the value represented by this memo is still
/// valid in the current revision; note that the value itself is
/// not needed for this check. If needed, this will walk each
/// dependency and recursively invoke `maybe_changed_since`, which
/// dependency and recursively invoke `maybe_changed_after`, which
/// may in turn re-execute the dependency. This can cause cycles to occur,
/// so the current query must be pushed onto the
/// stack to permit cycle detection and recovery: therefore,
Expand Down Expand Up @@ -786,7 +786,7 @@ where
QueryInputs::Tracked { inputs } => {
let changed_input = inputs
.iter()
.find(|&&input| db.maybe_changed_since(input, verified_at));
.find(|&&input| db.maybe_changed_after(input, verified_at));
if let Some(input) = changed_input {
debug!("validate_memoized_value: `{:?}` may have changed", input);

Expand Down
11 changes: 6 additions & 5 deletions src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,22 +78,23 @@ where
write!(fmt, "{}({:?})", Q::QUERY_NAME, key)
}

fn maybe_changed_since(
fn maybe_changed_after(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
input: DatabaseKeyIndex,
revision: Revision,
) -> bool {
assert_eq!(input.group_index, self.group_index);
assert_eq!(input.query_index, Q::QUERY_INDEX);
debug_assert!(revision < db.salsa_runtime().current_revision());
let slot = self
.slots
.read()
.get_index(input.key_index as usize)
.unwrap()
.1
.clone();
slot.maybe_changed_since(db, revision)
slot.maybe_changed_after(db, revision)
}

fn fetch(&self, db: &<Q as QueryDb<'_>>::DynDb, key: &Q::Key) -> Q::Value {
Expand Down Expand Up @@ -147,15 +148,15 @@ impl<Q> Slot<Q>
where
Q: Query,
{
fn maybe_changed_since(&self, _db: &<Q as QueryDb<'_>>::DynDb, revision: Revision) -> bool {
fn maybe_changed_after(&self, _db: &<Q as QueryDb<'_>>::DynDb, revision: Revision) -> bool {
debug!(
"maybe_changed_since(slot={:?}, revision={:?})",
"maybe_changed_after(slot={:?}, revision={:?})",
self, revision,
);

let changed_at = self.stamped_value.read().changed_at;

debug!("maybe_changed_since: changed_at = {:?}", changed_at);
debug!("maybe_changed_after: changed_at = {:?}", changed_at);

changed_at > revision
}
Expand Down
13 changes: 7 additions & 6 deletions src/interned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,17 +214,18 @@ where
write!(fmt, "{}({:?})", Q::QUERY_NAME, slot.value)
}

fn maybe_changed_since(
fn maybe_changed_after(
&self,
_db: &<Q as QueryDb<'_>>::DynDb,
db: &<Q as QueryDb<'_>>::DynDb,
input: DatabaseKeyIndex,
revision: Revision,
) -> bool {
assert_eq!(input.group_index, self.group_index);
assert_eq!(input.query_index, Q::QUERY_INDEX);
debug_assert!(revision < db.salsa_runtime().current_revision());
let intern_id = InternId::from(input.key_index);
let slot = self.lookup_value(intern_id);
slot.maybe_changed_since(revision)
slot.maybe_changed_after(revision)
}

fn fetch(&self, db: &<Q as QueryDb<'_>>::DynDb, key: &Q::Key) -> Q::Value {
Expand Down Expand Up @@ -331,7 +332,7 @@ where
interned_storage.fmt_index(Q::convert_db(db), index, fmt)
}

fn maybe_changed_since(
fn maybe_changed_after(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
input: DatabaseKeyIndex,
Expand All @@ -340,7 +341,7 @@ where
let group_storage =
<<Q as QueryDb<'_>>::DynDb as HasQueryGroup<Q::Group>>::group_storage(db);
let interned_storage = IQ::query_storage(Q::convert_group_storage(group_storage));
interned_storage.maybe_changed_since(Q::convert_db(db), input, revision)
interned_storage.maybe_changed_after(Q::convert_db(db), input, revision)
}

fn fetch(&self, db: &<Q as QueryDb<'_>>::DynDb, key: &Q::Key) -> Q::Value {
Expand Down Expand Up @@ -393,7 +394,7 @@ where
}

impl<K> Slot<K> {
fn maybe_changed_since(&self, revision: Revision) -> bool {
fn maybe_changed_after(&self, revision: Revision) -> bool {
self.interned_at > revision
}
}
Expand Down
Loading

0 comments on commit fc020de

Please sign in to comment.