From 91fc15e7cdd460226bac37650bb8648179c7438a Mon Sep 17 00:00:00 2001 From: Gregory Conrad Date: Sat, 1 Jul 2023 23:50:59 -0400 Subject: [PATCH] refactor: split up files and fix TODOs --- rearch/src/capsule_reader.rs | 61 +++++++++ rearch/src/gc.rs | 74 ---------- rearch/src/lib.rs | 123 +++++++++++------ rearch/src/side_effect_registrar.rs | 88 ++++++++++++ rearch/src/txn.rs | 203 ++++++++++++++++++++++++++++ 5 files changed, 433 insertions(+), 116 deletions(-) create mode 100644 rearch/src/capsule_reader.rs delete mode 100644 rearch/src/gc.rs create mode 100644 rearch/src/side_effect_registrar.rs create mode 100644 rearch/src/txn.rs diff --git a/rearch/src/capsule_reader.rs b/rearch/src/capsule_reader.rs new file mode 100644 index 0000000..fe2c713 --- /dev/null +++ b/rearch/src/capsule_reader.rs @@ -0,0 +1,61 @@ +use std::any::TypeId; + +use crate::{Capsule, ContainerWriteTxn}; + +/// Allows you to read the current data of capsules based on the given state of the container txn. +pub struct CapsuleReader<'scope, 'total> { + id: TypeId, + txn: &'scope mut ContainerWriteTxn<'total>, + // TODO mock utility, like MockCapsuleReaderBuilder::new().set(capsule, value).set(...).build() + // #[cfg(feature = "capsule-reader-mock")] + // mock: Option, +} + +impl<'scope, 'total> CapsuleReader<'scope, 'total> { + pub(crate) fn new(id: TypeId, txn: &'scope mut ContainerWriteTxn<'total>) -> Self { + Self { id, txn } + } + + /// Reads the current data of the supplied capsule, initializing it if needed. + /// Internally forms a dependency graph amongst capsules, so feel free to conditionally invoke + /// this function in case you only conditionally need a capsule's data. + /// + /// # Panics + /// Panics when a capsule attempts to read itself in its first build. + pub fn read(&mut self, capsule: C) -> C::Data { + let (this, other) = (self.id, TypeId::of::()); + if this == other { + return self.txn.try_read(&capsule).unwrap_or_else(|| { + let capsule_name = std::any::type_name::(); + panic!( + "Capsule {capsule_name} tried to read itself on its first build! {} {} {}", + "This is disallowed since the capsule doesn't have any data to read yet.", + "To avoid this issue, wrap the `read({capsule_name})` call in an if statement", + "with the `IsFirstBuildEffect`." + ); + }); + } + + // Get the value (and make sure the other manager is initialized!) + let data = self.txn.read_or_init(capsule); + + // Take care of some dependency housekeeping + self.txn.node_or_panic(other).dependents.insert(this); + self.txn.node_or_panic(this).dependencies.insert(other); + + data + } +} + +impl FnOnce<(A,)> for CapsuleReader<'_, '_> { + type Output = A::Data; + extern "rust-call" fn call_once(mut self, args: (A,)) -> Self::Output { + self.call_mut(args) + } +} + +impl FnMut<(A,)> for CapsuleReader<'_, '_> { + extern "rust-call" fn call_mut(&mut self, args: (A,)) -> Self::Output { + self.read(args.0) + } +} diff --git a/rearch/src/gc.rs b/rearch/src/gc.rs deleted file mode 100644 index cb6c3b1..0000000 --- a/rearch/src/gc.rs +++ /dev/null @@ -1,74 +0,0 @@ -use crate::ContainerWriteTxn; -impl ContainerWriteTxn<'_> { - // TODO maybe we can expose a singular method to do this in just one method in a new file - /* - #[must_use] - .start_garbage_collection() - // following few - .try_single_only(capsule) - .try_single_and_dependents(capsule) - .force_single_only(capsule) // unsafe - .force_single_and_dependents(capsule) // unsafe - // are the following two even possible easily? - .trim_dependencies() - .dont_trim_dependencies() - // and then if we want to thourough - .all_super_pure() - // better name for this one: - .commit(); // returns enum of not present, validation failed, success - // validation failed could have status where self as ip/sp and dependents as dne/ip/sp - */ - /* - /// Attempts to garbage collect the given Capsule and its dependent subgraph, disposing - /// the supplied Capsule and its dependent subgraph (and then returning `true`) only when - /// the supplied Capsule and its dependent subgraph consist only of super pure capsules. - pub fn try_garbage_collect_super_pure(&mut self) -> bool { - let id = TypeId::of::(); - let build_order = self.create_build_order_stack(id); - - let is_all_super_pure = build_order - .iter() - .all(|id| self.node_or_panic(*id).is_super_pure()); - - if is_all_super_pure { - for id in build_order { - self.dispose_single_node(id); - } - } - - is_all_super_pure - } - */ - - /* - /// Attempts to garbage collect the given Capsule and its dependent subgraph, disposing - /// the supplied Capsule and its dependent subgraph (and then returning `true`) only when: - /// - The dependent subgraph consists only of super pure capsules, or - /// - `dispose_impure_dependents` is set to true - /// - /// If you are not expecting the supplied Capsule to have dependents, - /// _set `dispose_impure_dependents` to false_, as setting it to true is *highly* unsafe. - /// In addition, in this case, it is also recommended to `assert!` the return value of this - /// function is true to ensure you didn't accidentally create other Capsule(s) which depend - /// on the supplied Capsule. - /// - /// # Safety - /// This is inherently unsafe because it violates the contract that capsules which - /// are not super pure will not be disposed, at least prior to their Container's disposal. - /// While invoking this method will never result in undefined behavior, - /// it can *easily* result in logic bugs, thus the unsafe marking. - /// This method is only exposed for the *very* few and specific use cases in which there - /// is a need to deeply integrate with rearch in order to prevent leaks, - /// such as when developing a UI framework and you need to listen to capsule updates. - pub unsafe fn force_garbage_collect( - dispose_impure_dependents: bool, - ) -> bool { - // handles these cases: - // - super pure, with impure dependents - // - impure, no dependents - // - impure, with super pure dependents - // - impure, with impure dependents - todo!() - } - */ -} diff --git a/rearch/src/lib.rs b/rearch/src/lib.rs index 04edbf8..0993143 100644 --- a/rearch/src/lib.rs +++ b/rearch/src/lib.rs @@ -43,12 +43,60 @@ pub use side_effect_registrar::*; mod txn; pub use txn::*; -mod gc; -pub use gc::*; +// TODO convert side effects to this format +/* +fn state(default: T) -> SideEffect { + move |register| { + let (data, rebuilder) = register.state(default); + (data, rebuilder) + } +} +fn lazy_state T>(init: F) -> SideEffect { + move |register| { + let (data, rebuilder) = register.state(LazyCell::new(init)); + (data.get_mut(), rebuilder) + } +} + +fn sync_persist(read: Read, write: Write) { + move |register| { + let ((state, set_state), write) = register(lazy_state(read), value(Arc::new(write))); + + let write = Arc::clone(write); + let persist = move |new_data| { + let persist_result = write(new_data); + set_state(persist_result); + }; + + (state, Arc::new(persist)) + } +} +*/ +// TODO attempt to see if we can rewrite SideEffectRebuilder without Box with new approach +// (but keep the Box inner part the same) +// TODO these next two require generic closures, perhaps ask about them again. +// https://github.com/rust-lang/rfcs/pull/1650 +// https://github.com/rust-lang/rust/issues/97362 +// TODO side effect registrar can also be a type alias to Box, +// and backwards compat can be maintained completely if we provide mock-creating API +// (because tuple side effect will always be treated as one) +// because ideally, registrar should be var-args. +// benchmark before and after so we don't make a bad time trade off +// TODO Consider turning CapsuleReader into type alias for Box +// for easy mocking and also easy read +// Either that or do the mocking feature thing +// benchmark before and after so we don't make a bad time trade off +// TODO capsule macro +// TODO side effect macro to remove the `move |register| {}` boilerplate // TODO aggressive garbage collection mode // (delete all created super pure capsules that aren't needed at end of a requested build) -// TODO capsule macro +// TODO listener function instead of exposed garbage collection. +// container.listen(|get| do_something(get(some_capsule))) +// returns a ListenerKeepAlive that removes listener once dropped +// internally implemented as an "impure" capsule that is dropped when keep alive drops +// what about the listener's dependencies? should they be trimmed if possible? +// maybe go off container's aggressiveness setting /// Capsules are blueprints for creating some immutable data /// and do not actually contain any data themselves. @@ -111,11 +159,6 @@ pub trait SideEffect: Send + 'static { where Self: 'a; - // TODO Can we change side effects to be like: - // register(effect1(1234), effect2(abc)) - // where effect1/2 take in own args, then return fn that takes in registrar to produce API - // OR, inner type and function to make rebuild easier? - /// Construct this side effect's build api, given: /// - A mutable reference to the current state of this side effect (&mut self) /// - A mechanism to trigger rebuilds that can also update the state of this side effect @@ -153,9 +196,6 @@ generate_tuple_side_effect_impl!(A, B, C, D, E, F); generate_tuple_side_effect_impl!(A, B, C, D, E, F, G); generate_tuple_side_effect_impl!(A, B, C, D, E, F, G, H); -// Using a trait object here to prevent a sea of complicated generics everywhere -// TODO maybe try making this static dispatch again? Box is gross. -// (Keep inner as Box, but outer should be impl) pub trait SideEffectRebuilder: Fn(Box) + Send + Sync + DynClone + 'static { @@ -221,7 +261,6 @@ impl Container { /// Internally, tries to read all supplied capsules with a read txn first (cheap), /// but if that fails (i.e., capsules' data not present in the container), /// spins up a write txn and initializes all needed capsules (which blocks). - // TODO add our fun lil var args impl hack to Container to make it easier to read capsules pub fn read(&self, capsules: CL) -> CL::Data { capsules.read(self) } @@ -243,7 +282,7 @@ macro_rules! generate_capsule_list_impl { fn read(self, container: &Container) -> Self::Data { let ($([]),*) = self; if let ($(Some([])),*) = - container.with_read_txn(|txn| ($(txn.try_read_raw::<$C>()),*)) { + container.with_read_txn(|txn| ($(txn.try_read(&[])),*)) { ($([]),*) } else { container.with_write_txn(|txn| ($(txn.read_or_init([])),*)) @@ -433,7 +472,7 @@ mod tests { let container = Container::new(); assert_eq!( (None, None), - container.with_read_txn(|txn| (txn.try_read(count), txn.try_read(count_plus_one))) + container.with_read_txn(|txn| (txn.try_read(&count), txn.try_read(&count_plus_one))) ); assert_eq!( 1, @@ -441,7 +480,7 @@ mod tests { ); assert_eq!( 0, - container.with_read_txn(|txn| txn.try_read(count).unwrap()) + container.with_read_txn(|txn| txn.try_read(&count).unwrap()) ); let container = Container::new(); @@ -551,45 +590,45 @@ mod tests { container.with_read_txn(|txn| { read_txn_counter += 1; - assert!(txn.try_read(stateful_a).is_none()); - assert_eq!(txn.try_read(a), None); - assert_eq!(txn.try_read(b), None); - assert_eq!(txn.try_read(c), None); - assert_eq!(txn.try_read(d), None); - assert_eq!(txn.try_read(e), None); - assert_eq!(txn.try_read(f), None); - assert_eq!(txn.try_read(g), None); - assert_eq!(txn.try_read(h), None); + assert!(txn.try_read(&stateful_a).is_none()); + assert_eq!(txn.try_read(&a), None); + assert_eq!(txn.try_read(&b), None); + assert_eq!(txn.try_read(&c), None); + assert_eq!(txn.try_read(&d), None); + assert_eq!(txn.try_read(&e), None); + assert_eq!(txn.try_read(&f), None); + assert_eq!(txn.try_read(&g), None); + assert_eq!(txn.try_read(&h), None); }); container.read((d, g)); container.with_read_txn(|txn| { read_txn_counter += 1; - assert!(txn.try_read(stateful_a).is_some()); - assert_eq!(txn.try_read(a).unwrap(), 0); - assert_eq!(txn.try_read(b).unwrap(), 1); - assert_eq!(txn.try_read(c).unwrap(), 2); - assert_eq!(txn.try_read(d).unwrap(), 2); - assert_eq!(txn.try_read(e).unwrap(), 1); - assert_eq!(txn.try_read(f).unwrap(), 1); - assert_eq!(txn.try_read(g).unwrap(), 3); - assert_eq!(txn.try_read(h).unwrap(), 1); + assert!(txn.try_read(&stateful_a).is_some()); + assert_eq!(txn.try_read(&a).unwrap(), 0); + assert_eq!(txn.try_read(&b).unwrap(), 1); + assert_eq!(txn.try_read(&c).unwrap(), 2); + assert_eq!(txn.try_read(&d).unwrap(), 2); + assert_eq!(txn.try_read(&e).unwrap(), 1); + assert_eq!(txn.try_read(&f).unwrap(), 1); + assert_eq!(txn.try_read(&g).unwrap(), 3); + assert_eq!(txn.try_read(&h).unwrap(), 1); }); container.read(stateful_a).1(10); container.with_read_txn(|txn| { read_txn_counter += 1; - assert!(txn.try_read(stateful_a).is_some()); - assert_eq!(txn.try_read(a).unwrap(), 10); - assert_eq!(txn.try_read(b).unwrap(), 11); - assert_eq!(txn.try_read(c), None); - assert_eq!(txn.try_read(d), None); - assert_eq!(txn.try_read(e).unwrap(), 11); - assert_eq!(txn.try_read(f).unwrap(), 11); - assert_eq!(txn.try_read(g), None); - assert_eq!(txn.try_read(h).unwrap(), 1); + assert!(txn.try_read(&stateful_a).is_some()); + assert_eq!(txn.try_read(&a).unwrap(), 10); + assert_eq!(txn.try_read(&b).unwrap(), 11); + assert_eq!(txn.try_read(&c), None); + assert_eq!(txn.try_read(&d), None); + assert_eq!(txn.try_read(&e).unwrap(), 11); + assert_eq!(txn.try_read(&f).unwrap(), 11); + assert_eq!(txn.try_read(&g), None); + assert_eq!(txn.try_read(&h).unwrap(), 1); }); assert_eq!(read_txn_counter, 3); diff --git a/rearch/src/side_effect_registrar.rs b/rearch/src/side_effect_registrar.rs new file mode 100644 index 0000000..e0230fe --- /dev/null +++ b/rearch/src/side_effect_registrar.rs @@ -0,0 +1,88 @@ +use std::{any::Any, cell::OnceCell}; + +use crate::{SideEffect, SideEffectRebuilder}; + +/// Registers the given side effect and returns its build api. +/// You can only call register once on purpose (it consumes self); +/// to register multiple side effects, simply pass them in together! +/// If you have a super pure capsule that you wish to make not super pure, +/// simply call `register()` with no arguments. +pub struct SideEffectRegistrar<'a> { + side_effect: &'a mut OnceCell>, + rebuild: Box>>, +} + +impl<'a> SideEffectRegistrar<'a> { + /// Creates a new `SideEffectRegistrar`. + /// + /// This is public only to enable easier mocking in your code; + /// do not use this method in a non-test context. + pub fn new( + side_effect: &'a mut OnceCell>, + rebuild: Box>>, + ) -> Self { + Self { + side_effect, + rebuild, + } + } +} + +// Empty register() for the no-op side effect +impl FnOnce<()> for SideEffectRegistrar<'_> { + type Output = (); + extern "rust-call" fn call_once(self, _: ()) -> Self::Output { + // Initialize with the no-op side effect + self.side_effect.get_or_init(|| Box::new(())); + + // Ensure side effect wasn't changed + assert!( + self.side_effect + .get_mut() + .expect("Side effect should've been initialized above") + .is::<()>(), + "You cannot change the side effect(s) passed to register()!" + ); + } +} + +const EFFECT_FAILED_CAST_MSG: &str = + "The SideEffect registered with SideEffectRegistrar cannot be changed!"; + +macro_rules! generate_side_effect_registrar_fn_impl { + ($($types:ident),+) => { + #[allow(unused_parens, non_snake_case)] + impl<'a, $($types: SideEffect),*> FnOnce<($($types,)*)> for SideEffectRegistrar<'a> { + type Output = ($($types::Api<'a>),*); + + extern "rust-call" fn call_once(self, args: ($($types,)*)) -> Self::Output { + let ($($types,)*) = args; + self.side_effect.get_or_init(|| Box::new(($($types),*))); + let effect = self + .side_effect + .get_mut() + .expect("Side effect should've been initialized above") + .downcast_mut::<($($types),*)>() + .expect(EFFECT_FAILED_CAST_MSG); + + effect.api(Box::new(move |mutation| { + (self.rebuild)(Box::new(|effect| { + let effect = effect + .downcast_mut::<($($types),*)>() + .expect(EFFECT_FAILED_CAST_MSG); + mutation(effect); + })); + })) + } + } + } +} + +generate_side_effect_registrar_fn_impl!(A); +generate_side_effect_registrar_fn_impl!(A, B); +generate_side_effect_registrar_fn_impl!(A, B, C); +generate_side_effect_registrar_fn_impl!(A, B, C, D); +generate_side_effect_registrar_fn_impl!(A, B, C, D, E); +generate_side_effect_registrar_fn_impl!(A, B, C, D, E, F); +generate_side_effect_registrar_fn_impl!(A, B, C, D, E, F, G); +generate_side_effect_registrar_fn_impl!(A, B, C, D, E, F, G, H); diff --git a/rearch/src/txn.rs b/rearch/src/txn.rs new file mode 100644 index 0000000..ad32677 --- /dev/null +++ b/rearch/src/txn.rs @@ -0,0 +1,203 @@ +use std::{ + any::{Any, TypeId}, + collections::HashSet, +}; + +use concread::hashmap::{HashMapReadTxn, HashMapWriteTxn}; + +use crate::{Capsule, CapsuleData, CapsuleManager, CapsuleRebuilder}; + +#[allow(clippy::module_name_repetitions)] +pub struct ContainerReadTxn<'a> { + data: HashMapReadTxn<'a, TypeId, Box>, +} + +impl<'a> ContainerReadTxn<'a> { + pub(crate) fn new(data: HashMapReadTxn<'a, TypeId, Box>) -> Self { + Self { data } + } +} + +impl ContainerReadTxn<'_> { + #[must_use] + pub fn try_read(&self, _capsule: &C) -> Option { + self.try_read_raw::() + } + + /// Tries a capsule read, but doesn't require an instance of the capsule itself + fn try_read_raw(&self) -> Option { + let id = TypeId::of::(); + self.data.get(&id).map(|data| { + let data: Box = data.clone(); + *data + .downcast::() + .expect("Types should be properly enforced due to generics") + }) + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct ContainerWriteTxn<'a> { + pub(crate) rebuilder: CapsuleRebuilder, + pub(crate) data: HashMapWriteTxn<'a, TypeId, Box>, + nodes: &'a mut std::collections::HashMap, +} + +impl<'a> ContainerWriteTxn<'a> { + pub(crate) fn new( + data: HashMapWriteTxn<'a, TypeId, Box>, + nodes: &'a mut std::collections::HashMap, + rebuilder: CapsuleRebuilder, + ) -> Self { + Self { + rebuilder, + data, + nodes, + } + } +} + +impl ContainerWriteTxn<'_> { + pub fn read_or_init(&mut self, capsule: C) -> C::Data { + let id = TypeId::of::(); + if !self.data.contains_key(&id) { + #[cfg(feature = "logging")] + log::debug!("Initializing {} ({:?})", std::any::type_name::(), id); + + self.build_capsule(capsule); + } + self.try_read_raw::() + .expect("Data should be present due to checking/building capsule above") + } + + #[must_use] + pub fn try_read(&self, _capsule: &C) -> Option { + self.try_read_raw::() + } + + /// Tries a capsule read, but doesn't require an instance of the capsule itself + fn try_read_raw(&self) -> Option { + let id = TypeId::of::(); + self.data.get(&id).map(|data| { + let data: Box = data.clone(); + *data + .downcast::() + .expect("Types should be properly enforced due to generics") + }) + } + + /// Triggers a first build or rebuild for the supplied capsule + fn build_capsule(&mut self, capsule: C) { + let id = TypeId::of::(); + + if let std::collections::hash_map::Entry::Vacant(e) = self.nodes.entry(id) { + e.insert(CapsuleManager::new(capsule)); + } + + self.build_capsule_or_panic(id); + } + + /// Forcefully builds the capsule with the supplied id. Panics if node is not in the graph + pub(crate) fn build_capsule_or_panic(&mut self, id: TypeId) { + self.build_single_node(id); + + // Since we have already built the node above (since *it must be built in this method*), + // we can skip it with skip(1) when we are handling the rest of the dependent subgraph + let build_order = { + let build_order = self.create_build_order_stack(id).into_iter().rev().skip(1); + self.garbage_collect_diposable_nodes(build_order) + }; + for id in build_order { + self.build_single_node(id); + } + } + + /// Gets the requested node or panics if it is not in the graph + pub(crate) fn node_or_panic(&mut self, id: TypeId) -> &mut CapsuleManager { + self.nodes + .get_mut(&id) + .expect("Requested node should be in the graph") + } + + /// Builds only the requested node. Panics if the node is not in the graph + fn build_single_node(&mut self, id: TypeId) { + // Remove old dependency info since it may change on this build + // We use std::mem::take below to prevent needing a clone on the existing dependencies + let node = self.node_or_panic(id); + let old_deps = std::mem::take(&mut node.dependencies); + for dep in old_deps { + self.node_or_panic(dep).dependents.remove(&id); + } + + // Trigger the build (which also populates its new dependencies in self) + (self.node_or_panic(id).build)(self); + } + + /// Forcefully disposes only the requested node, cleaning up the node's direct dependencies. + /// Panics if the node is not in the graph. + fn dispose_single_node(&mut self, id: TypeId) { + self.data.remove(&id); + self.nodes + .remove(&id) + .expect("Node should be in graph") + .dependencies + .iter() + .for_each(|dep| { + self.node_or_panic(*dep).dependents.remove(&id); + }); + } + + /// Creates the start node's dependent subgraph build order, including start, *as a stack*. + /// Thus, proper iteration order is done by popping off of the stack (in reverse order)! + fn create_build_order_stack(&mut self, start: TypeId) -> Vec { + // We need some more information alongside each node in order to do the topological sort + // - False is for the first visit, which adds all deps to be visited and then self again + // - True is for the second visit, which pushes node to the build order + let mut to_visit_stack = vec![(false, start)]; + let mut visited = HashSet::new(); + let mut build_order_stack = Vec::new(); + + while let Some((has_visited_before, node)) = to_visit_stack.pop() { + if has_visited_before { + // Already processed this node's dependents, so finally add to build order + build_order_stack.push(node); + } else if !visited.contains(&node) { + // New node, so mark this node to be added later and process dependents + visited.insert(node); + to_visit_stack.push((true, node)); // mark node to be added to build order later + self.node_or_panic(node) + .dependents + .iter() + .copied() + .filter(|dep| !visited.contains(dep)) + .for_each(|dep| to_visit_stack.push((false, dep))); + } + } + + build_order_stack + } + + /// Helper function that given a `build_order`, garbage collects all super pure nodes + /// that have no dependents (i.e., they are entirely disposable) + /// and returns the new build order without the (now garbage collected) super pure nodes. + /// While the build order specifies the order in which nodes must be built in to propagate + /// updates, the reverse of the build order specifies the order in which we can trim down + /// some fat through gc. + fn garbage_collect_diposable_nodes( + &mut self, + build_order: impl DoubleEndedIterator, + ) -> impl DoubleEndedIterator { + let mut non_disposable = Vec::new(); + + build_order.rev().for_each(|id| { + let is_disposable = self.node_or_panic(id).is_disposable(); + if is_disposable { + self.dispose_single_node(id); + } else { + non_disposable.push(id); + } + }); + + non_disposable.into_iter().rev() + } +}