perf: dynamically calculate topological order to prevent cache invalidation (#1968)

Author: Christopher-Chianelli

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

@@ -21,6 +21,7 @@ public abstract sealed class AbstractVariableReferenceGraph<Solution_, ChangeSet
 
     // These structures are immutable.
     protected final List<GraphNode<Solution_>> nodeList;
+    protected final BaseTopologicalOrderGraph.NodeTopologicalOrder[] nodeTopologicalOrders;
     protected final Map<VariableMetaModel<?, ?, ?>, Map<Object, GraphNode<Solution_>>> variableReferenceToContainingNodeMap;
     protected final Map<VariableMetaModel<?, ?, ?>, List<BiConsumer<AbstractVariableReferenceGraph<Solution_, ?>, Object>>> variableReferenceToBeforeProcessor;
     protected final Map<VariableMetaModel<?, ?, ?>, List<BiConsumer<AbstractVariableReferenceGraph<Solution_, ?>, Object>>> variableReferenceToAfterProcessor;
@@ -44,6 +45,7 @@ public abstract sealed class AbstractVariableReferenceGraph<Solution_, ChangeSet
         }
         graph = graphCreator.apply(instanceCount);
         graph.withNodeData(nodeList);
+        nodeTopologicalOrders = buildNodeTopologicalOrderArray(graph, nodeList.size());
 
         var visited = Collections.newSetFromMap(new IdentityHashMap<>());
         changeSet = createChangeSet(instanceCount);
@@ -62,6 +64,16 @@ public abstract sealed class AbstractVariableReferenceGraph<Solution_, ChangeSet
         }
     }
 
+    private BaseTopologicalOrderGraph.NodeTopologicalOrder[] buildNodeTopologicalOrderArray(BaseTopologicalOrderGraph graph,
+            int graphSize) {
+        var out = new BaseTopologicalOrderGraph.NodeTopologicalOrder[graphSize];
+
+        for (var i = 0; i < out.length; i++) {
+            out[i] = new BaseTopologicalOrderGraph.NodeTopologicalOrder(i, graph);
+        }
+        return out;
+    }
+
     protected abstract ChangeSet_ createChangeSet(int instanceCount);
 
     public @Nullable GraphNode<Solution_> lookupOrNull(VariableMetaModel<?, ?, ?> variableId, Object entity) {

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

@@ -15,6 +15,7 @@ final class AffectedEntitiesUpdater<Solution_>
     // From WorkingReferenceGraph.
     private final BaseTopologicalOrderGraph graph;
     private final List<GraphNode<Solution_>> nodeList; // Immutable.
+    private final BaseTopologicalOrderGraph.NodeTopologicalOrder[] nodeTopologicalOrders; // Immutable
     private final ChangedVariableNotifier<Solution_> changedVariableNotifier;
 
     // Internal state; expensive to create, therefore we reuse.
@@ -23,10 +24,12 @@ final class AffectedEntitiesUpdater<Solution_>
     private final PriorityQueue<BaseTopologicalOrderGraph.NodeTopologicalOrder> changeQueue;
 
     AffectedEntitiesUpdater(BaseTopologicalOrderGraph graph, List<GraphNode<Solution_>> nodeList,
+            BaseTopologicalOrderGraph.NodeTopologicalOrder[] nodeTopologicalOrders,
             Function<Object, List<GraphNode<Solution_>>> entityToContainingNode,
             int entityCount, ChangedVariableNotifier<Solution_> changedVariableNotifier) {
         this.graph = graph;
         this.nodeList = nodeList;
+        this.nodeTopologicalOrders = nodeTopologicalOrders;
         this.changedVariableNotifier = changedVariableNotifier;
         var instanceCount = nodeList.size();
         this.loopedTracker = new LoopedTracker(instanceCount,
@@ -87,7 +90,7 @@ public void accept(BitSet changed) {
                 while (iterator.hasNext()) {
                     var nextNodeForwardEdge = iterator.nextInt();
                     if (!visited.get(nextNodeForwardEdge)) {
-                        changeQueue.add(graph.getTopologicalOrder(nextNodeForwardEdge));
+                        changeQueue.add(nodeTopologicalOrders[nextNodeForwardEdge]);
                     }
                 }
             }
@@ -110,7 +113,7 @@ private void initializeChangeQueue(BitSet changed) {
         // This should never happen, since arrays in Java are limited
         // to slightly less than Integer.MAX_VALUE.
         for (var i = changed.nextSetBit(0); i >= 0; i = changed.nextSetBit(i + 1)) {
-            changeQueue.add(graph.getTopologicalOrder(i));
+            changeQueue.add(nodeTopologicalOrders[i]);
             if (i == Integer.MAX_VALUE) {
                 break; // or (i+1) would overflow
             }

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

@@ -28,9 +28,9 @@ public interface BaseTopologicalOrderGraph {
     boolean isLooped(LoopedTracker loopedTracker, int node);
 
     /**
-     * Returns a tuple containing node ID and a number corresponding to its topological order.
-     * In particular, after {@link TopologicalOrderGraph#commitChanges(java.util.BitSet)} is called, the following
-     * must be true for any pair of nodes A, B where:
+     * Returns a tuple containing node ID and a number corresponding to its topological order. In particular, after
+     * {@link TopologicalOrderGraph#commitChanges(java.util.BitSet)} is called, the following must be true for any pair of nodes
+     * A, B where:
      * <ul>
      * <li>A is a predecessor of B</li>
      * <li>`isLooped(A) == isLooped(B) == false`</li>
@@ -39,23 +39,38 @@ public interface BaseTopologicalOrderGraph {
      * <p>
      * Said number may not be unique.
      */
-    NodeTopologicalOrder getTopologicalOrder(int node);
+    int getTopologicalOrder(int node);
 
     /**
-     * Stores a graph node id along its topological order.
+     * Stores a graph node id and dynamically calculates its topological order.
      * Comparisons ignore node id and only use the topological order.
      * For instance, for x = (0, 0) and y = (1, 5), x is before y, whereas for
      * x = (0, 5) and y = (1, 0), y is before x. Note {@link BaseTopologicalOrderGraph}
      * is not guaranteed to return every topological order index (i.e.
      * it might be the case no nodes has order 0).
      */
-    record NodeTopologicalOrder(int nodeId, int order)
+    record NodeTopologicalOrder(int nodeId, BaseTopologicalOrderGraph graph)
             implements
                 Comparable<NodeTopologicalOrder> {
 
+        /**
+         * If the {@link BaseTopologicalOrderGraph#getTopologicalOrder(int)} of the node
+         * changes while inside a {@link java.util.PriorityQueue}, that {@link java.util.PriorityQueue}
+         * would be corrupted.
+         * <p>
+         * It is vital that all changes to a {@link TopologicalOrderGraph} happen
+         * before nodes are inserted into a {@link java.util.PriorityQueue}, and that
+         * the {@link java.util.PriorityQueue} is cleared before future changes to the
+         * {@link TopologicalOrderGraph}.
+         *
+         * @param other The node this node is being topologically compared to.
+         * @return a negative int if this node is topologically before the other node,
+         *         a positive int if this node is topologically after the other node,
+         *         0 if both nodes have the same topological order.
+         */
         @Override
         public int compareTo(NodeTopologicalOrder other) {
-            return order - other.order;
+            return graph.getTopologicalOrder(nodeId) - graph.getTopologicalOrder(other.nodeId);
         }
 
         @Override

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

@@ -253,7 +253,7 @@ private static <Solution_> boolean hasNoNonDeclarativeSourcesFromParent(
         graph.commitChanges(new BitSet());
         var sortedDeclarativeVariables = new ArrayList<>(declarativeShadowVariables);
         sortedDeclarativeVariables.sort(Comparator.<DeclarativeShadowVariableDescriptor<Solution_>> comparingInt(
-                variable -> graph.getTopologicalOrder(nameToIndex.get(variable.getVariableName())).order())
+                variable -> graph.getTopologicalOrder(nameToIndex.get(variable.getVariableName())))
                 .thenComparing(VariableDescriptor::getVariableName));
         return sortedDeclarativeVariables;
     }

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

@@ -14,15 +14,15 @@
 
 public class DefaultTopologicalOrderGraph implements TopologicalOrderGraph {
 
-    private final NodeTopologicalOrder[] nodeIdToTopologicalOrderMap;
+    private final int[] nodeIdToTopologicalOrderMap;
     private final Map<Integer, List<Integer>> componentMap;
     private final Set<Integer>[] forwardEdges;
     private final Set<Integer>[] backEdges;
     private final boolean[] isNodeInLoopedComponent;
 
     @SuppressWarnings({ "unchecked" })
     public DefaultTopologicalOrderGraph(final int size) {
-        this.nodeIdToTopologicalOrderMap = new NodeTopologicalOrder[size];
+        this.nodeIdToTopologicalOrderMap = new int[size];
         this.componentMap = CollectionUtils.newLinkedHashMap(size);
         this.forwardEdges = new Set[size];
         this.backEdges = new Set[size];
@@ -31,7 +31,7 @@ public DefaultTopologicalOrderGraph(final int size) {
             forwardEdges[i] = new HashSet<>();
             backEdges[i] = new HashSet<>();
             isNodeInLoopedComponent[i] = false;
-            nodeIdToTopologicalOrderMap[i] = new NodeTopologicalOrder(i, i);
+            nodeIdToTopologicalOrderMap[i] = i;
         }
     }
 
@@ -112,7 +112,7 @@ public boolean isLooped(LoopedTracker loopedTracker, int node) {
     }
 
     @Override
-    public NodeTopologicalOrder getTopologicalOrder(int node) {
+    public int getTopologicalOrder(int node) {
         return nodeIdToTopologicalOrderMap[node];
     }
 
@@ -141,7 +141,7 @@ public void commitChanges(BitSet changed) {
             var isComponentLooped = componentSize != 1;
             var componentNodes = new ArrayList<Integer>(componentSize);
             for (var node = component.nextSetBit(0); node >= 0; node = component.nextSetBit(node + 1)) {
-                nodeIdToTopologicalOrderMap[node] = new NodeTopologicalOrder(node, ordIndex);
+                nodeIdToTopologicalOrderMap[node] = ordIndex;
                 componentNodes.add(node);
                 componentMap.put(node, componentNodes);
 
@@ -206,7 +206,7 @@ public String toString() {
         var out = new StringBuilder();
         out.append("DefaultTopologicalOrderGraph{\n");
         for (var node = 0; node < forwardEdges.length; node++) {
-            out.append("    ").append(node).append("(").append(nodeIdToTopologicalOrderMap[node].order()).append(") -> ")
+            out.append("    ").append(node).append("(").append(nodeIdToTopologicalOrderMap[node]).append(") -> ")
                     .append(forwardEdges[node].stream()
                             .sorted()
                             .map(Object::toString)

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

@@ -35,7 +35,8 @@ public DefaultVariableReferenceGraph(VariableReferenceGraphBuilder<Solution_> ou
         // Otherwise its internal collections were observed being re-created so often
         // that the allocation of arrays would become a major bottleneck.
         affectedEntitiesUpdater =
-                new AffectedEntitiesUpdater<>(graph, nodeList, immutableEntityToVariableReferenceMap::get,
+                new AffectedEntitiesUpdater<>(graph, nodeList, nodeTopologicalOrders,
+                        immutableEntityToVariableReferenceMap::get,
                         outerGraph.entityToEntityId.size(), outerGraph.changedVariableNotifier);
     }
 

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

@@ -25,7 +25,7 @@ public FixedVariableReferenceGraph(VariableReferenceGraphBuilder<Solution_> oute
         // each node to changed.
         changedVariableNotifier = outerGraph.changedVariableNotifier;
         for (var node = 0; node < nodeList.size(); node++) {
-            changeSet.add(graph.getTopologicalOrder(node));
+            changeSet.add(nodeTopologicalOrders[node]);
             var variableReference = nodeList.get(node).variableReferences().get(0);
             var entityConsistencyState = variableReference.entityConsistencyState();
             if (variableReference.groupEntities() != null) {
@@ -55,7 +55,7 @@ public void markChanged(@NonNull GraphNode<Solution_> node) {
         // Before the graph is finalized, ignore changes, since
         // we don't know the topological order yet
         if (isFinalized) {
-            changeSet.add(graph.getTopologicalOrder(node.graphNodeId()));
+            changeSet.add(nodeTopologicalOrders[node.graphNodeId()]);
         }
     }
 
@@ -85,7 +85,7 @@ public void updateChanged() {
                             continue;
                         }
                         visited.set(nextNode);
-                        changeSet.add(graph.getTopologicalOrder(nextNode));
+                        changeSet.add(nodeTopologicalOrders[nextNode]);
                     }
                 }
             }

core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractTopologicalGraphTest.java

@@ -49,8 +49,8 @@ private static void assertTopologicalOrder(TopologicalOrderGraph graph, List<Int
                 var lowerMembers = expectedTopologicalNodeOrder[j];
                 for (var a : members) {
                     for (var b : lowerMembers) {
-                        var aOrder = graph.getTopologicalOrder(a).order();
-                        var bOrder = graph.getTopologicalOrder(b).order();
+                        var aOrder = graph.getTopologicalOrder(a);
+                        var bOrder = graph.getTopologicalOrder(b);
                         assertThat(aOrder)
                                 .withFailMessage(
                                         () -> "Expected topological order of %d (%d) to be greater than %d (%d) in graph %s"