From de7ac896bcb6a0ab8b3ec0f8a1342a68f96cc58f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 29 Oct 2021 05:23:20 -0400 Subject: [PATCH] put cycle participants into an arc Because it bugs me to clone the vector. Maybe silly, I admit, since cycle recovery is not the hot path. But by that same token, we now spend only 1 word for a null pointer instead of 4 words for a (usually empty) vector. --- src/derived/slot.rs | 21 +++++++++++---------- src/plumbing.rs | 7 +++++-- src/runtime.rs | 15 ++++++++------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/derived/slot.rs b/src/derived/slot.rs index 1e0b46d..fd777e2 100644 --- a/src/derived/slot.rs +++ b/src/derived/slot.rs @@ -4,6 +4,7 @@ use crate::durability::Durability; use crate::lru::LruIndex; use crate::lru::LruNode; use crate::plumbing::CycleError; +use crate::plumbing::CycleParticipants; use crate::plumbing::{CycleDetected, CycleRecoveryStrategy}; use crate::plumbing::{DatabaseOps, QueryFunction}; use crate::revision::Revision; @@ -203,7 +204,7 @@ where // just as we entered the cycle. Therefore there is no values to invalidate // and no need to call a cycle handler so we do not need to return the // actual cycle - Vec::new(), + None, ); return Ok(value); @@ -218,8 +219,8 @@ where Q::execute(db, self.key.clone()) }); - if !result.cycle.is_empty() { - result.value = Q::cycle_fallback(db, &result.cycle, &self.key); + if let Some(cycle) = &result.cycle { + result.value = Q::cycle_fallback(db, cycle, &self.key); } // We assume that query is side-effect free -- that is, does @@ -346,14 +347,14 @@ where "received result from other thread: cycle = {:?}", result.cycle ); - if result.cycle.is_empty() { - ProbeState::Retry - } else { + if let Some(cycle) = &result.cycle { ProbeState::UpToDate(Ok(StampedValue { - value: Q::cycle_fallback(db, &result.cycle, &self.key), + value: Q::cycle_fallback(db, &cycle, &self.key), durability: result.value.durability, changed_at: result.value.changed_at, })) + } else { + ProbeState::Retry } } @@ -504,7 +505,7 @@ where "received result from other thread: cycle = {:?}", result.cycle ); - if !result.cycle.is_empty() { + if result.cycle.is_some() { // Consider cycles to have changed. true } else { @@ -723,7 +724,7 @@ where /// Proceed with our panic guard by overwriting the placeholder for `key`. /// Once that completes, ensure that our deconstructor is not run once we /// are out of scope. - fn proceed(mut self, new_value: &StampedValue, cycle: Vec) { + fn proceed(mut self, new_value: &StampedValue, cycle: Option) { self.overwrite_placeholder(Some((new_value, cycle))); std::mem::forget(self) } @@ -733,7 +734,7 @@ where /// then notify them. fn overwrite_placeholder( &mut self, - new_value: Option<(&StampedValue, Vec)>, + new_value: Option<(&StampedValue, Option)>, ) { let mut write = self.slot.state.write(); diff --git a/src/plumbing.rs b/src/plumbing.rs index 24b71b9..22dd676 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -10,6 +10,7 @@ use crate::RuntimeId; use std::borrow::Borrow; use std::fmt::Debug; use std::hash::Hash; +use std::sync::Arc; pub use crate::derived::DependencyStorage; pub use crate::derived::MemoizedStorage; @@ -237,11 +238,13 @@ where Q::Key: Borrow; } +pub type CycleParticipants = Arc>; + /// The error returned when a query could not be resolved due to a cycle #[derive(Eq, PartialEq, Clone, Debug)] pub struct CycleError { /// The queries that were part of the cycle - pub(crate) cycle: Vec, + pub(crate) cycle: CycleParticipants, pub(crate) changed_at: Revision, pub(crate) durability: Durability, // pub(crate) recovery_strategy: CycleRecoveryStrategy, @@ -274,7 +277,7 @@ impl CycleError { impl std::fmt::Display for CycleError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { writeln!(f, "Internal error, cycle detected:\n")?; - for i in &self.cycle { + for i in &*self.cycle { writeln!(f, "{:?}", i)?; } Ok(()) diff --git a/src/runtime.rs b/src/runtime.rs index ae2286e..01ed922 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1,5 +1,5 @@ use crate::durability::Durability; -use crate::plumbing::{CycleDetected, CycleError, CycleRecoveryStrategy}; +use crate::plumbing::{CycleDetected, CycleError, CycleParticipants, CycleRecoveryStrategy}; use crate::revision::{AtomicRevision, Revision}; use crate::{Cancelled, Database, DatabaseKeyIndex, Event, EventKind}; use log::debug; @@ -45,7 +45,7 @@ pub struct Runtime { #[derive(Clone, Debug)] pub(crate) struct WaitResult { pub(crate) value: StampedValue<()>, - pub(crate) cycle: Vec, + pub(crate) cycle: Option, } impl Default for Runtime { @@ -309,10 +309,11 @@ impl Runtime { let mut stack = self.local_state.take_query_stack(); let mut dg = self.shared_state.dependency_graph.lock(); dg.for_each_cycle_participant(error.from, &mut stack, database_key_index, error.to, |aq| { - cycle_participants.push(aq.database_key_index) + cycle_participants.push(aq.database_key_index); }); + let cycle_participants = Arc::new(cycle_participants); dg.for_each_cycle_participant(error.from, &mut stack, database_key_index, error.to, |aq| { - aq.cycle = cycle_participants.clone() + aq.cycle = Some(cycle_participants.clone()); }); self.local_state.restore_query_stack(stack); let crs = self.mutual_cycle_recovery_strategy(db, &cycle_participants); @@ -501,7 +502,7 @@ struct ActiveQuery { dependencies: Option>, /// Stores the entire cycle, if one is found and this query is part of it. - cycle: Vec, + cycle: Option, } pub(crate) struct ComputedQueryResult { @@ -520,7 +521,7 @@ pub(crate) struct ComputedQueryResult { pub(crate) dependencies: Option>, /// The cycle if one occured while computing this value - pub(crate) cycle: Vec, + pub(crate) cycle: Option, } impl ActiveQuery { @@ -530,7 +531,7 @@ impl ActiveQuery { durability: max_durability, changed_at: Revision::start(), dependencies: Some(FxIndexSet::default()), - cycle: Vec::new(), + cycle: None, } }