kill the ChangedAt struct

It didn't seem like it was buying us much.
This commit is contained in:
Niko Matsakis 2019-06-21 20:12:51 -07:00
parent 0a5b6b0451
commit 30714022fd
6 changed files with 73 additions and 95 deletions

View file

@ -134,13 +134,18 @@ where
{ {
fn try_fetch(&self, db: &DB, key: &Q::Key) -> Result<Q::Value, CycleDetected> { fn try_fetch(&self, db: &DB, key: &Q::Key) -> Result<Q::Value, CycleDetected> {
let slot = self.slot(key); let slot = self.slot(key);
let StampedValue { value, changed_at } = slot.read(db)?; let StampedValue {
value,
is_constant,
changed_at,
} = slot.read(db)?;
if let Some(evicted) = self.lru_list.record_use(&slot) { if let Some(evicted) = self.lru_list.record_use(&slot) {
evicted.evict(); evicted.evict();
} }
db.salsa_runtime().report_query_read(slot, changed_at); db.salsa_runtime()
.report_query_read(slot, is_constant, changed_at);
Ok(value) Ok(value)
} }

View file

@ -8,7 +8,6 @@ use crate::plumbing::CycleDetected;
use crate::plumbing::GetQueryTable; use crate::plumbing::GetQueryTable;
use crate::plumbing::HasQueryGroup; use crate::plumbing::HasQueryGroup;
use crate::plumbing::QueryFunction; use crate::plumbing::QueryFunction;
use crate::runtime::ChangedAt;
use crate::runtime::FxIndexSet; use crate::runtime::FxIndexSet;
use crate::runtime::Revision; use crate::runtime::Revision;
use crate::runtime::Runtime; use crate::runtime::Runtime;
@ -223,7 +222,7 @@ where
// Careful: the "constant-ness" must also not have // Careful: the "constant-ness" must also not have
// changed, see the test `constant_to_non_constant`. // changed, see the test `constant_to_non_constant`.
let memo_was_constant = old_memo.constant_in_revision.is_some(); let memo_was_constant = old_memo.constant_in_revision.is_some();
if memo_was_constant == result.changed_at.is_constant if memo_was_constant == result.is_constant
&& MP::memoized_value_eq(&old_value, &result.value) && MP::memoized_value_eq(&old_value, &result.value)
{ {
debug!( debug!(
@ -231,14 +230,15 @@ where
self, old_memo.changed_at, self, old_memo.changed_at,
); );
assert!(old_memo.changed_at <= result.changed_at.revision); assert!(old_memo.changed_at <= result.changed_at);
result.changed_at.revision = old_memo.changed_at; result.changed_at = old_memo.changed_at;
} }
} }
} }
let new_value = StampedValue { let new_value = StampedValue {
value: result.value, value: result.value,
is_constant: result.is_constant,
changed_at: result.changed_at, changed_at: result.changed_at,
}; };
@ -253,7 +253,7 @@ where
self, result.changed_at, result.dependencies, self, result.changed_at, result.dependencies,
); );
let constant_in_revision = if result.changed_at.is_constant { let constant_in_revision = if result.is_constant {
Some(revision_now) Some(revision_now)
} else { } else {
None None
@ -280,7 +280,7 @@ where
panic_guard.memo = Some(Memo { panic_guard.memo = Some(Memo {
value, value,
changed_at: result.changed_at.revision, changed_at: result.changed_at,
verified_at: revision_now, verified_at: revision_now,
inputs, inputs,
constant_in_revision, constant_in_revision,
@ -717,10 +717,8 @@ where
} }
StampedValue { StampedValue {
changed_at: ChangedAt { is_constant,
is_constant, changed_at: self.changed_at,
revision: self.changed_at,
},
value, value,
} }
} }
@ -738,10 +736,8 @@ where
let is_constant = self.constant_in_revision.is_some(); let is_constant = self.constant_in_revision.is_some();
return Some(StampedValue { return Some(StampedValue {
changed_at: ChangedAt { is_constant,
is_constant, changed_at: self.changed_at,
revision: self.changed_at,
},
value: value.clone(), value: value.clone(),
}); });
} }
@ -840,7 +836,7 @@ where
std::mem::drop(state); std::mem::drop(state);
let value = rx.recv().unwrap_or_else(|_| db.on_propagated_panic()); let value = rx.recv().unwrap_or_else(|_| db.on_propagated_panic());
return value.changed_at.changed_since(revision); return value.changed_at > revision;
} }
// Consider a cycle to have changed. // Consider a cycle to have changed.
@ -903,10 +899,10 @@ where
debug!( debug!(
"maybe_changed_since({:?}: {:?} since (recomputed) value changed at {:?}", "maybe_changed_since({:?}: {:?} since (recomputed) value changed at {:?}",
self, self,
v.changed_at.changed_since(revision), v.changed_at > revision,
v.changed_at, v.changed_at,
); );
v.changed_at.changed_since(revision) v.changed_at > revision
} }
Err(CycleDetected) => true, Err(CycleDetected) => true,
}; };

View file

@ -4,7 +4,6 @@ use crate::plumbing::CycleDetected;
use crate::plumbing::InputQueryStorageOps; use crate::plumbing::InputQueryStorageOps;
use crate::plumbing::QueryStorageMassOps; use crate::plumbing::QueryStorageMassOps;
use crate::plumbing::QueryStorageOps; use crate::plumbing::QueryStorageOps;
use crate::runtime::ChangedAt;
use crate::runtime::Revision; use crate::runtime::Revision;
use crate::runtime::StampedValue; use crate::runtime::StampedValue;
use crate::Database; use crate::Database;
@ -100,18 +99,17 @@ where
// racing with somebody else to modify this same cell. // racing with somebody else to modify this same cell.
// (Otherwise, someone else might write a *newer* revision // (Otherwise, someone else might write a *newer* revision
// into the same cell while we block on the lock.) // into the same cell while we block on the lock.)
let changed_at = ChangedAt { let stamped_value = StampedValue {
value,
is_constant: is_constant.0, is_constant: is_constant.0,
revision: guard.new_revision(), changed_at: guard.new_revision(),
}; };
let stamped_value = StampedValue { value, changed_at };
match slots.entry(key.clone()) { match slots.entry(key.clone()) {
Entry::Occupied(entry) => { Entry::Occupied(entry) => {
let mut slot_stamped_value = entry.get().stamped_value.write(); let mut slot_stamped_value = entry.get().stamped_value.write();
if slot_stamped_value.changed_at.is_constant { if slot_stamped_value.is_constant {
guard.mark_constants_as_changed(); guard.mark_constants_as_changed();
} }
@ -140,16 +138,21 @@ where
None => panic!("no value set for {:?}({:?})", Q::default(), key), None => panic!("no value set for {:?}({:?})", Q::default(), key),
}; };
let StampedValue { value, changed_at } = slot.stamped_value.read().clone(); let StampedValue {
value,
is_constant,
changed_at,
} = slot.stamped_value.read().clone();
db.salsa_runtime().report_query_read(slot, changed_at); db.salsa_runtime()
.report_query_read(slot, is_constant, changed_at);
Ok(value) Ok(value)
} }
fn is_constant(&self, _db: &DB, key: &Q::Key) -> bool { fn is_constant(&self, _db: &DB, key: &Q::Key) -> bool {
self.slot(key) self.slot(key)
.map(|slot| slot.stamped_value.read().changed_at.is_constant) .map(|slot| slot.stamped_value.read().is_constant)
.unwrap_or(false) .unwrap_or(false)
} }
@ -215,7 +218,7 @@ where
debug!("maybe_changed_since: changed_at = {:?}", changed_at); debug!("maybe_changed_since: changed_at = {:?}", changed_at);
changed_at.changed_since(revision) changed_at > revision
} }
} }

View file

@ -5,7 +5,6 @@ use crate::plumbing::CycleDetected;
use crate::plumbing::HasQueryGroup; use crate::plumbing::HasQueryGroup;
use crate::plumbing::QueryStorageMassOps; use crate::plumbing::QueryStorageMassOps;
use crate::plumbing::QueryStorageOps; use crate::plumbing::QueryStorageOps;
use crate::runtime::ChangedAt;
use crate::runtime::Revision; use crate::runtime::Revision;
use crate::Query; use crate::Query;
use crate::{Database, DiscardIf, SweepStrategy}; use crate::{Database, DiscardIf, SweepStrategy};
@ -323,13 +322,8 @@ where
let slot = self.intern_index(db, key); let slot = self.intern_index(db, key);
let changed_at = slot.interned_at; let changed_at = slot.interned_at;
let index = slot.index; let index = slot.index;
db.salsa_runtime().report_query_read( db.salsa_runtime()
slot, .report_query_read(slot, false, changed_at);
ChangedAt {
is_constant: false,
revision: changed_at,
},
);
Ok(<Q::Value>::from_intern_id(index)) Ok(<Q::Value>::from_intern_id(index))
} }
@ -427,12 +421,10 @@ where
let group_storage = <DB as HasQueryGroup<Q::Group>>::group_storage(db); let group_storage = <DB as HasQueryGroup<Q::Group>>::group_storage(db);
let interned_storage = IQ::query_storage(group_storage); let interned_storage = IQ::query_storage(group_storage);
let slot = interned_storage.lookup_value(db, index); let slot = interned_storage.lookup_value(db, index);
let changed_at = ChangedAt {
is_constant: false,
revision: slot.interned_at,
};
let value = slot.value.clone(); let value = slot.value.clone();
db.salsa_runtime().report_query_read(slot, changed_at); let interned_at = slot.interned_at;
db.salsa_runtime()
.report_query_read(slot, false, interned_at);
Ok(value) Ok(value)
} }

View file

@ -332,11 +332,13 @@ where
let ActiveQuery { let ActiveQuery {
dependencies, dependencies,
changed_at, changed_at,
is_constant,
.. ..
} = active_query.complete(); } = active_query.complete();
ComputedQueryResult { ComputedQueryResult {
value, value,
is_constant,
changed_at, changed_at,
dependencies, dependencies,
} }
@ -353,10 +355,12 @@ where
pub(crate) fn report_query_read<'hack>( pub(crate) fn report_query_read<'hack>(
&self, &self,
database_slot: Arc<dyn DatabaseSlot<DB> + 'hack>, database_slot: Arc<dyn DatabaseSlot<DB> + 'hack>,
changed_at: ChangedAt, is_constant: bool,
changed_at: Revision,
) { ) {
let dependency = Dependency::new(database_slot); let dependency = Dependency::new(database_slot);
self.local_state.report_query_read(dependency, changed_at); self.local_state
.report_query_read(dependency, is_constant, changed_at);
} }
/// Reports that the query depends on some state unknown to salsa. /// Reports that the query depends on some state unknown to salsa.
@ -532,11 +536,12 @@ struct ActiveQuery<DB: Database> {
/// What query is executing /// What query is executing
database_key: DB::DatabaseKey, database_key: DB::DatabaseKey,
/// Maximum revision of all inputs thus far; /// True if all inputs were constant (and no untracked inputs).
/// we also track if all inputs have been constant. is_constant: bool,
///
/// If we see an untracked input, this is not terribly relevant. /// Maximum revision of all inputs observed. If we observe an
changed_at: ChangedAt, /// untracked read, this will be set to the most recent revision.
changed_at: Revision,
/// Set of subqueries that were accessed thus far, or `None` if /// Set of subqueries that were accessed thus far, or `None` if
/// there was an untracked the read. /// there was an untracked the read.
@ -547,12 +552,12 @@ pub(crate) struct ComputedQueryResult<DB: Database, V> {
/// Final value produced /// Final value produced
pub(crate) value: V, pub(crate) value: V,
/// Maximum revision of all inputs observed; `is_constant` is true /// True if all inputs were constant (and no untracked inputs).
/// if all inputs were constants. pub(crate) is_constant: bool,
///
/// If we observe an untracked read, this will be set to a /// Maximum revision of all inputs observed. If we observe an
/// non-constant value that changed in the most recent revision. /// untracked read, this will be set to the most recent revision.
pub(crate) changed_at: ChangedAt, pub(crate) changed_at: Revision,
/// Complete set of subqueries that were accessed, or `None` if /// Complete set of subqueries that were accessed, or `None` if
/// there was an untracked the read. /// there was an untracked the read.
@ -563,36 +568,29 @@ impl<DB: Database> ActiveQuery<DB> {
fn new(database_key: DB::DatabaseKey) -> Self { fn new(database_key: DB::DatabaseKey) -> Self {
ActiveQuery { ActiveQuery {
database_key, database_key,
changed_at: ChangedAt { is_constant: true,
is_constant: true, changed_at: Revision::start(),
revision: Revision::start(),
},
dependencies: Some(FxIndexSet::default()), dependencies: Some(FxIndexSet::default()),
} }
} }
fn add_read(&mut self, dependency: Dependency<DB>, changed_at: ChangedAt) { fn add_read(&mut self, dependency: Dependency<DB>, is_constant: bool, revision: Revision) {
let ChangedAt {
is_constant,
revision,
} = changed_at;
if let Some(set) = &mut self.dependencies { if let Some(set) = &mut self.dependencies {
set.insert(dependency); set.insert(dependency);
} }
self.changed_at.is_constant &= is_constant; self.is_constant &= is_constant;
self.changed_at.revision = self.changed_at.revision.max(revision); self.changed_at = self.changed_at.max(revision);
} }
fn add_untracked_read(&mut self, changed_at: Revision) { fn add_untracked_read(&mut self, changed_at: Revision) {
self.dependencies = None; self.dependencies = None;
self.changed_at.is_constant = false; self.is_constant = false;
self.changed_at.revision = changed_at; self.changed_at = changed_at;
} }
fn add_anon_read(&mut self, changed_at: Revision) { fn add_anon_read(&mut self, changed_at: Revision) {
self.changed_at.revision = self.changed_at.revision.max(changed_at); self.changed_at = self.changed_at.max(changed_at);
} }
} }
@ -640,31 +638,11 @@ impl std::fmt::Debug for Revision {
} }
} }
/// Records when a stamped value changed.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub struct ChangedAt {
// Will this value ever change again?
pub(crate) is_constant: bool,
// At which revision did this value last change? (If this value is
// the value of a constant input, this indicates when it became
// constant.)
pub(crate) revision: Revision,
}
impl ChangedAt {
/// True if a value is stored with this `ChangedAt` value has
/// changed after `revision`. This is invoked by query storage
/// when their dependents are asking them if they have changed.
pub(crate) fn changed_since(self, revision: Revision) -> bool {
self.revision > revision
}
}
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub(crate) struct StampedValue<V> { pub(crate) struct StampedValue<V> {
pub(crate) value: V, pub(crate) value: V,
pub(crate) changed_at: ChangedAt, pub(crate) is_constant: bool,
pub(crate) changed_at: Revision,
} }
struct DependencyGraph<DB: Database> { struct DependencyGraph<DB: Database> {

View file

@ -1,6 +1,5 @@
use crate::dependency::Dependency; use crate::dependency::Dependency;
use crate::runtime::ActiveQuery; use crate::runtime::ActiveQuery;
use crate::runtime::ChangedAt;
use crate::runtime::Revision; use crate::runtime::Revision;
use crate::Database; use crate::Database;
use std::cell::Ref; use std::cell::Ref;
@ -58,9 +57,14 @@ impl<DB: Database> LocalState<DB> {
.map(|active_query| active_query.database_key.clone()) .map(|active_query| active_query.database_key.clone())
} }
pub(super) fn report_query_read(&self, dependency: Dependency<DB>, changed_at: ChangedAt) { pub(super) fn report_query_read(
&self,
dependency: Dependency<DB>,
is_constant: bool,
changed_at: Revision,
) {
if let Some(top_query) = self.query_stack.borrow_mut().last_mut() { if let Some(top_query) = self.query_stack.borrow_mut().last_mut() {
top_query.add_read(dependency, changed_at); top_query.add_read(dependency, is_constant, changed_at);
} }
} }