Remove the use of a massive array

DefaultEdge[][] took 2GB of heap per thread in mid-size MJS problems.
Even though it did improve performance, such a hit would have made multi-threading practically impossible.
It would have also prevented us from solving large problems.

Author: triceo

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultTopologicalOrderGraph.java

@@ -32,24 +32,29 @@ public DefaultTopologicalOrderGraph(final int size) {
     }
 
     @Override
-    public void addEdge(Edge edge) {
-        var from = edge.from();
-        var to = edge.to();
-        forwardEdges[from].add(to);
-        backEdges[to].add(from);
+    public void addEdge(int fromNode, int toNode) {
+        forwardEdges[fromNode].add(toNode);
+        backEdges[toNode].add(fromNode);
     }
 
     @Override
-    public void removeEdge(Edge edge) {
-        var from = edge.from();
-        var to = edge.to();
-        forwardEdges[from].remove(to);
-        backEdges[to].remove(from);
+    public void removeEdge(int fromNode, int toNode) {
+        forwardEdges[fromNode].remove(toNode);
+        backEdges[toNode].remove(fromNode);
     }
 
     @Override
-    public PrimitiveIterator.OfInt nodeForwardEdges(int from) {
-        return componentMap.get(from).stream()
+    public void forEachEdge(EdgeConsumer edgeConsumer) {
+        for (var fromNode = 0; fromNode < forwardEdges.length; fromNode++) {
+            for (var toNode : forwardEdges[fromNode]) {
+                edgeConsumer.accept(fromNode, toNode);
+            }
+        }
+    }
+
+    @Override
+    public PrimitiveIterator.OfInt nodeForwardEdges(int fromNode) {
+        return componentMap.get(fromNode).stream()
                 .flatMap(member -> forwardEdges[member].stream())
                 .mapToInt(Integer::intValue)
                 .distinct().iterator();

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java

@@ -4,11 +4,13 @@
 import java.util.BitSet;
 import java.util.Collections;
 import java.util.IdentityHashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.IntFunction;
+import java.util.stream.Collectors;
 
 import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel;
 
@@ -24,7 +26,7 @@ final class DefaultVariableReferenceGraph<Solution_> implements VariableReferenc
     private final Map<VariableMetaModel<?, ?, ?>, List<BiConsumer<VariableReferenceGraph<Solution_>, Object>>> variableReferenceToAfterProcessor;
 
     // These structures are mutable.
-    private final DefaultEdge[][] edges;
+    private final BitSet[] edgesFromNode;
     private final TopologicalOrderGraph graph;
     private final BitSet changed;
 
@@ -38,7 +40,7 @@ public DefaultVariableReferenceGraph(VariableReferenceGraphBuilder<Solution_> ou
         variableReferenceToInstanceMap = mapOfMapsDeepCopyOf(outerGraph.variableReferenceToInstanceMap);
         variableReferenceToBeforeProcessor = mapOfListsDeepCopyOf(outerGraph.variableReferenceToBeforeProcessor);
         variableReferenceToAfterProcessor = mapOfListsDeepCopyOf(outerGraph.variableReferenceToAfterProcessor);
-        edges = new DefaultEdge[instanceCount][instanceCount];
+        edgesFromNode = new BitSet[instanceCount];
         graph = graphCreator.apply(instanceCount);
         graph.withNodeData(instanceList);
         changed = new BitSet(instanceCount);
@@ -82,15 +84,20 @@ public void addEdge(@NonNull EntityVariablePair<Solution_> from, @NonNull Entity
             return;
         }
 
-        var edge = edges[fromNodeId][toNodeId];
-        if (edge == null) {
-            edge = new DefaultEdge(fromNodeId, toNodeId);
-            edges[fromNodeId][toNodeId] = edge;
-            graph.addEdge(edge);
+        var hasEdge = false;
+        var presentEdges = edgesFromNode[fromNodeId];
+        if (presentEdges == null) {
+            presentEdges = new BitSet(edgesFromNode.length);
+            edgesFromNode[fromNodeId] = presentEdges;
+        } else {
+            hasEdge = presentEdges.get(toNodeId);
         }
-        edge.increaseCount();
 
-        markChanged(to);
+        if (!hasEdge) {
+            presentEdges.set(toNodeId);
+            graph.addEdge(fromNodeId, toNodeId);
+            markChanged(to);
+        }
     }
 
     @Override
@@ -101,13 +108,15 @@ public void removeEdge(@NonNull EntityVariablePair<Solution_> from, @NonNull Ent
             return;
         }
 
-        var edge = edges[fromNodeId][toNodeId];
-        if (edge.decreaseCount() == 0) {
-            graph.removeEdge(edge);
-            edges[fromNodeId][toNodeId] = null;
+        var presentEdges = edgesFromNode[fromNodeId];
+        if (presentEdges == null) {
+            return;
+        }
+        if (presentEdges.get(toNodeId)) {
+            graph.removeEdge(fromNodeId, toNodeId);
+            presentEdges.clear(toNodeId);
+            markChanged(to);
         }
-
-        markChanged(to);
     }
 
     @Override
@@ -153,28 +162,17 @@ public void afterVariableChanged(VariableMetaModel<?, ?, ?> variableReference, O
 
     @Override
     public String toString() {
-        var builder = new StringBuilder("{\n");
-        for (int from = 0; from < edges.length; from++) {
-            var row = edges[from];
-            var first = true;
-            for (int to = 0; to < row.length; to++) {
-                var edge = row[to];
-                if (edge != null) {
-                    if (first) {
-                        first = false;
-                        builder.append("    \"").append(instanceList.get(from)).append("\": [");
-                    } else {
-                        builder.append(", ");
-                    }
-                    builder.append("\"%s\"".formatted(instanceList.get(to)));
-                }
-            }
-            if (!first) {
-                builder.append("],\n");
-            }
-        }
-        builder.append("}");
-        return builder.toString();
+        var edgeList = new LinkedHashMap<EntityVariablePair<Solution_>, List<EntityVariablePair<Solution_>>>();
+        graph.forEachEdge((from, to) -> edgeList.computeIfAbsent(instanceList.get(from), k -> new ArrayList<>())
+                .add(instanceList.get(to)));
+        return edgeList.entrySet()
+                .stream()
+                .map(e -> e.getKey() + "->" + e.getValue())
+                .collect(Collectors.joining(
+                        "  ," + System.lineSeparator(),
+                        "{" + System.lineSeparator(),
+                        "}"));
+
     }
 
     @SuppressWarnings("unchecked")
@@ -195,54 +193,4 @@ private static <K1, V> Map<K1, List<V>> mapOfListsDeepCopyOf(Map<K1, List<V>> ma
         return Map.ofEntries(entryArray);
     }
 
-    public static final class DefaultEdge implements TopologicalOrderGraph.Edge {
-
-        private final int from;
-        private final int to;
-        private int count = 0;
-
-        public DefaultEdge(int from, int to) {
-            this.from = from;
-            this.to = to;
-        }
-
-        @Override
-        public int from() {
-            return from;
-        }
-
-        @Override
-        public int to() {
-            return to;
-        }
-
-        public void increaseCount() {
-            count++;
-        }
-
-        public int decreaseCount() {
-            return --count;
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            return o instanceof DefaultEdge other &&
-                    from == other.from &&
-                    to == other.to;
-        }
-
-        @Override
-        public int hashCode() {
-            var hash = 31;
-            hash += 31 * from;
-            hash += 31 * to * to; // Make sure order of nodes matters.
-            return hash;
-        }
-
-        @Override
-        public String toString() {
-            return "%d->%d".formatted(from, to);
-        }
-    }
-
 }

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/TopologicalOrderGraph.java

@@ -27,7 +27,7 @@ default <Solution_> void withNodeData(List<EntityVariablePair<Solution_>> nodes)
      * {@link #getTopologicalOrder(int)} is allowed to be invalid
      * when this method returns.
      */
-    void addEdge(Edge edge);
+    void addEdge(int from, int to);
 
     /**
      * Called when a graph edge is removed.
@@ -36,13 +36,14 @@ default <Solution_> void withNodeData(List<EntityVariablePair<Solution_>> nodes)
      * {@link #getTopologicalOrder(int)} is allowed to be invalid
      * when this method returns.
      */
-    void removeEdge(Edge edge);
+    void removeEdge(int from, int to);
 
-    interface Edge {
+    void forEachEdge(EdgeConsumer edgeConsumer);
 
-        int from();
+    @FunctionalInterface
+    interface EdgeConsumer {
 
-        int to();
+        void accept(int from, int to);
 
     }
 

core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java

@@ -191,19 +191,17 @@ public void removeEdge(VariableMetaModel<?, ?, ?> fromId, Object fromEntity, Var
         }
 
         @Override
-        public void addEdge(Edge edge) {
-            super.addEdge(edge);
-            var from = edge.from();
-            var to = edge.to();
-            addEdge(nodeToVariableMetamodel[from], nodeToEntities[from], nodeToVariableMetamodel[to], nodeToEntities[to]);
+        public void addEdge(int fromNode, int toNode) {
+            super.addEdge(fromNode, toNode);
+            addEdge(nodeToVariableMetamodel[fromNode], nodeToEntities[fromNode], nodeToVariableMetamodel[toNode],
+                    nodeToEntities[toNode]);
         }
 
         @Override
-        public void removeEdge(Edge edge) {
-            super.addEdge(edge);
-            var from = edge.from();
-            var to = edge.to();
-            removeEdge(nodeToVariableMetamodel[from], nodeToEntities[from], nodeToVariableMetamodel[to], nodeToEntities[to]);
+        public void removeEdge(int fromNode, int toNode) {
+            super.removeEdge(fromNode, toNode);
+            removeEdge(nodeToVariableMetamodel[fromNode], nodeToEntities[fromNode], nodeToVariableMetamodel[toNode],
+                    nodeToEntities[toNode]);
         }
     }
 

core/src/test/java/ai/timefold/solver/core/testdomain/declarative/dependency/TestdataDependencyConstraintProvider.java

@@ -19,8 +19,13 @@ public class TestdataDependencyConstraintProvider implements ConstraintProvider
                         .asConstraint("Invalid task"),
                 constraintFactory.forEach(TestdataDependencyValue.class)
                         .filter(task -> !task.isInvalid())
-                        .penalize(HardSoftScore.ONE_SOFT, t -> (int) Duration.between(t.getEntity().getStartTime(),
-                                t.getEndTime()).toMinutes())
+                        .penalize(HardSoftScore.ONE_SOFT, t -> {
+                            if (t.getEndTime() == null) {
+                                throw new IllegalStateException();
+                            }
+                            return (int) Duration.between(t.getEntity().getStartTime(),
+                                    t.getEndTime()).toMinutes();
+                        })
                         .asConstraint("Finish tasks as early as possible")
         };
     }