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.
This commit is contained in:
Niko Matsakis 2021-10-29 05:23:20 -04:00
parent 5bd2cbcb20
commit de7ac896bc
3 changed files with 24 additions and 19 deletions

View file

@ -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<Q::Value>, cycle: Vec<DatabaseKeyIndex>) {
fn proceed(mut self, new_value: &StampedValue<Q::Value>, cycle: Option<CycleParticipants>) {
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<Q::Value>, Vec<DatabaseKeyIndex>)>,
new_value: Option<(&StampedValue<Q::Value>, Option<CycleParticipants>)>,
) {
let mut write = self.slot.state.write();

View file

@ -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<S>;
}
pub type CycleParticipants = Arc<Vec<DatabaseKeyIndex>>;
/// 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<DatabaseKeyIndex>,
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(())

View file

@ -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<DatabaseKeyIndex>,
pub(crate) cycle: Option<CycleParticipants>,
}
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<FxIndexSet<DatabaseKeyIndex>>,
/// Stores the entire cycle, if one is found and this query is part of it.
cycle: Vec<DatabaseKeyIndex>,
cycle: Option<CycleParticipants>,
}
pub(crate) struct ComputedQueryResult<V> {
@ -520,7 +521,7 @@ pub(crate) struct ComputedQueryResult<V> {
pub(crate) dependencies: Option<FxIndexSet<DatabaseKeyIndex>>,
/// The cycle if one occured while computing this value
pub(crate) cycle: Vec<DatabaseKeyIndex>,
pub(crate) cycle: Option<CycleParticipants>,
}
impl ActiveQuery {
@ -530,7 +531,7 @@ impl ActiveQuery {
durability: max_durability,
changed_at: Revision::start(),
dependencies: Some(FxIndexSet::default()),
cycle: Vec::new(),
cycle: None,
}
}