mirrors/salsa
1
0
Fork 0
mirror of https://github.com/salsa-rs/salsa.git synced 2024-12-24 12:58:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

remove input policies

Browse source
This commit is contained in:
Niko Matsakis 2018-10-30 12:59:33 -04:00
parent 8a0f57d5c3
commit 13ae45d441
8 changed files with 43 additions and 240 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

162
src/input.rs
View file

@ -13,141 +13,36 @@ use log::debug;
use parking_lot::{RwLock, RwLockUpgradableReadGuard};
use rustc_hash::FxHashMap;
use std::collections::hash_map::Entry;
use std::marker::PhantomData;
/// Input queries store the result plus a list of the other queries
/// that they invoked. This means we can avoid recomputing them when
/// none of those inputs have changed.
pub struct InputStorage<DB, Q, IP>
pub struct InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
map: RwLock<FxHashMap<Q::Key, StampedValue<Q::Value>>>,
input_policy: PhantomData<IP>,
}
pub trait InputPolicy<DB, Q>
impl<DB, Q> Default for InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
{
fn compare_values() -> bool {
false
}
fn compare_value(_old_value: &Q::Value, _new_value: &Q::Value) -> bool {
panic!("should never be asked to compare values")
}
fn missing_value(key: &Q::Key) -> Q::Value {
panic!("no value set for {:?}({:?})", Q::default(), key)
}
}
/// The default policy for inputs:
///
/// - Each time a new value is set, trigger a new revision.
/// - On an attempt access a value that is not yet set, panic.
pub enum ExplicitInputPolicy {}
impl<DB, Q> InputPolicy<DB, Q> for ExplicitInputPolicy
where
Q: Query<DB>,
DB: Database,
{
}
/// Alternative policy for inputs:
///
/// - Each time a new value is set, trigger a new revision.
/// - On an attempt access a value that is not yet set, use `Default::default` to find
/// the value.
///
/// Requires that `Q::Value` implements the `Default` trait.
pub enum DefaultValueInputPolicy {}
impl<DB, Q> InputPolicy<DB, Q> for DefaultValueInputPolicy
where
Q: Query<DB>,
DB: Database,
Q::Value: Default,
{
fn missing_value(_key: &Q::Key) -> Q::Value {
<Q::Value>::default()
}
}
/// Alternative policy for inputs:
///
/// - Each time a new value is set, trigger a new revision
/// only if it is not equal to the old value.
/// - On an attempt access a value that is not yet set, panic.
///
/// Requires that `Q::Value` implements the `Eq` trait.
pub enum EqValueInputPolicy {}
impl<DB, Q> InputPolicy<DB, Q> for EqValueInputPolicy
where
Q: Query<DB>,
DB: Database,
Q::Value: Eq,
{
fn compare_values() -> bool {
true
}
fn compare_value(old_value: &Q::Value, new_value: &Q::Value) -> bool {
old_value == new_value
}
}
/// Alternative policy for inputs:
///
/// - Each time a new value is set, trigger a new revision
/// only if it is not equal to the old value.
/// - On an attempt access a value that is not yet set, use `Default::default`.
///
/// Requires that `Q::Value` implements the `Eq` trait.
pub enum DefaultEqValueInputPolicy {}
impl<DB, Q> InputPolicy<DB, Q> for DefaultEqValueInputPolicy
where
Q: Query<DB>,
DB: Database,
Q::Value: Default + Eq,
{
fn compare_values() -> bool {
true
}
fn compare_value(old_value: &Q::Value, new_value: &Q::Value) -> bool {
old_value == new_value
}
fn missing_value(_key: &Q::Key) -> Q::Value {
<Q::Value>::default()
}
}
impl<DB, Q, IP> Default for InputStorage<DB, Q, IP>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
fn default() -> Self {
InputStorage {
map: RwLock::new(FxHashMap::default()),
input_policy: PhantomData,
}
}
}
struct IsConstant(bool);
impl<DB, Q, IP> InputStorage<DB, Q, IP>
impl<DB, Q> InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
fn read<'q>(
&self,
@ -162,43 +57,18 @@ where
}
}
let value = IP::missing_value(key);
Ok(StampedValue {
value: value,
changed_at: ChangedAt {
is_constant: false,
revision: Revision::ZERO,
},
})
panic!("no value set for {:?}({:?})", Q::default(), key)
}
fn set_common(&self, db: &DB, key: &Q::Key, value: Q::Value, is_constant: IsConstant) {
let map = self.map.upgradable_read();
if IP::compare_values() {
if let Some(old_value) = map.get(key) {
if IP::compare_value(&old_value.value, &value) {
// If the value did not change, but it is now
// considered constant, we can just update
// `changed_at`. We don't have to trigger a new
// revision for this case: all the derived values are
// still intact, they just have conservative
// dependencies. The next revision, they may wind up
// with something more precise.
if is_constant.0 && !old_value.changed_at.is_constant {
let mut map = RwLockUpgradableReadGuard::upgrade(map);
let old_value = map.get_mut(key).unwrap();
old_value.changed_at.is_constant = true;
}
return;
}
}
}
let key = key.clone();
// This upgradable read just serves to gate against other
// people invoking `set`; we should probably remove it and
// instead acquire the query-write-lock, but that would
// require refactoring the `increment_revision` API a bit.
let map = self.map.upgradable_read();
// The value is changing, so even if we are setting this to a
// constant, we still need a new revision.
//
@ -243,11 +113,10 @@ where
}
}
impl<DB, Q, IP> QueryStorageOps<DB, Q> for InputStorage<DB, Q, IP>
impl<DB, Q> QueryStorageOps<DB, Q> for InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
fn try_fetch(
&self,
@ -314,20 +183,18 @@ where
}
}
impl<DB, Q, IP> QueryStorageMassOps<DB> for InputStorage<DB, Q, IP>
impl<DB, Q> QueryStorageMassOps<DB> for InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
fn sweep(&self, _db: &DB, _strategy: SweepStrategy) {}
}
impl<DB, Q, IP> InputQueryStorageOps<DB, Q> for InputStorage<DB, Q, IP>
impl<DB, Q> InputQueryStorageOps<DB, Q> for InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
fn set(&self, db: &DB, key: &Q::Key, value: Q::Value) {
log::debug!("{:?}({:?}) = {:?}", Q::default(), key, value);
@ -342,11 +209,10 @@ where
}
}
impl<DB, Q, IP> UncheckedMutQueryStorageOps<DB, Q> for InputStorage<DB, Q, IP>
impl<DB, Q> UncheckedMutQueryStorageOps<DB, Q> for InputStorage<DB, Q>
where
Q: Query<DB>,
DB: Database,
IP: InputPolicy<DB, Q>,
{
fn set_unchecked(&self, db: &DB, key: &Q::Key, value: Q::Value) {
let key = key.clone();

45
src/lib.rs
View file

@ -355,17 +355,6 @@ macro_rules! query_group {
// do nothing for `storage input`, presuming they did not write an explicit `use fn`
};
(
@query_fn[
storage((input $($flags:tt)*));
method_name($method_name:ident);
fn_path();
$($rest:tt)*
]
) => {
// do nothing for `storage input`, presuming they did not write an explicit `use fn`
};
(
@query_fn[
storage(input);
@ -380,20 +369,6 @@ macro_rules! query_group {
}
};
(
@query_fn[
storage((input $($flags:tt)*));
method_name($method_name:ident);
fn_path($fn_path:path);
$($rest:tt)*
]
) => {
// error for `storage input` with an explicit `use fn`
compile_error! {
"cannot have `storage input` combined with `use fn`"
}
};
(
@query_fn[
storage($($storage:ident)*);
@ -502,25 +477,7 @@ macro_rules! query_group {
(
@storage_ty[$DB:ident, $Self:ident, input]
) => {
$crate::plumbing::InputStorage<DB, Self, $crate::plumbing::ExplicitInputPolicy>
};
(
@storage_ty[$DB:ident, $Self:ident, (input default)]
) => {
$crate::plumbing::InputStorage<DB, Self, $crate::plumbing::DefaultValueInputPolicy>
};
(
@storage_ty[$DB:ident, $Self:ident, (input eq)]
) => {
$crate::plumbing::InputStorage<DB, Self, $crate::plumbing::EqValueInputPolicy>
};
(
@storage_ty[$DB:ident, $Self:ident, (input default eq)]
) => {
$crate::plumbing::InputStorage<DB, Self, $crate::plumbing::DefaultEqValueInputPolicy>
$crate::plumbing::InputStorage<DB, Self>
};
(

4
src/plumbing.rs
View file

@ -8,10 +8,6 @@ use std::hash::Hash;
pub use crate::derived::DependencyStorage;
pub use crate::derived::MemoizedStorage;
pub use crate::derived::VolatileStorage;
pub use crate::input::DefaultEqValueInputPolicy;
pub use crate::input::DefaultValueInputPolicy;
pub use crate::input::EqValueInputPolicy;
pub use crate::input::ExplicitInputPolicy;
pub use crate::input::InputStorage;
pub use crate::runtime::Revision;

18
tests/gc/derived_tests.rs
View file

@ -18,6 +18,7 @@ fn compute_one() {
let db = db::DatabaseImpl::default();
// Will compute fibonacci(5)
db.query(UseTriangular).set(5, false);
db.compute(5);
db.salsa_runtime().next_revision();
@ -27,7 +28,7 @@ fn compute_one() {
Triangular => (),
Fibonacci => (0, 1, 2, 3, 4, 5),
Compute => (5),
UseTriangular => (),
UseTriangular => (5),
Min => (),
Max => (),
}
@ -41,7 +42,7 @@ fn compute_one() {
Triangular => (),
Fibonacci => (5),
Compute => (5),
UseTriangular => (),
UseTriangular => (5),
Min => (),
Max => (),
}
@ -52,6 +53,7 @@ fn compute_switch() {
let db = db::DatabaseImpl::default();
// Will compute fibonacci(5)
db.query(UseTriangular).set(5, false);
assert_eq!(db.compute(5), 5);
// Change to triangular mode
@ -107,9 +109,9 @@ fn compute_switch() {
fn compute_all() {
let db = db::DatabaseImpl::default();
db.query(UseTriangular).set(1, true);
db.query(UseTriangular).set(3, true);
db.query(UseTriangular).set(5, true);
for i in 0..6 {
db.query(UseTriangular).set(i, (i % 2) != 0);
}
db.query(Min).set((), 0);
db.query(Max).set((), 6);
@ -125,7 +127,7 @@ fn compute_all() {
Fibonacci => (0, 2, 4),
Compute => (0, 1, 2, 3, 4, 5),
ComputeAll => (()),
UseTriangular => (1, 3, 5),
UseTriangular => (0, 1, 2, 3, 4, 5),
Min => (()),
Max => (()),
}
@ -140,7 +142,7 @@ fn compute_all() {
Fibonacci => (0, 2, 4),
Compute => (0, 1, 2, 3, 4, 5),
ComputeAll => (()),
UseTriangular => (1, 3, 5),
UseTriangular => (0, 1, 2, 3, 4, 5),
Min => (()),
Max => (()),
}
@ -155,7 +157,7 @@ fn compute_all() {
Fibonacci => (0, 2, 4),
Compute => (0, 1, 2, 3, 4),
ComputeAll => (()),
UseTriangular => (1, 3, 5),
UseTriangular => (0, 1, 2, 3, 4, 5),
Min => (()),
Max => (()),
}

2
tests/gc/group.rs
View file

@ -14,7 +14,7 @@ salsa::query_group! {
fn use_triangular(key: usize) -> bool {
type UseTriangular;
storage (input default);
storage input;
}
fn fibonacci(key: usize) -> usize {

33
tests/incremental/constants.rs
View file

@ -6,7 +6,7 @@ salsa::query_group! {
pub(crate) trait ConstantsDatabase: TestContext {
fn constants_input(key: char) -> usize {
type ConstantsInput;
storage (input default eq);
storage input;
}
fn constants_add(keys: (char, char)) -> usize {
@ -43,9 +43,10 @@ fn invalidate_constant_1() {
db.query(ConstantsInput).set_constant('a', 66);
}
/// Test that use can still `set` an input that is constant, so long
/// as you don't change the value.
/// Test that invoking `set` on a constant is an error, even if you
/// don't change the value.
#[test]
#[should_panic]
fn set_after_constant_same_value() {
let db = &TestContextImpl::default();
db.query(ConstantsInput).set_constant('a', 44);
@ -99,29 +100,3 @@ fn becomes_constant_with_change() {
assert_eq!(db.constants_add(('a', 'b')), 68);
assert!(db.query(ConstantsAdd).is_constant(('a', 'b')));
}
#[test]
fn becomes_constant_no_change() {
let db = &TestContextImpl::default();
db.query(ConstantsInput).set('a', 22);
db.query(ConstantsInput).set('b', 44);
assert_eq!(db.constants_add(('a', 'b')), 66);
assert!(!db.query(ConstantsAdd).is_constant(('a', 'b')));
db.assert_log(&["add(a, b)"]);
// 'a' is now constant, but the value did not change; this
// should not in and of itself trigger a new revision.
db.query(ConstantsInput).set_constant('a', 22);
assert_eq!(db.constants_add(('a', 'b')), 66);
assert!(!db.query(ConstantsAdd).is_constant(('a', 'b')));
db.assert_log(&[]); // no new revision, no new log entries
// 'b' is now constant, and its value DID change. This triggers a
// new revision, and at that point we figure out that we are
// constant.
db.query(ConstantsInput).set_constant('b', 45);
assert_eq!(db.constants_add(('a', 'b')), 67);
assert!(db.query(ConstantsAdd).is_constant(('a', 'b')));
db.assert_log(&["add(a, b)"]);
}

4
tests/incremental/memoized_dep_inputs.rs
View file

@ -15,7 +15,7 @@ salsa::query_group! {
}
fn dep_input1() -> usize {
type Input1;
storage (input default);
storage input;
}
fn dep_input2() -> usize {
type Input2;
@ -43,6 +43,8 @@ fn dep_derived1(db: &impl MemoizedDepInputsContext) -> usize {
fn revalidate() {
let db = &TestContextImpl::default();
db.query(Input1).set((), 0);
// Initial run starts from Memoized2:
let v = db.dep_memoized2();
assert_eq!(v, 0);

15
tests/incremental/memoized_inputs.rs
View file

@ -8,11 +8,11 @@ salsa::query_group! {
}
fn input1() -> usize {
type Input1;
storage (input default eq);
storage input;
}
fn input2() -> usize {
type Input2;
storage (input default eq);
storage input;
}
}
}
@ -26,6 +26,9 @@ fn max(db: &impl MemoizedInputsContext) -> usize {
fn revalidate() {
let db = &TestContextImpl::default();
db.query(Input1).set((), 0);
db.query(Input2).set((), 0);
let v = db.max();
assert_eq!(v, 0);
db.assert_log(&["Max invoked"]);
@ -61,12 +64,14 @@ fn revalidate() {
db.assert_log(&[]);
}
/// Test that invoking `set` on an input with the same value does not
/// trigger a new revision.
/// Test that invoking `set` on an input with the same value still
/// triggers a new revision.
#[test]
fn set_after_no_change() {
let db = &TestContextImpl::default();
db.query(Input2).set((), 0);
db.query(Input1).set((), 44);
let v = db.max();
assert_eq!(v, 44);
@ -75,5 +80,5 @@ fn set_after_no_change() {
db.query(Input1).set((), 44);
let v = db.max();
assert_eq!(v, 44);
db.assert_log(&[]);
db.assert_log(&["Max invoked"]);
}