From a3bbba618778a888dfbe6dadb08f1ca3dc318328 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 22 Jan 2019 23:33:22 +0300 Subject: [PATCH 1/2] allow to peek at values via debug query interface --- src/debug.rs | 35 +++++++++++++++++++++++++++++++++++ src/derived.rs | 18 ++++++++++++++++++ src/input.rs | 13 +++++++++++++ src/plumbing.rs | 6 ++++++ 4 files changed, 72 insertions(+) diff --git a/src/debug.rs b/src/debug.rs index 00cb2f24..601d3bab 100644 --- a/src/debug.rs +++ b/src/debug.rs @@ -14,6 +14,9 @@ pub trait DebugQueryTable { /// Key of this query. type Key; + /// Value of this query. + type Value; + /// True if salsa thinks that the value for `key` is a /// **constant**, meaning that it can never change, no matter what /// values the inputs take on from this point. @@ -23,6 +26,30 @@ pub trait DebugQueryTable { fn keys(&self) -> C where C: FromIterator; + + /// Get the (current) set of the entries in the query table. + fn entries(&self) -> C + where + C: FromIterator>; +} + +/// An entry from a query table, for debugging and inspecting the table state. +pub struct TableEntry { + /// key of the query + pub key: K, + /// value of the query, if it is stored + pub value: Option, + _for_future_use: (), +} + +impl TableEntry { + pub(crate) fn new(key: K, value: Option) -> TableEntry { + TableEntry { + key, + value, + _for_future_use: (), + } + } } impl DebugQueryTable for QueryTable<'_, DB, Q> @@ -31,6 +58,7 @@ where Q: Query, { type Key = Q::Key; + type Value = Q::Value; fn is_constant(&self, key: Q::Key) -> bool { self.storage.is_constant(self.db, &key) @@ -42,4 +70,11 @@ where { self.storage.keys(self.db) } + + fn entries(&self) -> C + where + C: FromIterator>, + { + self.storage.entries(self.db) + } } diff --git a/src/derived.rs b/src/derived.rs index 2754b37c..98116397 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -1,3 +1,4 @@ +use crate::debug::TableEntry; use crate::plumbing::CycleDetected; use crate::plumbing::QueryDescriptor; use crate::plumbing::QueryFunction; @@ -162,6 +163,13 @@ where waiting: Default::default(), } } + + fn value(&self) -> Option { + match self { + QueryState::InProgress { .. } => None, + QueryState::Memoized(memo) => memo.value.clone(), + } + } } struct Memo @@ -901,6 +909,16 @@ where let map = self.map.read(); map.keys().cloned().collect() } + + fn entries(&self, _db: &DB) -> C + where + C: std::iter::FromIterator>, + { + let map = self.map.read(); + map.iter() + .map(|(key, query_state)| TableEntry::new(key.clone(), query_state.value())) + .collect() + } } impl QueryStorageMassOps for DerivedStorage diff --git a/src/input.rs b/src/input.rs index 2ac519c1..f91dd3db 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,3 +1,4 @@ +use crate::debug::TableEntry; use crate::plumbing::CycleDetected; use crate::plumbing::InputQueryStorageOps; use crate::plumbing::QueryStorageMassOps; @@ -200,6 +201,18 @@ where let map = self.map.read(); map.keys().cloned().collect() } + + fn entries(&self, _db: &DB) -> C + where + C: std::iter::FromIterator>, + { + let map = self.map.read(); + map.iter() + .map(|(key, stamped_value)| { + TableEntry::new(key.clone(), Some(stamped_value.value.clone())) + }) + .collect() + } } impl QueryStorageMassOps for InputStorage diff --git a/src/plumbing.rs b/src/plumbing.rs index 61768bed..d4184806 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -1,5 +1,6 @@ #![allow(missing_docs)] +use crate::debug::TableEntry; use crate::Database; use crate::Query; use crate::QueryTable; @@ -133,6 +134,11 @@ where fn keys(&self, db: &DB) -> C where C: std::iter::FromIterator; + + /// Get the (current) set of the entries in the query storage + fn entries(&self, db: &DB) -> C + where + C: std::iter::FromIterator>; } /// An optional trait that is implemented for "user mutable" storage: From a5349b83306569f50f25cd0a6697d9745315c0fe Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 23 Jan 2019 14:21:54 +0300 Subject: [PATCH 2/2] remove debug keys in favor of entries --- src/debug.rs | 13 +------------ src/derived.rs | 8 -------- src/input.rs | 8 -------- src/plumbing.rs | 5 ----- tests/gc/derived_tests.rs | 10 ---------- tests/gc/discard_values.rs | 17 ++++++++++------- tests/gc/main.rs | 11 +++++++++++ tests/gc/shallow_constant_tests.rs | 23 +++++++++++++---------- 8 files changed, 35 insertions(+), 60 deletions(-) diff --git a/src/debug.rs b/src/debug.rs index 601d3bab..758fc5af 100644 --- a/src/debug.rs +++ b/src/debug.rs @@ -22,11 +22,6 @@ pub trait DebugQueryTable { /// values the inputs take on from this point. fn is_constant(&self, key: Self::Key) -> bool; - /// Get the (current) set of the keys in the query table. - fn keys(&self) -> C - where - C: FromIterator; - /// Get the (current) set of the entries in the query table. fn entries(&self) -> C where @@ -34,6 +29,7 @@ pub trait DebugQueryTable { } /// An entry from a query table, for debugging and inspecting the table state. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct TableEntry { /// key of the query pub key: K, @@ -64,13 +60,6 @@ where self.storage.is_constant(self.db, &key) } - fn keys(&self) -> C - where - C: FromIterator, - { - self.storage.keys(self.db) - } - fn entries(&self) -> C where C: FromIterator>, diff --git a/src/derived.rs b/src/derived.rs index 98116397..37826e0d 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -902,14 +902,6 @@ where } } - fn keys(&self, _db: &DB) -> C - where - C: std::iter::FromIterator, - { - let map = self.map.read(); - map.keys().cloned().collect() - } - fn entries(&self, _db: &DB) -> C where C: std::iter::FromIterator>, diff --git a/src/input.rs b/src/input.rs index f91dd3db..34976532 100644 --- a/src/input.rs +++ b/src/input.rs @@ -194,14 +194,6 @@ where .unwrap_or(false) } - fn keys(&self, _db: &DB) -> C - where - C: std::iter::FromIterator, - { - let map = self.map.read(); - map.keys().cloned().collect() - } - fn entries(&self, _db: &DB) -> C where C: std::iter::FromIterator>, diff --git a/src/plumbing.rs b/src/plumbing.rs index d4184806..44e16703 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -130,11 +130,6 @@ where /// Check if `key` is (currently) believed to be a constant. fn is_constant(&self, db: &DB, key: &Q::Key) -> bool; - /// Get the (current) set of the keys in the query storage - fn keys(&self, db: &DB) -> C - where - C: std::iter::FromIterator; - /// Get the (current) set of the entries in the query storage fn entries(&self, db: &DB) -> C where diff --git a/tests/gc/derived_tests.rs b/tests/gc/derived_tests.rs index 473d52eb..f58a345c 100644 --- a/tests/gc/derived_tests.rs +++ b/tests/gc/derived_tests.rs @@ -3,16 +3,6 @@ use crate::group::*; use salsa::debug::DebugQueryTable; use salsa::{Database, SweepStrategy}; -macro_rules! assert_keys { - ($db:expr, $($query:expr => ($($key:expr),*),)*) => { - $( - let mut keys = $db.query($query).keys::>(); - keys.sort(); - assert_eq!(keys, vec![$($key),*], "query {:?} had wrong keys", $query); - )* - }; -} - #[test] fn compute_one() { let mut db = db::DatabaseImpl::default(); diff --git a/tests/gc/discard_values.rs b/tests/gc/discard_values.rs index 4de16934..0c10c1da 100644 --- a/tests/gc/discard_values.rs +++ b/tests/gc/discard_values.rs @@ -9,7 +9,7 @@ fn sweep_default() { db.fibonacci(5); - let k: Vec<_> = db.query(FibonacciQuery).keys(); + let k: Vec<_> = db.query(FibonacciQuery).entries(); assert_eq!(k.len(), 6); db.salsa_runtime().next_revision(); @@ -20,9 +20,10 @@ 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()); - let mut k: Vec<_> = db.query(FibonacciQuery).keys(); - k.sort(); - assert_eq!(k, vec![3, 5]); + assert_keys! { + db, + FibonacciQuery => (3, 5), + } // Even though we ran the sweep, 5 is still in cache db.clear_log(); @@ -31,9 +32,11 @@ fn sweep_default() { // Same but we discard values this time. db.sweep_all(SweepStrategy::default().discard_values()); - let mut k: Vec<_> = db.query(FibonacciQuery).keys(); - k.sort(); - assert_eq!(k, vec![3, 5]); + db.sweep_all(SweepStrategy::default()); + assert_keys! { + db, + FibonacciQuery => (3, 5), + } // Now we have to recompute db.clear_log(); diff --git a/tests/gc/main.rs b/tests/gc/main.rs index 663623da..1cc88349 100644 --- a/tests/gc/main.rs +++ b/tests/gc/main.rs @@ -1,3 +1,14 @@ +macro_rules! assert_keys { + ($db:expr, $($query:expr => ($($key:expr),*),)*) => { + $( + let entries = $db.query($query).entries::>(); + let mut keys = entries.into_iter().map(|e| e.key).collect::>(); + keys.sort(); + assert_eq!(keys, vec![$($key),*], "query {:?} had wrong keys", $query); + )* + }; +} + mod db; mod derived_tests; mod discard_values; diff --git a/tests/gc/shallow_constant_tests.rs b/tests/gc/shallow_constant_tests.rs index a6405778..423ce010 100644 --- a/tests/gc/shallow_constant_tests.rs +++ b/tests/gc/shallow_constant_tests.rs @@ -13,7 +13,7 @@ fn one_rev() { db.fibonacci(5); - let k: Vec<_> = db.query(FibonacciQuery).keys(); + let k: Vec<_> = db.query(FibonacciQuery).entries(); assert_eq!(k.len(), 6); // Everything was used in this revision, so @@ -28,7 +28,7 @@ fn two_rev_nothing() { db.fibonacci(5); - let k: Vec<_> = db.query(FibonacciQuery).keys(); + let k: Vec<_> = db.query(FibonacciQuery).entries(); assert_eq!(k.len(), 6); db.salsa_runtime().next_revision(); @@ -37,7 +37,7 @@ fn two_rev_nothing() { // everything gets collected. db.sweep_all(SweepStrategy::default()); - let k: Vec<_> = db.query(FibonacciQuery).keys(); + let k: Vec<_> = db.query(FibonacciQuery).entries(); assert_eq!(k.len(), 0); } @@ -47,7 +47,7 @@ fn two_rev_one_use() { db.fibonacci(5); - let k: Vec<_> = db.query(FibonacciQuery).keys(); + let k: Vec<_> = db.query(FibonacciQuery).entries(); assert_eq!(k.len(), 6); db.salsa_runtime().next_revision(); @@ -58,8 +58,10 @@ fn two_rev_one_use() { // hence we keep `fibonacci(5)` but remove 0..=4. db.sweep_all(SweepStrategy::default()); - let k: Vec<_> = db.query(FibonacciQuery).keys(); - assert_eq!(k, vec![5]); + assert_keys! { + db, + FibonacciQuery => (5), + } } #[test] @@ -68,7 +70,7 @@ fn two_rev_two_uses() { db.fibonacci(5); - let k: Vec<_> = db.query(FibonacciQuery).keys(); + let k: Vec<_> = db.query(FibonacciQuery).entries(); assert_eq!(k.len(), 6); db.salsa_runtime().next_revision(); @@ -80,7 +82,8 @@ fn two_rev_two_uses() { // hence we keep 3 and 5 but remove the rest. db.sweep_all(SweepStrategy::default()); - let mut k: Vec<_> = db.query(FibonacciQuery).keys(); - k.sort(); - assert_eq!(k, vec![3, 5]); + assert_keys! { + db, + FibonacciQuery => (3, 5), + } }