Merge pull request #8187 from IoannisPanagiotas/fun-with-scc-again

An attempt to reduce SCC memory consumption for dense-ish graphs

algo/src/main/java/org/neo4j/gds/scc/Scc.java

@@ -41,7 +41,7 @@ public class Scc extends Algorithm<HugeLongArray> {
     private final HugeLongArray connectedComponents;
     private final HugeLongArray index;
     private final HugeLongArrayStack stack;
-    private final PagedLongStack todo; // stores pairs of (node-Id, TODO-Id)
+    private final PagedLongStack todo; // stores nodeIds either positive (edge visit) or negative (node visit)
     private final BitSet visited;
 
     public Scc(
@@ -69,7 +69,7 @@ public HugeLongArray compute() {
         index.fill(UNORDERED);
         connectedComponents.fill(UNORDERED);
 
-        graph.forEachNode(this::computePerNode);
+        graph.forEachNode(this::computePerNode); //this will visit 0 first
         progressTracker.endSubTask();
         return connectedComponents;
     }
@@ -83,42 +83,54 @@ private boolean computePerNode(long nodeId) {
             return true;
         }
 
-        todo.clear();
-
-        push(Action.VISIT_NODE, nodeId);
+        todo.push(-nodeId); //push nodeId as a node visit
 
         while (!todo.isEmpty()) {
-            var action = todo.pop();
             var node = todo.pop();
 
-            if (action == Action.VISIT_NODE.code) {
-                visitNode(node);
-            } else if (action == Action.VISIT_EDGE.code) {
+            if (node < 0) { // if the node is <0, we know we are going to visit a node as a node
+                distinguishNodeVisitType(-node);
+            } else if (node > 0) { //otherwise  if it's positive, then it 's an edge
                 visitEdge(node);
-            } else {
-                postVisitNode(node);
+            } else { //the 0 case
+                //-0 = 0 , so a 0 can indicate two things:
+                // (i) either a visit edge to 0
+                // (ii) or a node visit to 0 (here stuck must be empty: either it's the first action or the last)
+                if (todo.isEmpty()) {
+                    distinguishNodeVisitType(0);
+                } else {    //otherwise, it's an edge action, do so
+                    visitEdge(0);
+                }
             }
         }
         progressTracker.logProgress();
         return true;
     }
 
+    private void distinguishNodeVisitType(long node) {
+        if (index.get(node) != UNORDERED) { //last visit
+            postVisitNode(node);
+        } else {            //first visit
+            visitNode(node);
+        }
+    }
+
     private void visitNode(long nodeId) {
         final long stackSize = stack.size();
         index.set(nodeId, stackSize);
         stack.push(nodeId); // push to stack (at most one entry per vertex)
         boundaries.push(stackSize); // push to stack (at most one entry per vertex)
-        push(Action.POST_VISIT_NODE, nodeId);
+        todo.push(-nodeId);
         graph.forEachRelationship(nodeId, (s, t) -> {
-            push(Action.VISIT_EDGE, t);
+            todo.push(t);
             return true;
         });
     }
 
     private void visitEdge(long nodeId) {
         if (index.get(nodeId) == UNORDERED) {
-            push(Action.VISIT_NODE, nodeId);
-        } else if (!visited.get(nodeId)) {
+            todo.push(-nodeId); //organize a first visit to nodeId
+        } else if (!visited.get(nodeId)) {          //skip nodes already in a component
             while (index.get(nodeId) < boundaries.peek()) {
                 boundaries.pop();
             }
@@ -137,26 +149,5 @@ private void postVisitNode(long nodeId) {
         }
     }
 
-    /**
-     * pushes an action and a nodeId on the stack
-     *
-     * @param action
-     * @param value
-     */
-    private void push(Action action, long value) {
-        todo.push(value);
-        todo.push(action.code);
-    }
-
-    private enum Action {
-        VISIT_NODE(0L),
-        VISIT_EDGE(1L),
-        POST_VISIT_NODE(2L);
-
-        final long code;
 
-        Action(long code) {
-            this.code = code;
-        }
-    }
 }

algo/src/main/java/org/neo4j/gds/scc/SccAlgorithmFactory.java

@@ -68,7 +68,7 @@ public MemoryEstimation memoryEstimation(CONFIG configuration) {
             long relationshipCount = graphDimensions.relCountUpperBound();
             return MemoryRange.of(
                 PagedLongStack.memoryEstimation(nodeCount),
-                PagedLongStack.memoryEstimation(2 * Math.max(nodeCount, relationshipCount))
+                PagedLongStack.memoryEstimation(Math.max(nodeCount, relationshipCount))
                 //this bound is very-very-very loose
             );
         }));

algo/src/test/java/org/neo4j/gds/scc/SccAlgorithmFactoryTest.java

@@ -32,7 +32,7 @@ void shouldEstimateMemoryAccurately() {
         var config = SccStreamConfigImpl.builder().build();
         var factory = new SccAlgorithmFactory<>();
         var estimate = factory.memoryEstimation(config)
-            .estimate(GraphDimensions.of(100, 3000), config.concurrency());
+            .estimate(GraphDimensions.of(100, 6000), config.concurrency());
 
         var memoryUsage = estimate.memoryUsage();
         assertThat(memoryUsage.min).isEqualTo(36348L);