more orthogonal naming

src/derived.rs

@@ -11,7 +11,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;
@@ -953,7 +953,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 {:?}",
@@ -971,15 +972,18 @@ where
                     // revision, since we are holding the write lock
                     // when we read `revision_now`.
                     assert!(memo.verified_at <= revision_now);
-                    if strategy.keep_current_revision && memo.verified_at == revision_now {
-                        return true;
+                    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,
+                        },
                     }
-
-                    if !strategy.keep_values {
-                        memo.value = None;
-                    }
-
-                    strategy.keep_deps
                 }
             }
         });

src/lib.rs

@@ -205,57 +205,89 @@ 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,
-    keep_deps: bool,
-    keep_current_revision: bool,
+    discard_if: DiscardIf,
+    discard_what: DiscardWhat,
 }
 
 impl SweepStrategy {
-    /// Fully discards keys not used in the current revision
-    pub fn discard_old() -> SweepStrategy {
-        SweepStrategy::default().discard_deps()
+    /// 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()
     }
 
-    /// 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.
+    /// 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,
-            ..self
-        }
-    }
-    /// Causes us to discard both memoized *values* and *dependencies*.
-    pub fn discard_deps(self) -> SweepStrategy {
-        SweepStrategy {
-            keep_deps: false,
+            discard_what: self.discard_what.max(DiscardWhat::Values),
             ..self
         }
     }
 
-    /// Causes us to collect keys from all revisions.
+    /// Collects both values and information about dependencies.
     ///
-    /// By default, only keys not used in the current revision are collected.
-    pub fn discard_all_revisions(self) -> SweepStrategy {
+    /// Dependant queries will be recomputed even if all inputs to this query
+    /// stay the same.
+    pub fn discard_everything(self) -> SweepStrategy {
         SweepStrategy {
-            keep_current_revision: false,
+            discard_what: self.discard_what.max(DiscardWhat::Everything),
             ..self
         }
     }
-}
 
-impl Default for SweepStrategy {
-    fn default() -> Self {
+    /// Process all keys, not verefied at the current revision.
+    pub fn sweep_outdated(self) -> SweepStrategy {
         SweepStrategy {
-            keep_values: true,
-            keep_deps: true,
-            keep_current_revision: true,
+            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::discard_old());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     assert_keys! {
         db,
@@ -64,7 +64,7 @@ fn compute_switch() {
         MaxQuery => (),
     }
 
-    db.sweep_all(SweepStrategy::discard_old());
+    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::discard_old());
+    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::discard_old());
+    db.sweep_all(SweepStrategy::discard_outdated());
 
     assert_keys! {
         db,
@@ -137,7 +137,7 @@ fn compute_all() {
         MaxQuery => (()),
     }
 
-    db.sweep_all(SweepStrategy::discard_old());
+    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::discard_old());
+    db.sweep_all(SweepStrategy::discard_outdated());
     assert_keys! {
         db,
         FibonacciQuery => (3, 5),
@@ -34,9 +34,9 @@ fn sweep_default() {
     db.sweep_all(
         SweepStrategy::default()
             .discard_values()
-            .discard_all_revisions(),
+            .sweep_all_revisions(),
     );
-    db.sweep_all(SweepStrategy::discard_old());
+    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::discard_old());
+    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::discard_old());
+    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::discard_old());
+    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::discard_old());
+    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::discard_old();
+        let mut strategy = SweepStrategy::discard_outdated();
         if rng.gen_bool(0.5) {
             strategy = strategy.discard_values();
         }