Merge pull request #193 from github/new-cycle-detection

Per-path cycle detection

Author: Hendrik van Antwerpen

stack-graphs/include/stack-graphs.h

@@ -1072,9 +1072,9 @@ struct sg_forward_partial_path_stitcher *sg_forward_partial_path_stitcher_from_n
 // must ensure that `db` contains all possible extensions of any of those initial partial paths.
 // You can retrieve a list of those extensions via the `previous_phase_partial_paths` and
 // `previous_phase_partial_paths_length` fields.
-struct sg_forward_partial_path_stitcher *sg_forward_partial_path_stitcher_from_partial_paths(const struct sg_stack_graph *_graph,
+struct sg_forward_partial_path_stitcher *sg_forward_partial_path_stitcher_from_partial_paths(const struct sg_stack_graph *graph,
                                                                                              struct sg_partial_path_arena *partials,
-                                                                                             struct sg_partial_path_database *_db,
+                                                                                             struct sg_partial_path_database *db,
                                                                                              size_t count,
                                                                                              const struct sg_partial_path *initial_partial_paths);
 

stack-graphs/src/c.rs

@@ -1989,16 +1989,23 @@ pub extern "C" fn sg_forward_partial_path_stitcher_from_nodes(
 /// `previous_phase_partial_paths_length` fields.
 #[no_mangle]
 pub extern "C" fn sg_forward_partial_path_stitcher_from_partial_paths(
-    _graph: *const sg_stack_graph,
+    graph: *const sg_stack_graph,
     partials: *mut sg_partial_path_arena,
-    _db: *mut sg_partial_path_database,
+    db: *mut sg_partial_path_database,
     count: usize,
     initial_partial_paths: *const sg_partial_path,
 ) -> *mut sg_forward_partial_path_stitcher {
+    let graph = unsafe { &(*graph).inner };
     let partials = unsafe { &mut (*partials).inner };
+    let db = unsafe { &mut (*db).inner };
     let initial_partial_paths =
         unsafe { std::slice::from_raw_parts(initial_partial_paths as *const PartialPath, count) };
-    let stitcher = ForwardPartialPathStitcher::from_partial_paths(initial_partial_paths.to_vec());
+    let stitcher = ForwardPartialPathStitcher::from_partial_paths(
+        graph,
+        partials,
+        db,
+        initial_partial_paths.to_vec(),
+    );
     Box::into_raw(Box::new(InternalForwardPartialPathStitcher::new(
         stitcher, partials,
     ))) as *mut _

stack-graphs/src/cycles.rs

@@ -34,12 +34,20 @@ use std::collections::HashMap;
 use smallvec::SmallVec;
 
 use crate::arena::Handle;
+use crate::arena::List;
+use crate::arena::ListArena;
+use crate::graph::Edge;
 use crate::graph::Node;
+use crate::graph::StackGraph;
 use crate::partial::PartialPath;
+use crate::partial::PartialPaths;
 use crate::paths::Path;
+use crate::paths::PathResolutionError;
+use crate::stitching::Database;
+use crate::stitching::OwnedOrDatabasePath;
 
-/// Helps detect cycles in the path-finding algorithm.
-pub struct CycleDetector<P> {
+/// Helps detect similar paths in the path-finding algorithm.
+pub struct SimilarPathDetector<P> {
     paths: HashMap<PathKey, SmallVec<[P; 8]>>,
 }
 
@@ -86,13 +94,13 @@ impl HasPathKey for PartialPath {
 
 const MAX_SIMILAR_PATH_COUNT: usize = 7;
 
-impl<P> CycleDetector<P>
+impl<P> SimilarPathDetector<P>
 where
     P: HasPathKey,
 {
     /// Creates a new, empty cycle detector.
-    pub fn new() -> CycleDetector<P> {
-        CycleDetector {
+    pub fn new() -> SimilarPathDetector<P> {
+        SimilarPathDetector {
             paths: HashMap::new(),
         }
     }
@@ -127,3 +135,144 @@ where
         true
     }
 }
+
+// ----------------------------------------------------------------------------
+// Cycle detector
+
+pub trait Appendable {
+    type Ctx;
+
+    fn append_to(
+        &self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        ctx: &mut Self::Ctx,
+        path: &mut PartialPath,
+    ) -> Result<(), PathResolutionError>;
+    fn start_node(&self, ctx: &mut Self::Ctx) -> Handle<Node>;
+    fn end_node(&self, ctx: &mut Self::Ctx) -> Handle<Node>;
+}
+
+#[derive(Clone)]
+pub struct AppendingCycleDetector<A> {
+    appendages: List<A>,
+}
+
+pub type Appendables<A> = ListArena<A>;
+
+impl<A: Appendable + Clone> AppendingCycleDetector<A> {
+    pub fn new() -> Self {
+        Self {
+            appendages: List::empty(),
+        }
+    }
+
+    pub fn from(appendables: &mut Appendables<A>, appendage: A) -> Self {
+        let mut result = Self::new();
+        result.appendages.push_front(appendables, appendage);
+        result
+    }
+
+    pub fn append(
+        &mut self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        ctx: &mut A::Ctx,
+        appendables: &mut Appendables<A>,
+        appendage: A,
+    ) -> Result<(), PathResolutionError> {
+        let end_node = appendage.end_node(ctx);
+        self.appendages.push_front(appendables, appendage);
+
+        let mut maybe_cyclic_path = None;
+        let mut appendages = self.appendages;
+        loop {
+            // get prefix elements
+            let mut prefix_appendages = List::empty();
+            loop {
+                let appendable = appendages.pop_front(appendables).cloned();
+                match appendable {
+                    Some(appendage) => {
+                        let is_cycle = appendage.start_node(ctx) == end_node;
+                        prefix_appendages.push_front(appendables, appendage);
+                        if is_cycle {
+                            break;
+                        }
+                    }
+                    None => return Ok(()),
+                }
+            }
+
+            // build prefix path -- prefix starts at end_node, because this is a cycle
+            let mut prefix_path = PartialPath::from_node(graph, partials, end_node);
+            while let Some(appendage) = prefix_appendages.pop_front(appendables) {
+                prefix_path
+                    .resolve_to(graph, partials, appendage.start_node(ctx))
+                    .expect("resolving cycle prefix path failed");
+                appendage
+                    .append_to(graph, partials, ctx, &mut prefix_path)
+                    .expect("appending cycle prefix path failed");
+            }
+
+            // build cyclic path
+            let cyclic_path = maybe_cyclic_path
+                .unwrap_or_else(|| PartialPath::from_node(graph, partials, end_node));
+            prefix_path
+                .resolve_to(graph, partials, cyclic_path.start_node)
+                .expect("resolving cyclic path failed");
+            prefix_path.ensure_no_overlapping_variables(partials, &cyclic_path);
+            prefix_path
+                .concatenate(graph, partials, &cyclic_path)
+                .expect("concatenating cyclic path failed ");
+            if !prefix_path.is_productive(graph, partials) {
+                return Err(PathResolutionError::DisallowedCycle);
+            }
+            maybe_cyclic_path = Some(prefix_path);
+        }
+    }
+}
+
+impl Appendable for Edge {
+    type Ctx = ();
+
+    fn append_to(
+        &self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        _: &mut (),
+        path: &mut PartialPath,
+    ) -> Result<(), PathResolutionError> {
+        path.append(graph, partials, *self)
+    }
+
+    fn start_node(&self, _: &mut ()) -> Handle<Node> {
+        self.source
+    }
+
+    fn end_node(&self, _: &mut ()) -> Handle<Node> {
+        self.sink
+    }
+}
+
+impl Appendable for OwnedOrDatabasePath {
+    type Ctx = Database;
+
+    fn append_to(
+        &self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        db: &mut Database,
+        path: &mut PartialPath,
+    ) -> Result<(), PathResolutionError> {
+        path.ensure_no_overlapping_variables(partials, self.get(db));
+        path.concatenate(graph, partials, self.get(db))
+    }
+
+    fn start_node(&self, db: &mut Database) -> Handle<Node> {
+        self.get(db).start_node
+    }
+
+    fn end_node(&self, db: &mut Database) -> Handle<Node> {
+        self.get(db).end_node
+    }
+}

stack-graphs/src/lib.rs

@@ -55,6 +55,8 @@
 //! importantly, each “chunk” of the overall graph only depends on “local” information from the
 //! original source file.  (a.k.a., it’s incremental!)
 
+use std::time::{Duration, Instant};
+
 use thiserror::Error;
 
 pub mod arena;
@@ -85,6 +87,29 @@ impl CancellationFlag for NoCancellation {
     }
 }
 
+pub struct CancelAfterDuration {
+    limit: Duration,
+    start: Instant,
+}
+
+impl CancelAfterDuration {
+    pub fn new(limit: Duration) -> Self {
+        Self {
+            limit,
+            start: Instant::now(),
+        }
+    }
+}
+
+impl CancellationFlag for CancelAfterDuration {
+    fn check(&self, at: &'static str) -> Result<(), CancellationError> {
+        if self.start.elapsed() > self.limit {
+            return Err(CancellationError(at));
+        }
+        Ok(())
+    }
+}
+
 #[derive(Clone, Debug, Error)]
 #[error("Cancelled at \"{0}\"")]
 pub struct CancellationError(pub &'static str);