From ded1517a5c623be79e1ba6db0deee59cb0806778 Mon Sep 17 00:00:00 2001 From: Arvid Gerstmann Date: Wed, 30 Oct 2024 09:59:40 +0000 Subject: [PATCH] snap --- ferrunix-core/src/cycle_detection.rs | 199 +++++++++++++-------------- ferrunix-core/src/lib.rs | 1 + ferrunix-core/src/types.rs | 47 +++---- 3 files changed, 116 insertions(+), 131 deletions(-) diff --git a/ferrunix-core/src/cycle_detection.rs b/ferrunix-core/src/cycle_detection.rs index f00dbf5..cfd875a 100644 --- a/ferrunix-core/src/cycle_detection.rs +++ b/ferrunix-core/src/cycle_detection.rs @@ -80,24 +80,10 @@ pub(crate) struct DependencyValidator { /// The visitor callbacks. Those are necessary because we only want to register each type once /// we have collected them all. visitor: NonAsyncRwLock>, - /// Dependency graph. - graph: - NonAsyncRwLock>, - /// Cache of all previously visited types. To avoid infinite recursion and as an optimization. - visited: NonAsyncRwLock>, - /// All missing dependencies. - missing: NonAsyncRwLock>, /// Whether we have already visited all visitors. visitor_visited: AtomicBool, - /// Cached validation result. - validation_cache: NonAsyncRwLock< - Option< - Result< - Vec, - petgraph::algo::Cycle, - >, - >, - >, + /// Context for visitors. + context: NonAsyncRwLock, } impl DependencyValidator { @@ -105,11 +91,8 @@ impl DependencyValidator { pub(crate) fn new() -> Self { Self { visitor: NonAsyncRwLock::new(HashMap::new()), - graph: NonAsyncRwLock::new(petgraph::Graph::new()), - visited: NonAsyncRwLock::new(HashMap::new()), - missing: NonAsyncRwLock::new(HashMap::new()), visitor_visited: AtomicBool::new(false), - validation_cache: NonAsyncRwLock::new(None), + context: NonAsyncRwLock::new(VisitorContext::new()), } } @@ -118,28 +101,18 @@ impl DependencyValidator { where T: Registerable, { - let visitor = Visitor(|this, _visitors| { - if let Some(index) = this.visited.read().get(&TypeId::of::()) { + let visitor = Visitor(|_this, _visitors, context| { + if let Some(index) = context.visited.get(&TypeId::of::()) { return *index; } - let index = { - let mut graph = this.graph.write(); - graph.add_node(std::any::type_name::()) - }; + let index = context.graph.add_node(std::any::type_name::()); - { - let mut visited = this.visited.write(); - visited.insert(TypeId::of::(), index); - } + context.visited.insert(TypeId::of::(), index); index }); - // TODO: Make this lock free! - { - let _ = self.validation_cache.write().take(); - } self.visitor_visited.store(false, Ordering::Release); self.visitor.write().insert(TypeId::of::(), visitor); } @@ -160,21 +133,17 @@ impl DependencyValidator { >( &self, ) { - let visitor = Visitor(|this, visitors| { + let visitor = Visitor(|this, visitors, context| { // We already visited this type. - if let Some(index) = this.visited.read().get(&TypeId::of::()) { + if let Some(index) = context.visited.get(&TypeId::of::()) { return *index; } - let current = { - let mut graph = this.graph.write(); - graph.add_node(std::any::type_name::()) - }; + let current = context.graph.add_node(std::any::type_name::()); // We visited this type. This must be added before we visit dependencies. { - let mut visited = this.visited.write(); - visited.insert(TypeId::of::(), current); + context.visited.insert(TypeId::of::(), current); } let type_ids = @@ -182,24 +151,25 @@ impl DependencyValidator { for (type_id, type_name) in &type_ids { // We have been to the dependency type before, we don't need to do it again. - if let Some(index) = this.visited.read().get(type_id) { - this.graph.write().add_edge(current, *index, ()); + if let Some(index) = context.visited.get(type_id) { + context.graph.add_edge(current, *index, ()); continue; } // Never seen the type before, visit it. if let Some(visitor) = visitors.get(type_id) { - let index = (visitor.0)(this, visitors); - this.graph.write().add_edge(current, index, ()); + let index = (visitor.0)(this, visitors, context); + context.graph.add_edge(current, index, ()); continue; } { - let mut missing = this.missing.write(); - if let Some(ty) = missing.get_mut(&TypeId::of::()) { + if let Some(ty) = + context.missing.get_mut(&TypeId::of::()) + { ty.deps.push((*type_id, type_name)); } else { - missing.insert( + context.missing.insert( TypeId::of::(), MissingDependencies { ty: ( @@ -222,10 +192,6 @@ impl DependencyValidator { current }); - // TODO: Make this lock free! - { - let _ = self.validation_cache.write().take(); - } self.visitor_visited.store(false, Ordering::Release); self.visitor.write().insert(TypeId::of::(), visitor); } @@ -245,66 +211,62 @@ impl DependencyValidator { /// are fulfillable and there are no cycles in the graph. pub(crate) fn validate_all(&self) -> Result<(), ValidationError> { // This **must** be a separate `if`, otherwise the lock is held also in the `else`. - if let Some(cache) = &*self.validation_cache.read() { - // Validation is cached. - { - let missing = self.missing.read(); - if missing.len() > 0 { - let mut vec = Vec::with_capacity(missing.len()); - for (_, ty) in missing.iter() { - vec.push(ty.clone()); - } - return Err(ValidationError::Missing(vec)); - } - } - - // EARLY RETURN ABSOLUTELY REQUIRED! - return match cache { - Ok(_) => Ok(()), - Err(_err) => Err(ValidationError::Cycle), - }; - } + // if let Some(cache) = &*self.validation_cache.read() { + // // Validation is cached. + // { + // let missing = self.missing_cache.read(); + // if missing.len() > 0 { + // let mut vec = Vec::with_capacity(missing.len()); + // for (_, ty) in missing.iter() { + // vec.push(ty.clone()); + // } + // return Err(ValidationError::Missing(vec)); + // } + // } + + // // EARLY RETURN ABSOLUTELY REQUIRED! + // return match cache { + // Ok(_) => Ok(()), + // Err(_err) => Err(ValidationError::Cycle), + // }; + // } // Validation is **not** cached. - if self - .visitor_visited - .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) - .is_ok() + // if self + // .visitor_visited + // .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) + // .is_ok() + // { + let mut context = self.context.write(); + + // Make sure we have all types registered. { - // Make sure we have all types registered. let visitor = self.visitor.read(); for (_type_id, cb) in visitor.iter() { // To avoid a dead lock due to other visitors needing to be called, we pass in the // visitors hashmap. - (cb.0)(self, &visitor); + (cb.0)(self, &visitor, &mut context); } } - { - let missing = self.missing.read(); - if missing.len() > 0 { - let mut vec = Vec::with_capacity(missing.len()); - for (_, ty) in missing.iter() { - vec.push(ty.clone()); - } - return Err(ValidationError::Missing(vec)); + if !context.missing.is_empty() { + let mut vec = Vec::with_capacity(context.missing.len()); + for (_, ty) in &context.missing { + vec.push(ty.clone()); } + return Err(ValidationError::Missing(vec)); } - let graph = self.graph.read(); - let mut space = petgraph::algo::DfsSpace::new(&*graph); - let result = petgraph::algo::toposort(&*graph, Some(&mut space)); - let ret = match result { - Ok(_) => Ok(()), - Err(_) => Err(ValidationError::Cycle), - }; + let mut space = petgraph::algo::DfsSpace::new(&context.graph); + context.validation_cache = + Some(petgraph::algo::toposort(&context.graph, Some(&mut space))); - // Cache the result. - { - let mut cache = self.validation_cache.write(); - *cache = Some(result); - } + let ret = match context.validation_cache { + Some(Ok(_)) => Ok(()), + Some(Err(_)) => Err(ValidationError::Cycle), + _ => unreachable!("it's written above"), + }; ret } @@ -322,12 +284,49 @@ impl DependencyValidator { pub(crate) fn dotgraph(&self) -> Result { self.validate_all()?; - let graph = self.graph.read(); + let context = self.context.read(); let dot = petgraph::dot::Dot::with_config( - &*graph, + &context.graph, &[petgraph::dot::Config::EdgeNoLabel], ); Ok(format!("{dot:?}")) } } + +/// Context that's passed into every `visitor`. +pub(crate) struct VisitorContext { + /// Dependency graph. + graph: petgraph::Graph<&'static str, (), petgraph::Directed>, + /// All missing dependencies. + missing: HashMap, + /// Cache of all previously visited types. To avoid infinite recursion and as an optimization. + visited: HashMap, + /// Cached validation result. + validation_cache: Option< + Result< + Vec, + petgraph::algo::Cycle, + >, + >, +} + +impl VisitorContext { + /// Create a new default context. + pub fn new() -> Self { + Self { + graph: petgraph::Graph::new(), + missing: HashMap::new(), + visited: HashMap::new(), + validation_cache: None, + } + } + + /// Reset the context. + pub fn reset(&mut self) { + self.graph.clear(); + self.missing.clear(); + self.visited.clear(); + self.validation_cache = None; + } +} diff --git a/ferrunix-core/src/lib.rs b/ferrunix-core/src/lib.rs index b7731b8..226ede9 100644 --- a/ferrunix-core/src/lib.rs +++ b/ferrunix-core/src/lib.rs @@ -18,6 +18,7 @@ pub mod cycle_detection; pub mod dependencies; pub mod dependency_builder; pub mod error; +pub mod lazy_locked_cache; pub mod object_builder; pub mod registration; pub mod registry; diff --git a/ferrunix-core/src/types.rs b/ferrunix-core/src/types.rs index 689f41c..59bdc2e 100644 --- a/ferrunix-core/src/types.rs +++ b/ferrunix-core/src/types.rs @@ -11,12 +11,24 @@ mod private { pub struct SealToken; } +use std::any::TypeId; + +use crate::cycle_detection::{DependencyValidator, VisitorContext}; + +// Alias types used in [`DependencyValidator`]. +pub(crate) struct Visitor( + pub(crate) fn( + &DependencyValidator, + &HashMap, + &mut VisitorContext, + ) -> petgraph::graph::NodeIndex, +); + /// Types that are enabled when the `multithread` feature is set. #[cfg(all(feature = "multithread", not(feature = "tokio")))] mod sync { - use std::any::{Any, TypeId}; + use std::any::Any; - use crate::cycle_detection::DependencyValidator; use crate::object_builder::{SingletonGetter, TransientBuilder}; pub(crate) type OnceCell = once_cell::sync::OnceCell; @@ -45,14 +57,6 @@ mod sync { pub(crate) type BoxedSingletonGetter = Box; - // Alias types used in [`DependencyValidator`]. - pub(crate) struct Visitor( - pub(crate) fn( - &DependencyValidator, - &HashMap, - ) -> petgraph::graph::NodeIndex, - ); - /// A generic constructor for singletons. /// /// This is a marker trait to identify all valid constructors usable by singletons. @@ -129,9 +133,8 @@ mod sync { /// Types that are enabled when the `multithread` feature is **NOT** set. #[cfg(all(not(feature = "multithread"), not(feature = "tokio")))] mod unsync { - use std::any::{Any, TypeId}; + use std::any::Any; - use crate::cycle_detection::DependencyValidator; use crate::object_builder::{SingletonGetter, TransientBuilder}; pub(crate) type OnceCell = once_cell::unsync::OnceCell; @@ -178,14 +181,6 @@ mod unsync { pub(crate) type BoxedTransientBuilder = Box; pub(crate) type BoxedSingletonGetter = Box; - // Alias types used in [`DependencyValidator`]. - pub(crate) struct Visitor( - pub(crate) fn( - &DependencyValidator, - &HashMap, - ) -> petgraph::graph::NodeIndex, - ); - /// A generic constructor for singletons. /// /// This is a marker trait to identify all valid constructors usable by singletons. @@ -256,22 +251,12 @@ mod unsync { #[cfg(feature = "tokio")] mod tokio_ext { - use std::any::{Any, TypeId}; - - use crate::cycle_detection::DependencyValidator; + use std::any::Any; // Alias types used in [`Registry`]. pub(crate) type BoxedAny = Box; pub(crate) type RefAny = Ref; - // Alias types used in [`DependencyValidator`]. - pub(crate) struct Visitor( - pub(crate) fn( - &DependencyValidator, - &HashMap, - ) -> petgraph::graph::NodeIndex, - ); - // `RwLock` types. pub(crate) type NonAsyncRwLock = parking_lot::RwLock; pub(crate) type RwLock = ::tokio::sync::RwLock;