From dc1bbb317ba799865b5ad9e76e0e7b7f22d23b10 Mon Sep 17 00:00:00 2001 From: Arvid Gerstmann Date: Tue, 29 Oct 2024 14:32:36 +0000 Subject: [PATCH] improved cycle detection --- ferrunix-core/src/cycle_detection.rs | 20 ++- ferrunix-core/src/types.rs | 30 ++-- ferrunix/Cargo.toml | 1 + ferrunix/tests/it/main.rs | 1 + ferrunix/tests/it/stress.rs | 215 +++++++++++++++++++++++++++ 5 files changed, 247 insertions(+), 20 deletions(-) create mode 100644 ferrunix/tests/it/stress.rs diff --git a/ferrunix-core/src/cycle_detection.rs b/ferrunix-core/src/cycle_detection.rs index 8aa7100..f00dbf5 100644 --- a/ferrunix-core/src/cycle_detection.rs +++ b/ferrunix-core/src/cycle_detection.rs @@ -118,7 +118,7 @@ impl DependencyValidator { where T: Registerable, { - let visitor: Visitor = |this| { + let visitor = Visitor(|this, _visitors| { if let Some(index) = this.visited.read().get(&TypeId::of::()) { return *index; } @@ -134,7 +134,7 @@ impl DependencyValidator { } index - }; + }); // TODO: Make this lock free! { @@ -160,7 +160,7 @@ impl DependencyValidator { >( &self, ) { - let visitor: Visitor = |this| { + let visitor = Visitor(|this, visitors| { // We already visited this type. if let Some(index) = this.visited.read().get(&TypeId::of::()) { return *index; @@ -188,8 +188,8 @@ impl DependencyValidator { } // Never seen the type before, visit it. - if let Some(visitor) = this.visitor.read().get(type_id) { - let index = (visitor)(this); + if let Some(visitor) = visitors.get(type_id) { + let index = (visitor.0)(this, visitors); this.graph.write().add_edge(current, index, ()); continue; } @@ -220,7 +220,7 @@ impl DependencyValidator { } current - }; + }); // TODO: Make this lock free! { @@ -246,7 +246,6 @@ impl DependencyValidator { 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() { - eprintln!("use cache",); // Validation is cached. { let missing = self.missing.read(); @@ -267,18 +266,18 @@ impl DependencyValidator { } // Validation is **not** cached. - eprintln!("calculate validation",); if self .visitor_visited .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) .is_ok() { - eprintln!("VISIT!"); // Make sure we have all types registered. let visitor = self.visitor.read(); for (_type_id, cb) in visitor.iter() { - (cb)(self); + // To avoid a dead lock due to other visitors needing to be called, we pass in the + // visitors hashmap. + (cb.0)(self, &visitor); } } @@ -304,7 +303,6 @@ impl DependencyValidator { // Cache the result. { let mut cache = self.validation_cache.write(); - eprintln!("write to cache"); *cache = Some(result); } diff --git a/ferrunix-core/src/types.rs b/ferrunix-core/src/types.rs index 7aa5ab3..689f41c 100644 --- a/ferrunix-core/src/types.rs +++ b/ferrunix-core/src/types.rs @@ -14,7 +14,7 @@ mod private { /// Types that are enabled when the `multithread` feature is set. #[cfg(all(feature = "multithread", not(feature = "tokio")))] mod sync { - use std::any::Any; + use std::any::{Any, TypeId}; use crate::cycle_detection::DependencyValidator; use crate::object_builder::{SingletonGetter, TransientBuilder}; @@ -46,8 +46,12 @@ mod sync { Box; // Alias types used in [`DependencyValidator`]. - pub(crate) type Visitor = - fn(&DependencyValidator) -> petgraph::graph::NodeIndex; + pub(crate) struct Visitor( + pub(crate) fn( + &DependencyValidator, + &HashMap, + ) -> petgraph::graph::NodeIndex, + ); /// A generic constructor for singletons. /// @@ -125,7 +129,7 @@ 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; + use std::any::{Any, TypeId}; use crate::cycle_detection::DependencyValidator; use crate::object_builder::{SingletonGetter, TransientBuilder}; @@ -175,8 +179,12 @@ mod unsync { pub(crate) type BoxedSingletonGetter = Box; // Alias types used in [`DependencyValidator`]. - pub(crate) type Visitor = - fn(&DependencyValidator) -> petgraph::graph::NodeIndex; + pub(crate) struct Visitor( + pub(crate) fn( + &DependencyValidator, + &HashMap, + ) -> petgraph::graph::NodeIndex, + ); /// A generic constructor for singletons. /// @@ -248,7 +256,7 @@ mod unsync { #[cfg(feature = "tokio")] mod tokio_ext { - use std::any::Any; + use std::any::{Any, TypeId}; use crate::cycle_detection::DependencyValidator; @@ -257,8 +265,12 @@ mod tokio_ext { pub(crate) type RefAny = Ref; // Alias types used in [`DependencyValidator`]. - pub(crate) type Visitor = - fn(&DependencyValidator) -> petgraph::graph::NodeIndex; + pub(crate) struct Visitor( + pub(crate) fn( + &DependencyValidator, + &HashMap, + ) -> petgraph::graph::NodeIndex, + ); // `RwLock` types. pub(crate) type NonAsyncRwLock = parking_lot::RwLock; diff --git a/ferrunix/Cargo.toml b/ferrunix/Cargo.toml index 78de247..87afb08 100644 --- a/ferrunix/Cargo.toml +++ b/ferrunix/Cargo.toml @@ -32,6 +32,7 @@ thiserror = "1" tokio = { version = "=1.24.2", features = ["full"] } async-trait = "0.1" skeptic = { git = "https://github.com/Leandros/rust-skeptic.git", rev = "45d69d27c3ee1f0cf66072053bb6f88c1be0e07b" } +paste = "1.0" [build-dependencies] skeptic = { git = "https://github.com/Leandros/rust-skeptic.git", rev = "45d69d27c3ee1f0cf66072053bb6f88c1be0e07b" } diff --git a/ferrunix/tests/it/main.rs b/ferrunix/tests/it/main.rs index 3a9f57e..1f7f88c 100644 --- a/ferrunix/tests/it/main.rs +++ b/ferrunix/tests/it/main.rs @@ -2,6 +2,7 @@ mod common; mod cycle_test; +mod stress; mod validate_traits; #[cfg(all(feature = "derive", feature = "tokio"))] diff --git a/ferrunix/tests/it/stress.rs b/ferrunix/tests/it/stress.rs new file mode 100644 index 0000000..df929b0 --- /dev/null +++ b/ferrunix/tests/it/stress.rs @@ -0,0 +1,215 @@ +#![cfg(feature = "multithread")] +use std::sync::Arc; + +use ferrunix::Registry; + +macro_rules! make_type { + ($base:ident) => { + paste! { + #[allow(non_snake_case)] + #[derive(Debug, Default)] + pub(super) struct $base {} + + impl $base { + #[allow(non_snake_case)] + pub(super) fn register(registry: &ferrunix::Registry) { + registry.transient(|| $base::default()); + } + } + } + }; + + ($base:ident, $($deps:ident),*) => { + paste! { + #[allow(non_snake_case)] + #[derive(Debug, Default)] + pub(super) struct $base { + $( + pub(super) [<_ $deps>]: $deps, + )* + } + + impl $base { + #[allow(non_snake_case)] + pub(super) fn register(registry: &ferrunix::Registry) { + registry + .with_deps::<_, ($( + ferrunix::Transient<$deps>, + )*)>() + .transient(|($( + [<_ $deps>], + )*)| $base {$( + [<_ $deps>]: [<_ $deps>].get(), + )*}); + } + } + } + }; +} + +macro_rules! make_many_types { + ($modname:ident) => { + mod $modname { + use paste::paste; + + make_type!(Config); + make_type!(TypeZero, Dep0); + make_type!(Dep0, Dep1); + make_type!(Dep1, Dep2); + make_type!(Dep2, Dep3); + make_type!(Dep3, Dep4); + make_type!(Dep4); + + make_type!(TypeNoDeps0); + make_type!(TypeNoDeps1); + make_type!(TypeNoDeps2); + make_type!(TypeNoDeps3); + make_type!(TypeNoDeps4); + make_type!(TypeNoDeps5); + make_type!(TypeNoDeps6); + make_type!(TypeNoDeps7); + make_type!(TypeNoDeps8); + make_type!(TypeNoDeps9); + make_type!(TypeNoDeps10); + + make_type!( + TypeManyDeps0, + TypeNoDeps0, + TypeNoDeps1, + TypeNoDeps2, + TypeNoDeps3, + TypeNoDeps4, + TypeNoDeps5 + ); + + make_type!( + TypeManyDeps1, + TypeNoDeps6, + TypeNoDeps7, + TypeNoDeps8, + TypeNoDeps9, + TypeNoDeps10 + ); + + make_type!(TypeSingleDep0, Config); + make_type!(TypeSingleDep1, Config); + make_type!(TypeSingleDep2, Config); + make_type!(TypeSingleDep3, Config); + make_type!(TypeSingleDep4, Config); + make_type!(TypeSingleDep5, Config); + make_type!(TypeSingleDep6, Config); + make_type!(TypeSingleDep7, Config); + make_type!(TypeSingleDep8, Config); + make_type!(TypeSingleDep9, Config); + make_type!(TypeSingleDep10, Config); + } + }; +} + +macro_rules! register_all_types { + ($modname:ident, $reg:ident) => { + $modname::Config::register(&$reg); + $modname::TypeZero::register(&$reg); + $modname::Dep0::register(&$reg); + $modname::Dep1::register(&$reg); + $modname::Dep2::register(&$reg); + $modname::Dep3::register(&$reg); + $modname::Dep4::register(&$reg); + + $modname::TypeNoDeps0::register(&$reg); + $modname::TypeNoDeps1::register(&$reg); + $modname::TypeNoDeps2::register(&$reg); + $modname::TypeNoDeps3::register(&$reg); + $modname::TypeNoDeps4::register(&$reg); + $modname::TypeNoDeps5::register(&$reg); + $modname::TypeNoDeps6::register(&$reg); + $modname::TypeNoDeps7::register(&$reg); + $modname::TypeNoDeps8::register(&$reg); + $modname::TypeNoDeps9::register(&$reg); + $modname::TypeNoDeps10::register(&$reg); + + $modname::TypeManyDeps0::register(&$reg); + $modname::TypeManyDeps1::register(&$reg); + + $modname::TypeSingleDep0::register(&$reg); + $modname::TypeSingleDep1::register(&$reg); + $modname::TypeSingleDep2::register(&$reg); + $modname::TypeSingleDep3::register(&$reg); + $modname::TypeSingleDep4::register(&$reg); + $modname::TypeSingleDep5::register(&$reg); + $modname::TypeSingleDep6::register(&$reg); + $modname::TypeSingleDep7::register(&$reg); + $modname::TypeSingleDep8::register(&$reg); + $modname::TypeSingleDep9::register(&$reg); + $modname::TypeSingleDep10::register(&$reg); + + $reg.validate_all().unwrap(); + }; +} + +make_many_types!(manytypes0); +make_many_types!(manytypes1); +make_many_types!(manytypes2); +make_many_types!(manytypes3); +make_many_types!(manytypes4); +make_many_types!(manytypes5); +make_many_types!(manytypes6); + +#[test] +fn stress_registration() { + let registry = Arc::new(Registry::empty()); + + let handle0 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes0, registry); + }) + }; + let handle1 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes1, registry); + }) + }; + let handle2 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes2, registry); + }) + }; + let handle3 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes3, registry); + }) + }; + let handle4 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes4, registry); + }) + }; + let handle5 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes5, registry); + }) + }; + let handle6 = { + let registry = Arc::clone(®istry); + std::thread::spawn(move || { + register_all_types!(manytypes6, registry); + }) + }; + + handle0.join().unwrap(); + handle1.join().unwrap(); + handle2.join().unwrap(); + handle3.join().unwrap(); + handle4.join().unwrap(); + handle5.join().unwrap(); + handle6.join().unwrap(); + + registry.validate_all().unwrap(); + // println!("{}", registry.dotgraph().unwrap()); +}