8200134: Improve ModuleHashesBuilder

Reviewed-by: mchung, alanb

src/java.base/share/classes/jdk/internal/module/ModuleHashesBuilder.java

@@ -35,7 +35,6 @@ import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
@@ -76,16 +75,15 @@ public class ModuleHashesBuilder {
         // build a graph containing the packaged modules and
         // its transitive dependences matching --hash-modules
         Graph.Builder<String> builder = new Graph.Builder<>();
-        Deque<ResolvedModule> deque = new ArrayDeque<>(configuration.modules());
+        Deque<ResolvedModule> todo = new ArrayDeque<>(configuration.modules());
         Set<ResolvedModule> visited = new HashSet<>();
-        while (!deque.isEmpty()) {
+        ResolvedModule rm;
-            ResolvedModule rm = deque.pop();
+        while ((rm = todo.poll()) != null) {
-            if (!visited.contains(rm)) {
+            if (visited.add(rm)) {
-                visited.add(rm);
                 builder.addNode(rm.name());
                 for (ResolvedModule dm : rm.reads()) {
                     if (!visited.contains(dm)) {
-                        deque.push(dm);
+                        todo.push(dm);
                     }
                     builder.addEdge(rm.name(), dm.name());
                 }
@@ -212,17 +210,14 @@ public class ModuleHashesBuilder {
          * Returns all nodes reachable from the given set of roots.
          */
         public Set<T> dfs(Set<T> roots) {
-            Deque<T> deque = new LinkedList<>(roots);
+            ArrayDeque<T> todo = new ArrayDeque<>(roots);
             Set<T> visited = new HashSet<>();
-            while (!deque.isEmpty()) {
+            T u;
-                T u = deque.pop();
+            while ((u = todo.poll()) != null) {
-                if (!visited.contains(u)) {
+                if (visited.add(u) && contains(u)) {
-                    visited.add(u);
-                    if (contains(u)) {
                     adjacentNodes(u).stream()
                         .filter(v -> !visited.contains(v))
-                            .forEach(deque::push);
+                        .forEach(todo::push);
-                    }
                 }
             }
             return visited;
@@ -240,12 +235,10 @@ public class ModuleHashesBuilder {
             final Map<T, Set<T>> edges = new HashMap<>();
 
             public void addNode(T node) {
-                if (nodes.contains(node)) {
+                if (nodes.add(node)) {
-                    return;
-                }
-                nodes.add(node);
                     edges.computeIfAbsent(node, _e -> new HashSet<>());
                 }
+            }
 
             public void addEdge(T u, T v) {
                 addNode(u);
@@ -263,18 +256,16 @@ public class ModuleHashesBuilder {
      * Topological sort
      */
     private static class TopoSorter<T> {
-        final Deque<T> result = new LinkedList<>();
+        final Deque<T> result = new ArrayDeque<>();
-        final Deque<T> nodes;
         final Graph<T> graph;
 
         TopoSorter(Graph<T> graph) {
             this.graph = graph;
-            this.nodes = new LinkedList<>(graph.nodes);
             sort();
         }
 
         public void ordered(Consumer<T> action) {
-            result.iterator().forEachRemaining(action);
+            result.forEach(action);
         }
 
         public void reverse(Consumer<T> action) {
@@ -282,29 +273,26 @@ public class ModuleHashesBuilder {
         }
 
         private void sort() {
-            Deque<T> visited = new LinkedList<>();
+            Set<T> visited = new HashSet<>();
-            Deque<T> done = new LinkedList<>();
+            Deque<T> stack = new ArrayDeque<>();
-            T node;
+            graph.nodes.forEach(node -> visit(node, visited, stack));
-            while ((node = nodes.poll()) != null) {
-                if (!visited.contains(node)) {
-                    visit(node, visited, done);
-                }
-            }
         }
 
-        private void visit(T node, Deque<T> visited, Deque<T> done) {
+        private Set<T> children(T node) {
-            if (visited.contains(node)) {
+            return graph.edges().get(node);
-                if (!done.contains(node)) {
-                    throw new IllegalArgumentException("Cyclic detected: " +
-                        node + " " + graph.edges().get(node));
         }
-                return;
+
-            }
+        private void visit(T node, Set<T> visited, Deque<T> stack) {
-            visited.add(node);
+            if (visited.add(node)) {
-            graph.edges().get(node)
+                stack.push(node);
-                .forEach(x -> visit(x, visited, done));
+                children(node).forEach(child -> visit(child, visited, stack));
-            done.add(node);
+                stack.pop();
                 result.addLast(node);
             }
+            else if (stack.contains(node)) {
+                throw new IllegalArgumentException(
+                    "Cycle detected: " + node + " -> " + children(node));
+            }
+        }
     }
 }