From b8d057689022e95635955ed0cd25a2285b1202ec Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 17 Nov 2023 18:36:54 +0100 Subject: [PATCH 1/2] Add _mut methods for edge mutation for faster single-thread access --- crates/builder/src/graph/adj_list.rs | 89 +++++++++++++++++++++------- crates/builder/src/lib.rs | 21 +++++++ 2 files changed, 88 insertions(+), 22 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 71f793d..53c072d 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -71,6 +71,33 @@ impl AdjacencyList { }, }; } + + #[inline] + pub(crate) fn insert_mut(&mut self, source: NI, target: Target) { + let edges = self.edges[source.index()].get_mut().unwrap(); + + match self.layout { + CsrLayout::Sorted => match edges.binary_search(&target) { + Ok(i) => edges.insert(i, target), + Err(i) => edges.insert(i, target), + }, + CsrLayout::Unsorted => edges.push(target), + CsrLayout::Deduplicated => match edges.binary_search(&target) { + Ok(_) => {} + Err(i) => edges.insert(i, target), + }, + }; + } + + #[inline] + fn check_bounds(&self, node: NI) -> Result<(), crate::Error> { + if node >= self.node_count() { + return Err(crate::Error::MissingNode { + node: format!("{}", node.index()), + }); + }; + Ok(()) + } } #[derive(Debug)] @@ -345,26 +372,35 @@ impl EdgeMutation for DirectedALGraph { fn add_edge(&self, source: NI, target: NI) -> Result<(), crate::Error> { self.add_edge_with_value(source, target, ()) } + + fn add_edge_mut(&mut self, source: NI, target: NI) -> Result<(), crate::Error> { + self.add_edge_with_value_mut(source, target, ()) + } } impl EdgeMutationWithValues for DirectedALGraph { fn add_edge_with_value(&self, source: NI, target: NI, value: EV) -> Result<(), crate::Error> { - if source >= self.al_out.node_count() { - return Err(crate::Error::MissingNode { - node: format!("{}", source.index()), - }); - } - if target >= self.al_inc.node_count() { - return Err(crate::Error::MissingNode { - node: format!("{}", target.index()), - }); - } - + self.al_out.check_bounds(source)?; + self.al_inc.check_bounds(target)?; self.al_out.insert(source, Target::new(target, value)); self.al_inc.insert(target, Target::new(source, value)); Ok(()) } + + fn add_edge_with_value_mut( + &mut self, + source: NI, + target: NI, + value: EV, + ) -> Result<(), crate::Error> { + self.al_out.check_bounds(source)?; + self.al_inc.check_bounds(target)?; + self.al_out.insert_mut(source, Target::new(target, value)); + self.al_inc.insert_mut(target, Target::new(source, value)); + + Ok(()) + } } impl From<(E, CsrLayout)> for DirectedALGraph @@ -482,26 +518,35 @@ impl EdgeMutation for UndirectedALGraph { fn add_edge(&self, source: NI, target: NI) -> Result<(), crate::Error> { self.add_edge_with_value(source, target, ()) } + + fn add_edge_mut(&mut self, source: NI, target: NI) -> Result<(), crate::Error> { + self.add_edge_with_value_mut(source, target, ()) + } } impl EdgeMutationWithValues for UndirectedALGraph { fn add_edge_with_value(&self, source: NI, target: NI, value: EV) -> Result<(), crate::Error> { - if source >= self.al.node_count() { - return Err(crate::Error::MissingNode { - node: format!("{}", source.index()), - }); - } - if target >= self.al.node_count() { - return Err(crate::Error::MissingNode { - node: format!("{}", target.index()), - }); - } - + self.al.check_bounds(source)?; + self.al.check_bounds(target)?; self.al.insert(source, Target::new(target, value)); self.al.insert(target, Target::new(source, value)); Ok(()) } + + fn add_edge_with_value_mut( + &mut self, + source: NI, + target: NI, + value: EV, + ) -> Result<(), crate::Error> { + self.al.check_bounds(source)?; + self.al.check_bounds(target)?; + self.al.insert_mut(source, Target::new(target, value)); + self.al.insert_mut(target, Target::new(source, value)); + + Ok(()) + } } impl From<(E, CsrLayout)> for UndirectedALGraph diff --git a/crates/builder/src/lib.rs b/crates/builder/src/lib.rs index 9192131..cd4c93a 100644 --- a/crates/builder/src/lib.rs +++ b/crates/builder/src/lib.rs @@ -420,6 +420,16 @@ pub trait EdgeMutation { /// If either the source or the target node does not exist, /// the method will return [`Error::MissingNode`]. fn add_edge(&self, source: NI, target: NI) -> Result<(), Error>; + + /// Adds a new edge between the given source and target node. + /// + /// Does not require locking the node-local list due to `&mut self`. + /// + /// # Errors + /// + /// If either the source or the target node does not exist, + /// the method will return [`Error::MissingNode`]. + fn add_edge_mut(&mut self, source: NI, target: NI) -> Result<(), Error>; } /// Allows adding new edges to a graph. @@ -432,6 +442,17 @@ pub trait EdgeMutationWithValues { /// If either the source or the target node does not exist, /// the method will return [`Error::MissingNode`]. fn add_edge_with_value(&self, source: NI, target: NI, value: EV) -> Result<(), Error>; + + /// Adds a new edge between the given source and target node + /// and assigns the given value to it. + /// + /// Does not require locking the node-local list due to `&mut self`. + /// + /// # Errors + /// + /// If either the source or the target node does not exist, + /// the method will return [`Error::MissingNode`]. + fn add_edge_with_value_mut(&mut self, source: NI, target: NI, value: EV) -> Result<(), Error>; } #[repr(transparent)] From a28d250453fcf1964dc6fa853d5fc34731347fe7 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 17 Nov 2023 19:34:30 +0100 Subject: [PATCH 2/2] Avoid duplication layout application --- crates/builder/src/graph/adj_list.rs | 39 ++++++++++++---------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 53c072d..5e77c58 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -58,25 +58,28 @@ impl AdjacencyList { #[inline] pub(crate) fn insert(&self, source: NI, target: Target) { let mut edges = self.edges[source.index()].write().unwrap(); - - match self.layout { - CsrLayout::Sorted => match edges.binary_search(&target) { - Ok(i) => edges.insert(i, target), - Err(i) => edges.insert(i, target), - }, - CsrLayout::Unsorted => edges.push(target), - CsrLayout::Deduplicated => match edges.binary_search(&target) { - Ok(_) => {} - Err(i) => edges.insert(i, target), - }, - }; + Self::apply_layout(self.layout, &mut edges, target); } #[inline] pub(crate) fn insert_mut(&mut self, source: NI, target: Target) { let edges = self.edges[source.index()].get_mut().unwrap(); + Self::apply_layout(self.layout, edges, target); + } - match self.layout { + #[inline] + fn check_bounds(&self, node: NI) -> Result<(), crate::Error> { + if node >= self.node_count() { + return Err(crate::Error::MissingNode { + node: format!("{}", node.index()), + }); + }; + Ok(()) + } + + #[inline] + fn apply_layout(layout: CsrLayout, edges: &mut Vec>, target: Target) { + match layout { CsrLayout::Sorted => match edges.binary_search(&target) { Ok(i) => edges.insert(i, target), Err(i) => edges.insert(i, target), @@ -88,16 +91,6 @@ impl AdjacencyList { }, }; } - - #[inline] - fn check_bounds(&self, node: NI) -> Result<(), crate::Error> { - if node >= self.node_count() { - return Err(crate::Error::MissingNode { - node: format!("{}", node.index()), - }); - }; - Ok(()) - } } #[derive(Debug)]