From 5849af83ac65c9cda218809f482713dacef482eb Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 1 Oct 2018 05:49:18 -0400 Subject: [PATCH] track the "changed at" revision for every query read as well --- src/input.rs | 9 ++++----- src/memoized.rs | 24 ++++++++++-------------- src/runtime.rs | 21 +++++++++++++++------ src/volatile.rs | 19 ++++++++++++++----- 4 files changed, 43 insertions(+), 30 deletions(-) diff --git a/src/input.rs b/src/input.rs index 4056713..4006714 100644 --- a/src/input.rs +++ b/src/input.rs @@ -82,12 +82,11 @@ where key: &Q::Key, descriptor: &QC::QueryDescriptor, ) -> Result { - let StampedValue { - value, - changed_at: _, - } = self.read(query, key, &descriptor)?; + let StampedValue { value, changed_at } = self.read(query, key, &descriptor)?; - query.salsa_runtime().report_query_read(descriptor); + query + .salsa_runtime() + .report_query_read(descriptor, changed_at); Ok(value) } diff --git a/src/memoized.rs b/src/memoized.rs index 48ea422..22145b8 100644 --- a/src/memoized.rs +++ b/src/memoized.rs @@ -156,7 +156,7 @@ where // Query was not previously executed or value is potentially // stale. Let's execute! - let (value, inputs) = query + let (mut stamped_value, inputs) = query .salsa_runtime() .execute_query_implementation::(query, descriptor, key); @@ -173,10 +173,10 @@ where // really change, even if some of its inputs have. So we can // "backdate" its `changed_at` revision to be the same as the // old value. - let mut changed_at = revision_now; if let Some(QueryState::Memoized(old_memo)) = &old_value { - if old_memo.stamped_value.value == value { - changed_at = old_memo.stamped_value.changed_at; + if old_memo.stamped_value.value == stamped_value.value { + assert!(old_memo.stamped_value.changed_at <= stamped_value.changed_at); + stamped_value.changed_at = old_memo.stamped_value.changed_at; } } @@ -186,10 +186,7 @@ where let old_value = map_write.insert( key.clone(), QueryState::Memoized(Memo { - stamped_value: StampedValue { - value: value.clone(), - changed_at, - }, + stamped_value: stamped_value.clone(), inputs, verified_at: revision_now, }), @@ -203,7 +200,7 @@ where ); } - Ok(StampedValue { value, changed_at }) + Ok(stamped_value) } } @@ -218,12 +215,11 @@ where key: &Q::Key, descriptor: &QC::QueryDescriptor, ) -> Result { - let StampedValue { - value, - changed_at: _, - } = self.read(query, key, &descriptor)?; + let StampedValue { value, changed_at } = self.read(query, key, &descriptor)?; - query.salsa_runtime().report_query_read(descriptor); + query + .salsa_runtime() + .report_query_read(descriptor, changed_at); Ok(value) } diff --git a/src/runtime.rs b/src/runtime.rs index 9a58d7d..2f65571 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -92,7 +92,7 @@ where query: &QC, descriptor: &QC::QueryDescriptor, key: &Q::Key, - ) -> (Q::Value, QueryDescriptorSet) + ) -> (StampedValue, QueryDescriptorSet) where Q: Query, { @@ -111,7 +111,11 @@ where let value = Q::execute(query, key.clone()); // Extract accumulated inputs. - let ActiveQuery { subqueries, .. } = { + let ActiveQuery { + subqueries, + changed_at, + .. + } = { let mut local_state = self.local_state.borrow_mut(); // Sanity check: pushes and pops should be balanced. @@ -120,7 +124,7 @@ where local_state.query_stack.pop().unwrap() }; - (value, subqueries) + (StampedValue { value, changed_at }, subqueries) } /// Reports that the currently active query read the result from @@ -131,9 +135,9 @@ where /// - `descriptor`: the query whose result was read /// - `changed_revision`: the last revision in which the result of that /// query had changed - crate fn report_query_read(&self, descriptor: &QC::QueryDescriptor) { + crate fn report_query_read(&self, descriptor: &QC::QueryDescriptor, changed_at: Revision) { if let Some(top_query) = self.local_state.borrow_mut().query_stack.last_mut() { - top_query.add_read(descriptor); + top_query.add_read(descriptor, changed_at); } } @@ -172,6 +176,9 @@ struct ActiveQuery { /// What query is executing descriptor: QC::QueryDescriptor, + /// Records the maximum revision where any subquery changed + changed_at: Revision, + /// Each subquery subqueries: QueryDescriptorSet, } @@ -180,12 +187,14 @@ impl ActiveQuery { fn new(descriptor: QC::QueryDescriptor) -> Self { ActiveQuery { descriptor, + changed_at: Revision::ZERO, subqueries: QueryDescriptorSet::new(), } } - fn add_read(&mut self, subquery: &QC::QueryDescriptor) { + fn add_read(&mut self, subquery: &QC::QueryDescriptor, changed_at: Revision) { self.subqueries.insert(subquery.clone()); + self.changed_at = self.changed_at.max(changed_at); } } diff --git a/src/volatile.rs b/src/volatile.rs index 5312ffd..8e1ab33 100644 --- a/src/volatile.rs +++ b/src/volatile.rs @@ -1,4 +1,5 @@ use crate::runtime::Revision; +use crate::runtime::StampedValue; use crate::CycleDetected; use crate::Query; use crate::QueryContext; @@ -54,16 +55,24 @@ where return Err(CycleDetected); } - // FIXME: Should we even call `execute_query_implementation` - // here? Or should we just call `Q::execute`, and maybe - // separate out the `push`/`pop` operations. - let (value, _inputs) = query + let ( + StampedValue { + value, + changed_at: _, + }, + _inputs, + ) = query .salsa_runtime() .execute_query_implementation::(query, descriptor, key); + let was_in_progress = self.in_progress.lock().remove(key); assert!(was_in_progress); - query.salsa_runtime().report_query_read(descriptor); + let revision_now = query.salsa_runtime().current_revision(); + + query + .salsa_runtime() + .report_query_read(descriptor, revision_now); Ok(value) }