From d01d6ed511fc46b6bd8d7bc2de5e5896da008ed0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 19 Dec 2018 02:40:02 +0300 Subject: [PATCH] Make GC API more orthogonal and flexible Now, the effect of GC is a "product" of three parameters: * what values are affected (everything/everything except used) * are we removing values * are we removing deps SweepStrategy::default is now a no-op GC. --- src/derived.rs | 5 ++++- src/lib.rs | 30 +++++++++++++++++++++++++++++- tests/gc/derived_tests.rs | 10 +++++----- tests/gc/discard_values.rs | 10 +++++++--- tests/gc/shallow_constant_tests.rs | 8 ++++---- tests/parallel/stress.rs | 2 +- 6 files changed, 50 insertions(+), 15 deletions(-) diff --git a/src/derived.rs b/src/derived.rs index dcebf4f2..9e0e8c4e 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -971,12 +971,15 @@ 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; + } if !strategy.keep_values { memo.value = None; } - memo.verified_at == revision_now + strategy.keep_deps } } }); diff --git a/src/lib.rs b/src/lib.rs index 270a689b..78a2cfbf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -211,9 +211,16 @@ impl fmt::Debug for EventKind { #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct SweepStrategy { keep_values: bool, + keep_deps: bool, + keep_current_revision: bool, } impl SweepStrategy { + /// Fully discards keys not used in the current revision + pub fn discard_old() -> SweepStrategy { + SweepStrategy::default().discard_deps() + } + /// 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 @@ -224,11 +231,32 @@ impl SweepStrategy { ..self } } + /// Causes us to discard both memoized *values* and *dependencies*. + pub fn discard_deps(self) -> SweepStrategy { + SweepStrategy { + keep_deps: false, + ..self + } + } + + /// Causes us to collect keys from all revisions. + /// + /// By default, only keys not used in the current revision are collected. + pub fn discard_all_revisions(self) -> SweepStrategy { + SweepStrategy { + keep_current_revision: false, + ..self + } + } } impl Default for SweepStrategy { fn default() -> Self { - SweepStrategy { keep_values: true } + SweepStrategy { + keep_values: true, + keep_deps: true, + keep_current_revision: true, + } } } diff --git a/tests/gc/derived_tests.rs b/tests/gc/derived_tests.rs index 87f19162..e68a76e2 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::default()); + db.sweep_all(SweepStrategy::discard_old()); assert_keys! { db, @@ -64,7 +64,7 @@ fn compute_switch() { MaxQuery => (), } - db.sweep_all(SweepStrategy::default()); + db.sweep_all(SweepStrategy::discard_old()); // 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_old()); // 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_old()); assert_keys! { db, @@ -137,7 +137,7 @@ fn compute_all() { MaxQuery => (()), } - db.sweep_all(SweepStrategy::default()); + db.sweep_all(SweepStrategy::discard_old()); // 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 0c10c1da..659dc939 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::default()); + db.sweep_all(SweepStrategy::discard_old()); 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() + .discard_all_revisions(), + ); + db.sweep_all(SweepStrategy::discard_old()); assert_keys! { db, FibonacciQuery => (3, 5), diff --git a/tests/gc/shallow_constant_tests.rs b/tests/gc/shallow_constant_tests.rs index 423ce010..25c0abc4 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::default()); + db.sweep_all(SweepStrategy::discard_old()); 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_old()); 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_old()); 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_old()); assert_keys! { db, diff --git a/tests/parallel/stress.rs b/tests/parallel/stress.rs index 152cfbfe..e9ce67fd 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::default(); + let mut strategy = SweepStrategy::discard_old(); if rng.gen_bool(0.5) { strategy = strategy.discard_values(); }