diff --git a/crates/zrx-graph/src/graph.rs b/crates/zrx-graph/src/graph.rs index 499a9a5..f811633 100644 --- a/crates/zrx-graph/src/graph.rs +++ b/crates/zrx-graph/src/graph.rs @@ -190,7 +190,7 @@ impl Graph { #[allow(clippy::must_use_candidate)] impl Graph { - /// Returns the graph topology. + /// Returns a reference to the graph topology. #[inline] pub fn topology(&self) -> &Topology { &self.topology diff --git a/crates/zrx-graph/src/graph/operator.rs b/crates/zrx-graph/src/graph/operator.rs index c4dfcfc..e4d9bf0 100644 --- a/crates/zrx-graph/src/graph/operator.rs +++ b/crates/zrx-graph/src/graph/operator.rs @@ -25,5 +25,5 @@ //! Graph operators. +mod adjacent; mod map; -mod with; diff --git a/crates/zrx-graph/src/graph/operator/with.rs b/crates/zrx-graph/src/graph/operator/adjacent.rs similarity index 83% rename from crates/zrx-graph/src/graph/operator/with.rs rename to crates/zrx-graph/src/graph/operator/adjacent.rs index 0d35eec..5557d96 100644 --- a/crates/zrx-graph/src/graph/operator/with.rs +++ b/crates/zrx-graph/src/graph/operator/adjacent.rs @@ -23,7 +23,7 @@ // ---------------------------------------------------------------------------- -//! With operator. +//! Adjacent operator. use crate::graph::Graph; @@ -44,7 +44,7 @@ pub struct Adjacent<'a> { // ---------------------------------------------------------------------------- impl Graph { - /// Retrieve a reference to a node's data. + /// Retrieve a reference to a node and its adjacent nodes. /// /// # Examples /// @@ -63,24 +63,20 @@ impl Graph { /// builder.add_edge(a, b)?; /// builder.add_edge(b, c)?; /// - /// // Create graph from builder and retrieve nodes + /// // Create graph from builder and retrieve node /// let graph = builder.build(); - /// for node in &graph { - /// graph.with(node, |name, _| { - /// println!("{name:?}"); - /// }); - /// } + /// + /// // Obtain reference to node and adjacent nodes + /// let (data, adj) = graph.adjacent(a); /// # Ok(()) /// # } /// ``` #[inline] - pub fn with(&self, node: usize, f: F) -> R - where - F: FnOnce(&T, Adjacent) -> R, - { + #[must_use] + pub fn adjacent(&self, node: usize) -> (&'_ T, Adjacent<'_>) { let incoming = self.topology.incoming(); let outgoing = self.topology.outgoing(); - f( + ( &self.data[node], Adjacent { incoming: &incoming[node], @@ -89,7 +85,7 @@ impl Graph { ) } - /// Retrieve a mutable reference to a node's data. + /// Retrieve a mutable reference to a node and its adjacent nodes. /// /// # Examples /// @@ -110,22 +106,18 @@ impl Graph { /// /// // Create graph from builder and retrieve node /// let mut graph = builder.build(); - /// for node in &graph { - /// graph.with_mut(node, |name, _| { - /// println!("{name:?}"); - /// }); - /// } + /// + /// // Obtain mutable reference to node and adjacent nodes + /// let (data, adj) = graph.adjacent_mut(a); /// # Ok(()) /// # } /// ``` #[inline] - pub fn with_mut(&mut self, node: usize, f: F) -> R - where - F: FnOnce(&mut T, Adjacent) -> R, - { + #[must_use] + pub fn adjacent_mut(&mut self, node: usize) -> (&'_ mut T, Adjacent<'_>) { let incoming = self.topology.incoming(); let outgoing = self.topology.outgoing(); - f( + ( &mut self.data[node], Adjacent { incoming: &incoming[node], diff --git a/crates/zrx-graph/src/graph/traversal.rs b/crates/zrx-graph/src/graph/traversal.rs index 6996218..2127bc1 100644 --- a/crates/zrx-graph/src/graph/traversal.rs +++ b/crates/zrx-graph/src/graph/traversal.rs @@ -267,12 +267,25 @@ impl Traversal { Ok(()) } - /// Attempts to converge this traversal with another traversal. + /// Attempts to converge with the given traversal. /// - /// This method attempts to combine both traversals into a single traversal, - /// which is possible when they have common descendants. This is useful for - /// deduplication of computations that need to be carried out for traversals - /// that originate from different sources, but converge at some point. + /// This method attempts to merge both traversals into a single traversal, + /// which is possible when they have common descendants, a condition that + /// is always true for directed acyclic graphs with a single component. + /// There are several cases to consider when converging two traversals: + /// + /// - If traversals start from the same set of source nodes, they already + /// converged, so we just restart the traversal at these source nodes. + /// + /// - If traversals start from different source nodes, yet both have common + /// descendants, we converge at the first layer of common descendants, as + /// all descendants of them must be revisited in the combined traversal. + /// Ancestors of the common descendants that have already been visited in + /// either traversal don't need to be revisited, and thus are carried over + /// from both traversals in their current state. + /// + /// - If traversals are disjoint, they can't be converged, so we return the + /// given traversal back to the caller wrapped in [`Error::Disjoint`]. /// /// # Errors /// @@ -327,15 +340,15 @@ impl Traversal { topology: self.topology.clone(), }; - // If there are no common descendants, the traversals are disjoint, and - // we can't converge them, so we return the traversal to the caller + // If there are no common descendants, the traversals are disjoint and + // can't converge, so we return the given traversal back to the caller let mut iter = graph.common_descendants(&initial); let Some(common) = iter.next() else { return Err(Error::Disjoint(other)); }; // Create the combined traversal, and mark all already visited nodes - // that are ancestors of the common descendants as visited as well + // that are ancestors of the common descendants as visited let prior = mem::replace(self, Self::new(&self.topology, initial)); // Compute the visitable nodes for the combined traversal, which is the @@ -347,10 +360,10 @@ impl Traversal { let p = prior.dependencies[node]; let o = other.dependencies[node]; - // If the node has been visited in either traversal, and is not - // part of the common descendants, mark it as visited as well in - // the combined traversal, since we don't need to revisit nodes - // that are ancestors of the common descendants + // If the node has been visited in either traversal, and is not part + // of the first layer of common descendants, mark it as visited in + // the combined traversal, since we don't need to revisit ancestors + // that have already been visited in either traversal if (p == u8::MAX || o == u8::MAX) && !common.contains(&node) { self.complete(node)?; } else { @@ -368,12 +381,18 @@ impl Traversal { #[allow(clippy::must_use_candidate)] impl Traversal { - /// Returns the graph topology. + /// Returns a reference to the graph topology. #[inline] pub fn topology(&self) -> &Topology { &self.topology } + /// Returns a reference to the initial nodes. + #[inline] + pub fn initial(&self) -> &[usize] { + &self.initial + } + /// Returns the number of visitable nodes. #[inline] pub fn len(&self) -> usize { diff --git a/crates/zrx-id/src/id.rs b/crates/zrx-id/src/id.rs index 3ff647a..82fc14c 100644 --- a/crates/zrx-id/src/id.rs +++ b/crates/zrx-id/src/id.rs @@ -367,7 +367,9 @@ impl PartialEq for Id { /// ``` #[inline] fn eq(&self, other: &Self) -> bool { - self.hash == other.hash + // We first compare the precomputed hashes, which is extremly fast, as + // it saves us the comparison when the identifiers are different + self.hash == other.hash && self.format == other.format } } @@ -417,13 +419,7 @@ impl Ord for Id { /// ``` #[inline] fn cmp(&self, other: &Self) -> Ordering { - // Fast path - first, we compare for equality by using the precomputed - // hashes, as it's a simple and extremely fast integer comparison - if self.eq(other) { - Ordering::Equal - } else { - self.format.cmp(&other.format) - } + self.format.cmp(&other.format) } } diff --git a/crates/zrx-id/src/id/matcher/selector.rs b/crates/zrx-id/src/id/matcher/selector.rs index a5aad64..a6dcfb6 100644 --- a/crates/zrx-id/src/id/matcher/selector.rs +++ b/crates/zrx-id/src/id/matcher/selector.rs @@ -27,6 +27,7 @@ use ahash::AHasher; use std::borrow::Cow; +use std::cmp::Ordering; use std::fmt::{self, Debug, Display}; use std::hash::{Hash, Hasher}; use std::str::FromStr; @@ -104,7 +105,7 @@ pub use convert::TryToSelector; /// # Ok(()) /// # } /// ``` -#[derive(Clone, PartialOrd, Ord)] +#[derive(Clone)] pub struct Selector { /// Formatted string. format: Arc>, @@ -343,7 +344,9 @@ impl PartialEq for Selector { /// ``` #[inline] fn eq(&self, other: &Self) -> bool { - self.hash == other.hash + // We first compare the precomputed hashes, which is extremely fast, as + // it saves us the comparison when the identifiers are different + self.hash == other.hash && self.format == other.format } } @@ -351,6 +354,54 @@ impl Eq for Selector {} // ---------------------------------------------------------------------------- +impl PartialOrd for Selector { + /// Orders two selectors. + /// + /// # Examples + /// + /// ``` + /// # use std::error::Error; + /// # fn main() -> Result<(), Box> { + /// use zrx_id::Selector; + /// + /// // Create and compare selectors + /// let a: Selector = "zrs:::::**/*.md:".parse()?; + /// let b: Selector = "zrs:::::**/*.rs:".parse()?; + /// assert!(a < b); + /// # Ok(()) + /// # } + /// ``` + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Selector { + /// Orders two selectors. + /// + /// # Examples + /// + /// ``` + /// # use std::error::Error; + /// # fn main() -> Result<(), Box> { + /// use zrx_id::Selector; + /// + /// // Create and compare selectors + /// let a: Selector = "zrs:::::**/*.md:".parse()?; + /// let b: Selector = "zrs:::::**/*.rs:".parse()?; + /// assert!(a < b); + /// # Ok(()) + /// # } + /// ``` + #[inline] + fn cmp(&self, other: &Self) -> Ordering { + self.format.cmp(&other.format) + } +} + +// ---------------------------------------------------------------------------- + impl Display for Selector { /// Formats the selector for display. fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {