diff --git a/android/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java b/android/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java index d81edaf6678c..a6860929048e 100644 --- a/android/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java +++ b/android/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java @@ -155,6 +155,17 @@ public void hasCycle_multipleCycles() { assertThat(hasCycle(undirectedGraph)).isTrue(); } + @Test + public void hasCycle_deepPathGraph() { + for (MutableGraph graph : graphsToTest) { + for (int i = 0; i < 100000; i++) { + graph.putEdge(i, i + 1); + } + } + assertThat(hasCycle(directedNetwork)).isFalse(); + assertThat(hasCycle(undirectedNetwork)).isFalse(); + } + @Test public void hasCycle_twoParallelEdges() { for (MutableNetwork network : networksToTest) { @@ -176,4 +187,15 @@ public void hasCycle_cyclicMultigraph() { assertThat(hasCycle(directedNetwork)).isTrue(); assertThat(hasCycle(undirectedNetwork)).isTrue(); } + + @Test + public void hasCycle_deepPathNetwork() { + for (MutableNetwork network : networksToTest) { + for (int i = 0; i < 100000; i++) { + network.addEdge(i, i + 1, Integer.toString(i)); + } + } + assertThat(hasCycle(directedNetwork)).isFalse(); + assertThat(hasCycle(undirectedNetwork)).isFalse(); + } } diff --git a/android/guava/src/com/google/common/graph/Graphs.java b/android/guava/src/com/google/common/graph/Graphs.java index b3268015b2fe..948fd000181a 100644 --- a/android/guava/src/com/google/common/graph/Graphs.java +++ b/android/guava/src/com/google/common/graph/Graphs.java @@ -27,10 +27,13 @@ import com.google.common.collect.Iterators; import com.google.common.collect.Maps; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.util.ArrayDeque; import java.util.Collection; +import java.util.Deque; import java.util.HashSet; import java.util.Iterator; import java.util.Map; +import java.util.Queue; import java.util.Set; import javax.annotation.CheckForNull; @@ -68,7 +71,7 @@ public static boolean hasCycle(Graph graph) { Map visitedNodes = Maps.newHashMapWithExpectedSize(graph.nodes().size()); for (N node : graph.nodes()) { - if (subgraphHasCycle(graph, visitedNodes, node, null)) { + if (subgraphHasCycle(graph, visitedNodes, node)) { return true; } } @@ -94,34 +97,67 @@ public static boolean hasCycle(Network network) { } /** - * Performs a traversal of the nodes reachable from {@code node}. If we ever reach a node we've - * already visited (following only outgoing edges and without reusing edges), we know there's a - * cycle in the graph. + * Performs a traversal of the nodes reachable from {@code startNode}. If we ever reach a node + * we've already visited (following only outgoing edges and without reusing edges), we know + * there's a cycle in the graph. */ private static boolean subgraphHasCycle( - Graph graph, - Map visitedNodes, - N node, - @CheckForNull N previousNode) { - NodeVisitState state = visitedNodes.get(node); - if (state == NodeVisitState.COMPLETE) { - return false; - } - if (state == NodeVisitState.PENDING) { - return true; - } + Graph graph, Map visitedNodes, N startNode) { + Deque> stack = new ArrayDeque<>(); + stack.addLast(new NodeAndRemainingSuccessors<>(startNode)); + + while (!stack.isEmpty()) { + // To peek at the top two items, we need to temporarily remove one. + NodeAndRemainingSuccessors top = stack.removeLast(); + NodeAndRemainingSuccessors prev = stack.peekLast(); + stack.addLast(top); + + N node = top.node; + N previousNode = prev == null ? null : prev.node; + if (top.remainingSuccessors == null) { + NodeVisitState state = visitedNodes.get(node); + if (state == NodeVisitState.COMPLETE) { + stack.removeLast(); + continue; + } + if (state == NodeVisitState.PENDING) { + return true; + } - visitedNodes.put(node, NodeVisitState.PENDING); - for (N nextNode : graph.successors(node)) { - if (canTraverseWithoutReusingEdge(graph, nextNode, previousNode) - && subgraphHasCycle(graph, visitedNodes, nextNode, node)) { - return true; + visitedNodes.put(node, NodeVisitState.PENDING); + top.remainingSuccessors = new ArrayDeque<>(graph.successors(node)); + } + + if (!top.remainingSuccessors.isEmpty()) { + N nextNode = top.remainingSuccessors.remove(); + if (canTraverseWithoutReusingEdge(graph, nextNode, previousNode)) { + stack.addLast(new NodeAndRemainingSuccessors<>(nextNode)); + continue; + } } + + stack.removeLast(); + visitedNodes.put(node, NodeVisitState.COMPLETE); } - visitedNodes.put(node, NodeVisitState.COMPLETE); return false; } + private static final class NodeAndRemainingSuccessors { + final N node; + + /** + * The successors left to be visited, or {@code null} if we just added this {@code + * NodeAndRemainingSuccessors} instance to the stack. In the latter case, we'll compute the + * successors if we determine that we need them after we've performed the initial processing of + * the node. + */ + @CheckForNull Queue remainingSuccessors; + + NodeAndRemainingSuccessors(N node) { + this.node = node; + } + } + /** * Determines whether an edge has already been used during traversal. In the directed case a cycle * is always detected before reusing an edge, so no special logic is required. In the undirected diff --git a/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java b/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java index d81edaf6678c..a6860929048e 100644 --- a/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java +++ b/guava-tests/test/com/google/common/graph/GraphPropertiesTest.java @@ -155,6 +155,17 @@ public void hasCycle_multipleCycles() { assertThat(hasCycle(undirectedGraph)).isTrue(); } + @Test + public void hasCycle_deepPathGraph() { + for (MutableGraph graph : graphsToTest) { + for (int i = 0; i < 100000; i++) { + graph.putEdge(i, i + 1); + } + } + assertThat(hasCycle(directedNetwork)).isFalse(); + assertThat(hasCycle(undirectedNetwork)).isFalse(); + } + @Test public void hasCycle_twoParallelEdges() { for (MutableNetwork network : networksToTest) { @@ -176,4 +187,15 @@ public void hasCycle_cyclicMultigraph() { assertThat(hasCycle(directedNetwork)).isTrue(); assertThat(hasCycle(undirectedNetwork)).isTrue(); } + + @Test + public void hasCycle_deepPathNetwork() { + for (MutableNetwork network : networksToTest) { + for (int i = 0; i < 100000; i++) { + network.addEdge(i, i + 1, Integer.toString(i)); + } + } + assertThat(hasCycle(directedNetwork)).isFalse(); + assertThat(hasCycle(undirectedNetwork)).isFalse(); + } } diff --git a/guava/src/com/google/common/graph/Graphs.java b/guava/src/com/google/common/graph/Graphs.java index ae473bedd10b..3099279c46f6 100644 --- a/guava/src/com/google/common/graph/Graphs.java +++ b/guava/src/com/google/common/graph/Graphs.java @@ -27,11 +27,14 @@ import com.google.common.collect.Iterators; import com.google.common.collect.Maps; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.util.ArrayDeque; import java.util.Collection; +import java.util.Deque; import java.util.HashSet; import java.util.Iterator; import java.util.Map; import java.util.Optional; +import java.util.Queue; import java.util.Set; import javax.annotation.CheckForNull; @@ -69,7 +72,7 @@ public static boolean hasCycle(Graph graph) { Map visitedNodes = Maps.newHashMapWithExpectedSize(graph.nodes().size()); for (N node : graph.nodes()) { - if (subgraphHasCycle(graph, visitedNodes, node, null)) { + if (subgraphHasCycle(graph, visitedNodes, node)) { return true; } } @@ -95,34 +98,67 @@ public static boolean hasCycle(Network network) { } /** - * Performs a traversal of the nodes reachable from {@code node}. If we ever reach a node we've - * already visited (following only outgoing edges and without reusing edges), we know there's a - * cycle in the graph. + * Performs a traversal of the nodes reachable from {@code startNode}. If we ever reach a node + * we've already visited (following only outgoing edges and without reusing edges), we know + * there's a cycle in the graph. */ private static boolean subgraphHasCycle( - Graph graph, - Map visitedNodes, - N node, - @CheckForNull N previousNode) { - NodeVisitState state = visitedNodes.get(node); - if (state == NodeVisitState.COMPLETE) { - return false; - } - if (state == NodeVisitState.PENDING) { - return true; - } + Graph graph, Map visitedNodes, N startNode) { + Deque> stack = new ArrayDeque<>(); + stack.addLast(new NodeAndRemainingSuccessors<>(startNode)); + + while (!stack.isEmpty()) { + // To peek at the top two items, we need to temporarily remove one. + NodeAndRemainingSuccessors top = stack.removeLast(); + NodeAndRemainingSuccessors prev = stack.peekLast(); + stack.addLast(top); + + N node = top.node; + N previousNode = prev == null ? null : prev.node; + if (top.remainingSuccessors == null) { + NodeVisitState state = visitedNodes.get(node); + if (state == NodeVisitState.COMPLETE) { + stack.removeLast(); + continue; + } + if (state == NodeVisitState.PENDING) { + return true; + } - visitedNodes.put(node, NodeVisitState.PENDING); - for (N nextNode : graph.successors(node)) { - if (canTraverseWithoutReusingEdge(graph, nextNode, previousNode) - && subgraphHasCycle(graph, visitedNodes, nextNode, node)) { - return true; + visitedNodes.put(node, NodeVisitState.PENDING); + top.remainingSuccessors = new ArrayDeque<>(graph.successors(node)); + } + + if (!top.remainingSuccessors.isEmpty()) { + N nextNode = top.remainingSuccessors.remove(); + if (canTraverseWithoutReusingEdge(graph, nextNode, previousNode)) { + stack.addLast(new NodeAndRemainingSuccessors<>(nextNode)); + continue; + } } + + stack.removeLast(); + visitedNodes.put(node, NodeVisitState.COMPLETE); } - visitedNodes.put(node, NodeVisitState.COMPLETE); return false; } + private static final class NodeAndRemainingSuccessors { + final N node; + + /** + * The successors left to be visited, or {@code null} if we just added this {@code + * NodeAndRemainingSuccessors} instance to the stack. In the latter case, we'll compute the + * successors if we determine that we need them after we've performed the initial processing of + * the node. + */ + @CheckForNull Queue remainingSuccessors; + + NodeAndRemainingSuccessors(N node) { + this.node = node; + } + } + /** * Determines whether an edge has already been used during traversal. In the directed case a cycle * is always detected before reusing an edge, so no special logic is required. In the undirected