Merge pull request #1270 from lf-lang/better-cycle-detection

Do not include in cycles downstream reactions
lf-lang · Jul 5, 2022 · c986115 · c986115
2 parents 8eec152 + 96e38f5
commit c986115
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 16 deletions.
diff --git a/org.lflang/src/org/lflang/generator/ReactionInstanceGraph.java b/org.lflang/src/org/lflang/generator/ReactionInstanceGraph.java
@@ -32,7 +32,7 @@ STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 import java.util.Set;
 
 import org.lflang.generator.ReactionInstance.Runtime;
-import org.lflang.graph.DirectedGraph;
+import org.lflang.graph.PrecedenceGraph;
 import org.lflang.lf.Variable;
 
 /**
@@ -54,7 +54,7 @@ STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * @author{Marten Lohstroh <marten@berkeley.edu>}
  * @author{Edward A. Lee <eal@berkeley.edu>}
  */
-public class ReactionInstanceGraph extends DirectedGraph<ReactionInstance.Runtime> {
+public class ReactionInstanceGraph extends PrecedenceGraph<ReactionInstance.Runtime> {
 
     /**
      * Create a new graph by traversing the maps in the named instances 

diff --git a/org.lflang/src/org/lflang/generator/ReactorInstance.java b/org.lflang/src/org/lflang/generator/ReactorInstance.java
@@ -244,26 +244,34 @@ public void clearCaches(boolean includingRuntimes) {
     public Set<NamedInstance<?>> getCycles() {
         if (depth != 0) return root().getCycles();
         if (cachedCycles != null) return cachedCycles;
-        Set<ReactionInstance> reactions = new LinkedHashSet<>();
+        cachedCycles = new LinkedHashSet<>();
 
         ReactionInstanceGraph reactionRuntimes = assignLevels();
-        for (ReactionInstance.Runtime runtime : reactionRuntimes.nodes()) {
-            reactions.add(runtime.getReaction());
-        }
-        Set<PortInstance> ports = new LinkedHashSet<>();
-        // Need to figure out which ports are involved in the cycles.
-        // It may not be all ports that depend on this reaction.
-        for (ReactionInstance r : reactions) {
-            for (TriggerInstance<? extends Variable> p : r.effects) {
-                if (p instanceof PortInstance) {
-                    findPaths((PortInstance)p, reactions, ports);
+        if (reactionRuntimes.nodes().size() > 0) {
+            Set<ReactionInstance> reactions = new LinkedHashSet<>();
+            Set<PortInstance> ports = new LinkedHashSet<>();
+            // There are cycles. But the nodes set includes not
+            // just the cycles, but also nodes that are downstream of the
+            // cycles.  Use Tarjan's algorithm to get just the cycles.
+            var cycleNodes = reactionRuntimes.getCycles();
+            for (var cycle : cycleNodes) {
+                for (ReactionInstance.Runtime runtime : cycle) {
+                    reactions.add(runtime.getReaction());
+                }
+            }
+            // Need to figure out which ports are involved in the cycles.
+            // It may not be all ports that depend on this reaction.
+            for (ReactionInstance r : reactions) {
+                for (TriggerInstance<? extends Variable> p : r.effects) {
+                    if (p instanceof PortInstance) {
+                        findPaths((PortInstance)p, reactions, ports);
+                    }
                 }
             }
+            cachedCycles.addAll(reactions);
+            cachedCycles.addAll(ports);
         }
 
-        cachedCycles = new LinkedHashSet<>();
-        cachedCycles.addAll(reactions);
-        cachedCycles.addAll(ports);
         return cachedCycles;
     }