From 6d60798eb8a7141f83c355023fe34949d9a163ef Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 19 Jun 2019 19:59:03 +0300 Subject: [PATCH] Replace volatile query type with report_untracked_read fn --- Cargo.toml | 4 +- components/salsa-macros/Cargo.toml | 2 +- components/salsa-macros/src/lib.rs | 3 -- components/salsa-macros/src/query_group.rs | 8 +--- src/derived.rs | 48 ---------------------- src/plumbing.rs | 1 - src/runtime.rs | 6 ++- tests/cycles.rs | 4 +- tests/gc/volatile_tests.rs | 4 +- tests/incremental/memoized_volatile.rs | 2 +- tests/lru.rs | 6 +-- tests/storage_varieties/queries.rs | 2 +- 12 files changed, 18 insertions(+), 72 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6d67c4e6..a709d4fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "salsa" -version = "0.12.3" +version = "0.13.0" authors = ["Salsa developers"] edition = "2018" license = "Apache-2.0 OR MIT" @@ -16,7 +16,7 @@ lock_api = "0.2.0" indexmap = "1.0.1" log = "0.4.5" smallvec = "0.6.5" -salsa-macros = { version = "0.12.1", path = "components/salsa-macros" } +salsa-macros = { version = "0.13.0", path = "components/salsa-macros" } linked-hash-map = "0.5.2" [dev-dependencies] diff --git a/components/salsa-macros/Cargo.toml b/components/salsa-macros/Cargo.toml index 0053809c..0f8b4485 100644 --- a/components/salsa-macros/Cargo.toml +++ b/components/salsa-macros/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "salsa-macros" -version = "0.12.1" +version = "0.13.0" authors = ["Salsa developers"] edition = "2018" license = "Apache-2.0 OR MIT" diff --git a/components/salsa-macros/src/lib.rs b/components/salsa-macros/src/lib.rs index 150fcbcd..961cda3e 100644 --- a/components/salsa-macros/src/lib.rs +++ b/components/salsa-macros/src/lib.rs @@ -58,7 +58,6 @@ mod query_group; /// are described in detail in the section below. /// - `#[salsa::input]` /// - `#[salsa::memoized]` -/// - `#[salsa::volatile]` /// - `#[salsa::dependencies]` /// - Query execution: /// - `#[salsa::invoke(path::to::my_fn)]` -- for a non-input, this @@ -96,8 +95,6 @@ mod query_group; /// which can significantly reduce the amount of recomputation /// required in new revisions. This does require that the value /// implements `Eq`. -/// - `#[salsa::volatile]` -- indicates that the inputs are not fully -/// captured by salsa. The result will be recomputed once per revision. /// - `#[salsa::dependencies]` -- does not cache the value, so it will /// be recomputed every time it is needed. We do track the inputs, however, /// so if they have not changed, then things that rely on this query diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 7ec86927..e0515c95 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -54,10 +54,6 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream storage = QueryStorage::Memoized; num_storages += 1; } - "volatile" => { - storage = QueryStorage::Volatile; - num_storages += 1; - } "dependencies" => { storage = QueryStorage::Dependencies; num_storages += 1; @@ -359,7 +355,6 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream let storage = match &query.storage { QueryStorage::Memoized => quote!(salsa::plumbing::MemoizedStorage<#db, Self>), - QueryStorage::Volatile => quote!(salsa::plumbing::VolatileStorage<#db, Self>), QueryStorage::Dependencies => quote!(salsa::plumbing::DependencyStorage<#db, Self>), QueryStorage::Input => quote!(salsa::plumbing::InputStorage<#db, Self>), QueryStorage::Interned => quote!(salsa::plumbing::InternedStorage<#db, Self>), @@ -579,7 +574,6 @@ impl Query { #[derive(Debug, Clone, PartialEq, Eq)] enum QueryStorage { Memoized, - Volatile, Dependencies, Input, Interned, @@ -594,7 +588,7 @@ impl QueryStorage { | QueryStorage::Interned | QueryStorage::InternedLookup { .. } | QueryStorage::Transparent => false, - QueryStorage::Memoized | QueryStorage::Volatile | QueryStorage::Dependencies => true, + QueryStorage::Memoized | QueryStorage::Dependencies => true, } } } diff --git a/src/derived.rs b/src/derived.rs index 8cf1d7e4..eda76967 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -35,11 +35,6 @@ pub type MemoizedStorage = DerivedStorage; /// storage requirements. pub type DependencyStorage = DerivedStorage; -/// "Dependency" queries just track their dependencies and not the -/// actual value (which they produce on demand). This lessens the -/// storage requirements. -pub type VolatileStorage = DerivedStorage; - /// Handles storage where the value is 'derived' by executing a /// function (in contrast to "inputs"). pub struct DerivedStorage @@ -73,8 +68,6 @@ where fn should_memoize_value(key: &Q::Key) -> bool; fn memoized_value_eq(old_value: &Q::Value, new_value: &Q::Value) -> bool; - - fn should_track_inputs(key: &Q::Key) -> bool; } pub enum AlwaysMemoizeValue {} @@ -91,10 +84,6 @@ where fn memoized_value_eq(old_value: &Q::Value, new_value: &Q::Value) -> bool { old_value == new_value } - - fn should_track_inputs(_key: &Q::Key) -> bool { - true - } } pub enum NeverMemoizeValue {} @@ -110,34 +99,6 @@ where fn memoized_value_eq(_old_value: &Q::Value, _new_value: &Q::Value) -> bool { panic!("cannot reach since we never memoize") } - - fn should_track_inputs(_key: &Q::Key) -> bool { - true - } -} - -pub enum VolatileValue {} -impl MemoizationPolicy for VolatileValue -where - Q: QueryFunction, - DB: Database, -{ - fn should_memoize_value(_key: &Q::Key) -> bool { - // Why memoize? Well, if the "volatile" value really is - // constantly changing, we still want to capture its value - // until the next revision is triggered and ensure it doesn't - // change -- otherwise the system gets into an inconsistent - // state where the same query reports back different values. - true - } - - fn memoized_value_eq(_old_value: &Q::Value, _new_value: &Q::Value) -> bool { - false - } - - fn should_track_inputs(_key: &Q::Key) -> bool { - false - } } /// Defines the "current state" of query's memoized results. @@ -443,11 +404,6 @@ where // stale, or value is absent. Let's execute! let mut result = runtime.execute_query_implementation(db, database_key, || { info!("{:?}({:?}): executing query", Q::default(), key); - - if !self.should_track_inputs(key) { - runtime.report_untracked_read(); - } - Q::execute(db, key.clone()) }); @@ -649,10 +605,6 @@ where fn should_memoize_value(&self, key: &Q::Key) -> bool { MP::should_memoize_value(key) } - - fn should_track_inputs(&self, key: &Q::Key) -> bool { - MP::should_track_inputs(key) - } } struct PanicGuard<'db, DB, Q> diff --git a/src/plumbing.rs b/src/plumbing.rs index a0c11d5e..c2a0c4cd 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -11,7 +11,6 @@ use std::hash::Hash; pub use crate::derived::DependencyStorage; pub use crate::derived::MemoizedStorage; -pub use crate::derived::VolatileStorage; pub use crate::input::InputStorage; pub use crate::interned::InternedStorage; pub use crate::interned::LookupInternedStorage; diff --git a/src/runtime.rs b/src/runtime.rs index 2f98e259..b244394b 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -344,7 +344,11 @@ where self.local_state.report_query_read(database_key, changed_at); } - pub(crate) fn report_untracked_read(&self) { + /// Reports that the query depends on some state unknown to salsa. + /// + /// If query reports untracked read, it will be reexuted in the next + /// revision. + pub fn report_untracked_read(&self) { self.local_state .report_untracked_read(self.current_revision()); } diff --git a/tests/cycles.rs b/tests/cycles.rs index f4767f5b..77095aa4 100644 --- a/tests/cycles.rs +++ b/tests/cycles.rs @@ -15,9 +15,7 @@ trait Database: salsa::Database { // `a` and `b` depend on each other and form a cycle fn memoized_a(&self) -> (); fn memoized_b(&self) -> (); - #[salsa::volatile] fn volatile_a(&self) -> (); - #[salsa::volatile] fn volatile_b(&self) -> (); } @@ -30,10 +28,12 @@ fn memoized_b(db: &impl Database) -> () { } fn volatile_a(db: &impl Database) -> () { + db.salsa_runtime().report_untracked_read(); db.volatile_b() } fn volatile_b(db: &impl Database) -> () { + db.salsa_runtime().report_untracked_read(); db.volatile_a() } diff --git a/tests/gc/volatile_tests.rs b/tests/gc/volatile_tests.rs index 69c47954..86f4b9c3 100644 --- a/tests/gc/volatile_tests.rs +++ b/tests/gc/volatile_tests.rs @@ -5,12 +5,11 @@ use std::sync::Arc; /// Query group for tests for how interned keys interact with GC. #[salsa::query_group(Volatile)] -pub(crate) trait VolatileDatabase { +pub(crate) trait VolatileDatabase: Database { #[salsa::input] fn atomic_cell(&self) -> Arc; /// Underlying volatile query. - #[salsa::volatile] fn volatile(&self) -> usize; /// This just executes the intern query and returns the result. @@ -21,6 +20,7 @@ pub(crate) trait VolatileDatabase { } fn volatile(db: &impl VolatileDatabase) -> usize { + db.salsa_runtime().report_untracked_read(); db.atomic_cell().load(Ordering::SeqCst) } diff --git a/tests/incremental/memoized_volatile.rs b/tests/incremental/memoized_volatile.rs index de4e68c8..b48b1219 100644 --- a/tests/incremental/memoized_volatile.rs +++ b/tests/incremental/memoized_volatile.rs @@ -7,7 +7,6 @@ pub(crate) trait MemoizedVolatileContext: TestContext { // memoization. fn memoized2(&self) -> usize; fn memoized1(&self) -> usize; - #[salsa::volatile] fn volatile(&self) -> usize; } @@ -24,6 +23,7 @@ fn memoized1(db: &impl MemoizedVolatileContext) -> usize { fn volatile(db: &impl MemoizedVolatileContext) -> usize { db.log().add("Volatile invoked"); + db.salsa_runtime().report_untracked_read(); db.clock().increment() } diff --git a/tests/lru.rs b/tests/lru.rs index 93490d3f..4ec16f52 100644 --- a/tests/lru.rs +++ b/tests/lru.rs @@ -25,9 +25,8 @@ impl Drop for HotPotato { } #[salsa::query_group(QueryGroupStorage)] -trait QueryGroup { +trait QueryGroup: salsa::Database { fn get(&self, x: u32) -> Arc; - #[salsa::volatile] fn get_volatile(&self, x: u32) -> usize; } @@ -35,8 +34,9 @@ fn get(_db: &impl QueryGroup, x: u32) -> Arc { Arc::new(HotPotato::new(x)) } -fn get_volatile(_db: &impl QueryGroup, _x: u32) -> usize { +fn get_volatile(db: &impl QueryGroup, _x: u32) -> usize { static COUNTER: AtomicUsize = AtomicUsize::new(0); + db.salsa_runtime().report_untracked_read(); COUNTER.fetch_add(1, Ordering::SeqCst) } diff --git a/tests/storage_varieties/queries.rs b/tests/storage_varieties/queries.rs index eab2167b..81ddeff2 100644 --- a/tests/storage_varieties/queries.rs +++ b/tests/storage_varieties/queries.rs @@ -5,7 +5,6 @@ pub(crate) trait Counter: salsa::Database { #[salsa::query_group(GroupStruct)] pub(crate) trait Database: Counter { fn memoized(&self) -> usize; - #[salsa::volatile] fn volatile(&self) -> usize; } @@ -18,5 +17,6 @@ fn memoized(db: &impl Database) -> usize { /// Because this query is volatile, each time it is invoked, /// we will increment the counter. fn volatile(db: &impl Database) -> usize { + db.salsa_runtime().report_untracked_read(); db.increment() }