Bridges report size of components left

Co-authored-by: Veselin Nikolov <[email protected]>

algo/src/main/java/org/neo4j/gds/articulationpoints/ArticulationPointsMemoryEstimateDefinition.java

@@ -19,7 +19,6 @@
  */
 package org.neo4j.gds.articulationpoints;
 
-import org.neo4j.gds.bridges.Bridges;
 import org.neo4j.gds.collections.ha.HugeLongArray;
 import org.neo4j.gds.collections.ha.HugeObjectArray;
 import org.neo4j.gds.mem.Estimate;
@@ -32,7 +31,7 @@ public class ArticulationPointsMemoryEstimateDefinition implements MemoryEstimat
     @Override
     public MemoryEstimation memoryEstimation() {
 
-        var builder = MemoryEstimations.builder(Bridges.class);
+        var builder = MemoryEstimations.builder(ArticulationPoints.class);
         builder
             .perNode("tin", HugeLongArray::memoryEstimation)
             .perNode("low", HugeLongArray::memoryEstimation)
@@ -46,7 +45,6 @@ public MemoryEstimation memoryEstimation() {
                 HugeObjectArray.memoryEstimation(relationshipCount, Estimate.sizeOfInstance(ArticulationPoints.StackEvent.class))
             );
 
-
         }));
 
         return builder.build();

algo/src/main/java/org/neo4j/gds/bridges/Bridge.java

@@ -19,9 +19,9 @@
  */
 package org.neo4j.gds.bridges;
 
-public record Bridge(long from, long to) {
+public record Bridge(long from, long to,long[] remainingSizes) {
 
-    static Bridge create(long from, long to){
-        return new Bridge(Math.min(from,to), Math.max(from,to));
+    static Bridge create(long from, long to, long[] remainingSizes){
+        return new Bridge(Math.min(from,to), Math.max(from,to), remainingSizes);
     }
 }

algo/src/main/java/org/neo4j/gds/bridges/Bridges.java

@@ -28,7 +28,9 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.BiConsumer;
 
 public class Bridges extends Algorithm<BridgeResult> {
 
@@ -39,14 +41,26 @@ public class Bridges extends Algorithm<BridgeResult> {
     private long timer;
     private long stackIndex = -1;
     private List<Bridge> result = new ArrayList<>();
+    private final Optional<TreeSizeTracker> treeSizeTracker;
 
-    public Bridges(Graph graph, ProgressTracker progressTracker){
+    public static  Bridges create(Graph graph, ProgressTracker progressTracker, boolean shouldComputeComponents){
+
+        if (shouldComputeComponents) {
+            var treeSizeTracker = new TreeSizeTracker(graph.nodeCount());
+            return new Bridges(graph, progressTracker, Optional.of(treeSizeTracker));
+        }else{
+            return new Bridges(graph,progressTracker,Optional.empty());
+        }
+    }
+
+    private Bridges(Graph graph, ProgressTracker progressTracker, Optional<TreeSizeTracker> treeSizeTracker){
         super(progressTracker);
 
         this.graph = graph;
         this.visited = new BitSet(graph.nodeCount());
         this.tin = HugeLongArray.newArray(graph.nodeCount());
         this.low = HugeLongArray.newArray(graph.nodeCount());
+        this.treeSizeTracker = treeSizeTracker;
     }
 
     @Override
@@ -59,37 +73,50 @@ public BridgeResult compute() {
         //each edge may have at most one event to the stack at the same time
         var stack = HugeObjectArray.newArray(StackEvent.class, graph.relationshipCount());
 
+        BiConsumer<Long,Long> onLastChildVisit = (treeSizeTracker.isPresent()) ? treeSizeTracker.get()::recordTreeChild : (a,b)->{};
+        int listIndex=0;
         var n = graph.nodeCount();
-        for (int i = 0; i < n; ++i) {
-            if (!visited.get(i))
-                dfs(i, stack);
+        for (long i = 0; i < n; ++i) {
+            if (!visited.get(i)) {
+                dfs(i, stack,onLastChildVisit);
+
+                if (treeSizeTracker.isPresent()) {
+                    var tracker =treeSizeTracker.get();
+                    var listEndIndex = result.size();
+                    for (var j = listIndex; j < listEndIndex; ++j) {
+                        var currentBridge = result.get(j);
+                        var remainingSizes = tracker.recordBridge(currentBridge.to(),i);
+                        result.set(j,new Bridge(currentBridge.from(), currentBridge.to(),remainingSizes));
+                    }
+                    listIndex = listEndIndex;
+                }
+            }
         }
         progressTracker.endSubTask("Bridges");
         return new BridgeResult(result);
 
     }
 
-
-
-    private void dfs(long node, HugeObjectArray<StackEvent> stack) {
+    private void dfs(long node, HugeObjectArray<StackEvent> stack, BiConsumer<Long,Long> onLastChildVisit) {
         stack.set(++stackIndex, StackEvent.upcomingVisit(node,-1));
         while (stackIndex >= 0) {
             var stackEvent = stack.get(stackIndex--);
-            visitEvent(stackEvent, stack);
+            visitEvent(stackEvent, stack,onLastChildVisit);
         }
         progressTracker.logProgress();
     }
 
-    private void visitEvent(StackEvent event, HugeObjectArray<StackEvent> stack) {
+    private void visitEvent(StackEvent event, HugeObjectArray<StackEvent> stack, BiConsumer<Long,Long> onLastChildVisit) {
         if (event.lastVisit()) {
             var to = event.eventNode();
             var v = event.triggerNode();
             var lowV = low.get(v);
             var lowTo = low.get(to);
             low.set(v, Math.min(lowV, lowTo));
             var tinV = tin.get(v);
+            onLastChildVisit.accept(v,to);
             if (lowTo > tinV) {
-                result.add(new Bridge(v, to));
+                result.add(new Bridge(v, to, null));
             }
             progressTracker.logProgress();
             return;

algo/src/main/java/org/neo4j/gds/bridges/BridgesMemoryEstimateDefinition.java

@@ -28,6 +28,13 @@
 import org.neo4j.gds.mem.MemoryRange;
 
 public class BridgesMemoryEstimateDefinition implements MemoryEstimateDefinition {
+
+    private final boolean shouldComputeComponents;
+
+    public BridgesMemoryEstimateDefinition(boolean shouldComputeComponents) {
+        this.shouldComputeComponents = shouldComputeComponents;
+    }
+
     @Override
     public MemoryEstimation memoryEstimation() {
 
@@ -38,6 +45,13 @@ public MemoryEstimation memoryEstimation() {
             .perNode("visited", Estimate::sizeOfBitset)
                 .perNode("bridge", (v)->  v * Estimate.sizeOfInstance(Bridge.class));
 
+        if (shouldComputeComponents){
+            builder.add(
+                "component split",
+                 MemoryEstimations.builder(TreeSizeTracker.class)
+                    .perNode("treeSize", HugeLongArray::memoryEstimation)
+                     .build());
+        }
         builder.rangePerGraphDimension("stack", ((graphDimensions, concurrency) -> {
             long relationshipCount = graphDimensions.relCountUpperBound();
             return MemoryRange.of(

algo/src/main/java/org/neo4j/gds/bridges/TreeSizeTracker.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.gds.bridges;
+
+import org.neo4j.gds.collections.ha.HugeLongArray;
+
+ class TreeSizeTracker  {
+
+    private final HugeLongArray subTreeSize;
+
+    TreeSizeTracker(long nodeCount){
+        subTreeSize = HugeLongArray.newArray(nodeCount);
+        subTreeSize.setAll( v -> 1);
+    }
+    void recordTreeChild(long parent, long child) {
+        subTreeSize.addTo(parent,subTreeSize.get(child));
+    }
+
+    public long[] recordBridge( long child, long root) {
+        var childSubTree = subTreeSize.get(child);
+        var rootSubTree= subTreeSize.get(root) - childSubTree;
+        return  new long[]{rootSubTree, childSubTree};
+    }
+
+}

algo/src/test/java/org/neo4j/gds/articulationpoints/ArticulationPointsMemoryEstimateDefinitionTest.java

@@ -30,6 +30,6 @@ void shouldEstimateMemoryAccurately() {
 
         MemoryEstimationAssert.assertThat(memoryEstimation)
             .memoryRange(100, 6000, new Concurrency(1))
-            .hasSameMinAndMaxEqualTo(218752);
+            .hasSameMinAndMaxEqualTo(218760);
     }
 }

algo/src/test/java/org/neo4j/gds/bridges/BridgesLargestTest.java

@@ -74,31 +74,32 @@ class BridgesLargestTest {
     @Inject
     private TestGraph graph;
 
-    private Bridge bridge(String from, String to) {
-        return new Bridge(graph.toOriginalNodeId(from), graph.toOriginalNodeId(to));
+    private Bridge bridge(String from, String to, long[] remainingSizes) {
+        return new Bridge(graph.toOriginalNodeId(from), graph.toOriginalNodeId(to), remainingSizes);
     }
 
 
     @Test
     void shouldFindAllBridges() {
-        var bridges = new Bridges(graph, ProgressTracker.NULL_TRACKER);
+        var bridges =  Bridges.create(graph, ProgressTracker.NULL_TRACKER,true);
 
         var result = bridges.compute().bridges().stream()
             .map(b -> new Bridge(
             graph.toOriginalNodeId(b.from()),
-            graph.toOriginalNodeId(b.to())
+            graph.toOriginalNodeId(b.to()),
+                b.remainingSizes()
         )).toList();
 
-
         assertThat(result)
             .isNotNull()
+            .usingRecursiveFieldByFieldElementComparator()
             .containsExactlyInAnyOrder(
-                bridge("a1", "a2"),
-                bridge("a3", "a4"),
-                bridge("a3", "a7"),
-                bridge("a7", "a8"),
-                bridge("a10", "a11"),
-                bridge("a14", "a13")
+                bridge("a1", "a2",   new long[]{1,1}),
+                bridge("a3", "a4",   new long[]{3,1}),
+                bridge("a3", "a7",   new long[]{2,2}),
+                bridge("a7", "a8",   new long[]{3,1}),
+                bridge("a10", "a11", new long[]{5,4}),
+                bridge("a14", "a13", new long[]{8,1})
             );
     }
 }

algo/src/test/java/org/neo4j/gds/bridges/BridgesMemoryEstimateDefinitionTest.java

@@ -19,18 +19,31 @@
  */
 package org.neo4j.gds.bridges;
 
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.neo4j.gds.assertions.MemoryEstimationAssert;
 import org.neo4j.gds.core.concurrency.Concurrency;
 
+import java.util.stream.Stream;
+
 class BridgesMemoryEstimateDefinitionTest {
 
-    @Test
-    void shouldEstimateMemoryAccurately() {
-        var memoryEstimation = new BridgesMemoryEstimateDefinition().memoryEstimation();
+    static Stream<Arguments> memoryEstimationTuples() {
+        return Stream.of(
+                 Arguments.of(false, 221064L),
+                 Arguments.of(true,  221920L)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("memoryEstimationTuples")
+    void shouldEstimateMemoryAccurately(boolean should, long expectedMemory) {
+        var memoryEstimation = new BridgesMemoryEstimateDefinition(should).memoryEstimation();
 
         MemoryEstimationAssert.assertThat(memoryEstimation)
             .memoryRange(100, 6000, new Concurrency(1))
-            .hasSameMinAndMaxEqualTo(221056L);
+            .hasSameMinAndMaxEqualTo(expectedMemory);
     }
+
 }

algo/src/test/java/org/neo4j/gds/bridges/SmallBridgesTest.java

@@ -65,15 +65,32 @@ class GraphWithBridges {
 
 
         @Test
-        void shouldFindBridges() {
-            var bridges = new Bridges(graph, ProgressTracker.NULL_TRACKER);
+        void shouldFindJustBridges() {
+            var bridges = Bridges.create(graph, ProgressTracker.NULL_TRACKER,false);
+            var result = bridges.compute().bridges();
+
+            assertThat(result)
+                .isNotNull()
+                .usingRecursiveFieldByFieldElementComparator()
+                .containsExactlyInAnyOrder(
+                    Bridge.create(graph.toMappedNodeId("a"), graph.toMappedNodeId("d"),null),
+                    Bridge.create(graph.toMappedNodeId("d"), graph.toMappedNodeId("e"),null)
+                );
+        }
+
+
+
+        @Test
+        void shouldFindBridgesAndComponentSizes() {
+            var bridges = Bridges.create(graph, ProgressTracker.NULL_TRACKER,true);
             var result = bridges.compute().bridges();
 
             assertThat(result)
                 .isNotNull()
+                .usingRecursiveFieldByFieldElementComparator()
                 .containsExactlyInAnyOrder(
-                     Bridge.create(graph.toMappedNodeId("a"), graph.toMappedNodeId("d")),
-                     Bridge.create(graph.toMappedNodeId("d"), graph.toMappedNodeId("e"))
+                     Bridge.create(graph.toMappedNodeId("a"), graph.toMappedNodeId("d"),new long[]{3,2}),
+                     Bridge.create(graph.toMappedNodeId("d"), graph.toMappedNodeId("e"),new long[]{4,1})
                 );
         }
 
@@ -84,7 +101,7 @@ void shouldLogProgress(){
             var log = new GdsTestLog();
             var progressTracker = new TaskProgressTracker(progressTask, log, new Concurrency(1), EmptyTaskRegistryFactory.INSTANCE);
 
-            var bridges = new Bridges(graph, progressTracker);
+            var bridges = Bridges.create(graph, progressTracker,false);
             bridges.compute();
 
             Assertions.assertThat(log.getMessages(TestLog.INFO))
@@ -130,7 +147,7 @@ class GraphWithoutBridges {
 
         @Test
         void shouldFindBridges() {
-            var bridges = new Bridges(graph,ProgressTracker.NULL_TRACKER);
+            var bridges = Bridges.create(graph,ProgressTracker.NULL_TRACKER,false);
             var result = bridges.compute().bridges();
 
             assertThat(result)

applications/algorithms/centrality/src/main/java/org/neo4j/gds/applications/algorithms/centrality/CentralityAlgorithms.java

@@ -181,12 +181,12 @@ public BetwennessCentralityResult betweennessCentrality(
         );
     }
 
-    BridgeResult bridges(Graph graph, AlgoBaseConfig configuration) {
+    BridgeResult bridges(Graph graph, AlgoBaseConfig configuration, boolean shouldComputeComponents) {
 
         var task = BridgeProgressTaskCreator.progressTask(graph.nodeCount());
         var progressTracker = progressTrackerCreator.createProgressTracker(configuration, task);
 
-        var algorithm = new Bridges(graph, progressTracker);
+        var algorithm = Bridges.create(graph, progressTracker,shouldComputeComponents);
 
         return algorithmMachinery.runAlgorithmsAndManageProgressTracker(
             algorithm,

applications/algorithms/centrality/src/main/java/org/neo4j/gds/applications/algorithms/centrality/CentralityAlgorithmsEstimationModeBusinessFacade.java

@@ -82,12 +82,12 @@ public MemoryEstimateResult betweennessCentrality(
             memoryEstimation
         );
     }
-     MemoryEstimation bridges() {
-        return new BridgesMemoryEstimateDefinition().memoryEstimation();
+     MemoryEstimation bridges(boolean shouldComputeComponents) {
+        return new BridgesMemoryEstimateDefinition(shouldComputeComponents).memoryEstimation();
     }
 
-    public MemoryEstimateResult bridges(BridgesBaseConfig configuration, Object graphNameOrConfiguration) {
-        var memoryEstimation = bridges();
+    public MemoryEstimateResult bridges(BridgesBaseConfig configuration, Object graphNameOrConfiguration, boolean shouldComputeComponents) {
+        var memoryEstimation = bridges(shouldComputeComponents);
 
         return algorithmEstimationTemplate.estimate(
             configuration,