start threading through inputs and revision information

src/lib.rs

@@ -46,7 +46,11 @@ pub trait QueryContextStorageTypes: Sized {
     type QueryContextStorage: Default;
 }
 
-pub trait QueryDescriptor<QC>: Debug + Eq + Hash {}
+pub trait QueryDescriptor<QC>: Debug + Eq + Hash + Send + Sync {
+    /// Returns true if the value of this query may have changed since
+    /// the given revision.
+    fn maybe_changed_since(&self, revision: runtime::Revision) -> bool;
+}
 
 pub trait Query<QC: QueryContext>: Debug + Default + Sized + 'static {
     type Key: Clone + Debug + Hash + Eq + Send;

src/memoized.rs

@@ -1,6 +1,9 @@
+use crate::runtime::QueryDescriptorSet;
+use crate::runtime::Revision;
 use crate::CycleDetected;
 use crate::Query;
 use crate::QueryContext;
+use crate::QueryDescriptor;
 use crate::QueryStorageOps;
 use crate::QueryTable;
 use parking_lot::{RwLock, RwLockUpgradableReadGuard};
@@ -22,23 +25,43 @@ where
     Q: Query<QC>,
     QC: QueryContext,
 {
-    map: RwLock<FxHashMap<Q::Key, QueryState<Q::Value>>>,
+    map: RwLock<FxHashMap<Q::Key, QueryState<QC, Q>>>,
 }
 
 /// Defines the "current state" of query's memoized results.
 #[derive(Debug)]
-enum QueryState<V> {
+enum QueryState<QC, Q>
+where
+    Q: Query<QC>,
+    QC: QueryContext,
+{
     /// We are currently computing the result of this query; if we see
     /// this value in the table, it indeeds a cycle.
     InProgress,
 
     /// We have computed the query already, and here is the result.
-    Memoized(Memoized<V>),
+    Memoized(Memoized<QC, Q>),
 }
 
 #[derive(Debug)]
-struct Memoized<V> {
-    value: V,
+struct Memoized<QC, Q>
+where
+    Q: Query<QC>,
+    QC: QueryContext,
+{
+    value: Q::Value,
+
+    inputs: QueryDescriptorSet<QC>,
+
+    /// Last time that we checked our inputs to see if they have
+    /// changed. If this is equal to the current revision, then the
+    /// value is up to date. If not, we need to check our inputs and
+    /// see if any of them have changed since our last check -- if so,
+    /// we'll need to re-execute.
+    verified_at: Revision,
+
+    /// Last time that our value changed.
+    changed_at: Revision,
 }
 
 impl<QC, Q> Default for MemoizedStorage<QC, Q>
@@ -64,32 +87,91 @@ where
         key: &Q::Key,
         descriptor: impl FnOnce() -> QC::QueryDescriptor,
     ) -> Result<Q::Value, CycleDetected> {
-        {
+        let revision_now = query.salsa_runtime().current_revision();
+
+        let mut old_value = {
             let map_read = self.map.upgradable_read();
             if let Some(value) = map_read.get(key) {
-                return match value {
-                    QueryState::InProgress => Err(CycleDetected),
-                    QueryState::Memoized(m) => Ok(m.value.clone()),
-                };
+                match value {
+                    QueryState::InProgress => return Err(CycleDetected),
+                    QueryState::Memoized(m) => {
+                        if m.verified_at == revision_now {
+                            return Ok(m.value.clone());
+                        }
+                    }
+                }
             }
 
             let mut map_write = RwLockUpgradableReadGuard::upgrade(map_read);
-            map_write.insert(key.clone(), QueryState::InProgress);
+            map_write.insert(key.clone(), QueryState::InProgress)
+        };
+
+        // If we have an old-value, it *may* now be stale, since there
+        // has been a new revision since the last time we checked. So,
+        // first things first, let's walk over each of our previous
+        // inputs and check whether they are out of date.
+        if let Some(QueryState::Memoized(old_memo)) = &mut old_value {
+            if old_memo
+                .inputs
+                .iter()
+                .all(|old_input| !old_input.maybe_changed_since(old_memo.verified_at))
+            {
+                // If none of out inputs have changed since the last time we refreshed
+                // our value, then our value must still be good. We'll just patch
+                // the verified-at date and re-use it.
+                old_memo.verified_at = revision_now;
+                let value = old_memo.value.clone();
+
+                let mut map_write = self.map.write();
+                let placeholder = map_write.insert(key.clone(), old_value.unwrap());
+                assert!(
+                    match placeholder {
+                        Some(QueryState::InProgress) => true,
+                        _ => false,
+                    },
+                    "expected in-progress state",
+                );
+                return Ok(value);
+            }
         }
 
-        // If we get here, the query is in progress, and we are the
-        // ones tasked with finding its final value.
+        // Query was not previously executed or value is potentially
+        // stale. Let's execute!
         let descriptor = descriptor();
-        let value = query
+        let (value, inputs) = query
             .salsa_runtime()
             .execute_query_implementation::<Q>(query, descriptor, key);
 
+        // We assume that query is side-effect free -- that is, does
+        // not mutate the "inputs" to the query system. Sanity check
+        // that assumption here, at least to the best of our ability.
+        assert_eq!(
+            query.salsa_runtime().current_revision(),
+            revision_now,
+            "revision altered during query execution",
+        );
+
+        // If the new value is equal to the old one, then it didn't
+        // really change, even if some of its inputs have. So we can
+        // "backdate" our `changed_at` revision to be the same as the
+        // old value.
+        let mut changed_at = revision_now;
+        if let Some(QueryState::Memoized(old_memo)) = &old_value {
+            if old_memo.value == value {
+                changed_at = old_memo.changed_at;
+            }
+        }
+
         {
             let mut map_write = self.map.write();
+
             let old_value = map_write.insert(
                 key.clone(),
                 QueryState::Memoized(Memoized {
                     value: value.clone(),
+                    inputs,
+                    verified_at: revision_now,
+                    changed_at,
                 }),
             );
             assert!(
@@ -97,8 +179,7 @@ where
                     Some(QueryState::InProgress) => true,
                     _ => false,
                 },
-                "expected in-progress state, not {:?}",
-                old_value
+                "expected in-progress state",
             );
         }
 

src/runtime.rs

@@ -69,17 +69,31 @@ where
         query: &QC,
         descriptor: QC::QueryDescriptor,
         key: &Q::Key,
-    ) -> Q::Value
+    ) -> (Q::Value, QueryDescriptorSet<QC>)
     where
         Q: Query<QC>,
     {
-        self.local_state
-            .borrow_mut()
-            .query_stack
-            .push(ActiveQuery::new(descriptor));
+        // Push the active query onto the stack.
+        let push_len = {
+            let mut local_state = self.local_state.borrow_mut();
+            local_state.query_stack.push(ActiveQuery::new(descriptor));
+            local_state.query_stack.len()
+        };
+
+        // Execute user's code, accumulating inputs etc.
         let value = Q::execute(query, key.clone());
-        self.local_state.borrow_mut().query_stack.pop();
-        value
+
+        // Extract accumulated inputs.
+        let ActiveQuery { subqueries, .. } = {
+            let mut local_state = self.local_state.borrow_mut();
+
+            // Sanity check: pushes and pops should be balanced.
+            assert_eq!(local_state.query_stack.len(), push_len);
+
+            local_state.query_stack.pop().unwrap()
+        };
+
+        (value, subqueries)
     }
 
     /// Reports that the currently active query read the result from
@@ -134,28 +148,20 @@ struct ActiveQuery<QC: QueryContext> {
     /// What query is executing
     descriptor: QC::QueryDescriptor,
 
-    /// Each time we execute a subquery, it returns to us the revision
-    /// in which its value last changed. We track the maximum of these
-    /// to find the maximum revision in which *we* changed.
-    max_revision_read: Revision,
-
     /// Each subquery
-    subqueries: FxIndexSet<QC::QueryDescriptor>,
+    subqueries: QueryDescriptorSet<QC>,
 }
 
 impl<QC: QueryContext> ActiveQuery<QC> {
     fn new(descriptor: QC::QueryDescriptor) -> Self {
         ActiveQuery {
             descriptor,
-            max_revision_read: Revision::zero(),
-            subqueries: FxIndexSet::default(),
+            subqueries: QueryDescriptorSet::new(),
         }
     }
 
-    fn add_read(&mut self, subquery: QC::QueryDescriptor, changed_revision: Revision) {
-        if self.subqueries.insert(subquery) {
-            self.max_revision_read = self.max_revision_read.max(changed_revision);
-        }
+    fn add_read(&mut self, subquery: QC::QueryDescriptor, _changed_revision: Revision) {
+        self.subqueries.insert(subquery);
     }
 }
 
@@ -169,3 +175,35 @@ impl Revision {
         Revision { generation: 0 }
     }
 }
+
+/// An insertion-order-preserving set of queries. Used to track the
+/// inputs accessed during query execution.
+crate struct QueryDescriptorSet<QC: QueryContext> {
+    set: FxIndexSet<QC::QueryDescriptor>,
+}
+
+impl<QC: QueryContext> std::fmt::Debug for QueryDescriptorSet<QC> {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Debug::fmt(&self.set, fmt)
+    }
+}
+
+impl<QC: QueryContext> QueryDescriptorSet<QC> {
+    fn new() -> Self {
+        QueryDescriptorSet {
+            set: FxIndexSet::default(),
+        }
+    }
+
+    /// Add `descriptor` to the set. Returns true if `descriptor` is
+    /// newly added and false if `descriptor` was already a member.
+    fn insert(&mut self, descriptor: QC::QueryDescriptor) -> bool {
+        self.set.insert(descriptor)
+    }
+
+    /// Iterate over all queries in the set, in the order of their
+    /// first insertion.
+    pub fn iter(&self) -> impl Iterator<Item = &QC::QueryDescriptor> {
+        self.set.iter()
+    }
+}

src/transparent.rs

@@ -35,8 +35,9 @@ where
         // here? Or should we just call `Q::execute`, and maybe
         // separate out the `push`/`pop` operations.
         let descriptor = descriptor();
-        Ok(query
+        let (value, _inputs) = query
             .salsa_runtime()
-            .execute_query_implementation::<Q>(query, descriptor, key))
+            .execute_query_implementation::<Q>(query, descriptor, key);
+        Ok(value)
     }
 }