From 9387fd2f4d31210daa8805f0f0b8e4d22e73c3ea Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 27 Jan 2019 17:14:57 +0300 Subject: [PATCH] more orthogonal naming --- src/derived.rs | 24 ++++---- src/lib.rs | 94 ++++++++++++++++++++---------- tests/gc/derived_tests.rs | 10 ++-- tests/gc/discard_values.rs | 6 +- tests/gc/shallow_constant_tests.rs | 8 +-- tests/parallel/stress.rs | 2 +- 6 files changed, 90 insertions(+), 54 deletions(-) diff --git a/src/derived.rs b/src/derived.rs index 9e0e8c4e..a5f2ada4 100644 --- a/src/derived.rs +++ b/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 } } }); diff --git a/src/lib.rs b/src/lib.rs index 78a2cfbf..60a81e82 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -205,57 +205,89 @@ impl fmt::Debug for EventKind { } } +#[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 } } } diff --git a/tests/gc/derived_tests.rs b/tests/gc/derived_tests.rs index e68a76e2..ddaa7ee9 100644 --- a/tests/gc/derived_tests.rs +++ b/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. diff --git a/tests/gc/discard_values.rs b/tests/gc/discard_values.rs index 659dc939..2db2b60f 100644 --- a/tests/gc/discard_values.rs +++ b/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), diff --git a/tests/gc/shallow_constant_tests.rs b/tests/gc/shallow_constant_tests.rs index 25c0abc4..a5eef000 100644 --- a/tests/gc/shallow_constant_tests.rs +++ b/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, diff --git a/tests/parallel/stress.rs b/tests/parallel/stress.rs index e9ce67fd..8b702b8b 100644 --- a/tests/parallel/stress.rs +++ b/tests/parallel/stress.rs @@ -114,7 +114,7 @@ impl rand::distributions::Distribution for rand::distributions::Standard let key = rng.gen::() % 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(); }