Merge pull request #138 from matklad/flexible-gc

Make GC API more orthogonal and flexible

src/derived.rs

@@ -10,7 +10,7 @@ use crate::runtime::Revision;
 use crate::runtime::Runtime;
 use crate::runtime::RuntimeId;
 use crate::runtime::StampedValue;
-use crate::{Database, Event, EventKind, SweepStrategy};
+use crate::{Database, DiscardIf, DiscardWhat, Event, EventKind, SweepStrategy};
 use log::{debug, info};
 use parking_lot::Mutex;
 use parking_lot::RwLock;
@@ -952,7 +952,8 @@ where
                     true
                 }
 
-                // Otherwise, keep only if it was used in this revision.
+                // Otherwise, drop only value or the whole memo accoring to the
+                // strategy.
                 QueryState::Memoized(memo) => {
                     debug!(
                         "sweep({:?}({:?})): last verified at {:?}, current revision {:?}",
@@ -970,12 +971,18 @@ where
                     // revision, since we are holding the write lock
                     // when we read `revision_now`.
                     assert!(memo.verified_at <= revision_now);
-
-                    if !strategy.keep_values {
+                    match strategy.discard_if {
+                        DiscardIf::Never => true,
+                        DiscardIf::Outdated if memo.verified_at == revision_now => true,
+                        DiscardIf::Outdated | DiscardIf::Always => match strategy.discard_what {
+                            DiscardWhat::Nothing => true,
+                            DiscardWhat::Values => {
                                 memo.value = None;
+                                true
+                            }
+                            DiscardWhat::Everything => false,
+                        },
                     }
-
-                    memo.verified_at == revision_now
                 }
             }
         });

src/lib.rs

@@ -204,30 +204,90 @@ impl<DB: Database> fmt::Debug for EventKind<DB> {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+enum DiscardIf {
+    Never,
+    Outdated,
+    Always,
+}
+
+impl Default for DiscardIf {
+    fn default() -> DiscardIf {
+        DiscardIf::Never
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+enum DiscardWhat {
+    Nothing,
+    Values,
+    Everything,
+}
+
+impl Default for DiscardWhat {
+    fn default() -> DiscardWhat {
+        DiscardWhat::Nothing
+    }
+}
+
 /// The sweep strategy controls what data we will keep/discard when we
-/// do a GC-sweep. The default (`SweepStrategy::default`) is to keep
-/// all memoized values used in the current revision.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+/// do a GC-sweep. The default (`SweepStrategy::default`) is a no-op,
+/// use `SweepStrategy::discard_outdated` constructor or `discard_*`
+/// and `sweep_*` builder functions to construct useful strategies.
+#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
 pub struct SweepStrategy {
-    keep_values: bool,
+    discard_if: DiscardIf,
+    discard_what: DiscardWhat,
 }
 
 impl SweepStrategy {
-    /// Causes us to discard memoized *values* but keep the
-    /// *dependencies*. This means you will have to recompute the
-    /// results from any queries you execute but does permit you to
-    /// quickly determine if a value is still up to date.
+    /// Convenience function that discards all data not used thus far in the
+    /// current revision.
+    ///
+    /// Equivalent to `SweepStrategy::default().discard_everything()`.
+    pub fn discard_outdated() -> SweepStrategy {
+        SweepStrategy::default()
+            .discard_everything()
+            .sweep_outdated()
+    }
+
+    /// Collects query values.
+    ///
+    /// Query dependencies are left in the database, which allows to quickly
+    /// determine if the query is up to date, and avoid recomputing
+    /// dependencies.
     pub fn discard_values(self) -> SweepStrategy {
         SweepStrategy {
-            keep_values: false,
+            discard_what: self.discard_what.max(DiscardWhat::Values),
             ..self
         }
     }
+
+    /// Collects both values and information about dependencies.
+    ///
+    /// Dependant queries will be recomputed even if all inputs to this query
+    /// stay the same.
+    pub fn discard_everything(self) -> SweepStrategy {
+        SweepStrategy {
+            discard_what: self.discard_what.max(DiscardWhat::Everything),
+            ..self
+        }
     }
 
-impl Default for SweepStrategy {
-    fn default() -> Self {
-        SweepStrategy { keep_values: true }
+    /// Process all keys, not verefied at the current revision.
+    pub fn sweep_outdated(self) -> SweepStrategy {
+        SweepStrategy {
+            discard_if: self.discard_if.max(DiscardIf::Outdated),
+            ..self
+        }
+    }
+
+    /// Process all keys.
+    pub fn sweep_all_revisions(self) -> SweepStrategy {
+        SweepStrategy {
+            discard_if: self.discard_if.max(DiscardIf::Always),
+            ..self
+        }
     }
 }
 

tests/gc/derived_tests.rs

@@ -25,7 +25,7 @@ fn compute_one() {
 
     // Memoized, but will compute fibonacci(5) again
     db.compute(5);
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     assert_keys! {
         db,
@@ -64,7 +64,7 @@ fn compute_switch() {
         MaxQuery => (),
     }
 
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     // Now we just have `Triangular` and not `Fibonacci`
     assert_keys! {
@@ -80,7 +80,7 @@ fn compute_switch() {
     // Now run `compute` *again* in next revision.
     db.salsa_runtime().next_revision();
     assert_eq!(db.compute(5), 15);
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     // We keep triangular, but just the outermost one.
     assert_keys! {
@@ -109,7 +109,7 @@ fn compute_all() {
     db.compute_all();
     db.salsa_runtime().next_revision();
     db.compute_all();
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     assert_keys! {
         db,
@@ -137,7 +137,7 @@ fn compute_all() {
         MaxQuery => (()),
     }
 
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     // We no longer used `Compute(5)` and `Triangular(5)`; note that
     // `UseTriangularQuery(5)` is not collected, as it is an input.

tests/gc/discard_values.rs

@@ -19,7 +19,7 @@ fn sweep_default() {
 
     // fibonacci is a constant, so it will not be invalidated,
     // hence we keep 3 and 5 but remove the rest.
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
     assert_keys! {
         db,
         FibonacciQuery => (3, 5),
@@ -31,8 +31,12 @@ fn sweep_default() {
     db.assert_log(&[]);
 
     // Same but we discard values this time.
-    db.sweep_all(SweepStrategy::default().discard_values());
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(
+        SweepStrategy::default()
+            .discard_values()
+            .sweep_all_revisions(),
+    );
+    db.sweep_all(SweepStrategy::discard_outdated());
     assert_keys! {
         db,
         FibonacciQuery => (3, 5),

tests/gc/shallow_constant_tests.rs

@@ -18,7 +18,7 @@ fn one_rev() {
 
     // Everything was used in this revision, so
     // nothing gets collected.
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
     assert_eq!(k.len(), 6);
 }
 
@@ -35,7 +35,7 @@ fn two_rev_nothing() {
 
     // Nothing was used in this revision, so
     // everything gets collected.
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     let k: Vec<_> = db.query(FibonacciQuery).entries();
     assert_eq!(k.len(), 0);
@@ -56,7 +56,7 @@ fn two_rev_one_use() {
 
     // fibonacci is a constant, so it will not be invalidated,
     // hence we keep `fibonacci(5)` but remove 0..=4.
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     assert_keys! {
         db,
@@ -80,7 +80,7 @@ fn two_rev_two_uses() {
 
     // fibonacci is a constant, so it will not be invalidated,
     // hence we keep 3 and 5 but remove the rest.
-    db.sweep_all(SweepStrategy::default());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     assert_keys! {
         db,

tests/parallel/stress.rs

@@ -114,7 +114,7 @@ impl rand::distributions::Distribution<ReadOp> for rand::distributions::Standard
             let key = rng.gen::<usize>() % 10;
             return ReadOp::Get(query, key);
         }
-        let mut strategy = SweepStrategy::default();
+        let mut strategy = SweepStrategy::discard_outdated();
         if rng.gen_bool(0.5) {
             strategy = strategy.discard_values();
         }