From 5f4be3fc20c6ba01f1ec8d51bfd50c0312b4cd50 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 27 Oct 2023 12:03:29 +0200 Subject: [PATCH 01/15] Implement EdgeMutation for directed AL --- crates/builder/src/graph/adj_list.rs | 107 ++++++++++++++++++++++++++- crates/builder/src/lib.rs | 7 ++ crates/builder/src/prelude.rs | 2 + 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 7d2907d..2bb6a27 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -1,3 +1,4 @@ +use crate::EdgeMutation; use crate::{ index::Idx, prelude::Direction, prelude::Edges, prelude::NodeValues as NodeValuesTrait, CsrLayout, DirectedDegrees, DirectedNeighbors, DirectedNeighborsWithValues, Graph, Target, @@ -14,11 +15,16 @@ use rayon::prelude::*; #[derive(Debug)] pub struct AdjacencyList { edges: Vec>>, + layout: CsrLayout, } impl AdjacencyList { pub fn new(edges: Vec>>) -> Self { - Self { edges } + Self::with_layout(edges, CsrLayout::Unsorted) + } + + pub fn with_layout(edges: Vec>>, layout: CsrLayout) -> Self { + Self { edges, layout } } #[inline] @@ -66,6 +72,27 @@ impl AdjacencyList { // size and alignment. unsafe { std::slice::from_raw_parts(targets.as_ptr().cast(), targets.len()) } } + + #[inline] + fn insert(&mut self, source: NI, target: Target) { + match self.layout { + CsrLayout::Sorted => { + let edges = &mut self.edges[source.index()]; + match edges.binary_search(&target) { + Ok(i) => edges.insert(i, target), + Err(i) => edges.insert(i, target), + } + } + CsrLayout::Unsorted => self.edges[source.index()].push(target), + CsrLayout::Deduplicated => { + let edges = &mut self.edges[source.index()]; + match edges.binary_search(&target) { + Ok(_) => {} + Err(i) => edges.insert(i, target), + } + } + }; + } } impl From<(&'_ E, NI, Direction, CsrLayout)> for AdjacencyList @@ -123,7 +150,7 @@ where start.elapsed() ); - AdjacencyList::new(edges) + AdjacencyList::with_layout(edges, csr_layout) } } @@ -212,6 +239,26 @@ impl DirectedNeighborsWithValues for DirectedALGraph EdgeMutation for DirectedALGraph { + fn add_edge(&mut self, source: NI, target: NI) -> 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.insert(source, Target::new(target, ())); + self.al_inc.insert(target, Target::new(source, ())); + + Ok(()) + } +} + impl From<(E, CsrLayout)> for DirectedALGraph where NI: Idx, @@ -557,6 +604,62 @@ mod test { assert_eq!(g.node_value(2), &"baz"); } + #[test] + fn directed_al_graph_add_edge_unsorted() { + let mut g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges([(0, 2), (1, 2)]) + .build::>(); + + assert_eq!(g.out_neighbors(0).as_slice(), &[2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[2, 1]); + g.add_edge(0, 2).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[2, 1, 2]); + } + + #[test] + fn directed_al_graph_add_edge_sorted() { + let mut g = GraphBuilder::new() + .csr_layout(CsrLayout::Sorted) + .edges([(0, 2), (1, 2)]) + .build::>(); + + assert_eq!(g.out_neighbors(0).as_slice(), &[2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[1, 2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[1, 1, 2]); + } + + #[test] + fn directed_al_graph_add_edge_deduplicated() { + let mut g = GraphBuilder::new() + .csr_layout(CsrLayout::Deduplicated) + .edges([(0, 2), (1, 2), (1, 3)]) + .build::>(); + + assert_eq!(g.out_neighbors(0).as_slice(), &[2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[1, 2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[1, 2]); + g.add_edge(0, 3).expect("add edge failed"); + assert_eq!(g.out_neighbors(0).as_slice(), &[1, 2, 3]); + } + + #[test] + fn directed_al_graph_add_edge_missing_node() { + let mut g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges([(0, 2), (1, 2)]) + .build::>(); + + let err = g.add_edge(0, 3).unwrap_err(); + + assert!(matches!(err, crate::Error::MissingNode { node } if node == "3" )); + } + #[test] fn undirected_al_graph() { let g = GraphBuilder::new() diff --git a/crates/builder/src/lib.rs b/crates/builder/src/lib.rs index df133da..89c61f6 100644 --- a/crates/builder/src/lib.rs +++ b/crates/builder/src/lib.rs @@ -208,6 +208,9 @@ pub enum Error { InvalidNodeValues, #[error("invalid id size, expected {expected:?} bytes, got {actual:?} bytes")] InvalidIdType { expected: String, actual: String }, + + #[error("node {node:?} does not exist in the graph")] + MissingNode { node: String }, } impl From for Error { @@ -320,6 +323,10 @@ pub trait DirectedNeighborsWithValues { fn in_neighbors_with_values(&self, node: NI) -> Self::NeighborsIterator<'_>; } +pub trait EdgeMutation { + fn add_edge(&mut self, source: NI, target: NI) -> Result<(), Error>; +} + #[repr(transparent)] pub struct SharedMut(*mut T); unsafe impl Send for SharedMut {} diff --git a/crates/builder/src/prelude.rs b/crates/builder/src/prelude.rs index d2d4cb2..e410cd9 100644 --- a/crates/builder/src/prelude.rs +++ b/crates/builder/src/prelude.rs @@ -32,4 +32,6 @@ pub use crate::UndirectedDegrees; pub use crate::UndirectedNeighbors; pub use crate::UndirectedNeighborsWithValues; +pub use crate::EdgeMutation; + pub use crate::Error; From fd47132274d92245499853b91ca2fe07dd548a73 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 27 Oct 2023 16:27:51 +0200 Subject: [PATCH 02/15] Thread-safe directed adjacency list graph --- crates/builder/src/graph/adj_list.rs | 277 ++++++++++++++++++++------- crates/builder/src/lib.rs | 2 +- 2 files changed, 213 insertions(+), 66 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 2bb6a27..558aceb 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -6,7 +6,7 @@ use crate::{ }; use log::info; -use std::sync::Mutex; +use std::sync::{Mutex, MutexGuard}; use std::time::Instant; use crate::graph::csr::NodeValues; @@ -14,7 +14,7 @@ use rayon::prelude::*; #[derive(Debug)] pub struct AdjacencyList { - edges: Vec>>, + edges: Vec>>>, layout: CsrLayout, } @@ -24,6 +24,7 @@ impl AdjacencyList { } pub fn with_layout(edges: Vec>>, layout: CsrLayout) -> Self { + let edges = edges.into_iter().map(Mutex::new).collect::<_>(); Self { edges, layout } } @@ -38,25 +39,32 @@ impl AdjacencyList { NI: Send + Sync, EV: Send + Sync, { - NI::new(self.edges.par_iter().map(|v| v.len()).sum()) + NI::new( + self.edges + .par_iter() + .map(|v| v.lock().expect("Could not acquire lock").len()) + .sum(), + ) } #[inline] pub(crate) fn degree(&self, node: NI) -> NI { - NI::new(self.edges[node.index()].len()) + NI::new( + self.edges[node.index()] + .lock() + .expect("Could not aqcuire lock") + .len(), + ) } } -impl AdjacencyList { - #[inline] - pub(crate) fn targets_with_values(&self, node: NI) -> &[Target] { - self.edges[node.index()].as_slice() - } +#[derive(Debug)] +pub struct Targets<'slice, NI: Idx> { + targets: MutexGuard<'slice, Vec>>, } -impl AdjacencyList { - #[inline] - pub(crate) fn targets(&self, node: NI) -> &[NI] { +impl<'slice, NI: Idx> Targets<'slice, NI> { + pub fn as_slice(&self) -> &'slice [NI] { assert_eq!( std::mem::size_of::>(), std::mem::size_of::() @@ -65,36 +73,141 @@ impl AdjacencyList { std::mem::align_of::>(), std::mem::align_of::() ); - - let targets = self.edges[node.index()].as_slice(); - // SAFETY: The types Target and T are verified to have the same // size and alignment. - unsafe { std::slice::from_raw_parts(targets.as_ptr().cast(), targets.len()) } + unsafe { std::slice::from_raw_parts(self.targets.as_ptr().cast(), self.targets.len()) } + } +} + +pub struct TargetsIter<'slice, NI: Idx> { + _targets: Targets<'slice, NI>, + slice: std::slice::Iter<'slice, NI>, +} + +impl<'slice, NI: Idx> TargetsIter<'slice, NI> { + pub fn as_slice(&self) -> &'slice [NI] { + self.slice.as_slice() + } +} + +impl<'slice, NI: Idx> IntoIterator for Targets<'slice, NI> { + type Item = &'slice NI; + + type IntoIter = TargetsIter<'slice, NI>; + + fn into_iter(self) -> Self::IntoIter { + let slice = self.as_slice(); + TargetsIter { + _targets: self, + slice: slice.iter(), + } + } +} + +impl<'slice, NI: Idx> Iterator for TargetsIter<'slice, NI> { + type Item = &'slice NI; + + fn next(&mut self) -> Option { + self.slice.next() + } +} + +impl AdjacencyList { + #[inline] + pub(crate) fn targets<'list, 'slice>(&'list self, node: NI) -> Targets<'slice, NI> + where + 'list: 'slice, + { + let targets = self.edges[node.index()] + .lock() + .expect("Could not acquire lock"); + + Targets { targets } } #[inline] - fn insert(&mut self, source: NI, target: Target) { + fn insert(&self, source: NI, target: Target) { + let edges = &mut self.edges[source.index()] + .lock() + .expect("Could not aqcuire lock"); + match self.layout { - CsrLayout::Sorted => { - let edges = &mut self.edges[source.index()]; - match edges.binary_search(&target) { - Ok(i) => edges.insert(i, target), - Err(i) => edges.insert(i, target), - } - } - CsrLayout::Unsorted => self.edges[source.index()].push(target), - CsrLayout::Deduplicated => { - let edges = &mut self.edges[source.index()]; - match edges.binary_search(&target) { - Ok(_) => {} - Err(i) => edges.insert(i, target), - } - } + 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), + }, }; } } +#[derive(Debug)] +pub struct TargetsWithValues<'slice, NI: Idx, EV> { + targets: MutexGuard<'slice, Vec>>, +} + +impl<'slice, NI: Idx, EV> TargetsWithValues<'slice, NI, EV> { + pub fn as_slice(&self) -> &'slice [Target] { + // SAFETY: We can upcast the lifetime since the MutexGuard + // is not exposed, so it is not possible to deref mutable. + unsafe { std::slice::from_raw_parts(self.targets.as_ptr(), self.targets.len()) } + } +} + +pub struct TargetsWithValuesIter<'slice, NI: Idx, EV> { + _targets: TargetsWithValues<'slice, NI, EV>, + slice: std::slice::Iter<'slice, Target>, +} + +impl<'slice, NI: Idx, EV> TargetsWithValuesIter<'slice, NI, EV> { + pub fn as_slice(&self) -> &'slice [Target] { + self.slice.as_slice() + } +} + +impl<'slice, NI: Idx, EV> IntoIterator for TargetsWithValues<'slice, NI, EV> { + type Item = &'slice Target; + + type IntoIter = TargetsWithValuesIter<'slice, NI, EV>; + + fn into_iter(self) -> Self::IntoIter { + let slice = self.as_slice(); + TargetsWithValuesIter { + _targets: self, + slice: slice.iter(), + } + } +} + +impl<'slice, NI: Idx, EV> Iterator for TargetsWithValuesIter<'slice, NI, EV> { + type Item = &'slice Target; + + fn next(&mut self) -> Option { + self.slice.next() + } +} + +impl AdjacencyList { + #[inline] + pub(crate) fn targets_with_values<'list, 'slice>( + &'list self, + node: NI, + ) -> TargetsWithValues<'slice, NI, EV> + where + 'list: 'slice, + { + TargetsWithValues { + targets: self.edges[node.index()] + .lock() + .expect("Could not aqcuire lock"), + } + } +} + impl From<(&'_ E, NI, Direction, CsrLayout)> for AdjacencyList where NI: Idx, @@ -216,31 +329,31 @@ impl DirectedDegrees for DirectedALGraph { } impl DirectedNeighbors for DirectedALGraph { - type NeighborsIterator<'a> = std::slice::Iter<'a, NI> where NV: 'a; + type NeighborsIterator<'a> = TargetsIter<'a, NI> where NV: 'a; - fn out_neighbors(&self, node: NI) -> Self::NeighborsIterator<'_> { - self.al_out.targets(node).iter() + fn out_neighbors<'slice>(&self, node: NI) -> Self::NeighborsIterator<'_> { + self.al_out.targets(node).into_iter() } fn in_neighbors(&self, node: NI) -> Self::NeighborsIterator<'_> { - self.al_inc.targets(node).iter() + self.al_inc.targets(node).into_iter() } } impl DirectedNeighborsWithValues for DirectedALGraph { - type NeighborsIterator<'a> = std::slice::Iter<'a, Target> where NV: 'a, EV: 'a; + type NeighborsIterator<'a> = TargetsWithValuesIter<'a, NI, EV> where NV: 'a, EV: 'a; fn out_neighbors_with_values(&self, node: NI) -> Self::NeighborsIterator<'_> { - self.al_out.targets_with_values(node).iter() + self.al_out.targets_with_values(node).into_iter() } fn in_neighbors_with_values(&self, node: NI) -> Self::NeighborsIterator<'_> { - self.al_inc.targets_with_values(node).iter() + self.al_inc.targets_with_values(node).into_iter() } } impl EdgeMutation for DirectedALGraph { - fn add_edge(&mut self, source: NI, target: NI) -> Result<(), crate::Error> { + fn add_edge(&self, source: NI, target: NI) -> Result<(), crate::Error> { if source >= self.al_out.node_count() { return Err(crate::Error::MissingNode { node: format!("{}", source.index()), @@ -355,18 +468,18 @@ impl UndirectedDegrees for UndirectedALGraph { } impl UndirectedNeighbors for UndirectedALGraph { - type NeighborsIterator<'a> = std::slice::Iter<'a, NI> where NV: 'a; + type NeighborsIterator<'a> = TargetsIter<'a, NI> where NV: 'a; fn neighbors(&self, node: NI) -> Self::NeighborsIterator<'_> { - self.al.targets(node).iter() + self.al.targets(node).into_iter() } } impl UndirectedNeighborsWithValues for UndirectedALGraph { - type NeighborsIterator<'a> = std::slice::Iter<'a, Target> where NV: 'a, EV: 'a; + type NeighborsIterator<'a> = TargetsWithValuesIter<'a, NI, EV> where NV: 'a, EV: 'a; fn neighbors_with_values(&self, node: NI) -> Self::NeighborsIterator<'_> { - self.al.targets_with_values(node).iter() + self.al.targets_with_values(node).into_iter() } } @@ -441,8 +554,14 @@ mod test { /* node 1 */ vec![Target::new(0, 1337)], ]); - assert_eq!(list.targets_with_values(0), &[Target::new(1, 42)]); - assert_eq!(list.targets_with_values(1), &[Target::new(0, 1337)]); + assert_eq!( + list.targets_with_values(0).as_slice(), + &[Target::new(1, 42)] + ); + assert_eq!( + list.targets_with_values(1).as_slice(), + &[Target::new(0, 1337)] + ); } #[test] @@ -452,8 +571,8 @@ mod test { /* node 1 */ vec![Target::new(0, ())], ]); - assert_eq!(list.targets(0), &[1]); - assert_eq!(list.targets(1), &[0]); + assert_eq!(list.targets(0).as_slice(), &[1]); + assert_eq!(list.targets(1).as_slice(), &[0]); } #[test] @@ -465,13 +584,19 @@ mod test { assert_eq!( list.targets_with_values(0) - .iter() + .into_iter() .collect::>() .tap_mut(|v| v.sort_by_key(|t| t.target)), &[&Target::new(1, 42), &Target::new(2, 1337)] ); - assert_eq!(list.targets_with_values(1), &[Target::new(0, 84)]); - assert_eq!(list.targets_with_values(2), &[Target::new(0, 1337)]); + assert_eq!( + list.targets_with_values(1).as_slice(), + &[Target::new(0, 84)] + ); + assert_eq!( + list.targets_with_values(2).as_slice(), + &[Target::new(0, 1337)] + ); } #[test] @@ -483,13 +608,19 @@ mod test { assert_eq!( list.targets_with_values(0) - .iter() + .into_iter() .collect::>() .tap_mut(|v| v.sort_by_key(|t| t.target)), &[&Target::new(1, 42), &Target::new(2, 1337)] ); - assert_eq!(list.targets_with_values(1), &[Target::new(0, 42)]); - assert_eq!(list.targets_with_values(2), &[Target::new(0, 1337)]); + assert_eq!( + list.targets_with_values(1).as_slice(), + &[Target::new(0, 42)] + ); + assert_eq!( + list.targets_with_values(2).as_slice(), + &[Target::new(0, 1337)] + ); } #[test] @@ -505,7 +636,7 @@ mod test { assert_eq!( list.targets_with_values(0) - .iter() + .into_iter() .collect::>() .tap_mut(|v| v.sort_by_key(|t| t.target)), &[ @@ -517,14 +648,14 @@ mod test { ); assert_eq!( list.targets_with_values(1) - .iter() + .into_iter() .collect::>() .tap_mut(|v| v.sort_by_key(|t| t.target)), &[&Target::new(0, 42), &Target::new(0, 43)] ); assert_eq!( list.targets_with_values(2) - .iter() + .into_iter() .collect::>() .tap_mut(|v| v.sort_by_key(|t| t.target)), &[&Target::new(0, 1337), &Target::new(0, 1338)] @@ -545,8 +676,8 @@ mod test { let list = AdjacencyList::::from((&edges, 3, Direction::Outgoing, CsrLayout::Sorted)); - assert_eq!(list.targets(0), &[1, 2, 3]); - assert_eq!(list.targets(1), &[0, 2, 3]); + assert_eq!(list.targets(0).as_slice(), &[1, 2, 3]); + assert_eq!(list.targets(1).as_slice(), &[0, 2, 3]); } #[test] @@ -572,8 +703,8 @@ mod test { CsrLayout::Deduplicated, )); - assert_eq!(list.targets(0), &[1, 2, 3]); - assert_eq!(list.targets(1), &[0, 2, 3]); + assert_eq!(list.targets(0).as_slice(), &[1, 2, 3]); + assert_eq!(list.targets(1).as_slice(), &[0, 2, 3]); } #[test] @@ -606,7 +737,7 @@ mod test { #[test] fn directed_al_graph_add_edge_unsorted() { - let mut g = GraphBuilder::new() + let g = GraphBuilder::new() .csr_layout(CsrLayout::Unsorted) .edges([(0, 2), (1, 2)]) .build::>(); @@ -620,7 +751,7 @@ mod test { #[test] fn directed_al_graph_add_edge_sorted() { - let mut g = GraphBuilder::new() + let g = GraphBuilder::new() .csr_layout(CsrLayout::Sorted) .edges([(0, 2), (1, 2)]) .build::>(); @@ -634,7 +765,7 @@ mod test { #[test] fn directed_al_graph_add_edge_deduplicated() { - let mut g = GraphBuilder::new() + let g = GraphBuilder::new() .csr_layout(CsrLayout::Deduplicated) .edges([(0, 2), (1, 2), (1, 3)]) .build::>(); @@ -650,7 +781,7 @@ mod test { #[test] fn directed_al_graph_add_edge_missing_node() { - let mut g = GraphBuilder::new() + let g = GraphBuilder::new() .csr_layout(CsrLayout::Unsorted) .edges([(0, 2), (1, 2)]) .build::>(); @@ -660,6 +791,22 @@ mod test { assert!(matches!(err, crate::Error::MissingNode { node } if node == "3" )); } + #[test] + fn directed_al_graph_add_edge_parallel() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges([(0, 1), (0, 2), (0, 3)]) + .build::>(); + + std::thread::scope(|scope| { + for _ in 0..4 { + scope.spawn(|| g.add_edge(0, 1)); + } + }); + + assert_eq!(g.edge_count(), 7); + } + #[test] fn undirected_al_graph() { let g = GraphBuilder::new() diff --git a/crates/builder/src/lib.rs b/crates/builder/src/lib.rs index 89c61f6..181bd80 100644 --- a/crates/builder/src/lib.rs +++ b/crates/builder/src/lib.rs @@ -324,7 +324,7 @@ pub trait DirectedNeighborsWithValues { } pub trait EdgeMutation { - fn add_edge(&mut self, source: NI, target: NI) -> Result<(), Error>; + fn add_edge(&self, source: NI, target: NI) -> Result<(), Error>; } #[repr(transparent)] From 6b7566131b39fc4d9eb2406677dd8cc0deaef559 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Sun, 29 Oct 2023 11:58:06 +0100 Subject: [PATCH 03/15] Generalize add_edge to support edge values --- crates/builder/src/graph/adj_list.rs | 77 ++++++++++++++++++++-------- crates/builder/src/lib.rs | 4 ++ 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 558aceb..9cc5b4d 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -1,9 +1,9 @@ -use crate::EdgeMutation; use crate::{ index::Idx, prelude::Direction, prelude::Edges, prelude::NodeValues as NodeValuesTrait, CsrLayout, DirectedDegrees, DirectedNeighbors, DirectedNeighborsWithValues, Graph, Target, UndirectedDegrees, UndirectedNeighbors, UndirectedNeighborsWithValues, }; +use crate::{EdgeMutation, EdgeMutationWithValues}; use log::info; use std::sync::{Mutex, MutexGuard}; @@ -56,6 +56,25 @@ impl AdjacencyList { .len(), ) } + + #[inline] + pub(crate) fn insert(&self, source: NI, target: Target) { + let edges = &mut self.edges[source.index()] + .lock() + .expect("Could not aqcuire lock"); + + 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), + }, + }; + } } #[derive(Debug)] @@ -124,25 +143,6 @@ impl AdjacencyList { Targets { targets } } - - #[inline] - fn insert(&self, source: NI, target: Target) { - let edges = &mut self.edges[source.index()] - .lock() - .expect("Could not aqcuire lock"); - - 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), - }, - }; - } } #[derive(Debug)] @@ -354,6 +354,12 @@ impl DirectedNeighborsWithValues for DirectedALGraph EdgeMutation for DirectedALGraph { fn add_edge(&self, source: NI, target: NI) -> Result<(), crate::Error> { + self.add_edge_with_value(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()), @@ -365,8 +371,8 @@ impl EdgeMutation for DirectedALGraph { }); } - self.al_out.insert(source, Target::new(target, ())); - self.al_inc.insert(target, Target::new(source, ())); + self.al_out.insert(source, Target::new(target, value)); + self.al_inc.insert(target, Target::new(source, value)); Ok(()) } @@ -779,6 +785,33 @@ mod test { assert_eq!(g.out_neighbors(0).as_slice(), &[1, 2, 3]); } + #[test] + fn directed_al_graph_add_edge_with_value() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges_with_values([(0, 2, 4.2), (1, 2, 13.37)]) + .build::>(); + + assert_eq!( + g.out_neighbors_with_values(0).as_slice(), + &[Target::new(2, 4.2)] + ); + g.add_edge_with_value(0, 1, 19.84).expect("add edge failed"); + assert_eq!( + g.out_neighbors_with_values(0).as_slice(), + &[Target::new(2, 4.2), Target::new(1, 19.84)] + ); + g.add_edge_with_value(0, 2, 3.14).expect("add edge failed"); + assert_eq!( + g.out_neighbors_with_values(0).as_slice(), + &[ + Target::new(2, 4.2), + Target::new(1, 19.84), + Target::new(2, 3.14) + ] + ); + } + #[test] fn directed_al_graph_add_edge_missing_node() { let g = GraphBuilder::new() diff --git a/crates/builder/src/lib.rs b/crates/builder/src/lib.rs index 181bd80..0a8297a 100644 --- a/crates/builder/src/lib.rs +++ b/crates/builder/src/lib.rs @@ -327,6 +327,10 @@ pub trait EdgeMutation { fn add_edge(&self, source: NI, target: NI) -> Result<(), Error>; } +pub trait EdgeMutationWithValues { + fn add_edge_with_value(&self, source: NI, target: NI, value: EV) -> Result<(), Error>; +} + #[repr(transparent)] pub struct SharedMut(*mut T); unsafe impl Send for SharedMut {} From 1c98f5c93d00e6012564f72a91cf7fd806728ce3 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Sun, 29 Oct 2023 12:08:37 +0100 Subject: [PATCH 04/15] Document new mutation traits --- crates/builder/src/lib.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/builder/src/lib.rs b/crates/builder/src/lib.rs index 0a8297a..35bbf60 100644 --- a/crates/builder/src/lib.rs +++ b/crates/builder/src/lib.rs @@ -323,11 +323,26 @@ pub trait DirectedNeighborsWithValues { fn in_neighbors_with_values(&self, node: NI) -> Self::NeighborsIterator<'_>; } +/// Allows adding new edges to a graph. pub trait EdgeMutation { + /// Adds a new edge between the given source and target node. + /// + /// # Errors + /// + /// 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>; } +/// Allows adding new edges to a graph. pub trait EdgeMutationWithValues { + /// Adds a new edge between the given source and target node + /// and assigns the given value to it. + /// + /// # Errors + /// + /// 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>; } From c320eeac192ab7401461a1122a0d3e481041977c Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Mon, 30 Oct 2023 12:05:44 +0100 Subject: [PATCH 05/15] Impl mutation traits for UndirectedALGraph --- crates/builder/src/graph/adj_list.rs | 139 +++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 9cc5b4d..c0967d2 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -489,6 +489,32 @@ impl UndirectedNeighborsWithValues for UndirectedALGrap } } +impl EdgeMutation for UndirectedALGraph { + fn add_edge(&self, source: NI, target: NI) -> Result<(), crate::Error> { + self.add_edge_with_value(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.insert(source, Target::new(target, value)); + self.al.insert(target, Target::new(source, value)); + + Ok(()) + } +} + impl From<(E, CsrLayout)> for UndirectedALGraph where NI: Idx, @@ -840,6 +866,119 @@ mod test { assert_eq!(g.edge_count(), 7); } + #[test] + fn undirected_al_graph_add_edge_unsorted() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges([(0, 2), (1, 2)]) + .build::>(); + + assert_eq!(g.neighbors(0).as_slice(), &[2]); + assert_eq!(g.neighbors(1).as_slice(), &[2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[2, 1]); + assert_eq!(g.neighbors(1).as_slice(), &[2, 0]); + g.add_edge(0, 2).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[2, 1, 2]); + } + + #[test] + fn undirected_al_graph_add_edge_sorted() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Sorted) + .edges([(0, 2), (1, 2)]) + .build::>(); + + assert_eq!(g.neighbors(0).as_slice(), &[2]); + assert_eq!(g.neighbors(1).as_slice(), &[2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[1, 2]); + assert_eq!(g.neighbors(1).as_slice(), &[0, 2]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[1, 1, 2]); + } + + #[test] + fn undirected_al_graph_add_edge_deduplicated() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Deduplicated) + .edges([(0, 2), (1, 2), (1, 3)]) + .build::>(); + + assert_eq!(g.neighbors(0).as_slice(), &[2]); + assert_eq!(g.neighbors(1).as_slice(), &[2, 3]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[1, 2]); + assert_eq!(g.neighbors(1).as_slice(), &[0, 2, 3]); + g.add_edge(0, 1).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[1, 2]); + assert_eq!(g.neighbors(1).as_slice(), &[0, 2, 3]); + g.add_edge(0, 3).expect("add edge failed"); + assert_eq!(g.neighbors(0).as_slice(), &[1, 2, 3]); + } + + #[test] + fn undirected_al_graph_add_edge_with_value() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges_with_values([(0, 2, 4.2), (1, 2, 13.37)]) + .build::>(); + + assert_eq!( + g.neighbors_with_values(0).as_slice(), + &[Target::new(2, 4.2)] + ); + assert_eq!( + g.neighbors_with_values(1).as_slice(), + &[Target::new(2, 13.37)] + ); + g.add_edge_with_value(0, 1, 19.84).expect("add edge failed"); + assert_eq!( + g.neighbors_with_values(0).as_slice(), + &[Target::new(2, 4.2), Target::new(1, 19.84)] + ); + assert_eq!( + g.neighbors_with_values(1).as_slice(), + &[Target::new(2, 13.37), Target::new(0, 19.84)] + ); + g.add_edge_with_value(0, 2, 3.14).expect("add edge failed"); + assert_eq!( + g.neighbors_with_values(0).as_slice(), + &[ + Target::new(2, 4.2), + Target::new(1, 19.84), + Target::new(2, 3.14) + ] + ); + } + + #[test] + fn undirected_al_graph_add_edge_missing_node() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges([(0, 2), (1, 2)]) + .build::>(); + + let err = g.add_edge(0, 3).unwrap_err(); + + assert!(matches!(err, crate::Error::MissingNode { node } if node == "3" )); + } + + #[test] + fn undirected_al_graph_add_edge_parallel() { + let g = GraphBuilder::new() + .csr_layout(CsrLayout::Unsorted) + .edges([(0, 1), (0, 2), (0, 3)]) + .build::>(); + + std::thread::scope(|scope| { + for _ in 0..4 { + scope.spawn(|| g.add_edge(0, 1)); + } + }); + + assert_eq!(g.edge_count(), 7); + } #[test] fn undirected_al_graph() { let g = GraphBuilder::new() From 7b82e93c58c650d558373c00d7cffabefbed82a3 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Tue, 31 Oct 2023 12:43:28 +0100 Subject: [PATCH 06/15] Describe graph types in lib.rs --- crates/builder/src/lib.rs | 88 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/crates/builder/src/lib.rs b/crates/builder/src/lib.rs index 35bbf60..9192131 100644 --- a/crates/builder/src/lib.rs +++ b/crates/builder/src/lib.rs @@ -163,6 +163,92 @@ //! &[Target::new(0, 0.5)] //! ); //! ``` +//! +//! # Types of graphs +//! +//! The crate currently ships with two graph implementations: +//! +//! ## Compressed Sparse Row (CSR) +//! +//! [CSR](https://en.wikipedia.org/wiki/Sparse_matrix#Compressed_sparse_row_(CSR,_CRS_or_Yale_format)) +//! is a data structure used for representing a sparse matrix. Since graphs can be modelled as adjacency +//! matrix and are typically very sparse, i.e., not all possible pairs of nodes are connected +//! by an edge, the CSR representation is very well suited for representing a real-world graph topology. +//! +//! In our current implementation, we use two arrays two model the edges. One array stores the adjacency +//! lists for all nodes consecutively which requires `O(edge_count)` space. The other array stores the +//! offset for each node in the first array where the corresponding adjacency list can be found which +//! requires `O(node_count)` space. The degree of a node can be inferred from the offset array. +//! +//! Our CSR implementation is immutable, i.e., once built, the topology of the graph cannot be altered as +//! it would require inserting target ids and shifting all elements to the right which is expensive and +//! invalidates all offsets coming afterwards. However, building the CSR data structure from a list of +//! edges is implement very efficiently using multi-threading. +//! +//! However, due to inlining the all adjacency lists in one `Vec`, access becomes very cache-friendly, +//! as there is a chance that the adjacency list of the next node is already cached. Also, reading the +//! graph from multiple threads is safe, as there will be never be a concurrent mutable access. +//! +//! One can use [`DirectedCsrGraph`] or [`UndirectedCsrGraph`] to build a CSR-based graph: +//! +//! ``` +//! use graph_builder::prelude::*; +//! +//! let graph: DirectedCsrGraph = GraphBuilder::new() +//! .edges(vec![(0, 1), (0, 2), (1, 2), (1, 3), (2, 3)]) +//! .build(); +//! +//! assert_eq!(graph.node_count(), 4); +//! assert_eq!(graph.edge_count(), 5); +//! +//! assert_eq!(graph.out_degree(1), 2); +//! assert_eq!(graph.in_degree(1), 1); +//! +//! assert_eq!(graph.out_neighbors(1).as_slice(), &[2, 3]); +//! assert_eq!(graph.in_neighbors(1).as_slice(), &[0]); +//! ``` +//! +//! ## Adjacency List (AL) +//! +//! In the Adjacency List implementation, we essentially store the graph as `Vec>`. The outer +//! `Vec` has a length of `node_count` and at each index, we store the neighbors for that particular +//! node in its own, heap-allocated `Vec`. +//! +//! The downside of that representation is that - compared to CSR - it is expected to be slower, both +//! in building it and also in reading from it, as cache misses are becoming more likely due to the +//! isolated heap allocations for individual neighbor lists. +//! +//! However, in contrast to CSR, an adjacency list is mutable, i.e., it is possible to add edges to the +//! graph even after it has been built. This makes the data structure interesting for more flexible graph +//! construction frameworks or for algorithms that need to add new edges as part of the computation. +//! Currently, adding edges is constrained by source and target node already existing in the graph. +//! +//! Internally, the individual neighbor lists for each node are protected by a `Mutex` in order to support +//! parallel read and write operations on the graph topology. +//! +//! One can use [`DirectedALGraph`] or [`UndirectedALGraph`] to build a Adjacency-List-based graph: +//! +//! ``` +//! use graph_builder::prelude::*; +//! +//! let graph: DirectedALGraph = GraphBuilder::new() +//! .edges(vec![(0, 1), (0, 2), (1, 2), (1, 3), (2, 3)]) +//! .build(); +//! +//! assert_eq!(graph.node_count(), 4); +//! assert_eq!(graph.edge_count(), 5); +//! +//! assert_eq!(graph.out_degree(1), 2); +//! assert_eq!(graph.in_degree(1), 1); +//! +//! assert_eq!(graph.out_neighbors(1).as_slice(), &[2, 3]); +//! assert_eq!(graph.in_neighbors(1).as_slice(), &[0]); +//! +//! // Let's mutate the graph by adding another edge +//! graph.add_edge(1, 0); +//! assert_eq!(graph.edge_count(), 6); +//! assert_eq!(graph.out_neighbors(1).as_slice(), &[2, 3, 0]); +//! ``` pub mod builder; mod compat; @@ -173,6 +259,8 @@ pub mod input; pub mod prelude; pub use crate::builder::GraphBuilder; +pub use crate::graph::adj_list::DirectedALGraph; +pub use crate::graph::adj_list::UndirectedALGraph; pub use crate::graph::csr::CsrLayout; pub use crate::graph::csr::DirectedCsrGraph; pub use crate::graph::csr::UndirectedCsrGraph; From e8fde18fcce4a8bfb607f81e5f84f882f7cd5ac6 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Tue, 31 Oct 2023 13:12:03 +0100 Subject: [PATCH 07/15] Update graph_builder README.md --- crates/builder/README.md | 90 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 2 deletions(-) diff --git a/crates/builder/README.md b/crates/builder/README.md index cfeb049..add7fff 100644 --- a/crates/builder/README.md +++ b/crates/builder/README.md @@ -15,7 +15,7 @@ is tailored for fast and concurrent access to the graph topology. [Neo4j](https://github.com/neo4j/neo4j) developers. However, the library is __not__ an official product of Neo4j. -## What is a graph? +# What is a graph? A graph consists of nodes and edges where edges connect exactly two nodes. A graph can be either directed, i.e., an edge has a source and a target node @@ -30,7 +30,7 @@ In an undirected graph there is no distinction between source and target node. A neighbor of node `u` is any node `v` for which either an edge `(u, v)` or `(v, u)` exists. -## How to build a graph +# How to build a graph The library provides a builder that can be used to construct a graph from a given list of edges. @@ -161,4 +161,90 @@ assert_eq!( ); ``` +# Types of graphs + +The crate currently ships with two graph implementations: + +## Compressed Sparse Row (CSR) + +[CSR](https://en.wikipedia.org/wiki/Sparse_matrix#Compressed_sparse_row_(CSR,_CRS_or_Yale_format)) +is a data structure used for representing a sparse matrix. Since graphs can be modelled as adjacency +matrix and are typically very sparse, i.e., not all possible pairs of nodes are connected +by an edge, the CSR representation is very well suited for representing a real-world graph topology. + +In our current implementation, we use two arrays two model the edges. One array stores the adjacency +lists for all nodes consecutively which requires `O(edge_count)` space. The other array stores the +offset for each node in the first array where the corresponding adjacency list can be found which +requires `O(node_count)` space. The degree of a node can be inferred from the offset array. + +Our CSR implementation is immutable, i.e., once built, the topology of the graph cannot be altered as +it would require inserting target ids and shifting all elements to the right which is expensive and +invalidates all offsets coming afterwards. However, building the CSR data structure from a list of +edges is implement very efficiently using multi-threading. + +However, due to inlining the all adjacency lists in one `Vec`, access becomes very cache-friendly, +as there is a chance that the adjacency list of the next node is already cached. Also, reading the +graph from multiple threads is safe, as there will be never be a concurrent mutable access. + +One can use [`DirectedCsrGraph`] or [`UndirectedCsrGraph`] to build a CSR-based graph: + +```rust +use graph_builder::prelude::*; + +let graph: DirectedCsrGraph = GraphBuilder::new() + .edges(vec![(0, 1), (0, 2), (1, 2), (1, 3), (2, 3)]) + .build(); + +assert_eq!(graph.node_count(), 4); +assert_eq!(graph.edge_count(), 5); + +assert_eq!(graph.out_degree(1), 2); +assert_eq!(graph.in_degree(1), 1); + +assert_eq!(graph.out_neighbors(1).as_slice(), &[2, 3]); +assert_eq!(graph.in_neighbors(1).as_slice(), &[0]); +``` + +## Adjacency List (AL) + +In the Adjacency List implementation, we essentially store the graph as `Vec>`. The outer +`Vec` has a length of `node_count` and at each index, we store the neighbors for that particular +node in its own, heap-allocated `Vec`. + +The downside of that representation is that - compared to CSR - it is expected to be slower, both +in building it and also in reading from it, as cache misses are becoming more likely due to the +isolated heap allocations for individual neighbor lists. + +However, in contrast to CSR, an adjacency list is mutable, i.e., it is possible to add edges to the +graph even after it has been built. This makes the data structure interesting for more flexible graph +construction frameworks or for algorithms that need to add new edges as part of the computation. +Currently, adding edges is constrained by source and target node already existing in the graph. + +Internally, the individual neighbor lists for each node are protected by a `Mutex` in order to support +parallel read and write operations on the graph topology. + +One can use [`DirectedALGraph`] or [`UndirectedALGraph`] to build a Adjacency-List-based graph: + +```rust +use graph_builder::prelude::*; + +let graph: DirectedALGraph = GraphBuilder::new() + .edges(vec![(0, 1), (0, 2), (1, 2), (1, 3), (2, 3)]) + .build(); + +assert_eq!(graph.node_count(), 4); +assert_eq!(graph.edge_count(), 5); + +assert_eq!(graph.out_degree(1), 2); +assert_eq!(graph.in_degree(1), 1); + +assert_eq!(graph.out_neighbors(1).as_slice(), &[2, 3]); +assert_eq!(graph.in_neighbors(1).as_slice(), &[0]); + +// Let's mutate the graph by adding another edge +graph.add_edge(1, 0); +assert_eq!(graph.edge_count(), 6); +assert_eq!(graph.out_neighbors(1).as_slice(), &[2, 3, 0]); +``` + License: MIT From 2bfa5ef4c403cb264a26442daf87d79091c7cdf0 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Tue, 31 Oct 2023 14:51:13 +0100 Subject: [PATCH 08/15] Make clippy happy --- crates/builder/src/graph/adj_list.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index c0967d2..f934474 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -827,13 +827,13 @@ mod test { g.out_neighbors_with_values(0).as_slice(), &[Target::new(2, 4.2), Target::new(1, 19.84)] ); - g.add_edge_with_value(0, 2, 3.14).expect("add edge failed"); + g.add_edge_with_value(0, 2, 1.23).expect("add edge failed"); assert_eq!( g.out_neighbors_with_values(0).as_slice(), &[ Target::new(2, 4.2), Target::new(1, 19.84), - Target::new(2, 3.14) + Target::new(2, 1.23) ] ); } @@ -941,13 +941,13 @@ mod test { g.neighbors_with_values(1).as_slice(), &[Target::new(2, 13.37), Target::new(0, 19.84)] ); - g.add_edge_with_value(0, 2, 3.14).expect("add edge failed"); + g.add_edge_with_value(0, 2, 1.23).expect("add edge failed"); assert_eq!( g.neighbors_with_values(0).as_slice(), &[ Target::new(2, 4.2), Target::new(1, 19.84), - Target::new(2, 3.14) + Target::new(2, 1.23) ] ); } From 95383115aa3f5f56bd16738a11be436cb1098759 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:06:55 +0100 Subject: [PATCH 09/15] Just panic if the lock is poisoned Co-Authored-By: Paul Horn --- crates/builder/src/graph/adj_list.rs | 32 +++++++--------------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index f934474..90fb951 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -39,29 +39,17 @@ impl AdjacencyList { NI: Send + Sync, EV: Send + Sync, { - NI::new( - self.edges - .par_iter() - .map(|v| v.lock().expect("Could not acquire lock").len()) - .sum(), - ) + NI::new(self.edges.par_iter().map(|v| v.lock().unwrap().len()).sum()) } #[inline] pub(crate) fn degree(&self, node: NI) -> NI { - NI::new( - self.edges[node.index()] - .lock() - .expect("Could not aqcuire lock") - .len(), - ) + NI::new(self.edges[node.index()].lock().unwrap().len()) } #[inline] pub(crate) fn insert(&self, source: NI, target: Target) { - let edges = &mut self.edges[source.index()] - .lock() - .expect("Could not aqcuire lock"); + let edges = &mut self.edges[source.index()].lock().unwrap(); match self.layout { CsrLayout::Sorted => match edges.binary_search(&target) { @@ -137,9 +125,7 @@ impl AdjacencyList { where 'list: 'slice, { - let targets = self.edges[node.index()] - .lock() - .expect("Could not acquire lock"); + let targets = self.edges[node.index()].lock().unwrap(); Targets { targets } } @@ -201,9 +187,7 @@ impl AdjacencyList { 'list: 'slice, { TargetsWithValues { - targets: self.edges[node.index()] - .lock() - .expect("Could not aqcuire lock"), + targets: self.edges[node.index()].lock().unwrap(), } } } @@ -226,13 +210,13 @@ where if matches!(direction, Direction::Outgoing | Direction::Undirected) { thread_safe_vec[s.index()] .lock() - .expect("Cannot lock node-local list") + .unwrap() .push(Target::new(t, v)); } if matches!(direction, Direction::Incoming | Direction::Undirected) { thread_safe_vec[t.index()] .lock() - .expect("Cannot lock node-local list") + .unwrap() .push(Target::new(s, v)); } }); @@ -243,7 +227,7 @@ where thread_safe_vec .into_par_iter() .map(|list| { - let mut list = list.into_inner().expect("Cannot move out of Mutex"); + let mut list = list.into_inner().unwrap(); match csr_layout { CsrLayout::Sorted => list.sort_unstable_by_key(|t| t.target), From 8bc386dcdcb4a1a74f95f5480085aa5693b55bd0 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:09:35 +0100 Subject: [PATCH 10/15] Remove unnecessary &mut Co-Authored-By: Paul Horn --- crates/builder/src/graph/adj_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 90fb951..6839e79 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -49,7 +49,7 @@ impl AdjacencyList { #[inline] pub(crate) fn insert(&self, source: NI, target: Target) { - let edges = &mut self.edges[source.index()].lock().unwrap(); + let mut edges = self.edges[source.index()].lock().unwrap(); match self.layout { CsrLayout::Sorted => match edges.binary_search(&target) { From cacbad2ab3fc737b7d73ae8585050ce15ca301e6 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:13:32 +0100 Subject: [PATCH 11/15] Remove extra lifetime + constraint from targets methods Co-Authored-By: Paul Horn --- crates/builder/src/graph/adj_list.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 6839e79..0312cdf 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -121,10 +121,7 @@ impl<'slice, NI: Idx> Iterator for TargetsIter<'slice, NI> { impl AdjacencyList { #[inline] - pub(crate) fn targets<'list, 'slice>(&'list self, node: NI) -> Targets<'slice, NI> - where - 'list: 'slice, - { + pub(crate) fn targets(&self, node: NI) -> Targets<'_, NI> { let targets = self.edges[node.index()].lock().unwrap(); Targets { targets } @@ -179,13 +176,7 @@ impl<'slice, NI: Idx, EV> Iterator for TargetsWithValuesIter<'slice, NI, EV> { impl AdjacencyList { #[inline] - pub(crate) fn targets_with_values<'list, 'slice>( - &'list self, - node: NI, - ) -> TargetsWithValues<'slice, NI, EV> - where - 'list: 'slice, - { + pub(crate) fn targets_with_values(&self, node: NI) -> TargetsWithValues<'_, NI, EV> { TargetsWithValues { targets: self.edges[node.index()].lock().unwrap(), } From 6b59cac24add39dfb5e6d7fc37b93ab8ca546143 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:19:15 +0100 Subject: [PATCH 12/15] Extend SAFETY note Co-Authored-By: Paul Horn --- crates/builder/src/graph/adj_list.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 0312cdf..25affc2 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -82,6 +82,8 @@ impl<'slice, NI: Idx> Targets<'slice, NI> { ); // SAFETY: The types Target and T are verified to have the same // size and alignment. + // We can upcast the lifetime since the MutexGuard + // is not exposed, so it is not possible to deref mutable. unsafe { std::slice::from_raw_parts(self.targets.as_ptr().cast(), self.targets.len()) } } } From 6909e3b7082404d62173e91d53831ef30bd6999f Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:20:07 +0100 Subject: [PATCH 13/15] Remove unneeded lifetime Co-Authored-By: Paul Horn --- crates/builder/src/graph/adj_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 25affc2..7dbef13 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -308,7 +308,7 @@ impl DirectedDegrees for DirectedALGraph { impl DirectedNeighbors for DirectedALGraph { type NeighborsIterator<'a> = TargetsIter<'a, NI> where NV: 'a; - fn out_neighbors<'slice>(&self, node: NI) -> Self::NeighborsIterator<'_> { + fn out_neighbors(&self, node: NI) -> Self::NeighborsIterator<'_> { self.al_out.targets(node).into_iter() } From 907d727785c64a51287602704f5516ad35668b70 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:21:34 +0100 Subject: [PATCH 14/15] Export EdgeMutationWithValues Co-Authored-By: Paul Horn --- crates/builder/src/prelude.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/builder/src/prelude.rs b/crates/builder/src/prelude.rs index e410cd9..1708ceb 100644 --- a/crates/builder/src/prelude.rs +++ b/crates/builder/src/prelude.rs @@ -33,5 +33,6 @@ pub use crate::UndirectedNeighbors; pub use crate::UndirectedNeighborsWithValues; pub use crate::EdgeMutation; +pub use crate::EdgeMutationWithValues; pub use crate::Error; From 9766b7a7d81d1e5efc8e25ffa30b30b023733af3 Mon Sep 17 00:00:00 2001 From: Martin Junghanns Date: Fri, 3 Nov 2023 12:48:20 +0100 Subject: [PATCH 15/15] Statically check that AL is Send + Sync Co-Authored-By: Paul Horn --- crates/builder/src/graph/adj_list.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/builder/src/graph/adj_list.rs b/crates/builder/src/graph/adj_list.rs index 7dbef13..5fcf9cd 100644 --- a/crates/builder/src/graph/adj_list.rs +++ b/crates/builder/src/graph/adj_list.rs @@ -18,6 +18,14 @@ pub struct AdjacencyList { layout: CsrLayout, } +const _: () = { + const fn is_send() {} + const fn is_sync() {} + + is_send::>(); + is_sync::>(); +}; + impl AdjacencyList { pub fn new(edges: Vec>>) -> Self { Self::with_layout(edges, CsrLayout::Unsorted)