From c2c75b8a51dd5b7ddca1b42b08b6f0137d22dc24 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Fri, 13 Jun 2025 11:31:42 -0400 Subject: [PATCH 1/7] perf: add specialized variable reference graph when all edges are fixed --- .../AbstractVariableReferenceGraph.java | 167 ++++++++++++++++++ .../DefaultShadowVariableSessionFactory.java | 34 ++-- .../DefaultVariableReferenceGraph.java | 157 +--------------- .../FixedVariableReferenceGraph.java | 85 +++++++++ .../variable/declarative/GraphChangeType.java | 17 ++ .../declarative/VariableReferenceGraph.java | 3 +- .../VariableReferenceGraphBuilder.java | 17 +- 7 files changed, 311 insertions(+), 169 deletions(-) create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphChangeType.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java new file mode 100644 index 00000000000..f6787e9e377 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java @@ -0,0 +1,167 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import java.util.ArrayList; +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.IntFunction; +import java.util.stream.Collectors; + +import ai.timefold.solver.core.impl.util.DynamicIntArray; +import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; + +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; + +public sealed abstract class AbstractVariableReferenceGraph implements VariableReferenceGraph + permits DefaultVariableReferenceGraph, FixedVariableReferenceGraph { + // These structures are immutable. + protected final List> instanceList; + protected final Map, Map>> variableReferenceToInstanceMap; + protected final Map, List, Object>>> variableReferenceToBeforeProcessor; + protected final Map, List, Object>>> variableReferenceToAfterProcessor; + + // These structures are mutable. + protected final DynamicIntArray[] edgeCount; + protected final ChangeSet_ changeSet; + protected final TopologicalOrderGraph graph; + + public AbstractVariableReferenceGraph(VariableReferenceGraphBuilder outerGraph, + IntFunction graphCreator) { + instanceList = List.copyOf(outerGraph.instanceList); + var instanceCount = instanceList.size(); + // Often the maps are a singleton; we improve performance by actually making it so. + variableReferenceToInstanceMap = mapOfMapsDeepCopyOf(outerGraph.variableReferenceToInstanceMap); + variableReferenceToBeforeProcessor = mapOfListsDeepCopyOf(outerGraph.variableReferenceToBeforeProcessor); + variableReferenceToAfterProcessor = mapOfListsDeepCopyOf(outerGraph.variableReferenceToAfterProcessor); + edgeCount = new DynamicIntArray[instanceCount]; + for (int i = 0; i < instanceCount; i++) { + edgeCount[i] = new DynamicIntArray(instanceCount); + } + graph = graphCreator.apply(instanceCount); + graph.withNodeData(instanceList); + + var visited = Collections.newSetFromMap(new IdentityHashMap<>()); + changeSet = createChangeSet(instanceCount); + for (var instance : instanceList) { + var entity = instance.entity(); + if (visited.add(entity)) { + for (var variableId : outerGraph.variableReferenceToAfterProcessor.keySet()) { + afterVariableChanged(variableId, entity); + } + } + } + for (var fixedEdgeEntry : outerGraph.fixedEdges.entrySet()) { + for (var toEdge : fixedEdgeEntry.getValue()) { + addEdge(fixedEdgeEntry.getKey(), toEdge); + } + } + } + + abstract protected ChangeSet_ createChangeSet(int instanceCount); + + @Override + public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { + var map = variableReferenceToInstanceMap.get(variableId); + if (map == null) { + return null; + } + return map.get(entity); + } + + @Override + public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { + var fromNodeId = from.graphNodeId(); + var toNodeId = to.graphNodeId(); + if (fromNodeId == toNodeId) { + return; + } + + var count = edgeCount[fromNodeId].get(toNodeId); + if (count == 0) { + graph.addEdge(fromNodeId, toNodeId); + } + edgeCount[fromNodeId].set(toNodeId, count + 1); + markChanged(to); + } + + @Override + public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { + var fromNodeId = from.graphNodeId(); + var toNodeId = to.graphNodeId(); + if (fromNodeId == toNodeId) { + return; + } + + var count = edgeCount[fromNodeId].get(toNodeId); + if (count == 1) { + graph.removeEdge(fromNodeId, toNodeId); + } + edgeCount[fromNodeId].set(toNodeId, count - 1); + markChanged(to); + } + + @Override + public void beforeVariableChanged(VariableMetaModel variableReference, Object entity) { + if (variableReference.entity().type().isInstance(entity)) { + processEntity(variableReferenceToBeforeProcessor.getOrDefault(variableReference, Collections.emptyList()), entity); + } + } + + private void processEntity(List, Object>> processorList, Object entity) { + var processorCount = processorList.size(); + // Avoid creation of iterators on the hot path. + // The short-lived instances were observed to cause considerable GC pressure. + for (int i = 0; i < processorCount; i++) { + processorList.get(i).accept(this, entity); + } + } + + @Override + public void afterVariableChanged(VariableMetaModel variableReference, Object entity) { + if (variableReference.entity().type().isInstance(entity)) { + var node = lookupOrNull(variableReference, entity); + if (node != null) { + markChanged(node); + } + processEntity(variableReferenceToAfterProcessor.getOrDefault(variableReference, Collections.emptyList()), entity); + } + } + + @Override + public String toString() { + var edgeList = new LinkedHashMap, List>>(); + 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") + static Map> mapOfMapsDeepCopyOf(Map> map) { + var entryArray = map.entrySet() + .stream() + .map(e -> Map.entry(e.getKey(), Map.copyOf(e.getValue()))) + .toArray(Map.Entry[]::new); + return Map.ofEntries(entryArray); + } + + @SuppressWarnings("unchecked") + static Map> mapOfListsDeepCopyOf(Map> map) { + var entryArray = map.entrySet() + .stream() + .map(e -> Map.entry(e.getKey(), List.copyOf(e.getValue()))) + .toArray(Map.Entry[]::new); + return Map.ofEntries(entryArray); + } + +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index fe820a372b0..fcaf3446f81 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -113,12 +113,13 @@ private static void createSourceChangeProcessors( if (!sourcePart.isDeclarative()) { if (sourcePart.onRootEntity()) { // No need for inverse set; source and target entity are the same. - variableReferenceGraphBuilder.addAfterProcessor(toVariableId, (graph, entity) -> { - var changed = graph.lookupOrNull(fromVariableId, entity); - if (changed != null) { - graph.markChanged(changed); - } - }); + variableReferenceGraphBuilder.addAfterProcessor(GraphChangeType.NO_CHANGE, toVariableId, + (graph, entity) -> { + var changed = graph.lookupOrNull(fromVariableId, entity); + if (changed != null) { + graph.markChanged(changed); + } + }); } else { // Need to create an inverse set from source to target var inverseMap = new IdentityHashMap>(); @@ -129,14 +130,15 @@ private static void createSourceChangeProcessors( .computeIfAbsent(shadowEntity, ignored -> new ArrayList<>()).add(rootEntity)); } } - variableReferenceGraphBuilder.addAfterProcessor(toVariableId, (graph, entity) -> { - for (var item : inverseMap.getOrDefault(entity, Collections.emptyList())) { - var changed = graph.lookupOrNull(fromVariableId, item); - if (changed != null) { - graph.markChanged(changed); - } - } - }); + variableReferenceGraphBuilder.addAfterProcessor(GraphChangeType.NO_CHANGE, toVariableId, + (graph, entity) -> { + for (var item : inverseMap.getOrDefault(entity, Collections.emptyList())) { + var changed = graph.lookupOrNull(fromVariableId, item); + if (changed != null) { + graph.markChanged(changed); + } + } + }); } } } @@ -152,7 +154,7 @@ private static void createAliasToVariableChangeProcessors( if (!alias.isDeclarative() && alias.affectGraphEdges()) { // Exploit the same fact as above - variableReferenceGraphBuilder.addBeforeProcessor(sourceVariableId, + variableReferenceGraphBuilder.addBeforeProcessor(GraphChangeType.REMOVE_EDGE, sourceVariableId, (graph, toEntity) -> { // from/to can be null in extended models // ex: previous is used as a source, but only an extended class @@ -172,7 +174,7 @@ private static void createAliasToVariableChangeProcessors( } graph.removeEdge(from, to); }); - variableReferenceGraphBuilder.addAfterProcessor(sourceVariableId, + variableReferenceGraphBuilder.addAfterProcessor(GraphChangeType.ADD_EDGE, sourceVariableId, (graph, toEntity) -> { var to = graph.lookupOrNull(toVariableId, toEntity); if (to == null) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java index 73179d38aea..b726c634cf4 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java @@ -2,70 +2,28 @@ import java.util.ArrayList; 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.impl.util.DynamicIntArray; -import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NonNull; -import org.jspecify.annotations.Nullable; - -final class DefaultVariableReferenceGraph implements VariableReferenceGraph { - - // These structures are immutable. - private final List> instanceList; - private final Map, Map>> variableReferenceToInstanceMap; - private final Map, List, Object>>> variableReferenceToBeforeProcessor; - private final Map, List, Object>>> variableReferenceToAfterProcessor; +final class DefaultVariableReferenceGraph extends AbstractVariableReferenceGraph + implements VariableReferenceGraph { // These structures are mutable. - private final DynamicIntArray[] edgeCount; - private final TopologicalOrderGraph graph; - private final BitSet changed; - private final Consumer affectedEntitiesUpdater; public DefaultVariableReferenceGraph(VariableReferenceGraphBuilder outerGraph, IntFunction graphCreator) { - instanceList = List.copyOf(outerGraph.instanceList); - var instanceCount = instanceList.size(); - // Often the maps are a singleton; we improve performance by actually making it so. - variableReferenceToInstanceMap = mapOfMapsDeepCopyOf(outerGraph.variableReferenceToInstanceMap); - variableReferenceToBeforeProcessor = mapOfListsDeepCopyOf(outerGraph.variableReferenceToBeforeProcessor); - variableReferenceToAfterProcessor = mapOfListsDeepCopyOf(outerGraph.variableReferenceToAfterProcessor); - edgeCount = new DynamicIntArray[instanceCount]; - for (int i = 0; i < instanceCount; i++) { - edgeCount[i] = new DynamicIntArray(instanceCount); - } - graph = graphCreator.apply(instanceCount); - graph.withNodeData(instanceList); - changed = new BitSet(instanceCount); + super(outerGraph, graphCreator); var entityToVariableReferenceMap = new IdentityHashMap>>(); - var visited = Collections.newSetFromMap(new IdentityHashMap<>()); for (var instance : instanceList) { var entity = instance.entity(); - if (visited.add(entity)) { - for (var variableId : outerGraph.variableReferenceToAfterProcessor.keySet()) { - afterVariableChanged(variableId, entity); - } - } entityToVariableReferenceMap.computeIfAbsent(entity, ignored -> new ArrayList<>()) .add(instance); } - for (var fixedEdgeEntry : outerGraph.fixedEdges.entrySet()) { - for (var toEdge : fixedEdgeEntry.getValue()) { - addEdge(fixedEdgeEntry.getKey(), toEdge); - } - } // Immutable optimized version of the map, now that it won't be updated anymore. var immutableEntityToVariableReferenceMap = mapOfListsDeepCopyOf(entityToVariableReferenceMap); // This mutable structure is created once, and reused from there on. @@ -76,118 +34,21 @@ public DefaultVariableReferenceGraph(VariableReferenceGraphBuilder ou } @Override - public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { - var map = variableReferenceToInstanceMap.get(variableId); - if (map == null) { - return null; - } - return map.get(entity); - } - - @Override - public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - var fromNodeId = from.graphNodeId(); - var toNodeId = to.graphNodeId(); - if (fromNodeId == toNodeId) { - return; - } - - var count = edgeCount[fromNodeId].get(toNodeId); - if (count == 0) { - graph.addEdge(fromNodeId, toNodeId); - } - edgeCount[fromNodeId].set(toNodeId, count + 1); - markChanged(to); - } - - @Override - public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - var fromNodeId = from.graphNodeId(); - var toNodeId = to.graphNodeId(); - if (fromNodeId == toNodeId) { - return; - } - - var count = edgeCount[fromNodeId].get(toNodeId); - if (count == 1) { - graph.removeEdge(fromNodeId, toNodeId); - } - edgeCount[fromNodeId].set(toNodeId, count - 1); - markChanged(to); + protected BitSet createChangeSet(int instanceCount) { + return new BitSet(instanceCount); } @Override public void markChanged(@NonNull EntityVariablePair node) { - changed.set(node.graphNodeId()); + changeSet.set(node.graphNodeId()); } @Override public void updateChanged() { - if (changed.isEmpty()) { + if (changeSet.isEmpty()) { return; } - graph.commitChanges(changed); - affectedEntitiesUpdater.accept(changed); + graph.commitChanges(changeSet); + affectedEntitiesUpdater.accept(changeSet); } - - @Override - public void beforeVariableChanged(VariableMetaModel variableReference, Object entity) { - if (variableReference.entity().type().isInstance(entity)) { - processEntity(variableReferenceToBeforeProcessor.getOrDefault(variableReference, Collections.emptyList()), entity); - } - } - - private void processEntity(List, Object>> processorList, Object entity) { - var processorCount = processorList.size(); - // Avoid creation of iterators on the hot path. - // The short-lived instances were observed to cause considerable GC pressure. - for (int i = 0; i < processorCount; i++) { - processorList.get(i).accept(this, entity); - } - } - - @Override - public void afterVariableChanged(VariableMetaModel variableReference, Object entity) { - if (variableReference.entity().type().isInstance(entity)) { - var node = lookupOrNull(variableReference, entity); - if (node != null) { - markChanged(node); - } - processEntity(variableReferenceToAfterProcessor.getOrDefault(variableReference, Collections.emptyList()), entity); - } - } - - @Override - public String toString() { - var edgeList = new LinkedHashMap, List>>(); - 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") - private static Map> mapOfMapsDeepCopyOf(Map> map) { - var entryArray = map.entrySet() - .stream() - .map(e -> Map.entry(e.getKey(), Map.copyOf(e.getValue()))) - .toArray(Map.Entry[]::new); - return Map.ofEntries(entryArray); - } - - @SuppressWarnings("unchecked") - private static Map> mapOfListsDeepCopyOf(Map> map) { - var entryArray = map.entrySet() - .stream() - .map(e -> Map.entry(e.getKey(), List.copyOf(e.getValue()))) - .toArray(Map.Entry[]::new); - return Map.ofEntries(entryArray); - } - } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java new file mode 100644 index 00000000000..3765d3a8610 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java @@ -0,0 +1,85 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import java.util.BitSet; +import java.util.Objects; +import java.util.PriorityQueue; +import java.util.function.IntFunction; + +import org.jspecify.annotations.NonNull; + +public final class FixedVariableReferenceGraph + extends AbstractVariableReferenceGraph> + implements VariableReferenceGraph { + // These are immutable + private final ChangedVariableNotifier changedVariableNotifier; + + // These are mutable + private boolean isFinalized = false; + + public FixedVariableReferenceGraph(VariableReferenceGraphBuilder outerGraph, + IntFunction graphCreator) { + super(outerGraph, graphCreator); + // We don't use a bit set to store changes, so pass a one-use instance + graph.commitChanges(new BitSet(instanceList.size())); + isFinalized = true; + + // Now that we know the topological order of nodes, add + // each node to changed. + for (var node = 0; node < instanceList.size(); node++) { + changeSet.add(graph.getTopologicalOrder(node)); + } + changedVariableNotifier = outerGraph.changedVariableNotifier; + } + + @Override + protected PriorityQueue createChangeSet(int instanceCount) { + return new PriorityQueue<>(instanceCount); + } + + @Override + public void markChanged(@NonNull EntityVariablePair 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())); + } + } + + @Override + public void updateChanged() { + BitSet visited; + if (!changeSet.isEmpty()) { + visited = new BitSet(instanceList.size()); + visited.set(changeSet.peek().nodeId()); + } else { + return; + } + + // NOTE: This assumes the user did not add any fixed loops to + // their graph (i.e. have two variables ALWAYS depend on one-another). + while (!changeSet.isEmpty()) { + var changedNode = changeSet.poll(); + var entityVariable = instanceList.get(changedNode.nodeId()); + var entity = entityVariable.entity(); + var shadowVariableReference = entityVariable.variableReference(); + var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); + var newValue = shadowVariableReference.calculator().apply(entity); + var isVariableChanged = !Objects.equals(oldValue, newValue); + if (isVariableChanged) { + var variableDescriptor = shadowVariableReference.variableDescriptor(); + changedVariableNotifier.beforeVariableChanged().accept(variableDescriptor, entity); + variableDescriptor.setValue(entity, newValue); + changedVariableNotifier.afterVariableChanged().accept(variableDescriptor, entity); + + for (var iterator = graph.nodeForwardEdges(changedNode.nodeId()); iterator.hasNext();) { + var nextNode = iterator.next(); + if (visited.get(nextNode)) { + continue; + } + visited.set(nextNode); + changeSet.add(graph.getTopologicalOrder(nextNode)); + } + } + } + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphChangeType.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphChangeType.java new file mode 100644 index 00000000000..2b709d3368c --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphChangeType.java @@ -0,0 +1,17 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +public enum GraphChangeType { + NO_CHANGE(false), + ADD_EDGE(true), + REMOVE_EDGE(true); + + private final boolean affectsGraph; + + GraphChangeType(boolean affectsGraph) { + this.affectsGraph = affectsGraph; + } + + public boolean affectsGraph() { + return affectsGraph; + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java index 0d12ff2e882..d77d4ed68fe 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java @@ -6,7 +6,8 @@ import org.jspecify.annotations.Nullable; public sealed interface VariableReferenceGraph - permits DefaultVariableReferenceGraph, EmptyVariableReferenceGraph { + permits AbstractVariableReferenceGraph, DefaultVariableReferenceGraph, EmptyVariableReferenceGraph, + FixedVariableReferenceGraph { @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java index 2e635c5bde4..8ec2ed73df7 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java @@ -21,6 +21,7 @@ public final class VariableReferenceGraphBuilder { final List> instanceList; final Map, List>> fixedEdges; final Map, Map>> variableReferenceToInstanceMap; + boolean isGraphFixed; public VariableReferenceGraphBuilder(ChangedVariableNotifier changedVariableNotifier) { this.changedVariableNotifier = changedVariableNotifier; @@ -29,6 +30,7 @@ public VariableReferenceGraphBuilder(ChangedVariableNotifier changedV variableReferenceToBeforeProcessor = new HashMap<>(); variableReferenceToAfterProcessor = new HashMap<>(); fixedEdges = new HashMap<>(); + isGraphFixed = true; } public void addVariableReferenceEntity(Entity_ entity, VariableUpdaterInfo variableReference) { @@ -54,14 +56,16 @@ public void addFixedEdge(@NonNull EntityVariablePair from, @NonNull E fixedEdges.computeIfAbsent(from, k -> new ArrayList<>()).add(to); } - public void addBeforeProcessor(VariableMetaModel variableId, + public void addBeforeProcessor(GraphChangeType graphChangeType, VariableMetaModel variableId, BiConsumer, Object> consumer) { + isGraphFixed &= !graphChangeType.affectsGraph(); variableReferenceToBeforeProcessor.computeIfAbsent(variableId, k -> new ArrayList<>()) .add(consumer); } - public void addAfterProcessor(VariableMetaModel variableId, + public void addAfterProcessor(GraphChangeType graphChangeType, VariableMetaModel variableId, BiConsumer, Object> consumer) { + isGraphFixed &= !graphChangeType.affectsGraph(); variableReferenceToAfterProcessor.computeIfAbsent(variableId, k -> new ArrayList<>()) .add(consumer); } @@ -72,8 +76,13 @@ public VariableReferenceGraph build(IntFunction(this, graphCreator); + if (instanceList.isEmpty()) { + return EmptyVariableReferenceGraph.INSTANCE; + } + if (isGraphFixed) { + return new FixedVariableReferenceGraph<>(this, graphCreator); + } + return new DefaultVariableReferenceGraph<>(this, graphCreator); } public @NonNull EntityVariablePair lookupOrError(VariableMetaModel variableId, Object entity) { From 72b5795a56dc75dae04b914beed999cde54a70b6 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Mon, 23 Jun 2025 20:53:23 -0400 Subject: [PATCH 2/7] perf: Use a simplified updater when there is only a single direction an update can go - If only one entity has declarative variables, and all sources are either of the form "previous.*", "undirectional" or "declarative", then create a cascading updater using next to get successor and (index, id(entity)) for the initial sort. - If only one entity has declarative variables, and all sources are either of the form "next.*", "undirectional" or "declarative", then create a cascading updater using previous to get successor and (-index, id(entity)) for the initial sort. - If there are no dynamic edges, use a fixed topological order for each variable. - Otherwise use the current approach. Undirectional mean variables that don't create edges in the graph (such as accessing a fact on the inverse or genuine variables). --- .../declarative/ChangedVariableNotifier.java | 21 ++- .../DefaultShadowVariableSessionFactory.java | 120 +++++++++++++- .../variable/declarative/GraphStructure.java | 87 ++++++++++ .../declarative/ParentVariableType.java | 35 ++++ .../declarative/RootVariableSource.java | 83 ++++++++-- ...rectionalParentVariableReferenceGraph.java | 149 ++++++++++++++++++ .../declarative/VariableReferenceGraph.java | 2 +- .../declarative/GraphStructureTest.java | 48 ++++++ .../FollowerValuesShadowVariableTest.java | 2 +- .../simple_list/SimpleListTest.java | 63 ++++++++ .../TestdataDeclarativeExtendedBaseValue.java | 2 +- .../TestdataDeclarativeExtendedSolution.java | 11 ++ .../follower/TestdataFollowerSolution.java | 2 +- ...clarativeSimpleListConstraintProvider.java | 20 +++ .../TestdataDeclarativeSimpleListEntity.java | 53 +++++++ ...TestdataDeclarativeSimpleListSolution.java | 67 ++++++++ .../TestdataDeclarativeSimpleListValue.java | 103 ++++++++++++ 17 files changed, 849 insertions(+), 19 deletions(-) create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java create mode 100644 core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListConstraintProvider.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListEntity.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListSolution.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListValue.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java index 7567bbcef6b..ba537ccc9c8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java @@ -2,15 +2,21 @@ import java.util.function.BiConsumer; +import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; +import org.jspecify.annotations.Nullable; + public record ChangedVariableNotifier(BiConsumer, Object> beforeVariableChanged, - BiConsumer, Object> afterVariableChanged) { + BiConsumer, Object> afterVariableChanged, + @Nullable ListVariableStateSupply listVariableStateSupply) { + private static final ChangedVariableNotifier EMPTY = new ChangedVariableNotifier<>((a, b) -> { }, (a, b) -> { - }); + }, + null); @SuppressWarnings("unchecked") public static ChangedVariableNotifier empty() { @@ -18,9 +24,18 @@ public static ChangedVariableNotifier empty() { } public static ChangedVariableNotifier of(InnerScoreDirector scoreDirector) { + var listVariable = scoreDirector.getSolutionDescriptor().getListVariableDescriptor(); + if (listVariable == null) { + return new ChangedVariableNotifier<>( + scoreDirector::beforeVariableChanged, + scoreDirector::afterVariableChanged, + null); + } + var listVariableStateSupply = scoreDirector.getListVariableStateSupply(listVariable); return new ChangedVariableNotifier<>( scoreDirector::beforeVariableChanged, - scoreDirector::afterVariableChanged); + scoreDirector::afterVariableChanged, + listVariableStateSupply); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index fcaf3446f81..5409e76bd61 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -1,23 +1,35 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; import java.util.ArrayList; +import java.util.BitSet; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.function.IntFunction; +import java.util.function.ToIntFunction; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply; +import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; +import ai.timefold.solver.core.impl.util.CollectionUtils; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NullMarked; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @NullMarked public class DefaultShadowVariableSessionFactory { + private final static Logger LOGGER = LoggerFactory.getLogger(DefaultShadowVariableSessionFactory.class); private final SolutionDescriptor solutionDescriptor; private final InnerScoreDirector scoreDirector; private final IntFunction graphCreator; @@ -36,10 +48,112 @@ public static VariableReferenceGraph buildGraph( SolutionDescriptor solutionDescriptor, VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { - var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); - if (declarativeShadowVariableDescriptors.isEmpty()) { - return EmptyVariableReferenceGraph.INSTANCE; + var graphStructure = GraphStructure.determineGraphStructure(solutionDescriptor); + LOGGER.debug("Graph structure: {}", graphStructure); + return switch (graphStructure) { + case EMPTY -> EmptyVariableReferenceGraph.INSTANCE; + case SINGLE_DIRECTIONAL_PARENT -> { + var listVariableStateSupply = variableReferenceGraphBuilder.changedVariableNotifier.listVariableStateSupply(); + if (listVariableStateSupply == null) { + yield buildArbitraryGraph(solutionDescriptor, variableReferenceGraphBuilder, entities, graphCreator); + } + yield buildSingleDirectionalParentGraph(solutionDescriptor, + variableReferenceGraphBuilder.changedVariableNotifier, listVariableStateSupply, + entities); + } + case NO_DYNAMIC_EDGES, ARBITRARY -> + buildArbitraryGraph(solutionDescriptor, variableReferenceGraphBuilder, entities, graphCreator); + }; + } + + private static VariableReferenceGraph buildSingleDirectionalParentGraph( + SolutionDescriptor solutionDescriptor, + ChangedVariableNotifier changedVariableNotifier, + ListVariableStateSupply listVariableStateSupply, + Object[] entities) { + var declarativeShadowVariables = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); + Map nameToIndex = new LinkedHashMap<>(); + for (var declarativeShadowVariable : declarativeShadowVariables) { + nameToIndex.put(declarativeShadowVariable.getVariableName(), nameToIndex.size()); + } + var graph = new DefaultTopologicalOrderGraph(nameToIndex.size()); + ParentVariableType parentVariableType = null; + for (var declarativeShadowVariable : declarativeShadowVariables) { + var toIndex = nameToIndex.get(declarativeShadowVariable.getVariableName()); + var visited = new HashSet(); + for (var source : declarativeShadowVariable.getSources()) { + switch (source.parentVariableType()) { + case PREVIOUS, NEXT -> { + parentVariableType = source.parentVariableType(); + } + default -> { + } + } + var variableReferences = source.variableSourceReferences(); + if (variableReferences.size() != 1) { + // variableReferences is from directional variable + continue; + } + var variableReference = variableReferences.get(0); + var sourceDeclarativeVariable = variableReference.downstreamDeclarativeVariableMetamodel(); + if (sourceDeclarativeVariable != null) { + var fromIndex = nameToIndex.get(sourceDeclarativeVariable.name()); + if (visited.add(fromIndex)) { + graph.addEdge(fromIndex, toIndex); + } + } + } } + graph.commitChanges(new BitSet()); + var sortedDeclarativeVariables = new ArrayList<>(declarativeShadowVariables); + sortedDeclarativeVariables.sort(Comparator.> comparingInt( + variable -> graph.getTopologicalOrder(nameToIndex.get(variable.getVariableName())).order()) + .thenComparing(VariableDescriptor::getVariableName)); + + assert parentVariableType != null; + Function successorFunction; + Comparator comparator; + // Needed since otherwise values with the same index on different entities + // would be considered the same value. + Map entityToIndexMap = CollectionUtils.newIdentityHashMap(entities.length); + ToIntFunction indexFunction = value -> { + var out = listVariableStateSupply.getIndex(value); + if (out == null) { + return 0; + } + return out; + }; + + for (var entity : entities) { + entityToIndexMap.put(entity, entityToIndexMap.size()); + } + + switch (parentVariableType) { + case PREVIOUS -> { + successorFunction = listVariableStateSupply::getNextElement; + comparator = Comparator.comparingInt(indexFunction) + .thenComparing(entityToIndexMap::get); + } + case NEXT -> { + successorFunction = listVariableStateSupply::getPreviousElement; + comparator = Comparator.comparingInt(indexFunction) + .thenComparing(entityToIndexMap::get).reversed(); + } + default -> { + throw new IllegalStateException( + "Impossible state: expected parentVariableType to be previous or next but was %s." + .formatted(parentVariableType)); + } + } + return new SingleDirectionalParentVariableReferenceGraph<>(sortedDeclarativeVariables, successorFunction, comparator, + changedVariableNotifier, entities); + } + + private static VariableReferenceGraph buildArbitraryGraph( + SolutionDescriptor solutionDescriptor, + VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, + IntFunction graphCreator) { + var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); var variableIdToUpdater = new HashMap, VariableUpdaterInfo>(); // Create graph node for each entity/declarative shadow variable pair. diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java new file mode 100644 index 00000000000..3e5e1ea3138 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java @@ -0,0 +1,87 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import java.util.Arrays; + +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public enum GraphStructure { + /** + * A graph structure that only accepts the empty graph. + */ + EMPTY, + + /** + * A graph structure without dynamic edges. + */ + NO_DYNAMIC_EDGES, + + /** + * A graph structure where there is at most + * one directional parent for each graph node. + */ + SINGLE_DIRECTIONAL_PARENT, + + /** + * A graph structure that accepts all graphs. + */ + ARBITRARY; + + private static final Logger LOGGER = LoggerFactory.getLogger(GraphStructure.class); + + public static GraphStructure determineGraphStructure(SolutionDescriptor solutionDescriptor) { + var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); + if (declarativeShadowVariableDescriptors.isEmpty()) { + return EMPTY; + } + var multipleDeclarativeEntityClasses = declarativeShadowVariableDescriptors.stream() + .map(variable -> variable.getEntityDescriptor().getEntityClass()) + .distinct().count() > 1; + + if (multipleDeclarativeEntityClasses) { + // Inverse might become directional if it has + // declarative variables; ARBITRARY does optimize + // the graph to NO_DYNAMIC_EDGES if there are no variable listeners that + // add/remove edges. + return ARBITRARY; + } + + var rootVariableSources = declarativeShadowVariableDescriptors.stream() + .flatMap(descriptor -> Arrays.stream(descriptor.getSources())) + .toList(); + ParentVariableType directionalType = null; + for (var variableSource : rootVariableSources) { + var parentVariableType = variableSource.parentVariableType(); + LOGGER.debug("{} has parentVariableType {}", variableSource, parentVariableType); + switch (parentVariableType) { + case GROUP, INDIRECT_DIRECTIONAL, CHAINED_INVERSE -> { + // CHAINED_INVERSE is arbitrary, since we don't have + // the concept of "index" to help us identify topological + // order without creating a graph. + return GraphStructure.ARBITRARY; + } + case NEXT, PREVIOUS -> { + if (directionalType == null) { + directionalType = parentVariableType; + } else if (directionalType != parentVariableType) { + return GraphStructure.ARBITRARY; + } + } + case UNDIRECTIONAL -> { + // Do nothing + } + default -> { + throw new IllegalStateException("Unhandled case %s".formatted(variableSource.parentVariableType())); + } + } + } + + if (directionalType == null) { + return NO_DYNAMIC_EDGES; + } else { + return SINGLE_DIRECTIONAL_PARENT; + } + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java new file mode 100644 index 00000000000..6d96ce5e0ce --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java @@ -0,0 +1,35 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +public enum ParentVariableType { + /** + * Next element variable accessed from the root object. + */ + NEXT, + + /** + * Previous element variable accessed from the root object. + */ + PREVIOUS, + + /** + * Previous element variable accessed from the root object in a chained model. + */ + CHAINED_INVERSE, + + /** + * Next/previous element variable accessed + * from a fact. + */ + INDIRECT_DIRECTIONAL, + + /** + * Variables accessed from a group. + */ + GROUP, + + /** + * Declarative variables, inverse variables, and + * genuine variables. + */ + UNDIRECTIONAL +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index 5ef7139fe97..c8a80f4bc00 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -8,6 +8,11 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; +import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; +import ai.timefold.solver.core.api.domain.variable.NextElementShadowVariable; +import ai.timefold.solver.core.api.domain.variable.PlanningVariable; +import ai.timefold.solver.core.api.domain.variable.PlanningVariableGraphType; +import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; import ai.timefold.solver.core.api.domain.variable.ShadowVariable; import ai.timefold.solver.core.config.util.ConfigUtils; import ai.timefold.solver.core.impl.domain.common.ReflectionHelper; @@ -25,7 +30,9 @@ public record RootVariableSource( Class rootEntity, List listMemberAccessors, BiConsumer> valueEntityFunction, - List variableSourceReferences) { + List variableSourceReferences, + String variablePath, + ParentVariableType parentVariableType) { public static final String COLLECTION_REFERENCE_SUFFIX = "[]"; public static final String MEMBER_SEPERATOR_REGEX = "\\."; @@ -66,6 +73,7 @@ public static RootVariableSource from( boolean isAfterVariable = false; Class currentEntity = rootEntityClass; var factCountSinceLastVariable = 0; + ParentVariableType parentVariableType = null; for (var iterator = pathIterator(rootEntityClass, variablePath); iterator.hasNext();) { var pathPart = iterator.next(); @@ -94,6 +102,7 @@ public static RootVariableSource from( memberAccessor.getType(), memberAccessor.getGenericType(), ShadowSources.class, memberAccessor.getName()); + parentVariableType = ParentVariableType.GROUP; hasListMemberAccessor = true; } else { var memberAccessor = getMemberAccessor(pathPart.member(), @@ -116,6 +125,10 @@ public static RootVariableSource from( isAfterVariable = true; factCountSinceLastVariable = 0; + + if (parentVariableType == null) { + parentVariableType = determineParentVariableType(chainToVariable, memberAccessor); + } } else { factCountSinceLastVariable++; if (factCountSinceLastVariable == 2) { @@ -171,7 +184,9 @@ public static RootVariableSource from( return new RootVariableSource<>(rootEntityClass, listMemberAccessors, valueEntityFunction, - variableSourceReferences); + variableSourceReferences, + variablePath, + parentVariableType); } public @NonNull BiConsumer> getEntityVisitor(List chainToEntity) { @@ -314,16 +329,61 @@ public static boolean isVariable(PlanningSolutionMetaModel metaModel, Class chain, MemberAccessor memberAccessor) { + var isIndirect = chain.size() > 1; + var declaringClass = memberAccessor.getDeclaringClass(); + var memberName = memberAccessor.getName(); + if (getAnnotation(declaringClass, memberName, PreviousElementShadowVariable.class) != null) { + if (isIndirect) { + return ParentVariableType.INDIRECT_DIRECTIONAL; + } + return ParentVariableType.PREVIOUS; + } + if (getAnnotation(declaringClass, memberName, NextElementShadowVariable.class) != null) { + if (isIndirect) { + return ParentVariableType.INDIRECT_DIRECTIONAL; + } + return ParentVariableType.NEXT; + } + if (getAnnotation(declaringClass, memberName, InverseRelationShadowVariable.class) != null) { + // inverse can be both directional and undirectional; + // it is directional in chained models, undirectional otherwise + var inverseVariable = getAnnotation(declaringClass, memberName, InverseRelationShadowVariable.class); + var sourceClass = memberAccessor.getType(); + var variableName = inverseVariable.sourceVariableName(); + PlanningVariable sourcePlanningVariable = getAnnotation(sourceClass, variableName, PlanningVariable.class); + if (sourcePlanningVariable == null) { + return ParentVariableType.UNDIRECTIONAL; + } + if (sourcePlanningVariable.graphType() == PlanningVariableGraphType.CHAINED) { + if (isIndirect) { + return ParentVariableType.INDIRECT_DIRECTIONAL; + } + return ParentVariableType.CHAINED_INVERSE; + } + return ParentVariableType.UNDIRECTIONAL; + } + return ParentVariableType.UNDIRECTIONAL; + } + private static T getAnnotation(Class declaringClass, String memberName, Class annotationClass) { - var field = ReflectionHelper.getDeclaredField(declaringClass, memberName); - var getterMethod = ReflectionHelper.getDeclaredGetterMethod(declaringClass, memberName); + var currentClass = declaringClass; - if (field != null && field.getAnnotation(annotationClass) != null) { - return field.getAnnotation(annotationClass); - } - if (getterMethod != null && getterMethod.getAnnotation(annotationClass) != null) { - return getterMethod.getAnnotation(annotationClass); + while (currentClass != null) { + var field = ReflectionHelper.getDeclaredField(currentClass, memberName); + var getterMethod = ReflectionHelper.getDeclaredGetterMethod(currentClass, memberName); + + if (field != null && field.getAnnotation(annotationClass) != null) { + return field.getAnnotation(annotationClass); + } + if (getterMethod != null && getterMethod.getAnnotation(annotationClass) != null) { + return getterMethod.getAnnotation(annotationClass); + } + + // Need to also check superclass to support extended models; + // the subclass might have overridden an annotated method. + currentClass = currentClass.getSuperclass(); } return null; } @@ -337,4 +397,9 @@ private static boolean isDeclarativeShadowVariable(MemberAccessor memberAccessor return !shadowVariable.supplierName().isEmpty(); } + @Override + public @NonNull String toString() { + return variablePath; + } + } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java new file mode 100644 index 00000000000..f4e838962eb --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java @@ -0,0 +1,149 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.Function; + +import ai.timefold.solver.core.impl.util.CollectionUtils; +import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; + +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; + +public final class SingleDirectionalParentVariableReferenceGraph implements VariableReferenceGraph { + private final Map, Map>> variableReferenceToInstanceMap; + private final Set> monitoredSourceVariableSet; + private final List> sortedVariableUpdaterInfoList; + private final Set changedSet; + private final Function successorFunction; + private final Comparator comparator; + private final ChangedVariableNotifier changedVariableNotifier; + private final ShadowVariableLoopedVariableDescriptor loopedDescriptor; + + public SingleDirectionalParentVariableReferenceGraph( + List> sortedDeclarativeShadowVariableDescriptors, + Function successorFunction, Comparator comparator, + ChangedVariableNotifier changedVariableNotifier, + Object[] entities) { + variableReferenceToInstanceMap = CollectionUtils.newHashMap(sortedDeclarativeShadowVariableDescriptors.size()); + sortedVariableUpdaterInfoList = new ArrayList<>(sortedDeclarativeShadowVariableDescriptors.size()); + monitoredSourceVariableSet = new HashSet<>(); + changedSet = Collections.newSetFromMap(CollectionUtils.newIdentityHashMap(entities.length)); + + this.successorFunction = successorFunction; + this.comparator = comparator; + this.changedVariableNotifier = changedVariableNotifier; + this.loopedDescriptor = + sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getShadowVariableLoopedDescriptor(); + + for (var variableDescriptor : sortedDeclarativeShadowVariableDescriptors) { + var variableMetaModel = variableDescriptor.getVariableMetaModel(); + var objectMap = CollectionUtils.newIdentityHashMap(entities.length); + var variableUpdaterInfo = new VariableUpdaterInfo<>( + variableMetaModel, + variableDescriptor, + loopedDescriptor, + variableDescriptor.getMemberAccessor(), + variableDescriptor.getCalculator()::executeGetter); + sortedVariableUpdaterInfoList.add(variableUpdaterInfo); + variableReferenceToInstanceMap.put(variableMetaModel, CollectionUtils.newIdentityHashMap(entities.length)); + + for (var source : variableDescriptor.getSources()) { + for (var sourceReference : source.variableSourceReferences()) { + monitoredSourceVariableSet.add(sourceReference.variableMetaModel()); + } + } + + for (var entity : entities) { + // No graph, so no graph id + if (variableDescriptor.getEntityDescriptor().getEntityClass().isInstance(entity)) { + objectMap.put(entity, new EntityVariablePair<>(entity, variableUpdaterInfo, -1)); + changedSet.add(entity); + } + } + } + } + + @Override + public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { + return variableReferenceToInstanceMap.get(variableId).get(entity); + } + + @Override + public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { + markChanged(to); + } + + @Override + public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { + markChanged(to); + } + + @Override + public void markChanged(@NonNull EntityVariablePair node) { + changedSet.add(node.entity()); + } + + @Override + public void updateChanged() { + var changedSorted = new TreeSet<>(comparator); + var visited = Collections.newSetFromMap(new IdentityHashMap<>()); + changedSorted.addAll(changedSet); + for (var changed : changedSorted) { + if (visited.contains(changed)) { + continue; + } + + var current = changed; + while (current != null) { + visited.add(current); + var anyChanged = false; + if (loopedDescriptor != null) { + var oldValue = loopedDescriptor.getValue(current); + if (!Objects.equals(oldValue, false)) { + anyChanged = true; + changedVariableNotifier.beforeVariableChanged().accept(loopedDescriptor, current); + loopedDescriptor.setValue(current, false); + changedVariableNotifier.afterVariableChanged().accept(loopedDescriptor, current); + } + } + for (var updater : sortedVariableUpdaterInfoList) { + var oldValue = updater.memberAccessor().executeGetter(current); + var newValue = updater.calculator().apply(current); + if (!Objects.equals(oldValue, newValue)) { + anyChanged = true; + changedVariableNotifier.beforeVariableChanged().accept(updater.variableDescriptor(), current); + updater.memberAccessor().executeSetter(current, newValue); + changedVariableNotifier.afterVariableChanged().accept(updater.variableDescriptor(), current); + } + } + if (anyChanged) { + current = successorFunction.apply(current); + } else { + current = null; + } + } + } + changedSet.clear(); + } + + @Override + public void beforeVariableChanged(VariableMetaModel variableReference, Object entity) { + // Do nothing + } + + @Override + public void afterVariableChanged(VariableMetaModel variableReference, Object entity) { + if (monitoredSourceVariableSet.contains(variableReference)) { + changedSet.add(entity); + } + } +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java index d77d4ed68fe..fdd88c4e46c 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java @@ -7,7 +7,7 @@ public sealed interface VariableReferenceGraph permits AbstractVariableReferenceGraph, DefaultVariableReferenceGraph, EmptyVariableReferenceGraph, - FixedVariableReferenceGraph { + FixedVariableReferenceGraph, SingleDirectionalParentVariableReferenceGraph { @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java new file mode 100644 index 00000000000..f29b7c87a8f --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java @@ -0,0 +1,48 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import static org.assertj.core.api.Assertions.assertThat; + +import ai.timefold.solver.core.testdomain.TestdataSolution; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentSolution; +import ai.timefold.solver.core.testdomain.declarative.extended.TestdataDeclarativeExtendedSolution; +import ai.timefold.solver.core.testdomain.declarative.follower.TestdataFollowerSolution; +import ai.timefold.solver.core.testdomain.declarative.simple_list.TestdataDeclarativeSimpleListSolution; + +import org.junit.jupiter.api.Test; + +public class GraphStructureTest { + @Test + public void simpleListStructure() { + assertThat(GraphStructure.determineGraphStructure( + TestdataDeclarativeSimpleListSolution.buildSolutionDescriptor())) + .isEqualTo(GraphStructure.SINGLE_DIRECTIONAL_PARENT); + } + + @Test + public void extendedSimpleListStructure() { + assertThat(GraphStructure.determineGraphStructure( + TestdataDeclarativeExtendedSolution.buildSolutionDescriptor())) + .isEqualTo(GraphStructure.SINGLE_DIRECTIONAL_PARENT); + } + + @Test + public void concurrentValuesStructure() { + assertThat(GraphStructure.determineGraphStructure( + TestdataConcurrentSolution.buildSolutionDescriptor())) + .isEqualTo(GraphStructure.ARBITRARY); + } + + @Test + public void followerStructure() { + assertThat(GraphStructure.determineGraphStructure( + TestdataFollowerSolution.buildSolutionDescriptor())) + .isEqualTo(GraphStructure.NO_DYNAMIC_EDGES); + } + + @Test + public void emptyStructure() { + assertThat(GraphStructure.determineGraphStructure( + TestdataSolution.buildSolutionDescriptor())) + .isEqualTo(GraphStructure.EMPTY); + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/follower/FollowerValuesShadowVariableTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/follower/FollowerValuesShadowVariableTest.java index 9cf30069715..76b8375c2e3 100644 --- a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/follower/FollowerValuesShadowVariableTest.java +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/follower/FollowerValuesShadowVariableTest.java @@ -65,7 +65,7 @@ void testMove() { followerB1, followerB2), List.of(value1, value2)); - var solutionDescriptor = TestdataFollowerSolution.getSolutionDescriptor(); + var solutionDescriptor = TestdataFollowerSolution.buildSolutionDescriptor(); var variableMetamodel = solutionDescriptor.getMetaModel().entity(TestdataLeaderEntity.class).variable("value"); var moveAsserter = MoveAsserter.create(solutionDescriptor); diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java new file mode 100644 index 00000000000..a06aa5cfee6 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java @@ -0,0 +1,63 @@ +package ai.timefold.solver.core.preview.api.variable.declarative.simple_list; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import ai.timefold.solver.core.api.solver.SolverFactory; +import ai.timefold.solver.core.config.solver.EnvironmentMode; +import ai.timefold.solver.core.config.solver.PreviewFeature; +import ai.timefold.solver.core.config.solver.SolverConfig; +import ai.timefold.solver.core.config.solver.termination.TerminationConfig; +import ai.timefold.solver.core.testdomain.TestdataObject; +import ai.timefold.solver.core.testdomain.declarative.simple_list.TestdataDeclarativeSimpleListConstraintProvider; +import ai.timefold.solver.core.testdomain.declarative.simple_list.TestdataDeclarativeSimpleListEntity; +import ai.timefold.solver.core.testdomain.declarative.simple_list.TestdataDeclarativeSimpleListSolution; +import ai.timefold.solver.core.testdomain.declarative.simple_list.TestdataDeclarativeSimpleListValue; + +import org.junit.jupiter.api.Test; + +public class SimpleListTest { + @Test + public void simpleList() { + var solverConfig = new SolverConfig() + .withSolutionClass(TestdataDeclarativeSimpleListSolution.class) + .withEntityClasses(TestdataDeclarativeSimpleListEntity.class, TestdataDeclarativeSimpleListValue.class) + .withConstraintProviderClass(TestdataDeclarativeSimpleListConstraintProvider.class) + .withPreviewFeature(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES) + .withEnvironmentMode(EnvironmentMode.FULL_ASSERT) + .withTerminationConfig(new TerminationConfig().withBestScoreLimit("-344")); + + var entityList = List.of(new TestdataDeclarativeSimpleListEntity("e1", 0, 0)); + var valueList = List.of( + new TestdataDeclarativeSimpleListValue("v1", 1, 60), + new TestdataDeclarativeSimpleListValue("v2", 2, 120), + new TestdataDeclarativeSimpleListValue("v3", 3, 30)); + var problem = new TestdataDeclarativeSimpleListSolution(entityList, valueList); + var solverFactory = SolverFactory. create(solverConfig); + var solver = solverFactory.buildSolver(); + var solution = solver.solve(problem); + + // Note that we minimize the end time of all values, and not + // the end time of the last value. Since duration is cumulative and the + // difference in duration is larger than the difference in positions, + // the solver wants to do the shortest duration first, and thus the + // best solution is the one that maximizes distance. + assertThat(solution.getEntityList().get(0).getValues()) + .map(TestdataObject::getCode) + .containsExactly("v3", "v1", "v2"); + + var v1 = solution.getValueList().get(0); + var v2 = solution.getValueList().get(1); + var v3 = solution.getValueList().get(2); + + assertThat(v3.getStartTime()).isEqualTo(3); + assertThat(v3.getEndTime()).isEqualTo(33); + + assertThat(v1.getStartTime()).isEqualTo(35); + assertThat(v1.getEndTime()).isEqualTo(95); + + assertThat(v2.getStartTime()).isEqualTo(96); + assertThat(v2.getEndTime()).isEqualTo(216); + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedBaseValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedBaseValue.java index d50ef5af5ba..564b925826b 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedBaseValue.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedBaseValue.java @@ -6,7 +6,6 @@ @PlanningEntity public class TestdataDeclarativeExtendedBaseValue extends TestdataObject { - @PreviousElementShadowVariable(sourceVariableName = "values") TestdataDeclarativeExtendedBaseValue previous; public TestdataDeclarativeExtendedBaseValue() { @@ -17,6 +16,7 @@ public TestdataDeclarativeExtendedBaseValue(String code) { super(code); } + @PreviousElementShadowVariable(sourceVariableName = "values") public TestdataDeclarativeExtendedBaseValue getPrevious() { return previous; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedSolution.java index ade7e29f678..75a7a616a2b 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/extended/TestdataDeclarativeExtendedSolution.java @@ -1,16 +1,27 @@ package ai.timefold.solver.core.testdomain.declarative.extended; import java.util.List; +import java.util.Set; import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty; import ai.timefold.solver.core.api.domain.solution.PlanningScore; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; import ai.timefold.solver.core.api.score.buildin.hardsoft.HardSoftScore; +import ai.timefold.solver.core.config.solver.PreviewFeature; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; import ai.timefold.solver.core.testdomain.TestdataObject; @PlanningSolution public class TestdataDeclarativeExtendedSolution extends TestdataObject { + public static SolutionDescriptor buildSolutionDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(Set.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataDeclarativeExtendedSolution.class, + TestdataDeclarativeExtendedEntity.class, + TestdataDeclarativeExtendedBaseValue.class, + TestdataDeclarativeExtendedSubclassValue.class); + } + @PlanningEntityCollectionProperty List entities; diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/follower/TestdataFollowerSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/follower/TestdataFollowerSolution.java index 10b64959b8d..7a020967c0f 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/follower/TestdataFollowerSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/follower/TestdataFollowerSolution.java @@ -17,7 +17,7 @@ @PlanningSolution public class TestdataFollowerSolution extends TestdataObject { - public static SolutionDescriptor getSolutionDescriptor() { + public static SolutionDescriptor buildSolutionDescriptor() { return SolutionDescriptor.buildSolutionDescriptor(Set.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), TestdataFollowerSolution.class, TestdataLeaderEntity.class, TestdataFollowerEntity.class); } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListConstraintProvider.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListConstraintProvider.java new file mode 100644 index 00000000000..67c49651fa8 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListConstraintProvider.java @@ -0,0 +1,20 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_list; + +import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; +import ai.timefold.solver.core.api.score.stream.Constraint; +import ai.timefold.solver.core.api.score.stream.ConstraintFactory; +import ai.timefold.solver.core.api.score.stream.ConstraintProvider; + +import org.jspecify.annotations.NonNull; + +public class TestdataDeclarativeSimpleListConstraintProvider implements ConstraintProvider { + @Override + public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory constraintFactory) { + return new Constraint[] { + constraintFactory.forEach(TestdataDeclarativeSimpleListValue.class) + .penalize(SimpleScore.ONE, + value -> value.endTime) + .asConstraint("Minimize end time") + }; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListEntity.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListEntity.java new file mode 100644 index 00000000000..7085d1d8bd8 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListEntity.java @@ -0,0 +1,53 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_list; + +import java.util.ArrayList; +import java.util.List; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.PlanningListVariable; +import ai.timefold.solver.core.testdomain.TestdataObject; + +@PlanningEntity +public class TestdataDeclarativeSimpleListEntity extends TestdataObject { + @PlanningListVariable + List values; + + int position; + int startTime; + + public TestdataDeclarativeSimpleListEntity() { + this.values = new ArrayList<>(); + } + + public TestdataDeclarativeSimpleListEntity(String code, int position, int startTime) { + super(code); + this.values = new ArrayList<>(); + this.position = position; + this.startTime = startTime; + } + + public List getValues() { + return values; + } + + public void setValues( + List values) { + this.values = values; + } + + public int getPosition() { + return position; + } + + public void setPosition(int position) { + this.position = position; + } + + public int getStartTime() { + return startTime; + } + + public void setStartTime(int startTime) { + this.startTime = startTime; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListSolution.java new file mode 100644 index 00000000000..5bb84058815 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListSolution.java @@ -0,0 +1,67 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_list; + +import java.util.List; +import java.util.Set; + +import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty; +import ai.timefold.solver.core.api.domain.solution.PlanningScore; +import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; +import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; +import ai.timefold.solver.core.config.solver.PreviewFeature; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; + +@PlanningSolution +public class TestdataDeclarativeSimpleListSolution { + public static SolutionDescriptor buildSolutionDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(Set.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataDeclarativeSimpleListSolution.class, + TestdataDeclarativeSimpleListEntity.class, + TestdataDeclarativeSimpleListValue.class); + } + + @PlanningEntityCollectionProperty + List entityList; + + @PlanningEntityCollectionProperty + @ValueRangeProvider + List valueList; + + @PlanningScore + SimpleScore score; + + public TestdataDeclarativeSimpleListSolution() { + } + + public TestdataDeclarativeSimpleListSolution(List entityList, + List valueList) { + this.entityList = entityList; + this.valueList = valueList; + } + + public List getEntityList() { + return entityList; + } + + public void setEntityList( + List entityList) { + this.entityList = entityList; + } + + public List getValueList() { + return valueList; + } + + public void setValueList( + List valueList) { + this.valueList = valueList; + } + + public SimpleScore getScore() { + return score; + } + + public void setScore(SimpleScore score) { + this.score = score; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListValue.java new file mode 100644 index 00000000000..9867a8793be --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_list/TestdataDeclarativeSimpleListValue.java @@ -0,0 +1,103 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_list; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; +import ai.timefold.solver.core.api.domain.variable.PreviousElementShadowVariable; +import ai.timefold.solver.core.api.domain.variable.ShadowVariable; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; +import ai.timefold.solver.core.testdomain.TestdataObject; + +@PlanningEntity +public class TestdataDeclarativeSimpleListValue extends TestdataObject { + int position; + int duration; + + @PreviousElementShadowVariable(sourceVariableName = "values") + TestdataDeclarativeSimpleListValue previous; + + @InverseRelationShadowVariable(sourceVariableName = "values") + TestdataDeclarativeSimpleListEntity entity; + + @ShadowVariable(supplierName = "startTimeSupplier") + Integer startTime; + + @ShadowVariable(supplierName = "endTimeSupplier") + Integer endTime; + + public TestdataDeclarativeSimpleListValue() { + } + + public TestdataDeclarativeSimpleListValue(String code, int position, int duration) { + super(code); + this.position = position; + this.duration = duration; + } + + public int getPosition() { + return position; + } + + public void setPosition(int position) { + this.position = position; + } + + public int getDuration() { + return duration; + } + + public void setDuration(int duration) { + this.duration = duration; + } + + public TestdataDeclarativeSimpleListValue getPrevious() { + return previous; + } + + public void setPrevious(TestdataDeclarativeSimpleListValue previous) { + this.previous = previous; + } + + public TestdataDeclarativeSimpleListEntity getEntity() { + return entity; + } + + public void setEntity(TestdataDeclarativeSimpleListEntity entity) { + this.entity = entity; + } + + public Integer getStartTime() { + return startTime; + } + + public void setStartTime(Integer startTime) { + this.startTime = startTime; + } + + public Integer getEndTime() { + return endTime; + } + + public void setEndTime(Integer endTime) { + this.endTime = endTime; + } + + @ShadowSources({ "entity", "previous.endTime" }) + public Integer startTimeSupplier() { + if (entity == null) { + return null; + } + if (previous == null) { + return entity.startTime + Math.abs(position - entity.position); + } + return previous.endTime + Math.abs(position - previous.position); + } + + @ShadowSources("startTime") + public Integer endTimeSupplier() { + if (startTime == null) { + return null; + } + return startTime + duration; + } + +} From bad8b89ab52a0f623b4e23e9ee9174dba8477e9f Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Wed, 25 Jun 2025 18:28:26 -0400 Subject: [PATCH 3/7] perf: Update certain types of declaretive variables on afterVariableChanged --- .../descriptor/SolutionDescriptor.java | 5 +- .../AbstractVariableReferenceGraph.java | 5 + .../declarative/ChangedVariableNotifier.java | 13 +- .../DefaultShadowVariableSession.java | 37 +++++- .../DefaultShadowVariableSessionFactory.java | 99 ++++++++------- .../EmptyVariableReferenceGraph.java | 6 + .../variable/declarative/GraphStructure.java | 77 +++++++----- .../declarative/ParentVariableType.java | 59 +++++++-- .../declarative/RootVariableSource.java | 45 ++++--- ...rectionalParentVariableReferenceGraph.java | 119 ++++++++---------- .../declarative/VariableReferenceGraph.java | 9 ++ .../declarative/GraphStructureTest.java | 50 ++++++-- .../simple_chained/SimpleChainedTest.java | 50 ++++++++ ...ataChainedSimpleVarConstraintProvider.java | 20 +++ .../TestdataChainedSimpleVarEntity.java | 46 +++++++ .../TestdataChainedSimpleVarSolution.java | 75 +++++++++++ .../TestdataChainedSimpleVarValue.java | 59 +++++++++ 17 files changed, 584 insertions(+), 190 deletions(-) create mode 100644 core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarConstraintProvider.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarEntity.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarSolution.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarValue.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java index 67d6f42376c..f5fad68ff78 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java @@ -18,6 +18,7 @@ import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -1287,7 +1288,7 @@ public List> getAllShadowVariableDescriptors } public List> getDeclarativeShadowVariableDescriptors() { - var out = new ArrayList>(); + var out = new HashSet>(); for (var entityDescriptor : entityDescriptorMap.values()) { entityDescriptor.getShadowVariableDescriptors(); for (var shadowVariableDescriptor : entityDescriptor.getShadowVariableDescriptors()) { @@ -1296,7 +1297,7 @@ public List> getDeclarativeShadow } } } - return out; + return new ArrayList<>(out); } public ProblemSizeStatistics getProblemSizeStatistics(Solution_ solution) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java index f6787e9e377..769b656a8fc 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java @@ -131,6 +131,11 @@ public void afterVariableChanged(VariableMetaModel variableReference, O } } + @Override + public boolean shouldQueueAfterEvents() { + return false; + } + @Override public String toString() { var edgeList = new LinkedHashMap, List>>(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java index ba537ccc9c8..d1fa5bfd5ba 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ChangedVariableNotifier.java @@ -2,7 +2,6 @@ import java.util.function.BiConsumer; -import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; @@ -10,7 +9,7 @@ public record ChangedVariableNotifier(BiConsumer, Object> beforeVariableChanged, BiConsumer, Object> afterVariableChanged, - @Nullable ListVariableStateSupply listVariableStateSupply) { + @Nullable InnerScoreDirector innerScoreDirector) { private static final ChangedVariableNotifier EMPTY = new ChangedVariableNotifier<>((a, b) -> { }, @@ -24,18 +23,10 @@ public static ChangedVariableNotifier empty() { } public static ChangedVariableNotifier of(InnerScoreDirector scoreDirector) { - var listVariable = scoreDirector.getSolutionDescriptor().getListVariableDescriptor(); - if (listVariable == null) { - return new ChangedVariableNotifier<>( - scoreDirector::beforeVariableChanged, - scoreDirector::afterVariableChanged, - null); - } - var listVariableStateSupply = scoreDirector.getListVariableStateSupply(listVariable); return new ChangedVariableNotifier<>( scoreDirector::beforeVariableChanged, scoreDirector::afterVariableChanged, - listVariableStateSupply); + scoreDirector); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java index b49b7523227..3c0565d22ed 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java @@ -1,17 +1,38 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; +import java.util.ArrayList; +import java.util.List; + import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.supply.Supply; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; @NullMarked public final class DefaultShadowVariableSession implements Supply { final VariableReferenceGraph graph; + final boolean shouldQueueAfterEvents; + + record VariableChangedEvent(VariableMetaModel variableMetaModel, Object entity) { + } + + @Nullable + final List> variableChangedList; + + boolean isUpdating; public DefaultShadowVariableSession(VariableReferenceGraph graph) { this.graph = graph; + this.isUpdating = false; + if (graph.shouldQueueAfterEvents()) { + this.shouldQueueAfterEvents = true; + this.variableChangedList = new ArrayList<>(); + } else { + this.shouldQueueAfterEvents = false; + this.variableChangedList = null; + } } public void beforeVariableChanged(VariableDescriptor variableDescriptor, Object entity) { @@ -30,11 +51,23 @@ public void beforeVariableChanged(VariableMetaModel variableMet } public void afterVariableChanged(VariableMetaModel variableMetaModel, Object entity) { - graph.afterVariableChanged(variableMetaModel, - entity); + if (shouldQueueAfterEvents && !isUpdating) { + variableChangedList.add(new VariableChangedEvent<>(variableMetaModel, entity)); + } else { + graph.afterVariableChanged(variableMetaModel, + entity); + } } public void updateVariables() { + if (shouldQueueAfterEvents) { + isUpdating = true; + for (var variableChanged : variableChangedList) { + graph.afterVariableChanged(variableChanged.variableMetaModel, variableChanged.entity); + } + variableChangedList.clear(); + isUpdating = false; + } graph.updateChanged(); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index 5409e76bd61..5cffb1f562f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -11,19 +11,21 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.function.IntFunction; -import java.util.function.ToIntFunction; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; -import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.inverserelation.SingletonInverseVariableDemand; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; -import ai.timefold.solver.core.impl.util.CollectionUtils; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; +import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,17 +50,18 @@ public static VariableReferenceGraph buildGraph( SolutionDescriptor solutionDescriptor, VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { - var graphStructure = GraphStructure.determineGraphStructure(solutionDescriptor); - LOGGER.debug("Graph structure: {}", graphStructure); - return switch (graphStructure) { + var graphStructureAndDirection = GraphStructure.determineGraphStructure(solutionDescriptor, entities); + LOGGER.trace("Graph structure: {}", graphStructureAndDirection); + return switch (graphStructureAndDirection.structure()) { case EMPTY -> EmptyVariableReferenceGraph.INSTANCE; case SINGLE_DIRECTIONAL_PARENT -> { - var listVariableStateSupply = variableReferenceGraphBuilder.changedVariableNotifier.listVariableStateSupply(); - if (listVariableStateSupply == null) { + var scoreDirector = variableReferenceGraphBuilder.changedVariableNotifier.innerScoreDirector(); + if (scoreDirector == null) { yield buildArbitraryGraph(solutionDescriptor, variableReferenceGraphBuilder, entities, graphCreator); } yield buildSingleDirectionalParentGraph(solutionDescriptor, - variableReferenceGraphBuilder.changedVariableNotifier, listVariableStateSupply, + variableReferenceGraphBuilder.changedVariableNotifier, + graphStructureAndDirection, entities); } case NO_DYNAMIC_EDGES, ARBITRARY -> @@ -69,26 +72,32 @@ yield buildSingleDirectionalParentGraph(solutionDescriptor, private static VariableReferenceGraph buildSingleDirectionalParentGraph( SolutionDescriptor solutionDescriptor, ChangedVariableNotifier changedVariableNotifier, - ListVariableStateSupply listVariableStateSupply, + GraphStructure.GraphStructureAndDirection graphStructureAndDirection, Object[] entities) { var declarativeShadowVariables = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); + var sortedDeclarativeVariables = topologicallySortedDeclarativeShadowVariables(declarativeShadowVariables); + + Function successorFunction = + getSuccessorFunction(solutionDescriptor, Objects.requireNonNull(changedVariableNotifier.innerScoreDirector()), + Objects.requireNonNull(graphStructureAndDirection.parentMetaModel()), + Objects.requireNonNull(graphStructureAndDirection.direction())); + + return new SingleDirectionalParentVariableReferenceGraph<>(sortedDeclarativeVariables, successorFunction, + changedVariableNotifier, entities); + } + + private static @NonNull List> + topologicallySortedDeclarativeShadowVariables( + List> declarativeShadowVariables) { Map nameToIndex = new LinkedHashMap<>(); for (var declarativeShadowVariable : declarativeShadowVariables) { nameToIndex.put(declarativeShadowVariable.getVariableName(), nameToIndex.size()); } var graph = new DefaultTopologicalOrderGraph(nameToIndex.size()); - ParentVariableType parentVariableType = null; for (var declarativeShadowVariable : declarativeShadowVariables) { var toIndex = nameToIndex.get(declarativeShadowVariable.getVariableName()); var visited = new HashSet(); for (var source : declarativeShadowVariable.getSources()) { - switch (source.parentVariableType()) { - case PREVIOUS, NEXT -> { - parentVariableType = source.parentVariableType(); - } - default -> { - } - } var variableReferences = source.variableSourceReferences(); if (variableReferences.size() != 1) { // variableReferences is from directional variable @@ -109,44 +118,38 @@ private static VariableReferenceGraph buildSingleDirectio sortedDeclarativeVariables.sort(Comparator.> comparingInt( variable -> graph.getTopologicalOrder(nameToIndex.get(variable.getVariableName())).order()) .thenComparing(VariableDescriptor::getVariableName)); + return sortedDeclarativeVariables; + } - assert parentVariableType != null; - Function successorFunction; - Comparator comparator; - // Needed since otherwise values with the same index on different entities - // would be considered the same value. - Map entityToIndexMap = CollectionUtils.newIdentityHashMap(entities.length); - ToIntFunction indexFunction = value -> { - var out = listVariableStateSupply.getIndex(value); - if (out == null) { - return 0; - } - return out; - }; - - for (var entity : entities) { - entityToIndexMap.put(entity, entityToIndexMap.size()); - } - - switch (parentVariableType) { - case PREVIOUS -> { - successorFunction = listVariableStateSupply::getNextElement; - comparator = Comparator.comparingInt(indexFunction) - .thenComparing(entityToIndexMap::get); + private static @NonNull Function getSuccessorFunction( + SolutionDescriptor solutionDescriptor, InnerScoreDirector scoreDirector, + VariableMetaModel parentMetaModel, ParentVariableType parentVariableType) { + return switch (parentVariableType) { + case PREVIOUS -> + scoreDirector.getListVariableStateSupply(solutionDescriptor.getListVariableDescriptor())::getNextElement; + case NEXT -> + scoreDirector.getListVariableStateSupply(solutionDescriptor.getListVariableDescriptor())::getPreviousElement; + case CHAINED_PREVIOUS -> { + var entityDescriptor = solutionDescriptor.getEntityDescriptorStrict(parentMetaModel.entity().type()); + var variableDescriptor = entityDescriptor.getVariableDescriptor(parentMetaModel.name()); + var inverseSupply = + scoreDirector.getSupplyManager().demand(new SingletonInverseVariableDemand<>(variableDescriptor)); + yield inverseSupply::getInverseSingleton; } - case NEXT -> { - successorFunction = listVariableStateSupply::getPreviousElement; - comparator = Comparator.comparingInt(indexFunction) - .thenComparing(entityToIndexMap::get).reversed(); + case CHAINED_NEXT -> { + var entityDescriptor = solutionDescriptor.getEntityDescriptorStrict(parentMetaModel.entity().type()); + var inverseVariable = (InverseRelationShadowVariableDescriptor) entityDescriptor + .getShadowVariableDescriptor(parentMetaModel.name()); + var sourceVariable = inverseVariable.getSourceVariableDescriptorList().get(0); + var entityType = sourceVariable.getEntityDescriptor().getEntityClass(); + yield old -> entityType.isInstance(old) ? sourceVariable.getValue(old) : null; } default -> { throw new IllegalStateException( "Impossible state: expected parentVariableType to be previous or next but was %s." .formatted(parentVariableType)); } - } - return new SingleDirectionalParentVariableReferenceGraph<>(sortedDeclarativeVariables, successorFunction, comparator, - changedVariableNotifier, entities); + }; } private static VariableReferenceGraph buildArbitraryGraph( diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java index 6cd6b2db24d..870388c13ba 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java @@ -45,6 +45,12 @@ public void afterVariableChanged(VariableMetaModel variableReference, O // No need to do anything. } + @Override + public boolean shouldQueueAfterEvents() { + // Queuing involves more work, so don't + return false; + } + @Override public String toString() { return "{}"; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java index 3e5e1ea3138..bcf9c0527fc 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java @@ -3,7 +3,10 @@ import java.util.Arrays; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.util.MutableInt; +import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -14,13 +17,20 @@ public enum GraphStructure { EMPTY, /** - * A graph structure without dynamic edges. + * A graph structure without dynamic edges. The topological order + * of such a graph is fixed, since edges are neither added nor removed. */ NO_DYNAMIC_EDGES, /** * A graph structure where there is at most - * one directional parent for each graph node. + * one directional parent for each graph node, and + * no indirect parents. + * For example, when the only input variable from + * a different entity is previous. This allows us + * to use a successor function to find affected entities. + * Since there is at most a single parent node, such a graph + * cannot be looped. */ SINGLE_DIRECTIONAL_PARENT, @@ -31,57 +41,68 @@ public enum GraphStructure { private static final Logger LOGGER = LoggerFactory.getLogger(GraphStructure.class); - public static GraphStructure determineGraphStructure(SolutionDescriptor solutionDescriptor) { + public record GraphStructureAndDirection(GraphStructure structure, + @Nullable VariableMetaModel parentMetaModel, + @Nullable ParentVariableType direction) { + } + + public static GraphStructureAndDirection determineGraphStructure( + SolutionDescriptor solutionDescriptor, + Object... entities) { var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); if (declarativeShadowVariableDescriptors.isEmpty()) { - return EMPTY; + return new GraphStructureAndDirection(EMPTY, null, null); } var multipleDeclarativeEntityClasses = declarativeShadowVariableDescriptors.stream() .map(variable -> variable.getEntityDescriptor().getEntityClass()) .distinct().count() > 1; - if (multipleDeclarativeEntityClasses) { - // Inverse might become directional if it has - // declarative variables; ARBITRARY does optimize - // the graph to NO_DYNAMIC_EDGES if there are no variable listeners that - // add/remove edges. - return ARBITRARY; - } - var rootVariableSources = declarativeShadowVariableDescriptors.stream() .flatMap(descriptor -> Arrays.stream(descriptor.getSources())) .toList(); ParentVariableType directionalType = null; + VariableMetaModel parentMetaModel = null; for (var variableSource : rootVariableSources) { var parentVariableType = variableSource.parentVariableType(); - LOGGER.debug("{} has parentVariableType {}", variableSource, parentVariableType); + LOGGER.trace("{} has parentVariableType {}", variableSource, parentVariableType); switch (parentVariableType) { - case GROUP, INDIRECT_DIRECTIONAL, CHAINED_INVERSE -> { - // CHAINED_INVERSE is arbitrary, since we don't have - // the concept of "index" to help us identify topological - // order without creating a graph. - return GraphStructure.ARBITRARY; + case GROUP -> { + var groupMemberCount = new MutableInt(0); + for (var entity : entities) { + if (variableSource.rootEntity().isInstance(entity)) { + variableSource.valueEntityFunction().accept(entity, fromEntity -> { + groupMemberCount.increment(); + }); + } + } + if (groupMemberCount.intValue() != 0) { + return new GraphStructureAndDirection(ARBITRARY, null, null); + } + // The group variable is unused/always empty } - case NEXT, PREVIOUS -> { - if (directionalType == null) { + case INDIRECT, INVERSE -> { + return new GraphStructureAndDirection(ARBITRARY, null, null); + } + case NEXT, PREVIOUS, CHAINED_NEXT, CHAINED_PREVIOUS -> { + if (parentMetaModel == null) { + parentMetaModel = variableSource.variableSourceReferences().get(0).variableMetaModel(); directionalType = parentVariableType; - } else if (directionalType != parentVariableType) { - return GraphStructure.ARBITRARY; + } else if (!parentMetaModel.equals(variableSource.variableSourceReferences().get(0).variableMetaModel())) { + return new GraphStructureAndDirection(ARBITRARY, null, null); } } - case UNDIRECTIONAL -> { + case NO_PARENT -> { // Do nothing } - default -> { - throw new IllegalStateException("Unhandled case %s".formatted(variableSource.parentVariableType())); - } } } if (directionalType == null) { - return NO_DYNAMIC_EDGES; + return new GraphStructureAndDirection(NO_DYNAMIC_EDGES, null, null); } else { - return SINGLE_DIRECTIONAL_PARENT; + // Cannot use a single successor function if there are multiple entity classes + return multipleDeclarativeEntityClasses ? new GraphStructureAndDirection(ARBITRARY, null, null) + : new GraphStructureAndDirection(SINGLE_DIRECTIONAL_PARENT, parentMetaModel, directionalType); } } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java index 6d96ce5e0ce..10274c99e02 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java @@ -1,35 +1,72 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; public enum ParentVariableType { + /** + * A variable accessed from the root object. + */ + NO_PARENT(false, false), + + /** + * A variable accessed from the inverse. + */ + INVERSE(false, false), + /** * Next element variable accessed from the root object. */ - NEXT, + NEXT(true, false), /** * Previous element variable accessed from the root object. */ - PREVIOUS, + PREVIOUS(true, false), /** - * Previous element variable accessed from the root object in a chained model. + * PREVIOUS element variable accessed from the root object in a chained model. + * Note: it might be impossible, since a chained model uses separate classes + * for the anchor and values, and the anchor does not have a planning + * variable on it. */ - CHAINED_INVERSE, + CHAINED_PREVIOUS(true, false), /** - * Next/previous element variable accessed - * from a fact. + * Next element variable accessed from the root object in a chained model. */ - INDIRECT_DIRECTIONAL, + CHAINED_NEXT(true, false), + + /** + * A variable accessed indirectly from a fact or variable. + */ + INDIRECT(false, true), /** * Variables accessed from a group. */ - GROUP, + GROUP(false, true); + + /** + * True if the parent variable has a well-defined successor function. + * For instance, the successor of a variable with a previous variable + * is next. + */ + private final boolean isDirectional; /** - * Declarative variables, inverse variables, and - * genuine variables. + * True if the variable is accessed indirectly from a fact or + * a group. */ - UNDIRECTIONAL + private final boolean isIndirect; + + ParentVariableType(boolean isDirectional, boolean isIndirect) { + this.isDirectional = isDirectional; + this.isIndirect = isIndirect; + } + + public boolean isDirectional() { + return isDirectional; + } + + public boolean isIndirect() { + return isIndirect; + } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index c8a80f4bc00..fe578069ae6 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -5,6 +5,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Objects; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -181,6 +182,18 @@ public static RootVariableSource from( assertIsValidVariableReference(rootEntityClass, variablePath, variableSourceReference); } + if (parentVariableType != ParentVariableType.GROUP && variableSourceReferences.size() == 1) { + // No variables are accessed from the parent, so there no + // parent variable. + parentVariableType = ParentVariableType.NO_PARENT; + } + + if (!parentVariableType.isIndirect() && chainToVariable.size() > 2) { + // Child variable is accessed from a fact from the parent, + // so it is an indirect variable. + parentVariableType = ParentVariableType.INDIRECT; + } + return new RootVariableSource<>(rootEntityClass, listMemberAccessors, valueEntityFunction, @@ -333,37 +346,41 @@ private static ParentVariableType determineParentVariableType(List 1; var declaringClass = memberAccessor.getDeclaringClass(); var memberName = memberAccessor.getName(); + if (isIndirect) { + return ParentVariableType.INDIRECT; + } if (getAnnotation(declaringClass, memberName, PreviousElementShadowVariable.class) != null) { - if (isIndirect) { - return ParentVariableType.INDIRECT_DIRECTIONAL; - } return ParentVariableType.PREVIOUS; } if (getAnnotation(declaringClass, memberName, NextElementShadowVariable.class) != null) { - if (isIndirect) { - return ParentVariableType.INDIRECT_DIRECTIONAL; - } return ParentVariableType.NEXT; } if (getAnnotation(declaringClass, memberName, InverseRelationShadowVariable.class) != null) { // inverse can be both directional and undirectional; // it is directional in chained models, undirectional otherwise - var inverseVariable = getAnnotation(declaringClass, memberName, InverseRelationShadowVariable.class); + var inverseVariable = + Objects.requireNonNull(getAnnotation(declaringClass, memberName, InverseRelationShadowVariable.class)); var sourceClass = memberAccessor.getType(); var variableName = inverseVariable.sourceVariableName(); PlanningVariable sourcePlanningVariable = getAnnotation(sourceClass, variableName, PlanningVariable.class); if (sourcePlanningVariable == null) { - return ParentVariableType.UNDIRECTIONAL; + // Must have a PlanningListVariable instead + return ParentVariableType.INVERSE; } if (sourcePlanningVariable.graphType() == PlanningVariableGraphType.CHAINED) { - if (isIndirect) { - return ParentVariableType.INDIRECT_DIRECTIONAL; - } - return ParentVariableType.CHAINED_INVERSE; + return ParentVariableType.CHAINED_NEXT; + } else { + return ParentVariableType.INVERSE; + } + } + if (getAnnotation(declaringClass, memberName, PlanningVariable.class) != null) { + PlanningVariable planningVariable = + Objects.requireNonNull(getAnnotation(declaringClass, memberName, PlanningVariable.class)); + if (planningVariable.graphType() == PlanningVariableGraphType.CHAINED) { + return ParentVariableType.CHAINED_PREVIOUS; } - return ParentVariableType.UNDIRECTIONAL; } - return ParentVariableType.UNDIRECTIONAL; + return ParentVariableType.NO_PARENT; } private static T getAnnotation(Class declaringClass, String memberName, diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java index f4e838962eb..fca6a3a8f77 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java @@ -1,138 +1,118 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; +import java.util.Arrays; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.TreeSet; import java.util.function.Function; -import ai.timefold.solver.core.impl.util.CollectionUtils; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; public final class SingleDirectionalParentVariableReferenceGraph implements VariableReferenceGraph { - private final Map, Map>> variableReferenceToInstanceMap; private final Set> monitoredSourceVariableSet; - private final List> sortedVariableUpdaterInfoList; - private final Set changedSet; + private final VariableUpdaterInfo[] sortedVariableUpdaterInfos; private final Function successorFunction; - private final Comparator comparator; private final ChangedVariableNotifier changedVariableNotifier; - private final ShadowVariableLoopedVariableDescriptor loopedDescriptor; public SingleDirectionalParentVariableReferenceGraph( List> sortedDeclarativeShadowVariableDescriptors, - Function successorFunction, Comparator comparator, + Function successorFunction, ChangedVariableNotifier changedVariableNotifier, Object[] entities) { - variableReferenceToInstanceMap = CollectionUtils.newHashMap(sortedDeclarativeShadowVariableDescriptors.size()); - sortedVariableUpdaterInfoList = new ArrayList<>(sortedDeclarativeShadowVariableDescriptors.size()); + var entityClass = sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getEntityClass(); + // noinspection unchecked + sortedVariableUpdaterInfos = new VariableUpdaterInfo[sortedDeclarativeShadowVariableDescriptors.size()]; monitoredSourceVariableSet = new HashSet<>(); - changedSet = Collections.newSetFromMap(CollectionUtils.newIdentityHashMap(entities.length)); this.successorFunction = successorFunction; - this.comparator = comparator; this.changedVariableNotifier = changedVariableNotifier; - this.loopedDescriptor = + var shadowEntities = Arrays.stream(entities).filter(entityClass::isInstance).toArray(); + var loopedDescriptor = sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getShadowVariableLoopedDescriptor(); + var updaterIndex = 0; for (var variableDescriptor : sortedDeclarativeShadowVariableDescriptors) { var variableMetaModel = variableDescriptor.getVariableMetaModel(); - var objectMap = CollectionUtils.newIdentityHashMap(entities.length); var variableUpdaterInfo = new VariableUpdaterInfo<>( variableMetaModel, variableDescriptor, loopedDescriptor, variableDescriptor.getMemberAccessor(), variableDescriptor.getCalculator()::executeGetter); - sortedVariableUpdaterInfoList.add(variableUpdaterInfo); - variableReferenceToInstanceMap.put(variableMetaModel, CollectionUtils.newIdentityHashMap(entities.length)); + sortedVariableUpdaterInfos[updaterIndex++] = variableUpdaterInfo; for (var source : variableDescriptor.getSources()) { for (var sourceReference : source.variableSourceReferences()) { monitoredSourceVariableSet.add(sourceReference.variableMetaModel()); } } + } - for (var entity : entities) { - // No graph, so no graph id - if (variableDescriptor.getEntityDescriptor().getEntityClass().isInstance(entity)) { - objectMap.put(entity, new EntityVariablePair<>(entity, variableUpdaterInfo, -1)); - changedSet.add(entity); - } + for (var shadowEntity : shadowEntities) { + updateChanged(shadowEntity); + } + if (loopedDescriptor != null) { + for (var shadowEntity : shadowEntities) { + changedVariableNotifier.beforeVariableChanged().accept(loopedDescriptor, shadowEntity); + loopedDescriptor.setValue(shadowEntity, false); + changedVariableNotifier.afterVariableChanged().accept(loopedDescriptor, shadowEntity); } } } @Override public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { - return variableReferenceToInstanceMap.get(variableId).get(entity); + throw new IllegalStateException("Impossible state: cannot lookup in a %s graph." + .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); } @Override public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - markChanged(to); + throw new IllegalStateException("Impossible state: cannot modify an %s graph." + .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); } @Override public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - markChanged(to); + throw new IllegalStateException("Impossible state: cannot modify an %s graph." + .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); } @Override public void markChanged(@NonNull EntityVariablePair node) { - changedSet.add(node.entity()); + throw new IllegalStateException("Impossible state: cannot mark changed an %s graph." + .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); } @Override public void updateChanged() { - var changedSorted = new TreeSet<>(comparator); - var visited = Collections.newSetFromMap(new IdentityHashMap<>()); - changedSorted.addAll(changedSet); - for (var changed : changedSorted) { - if (visited.contains(changed)) { - continue; - } + // Do nothing; afterVariableChanged do the update + } - var current = changed; - while (current != null) { - visited.add(current); - var anyChanged = false; - if (loopedDescriptor != null) { - var oldValue = loopedDescriptor.getValue(current); - if (!Objects.equals(oldValue, false)) { - anyChanged = true; - changedVariableNotifier.beforeVariableChanged().accept(loopedDescriptor, current); - loopedDescriptor.setValue(current, false); - changedVariableNotifier.afterVariableChanged().accept(loopedDescriptor, current); - } - } - for (var updater : sortedVariableUpdaterInfoList) { - var oldValue = updater.memberAccessor().executeGetter(current); - var newValue = updater.calculator().apply(current); - if (!Objects.equals(oldValue, newValue)) { - anyChanged = true; - changedVariableNotifier.beforeVariableChanged().accept(updater.variableDescriptor(), current); - updater.memberAccessor().executeSetter(current, newValue); - changedVariableNotifier.afterVariableChanged().accept(updater.variableDescriptor(), current); - } - } - if (anyChanged) { - current = successorFunction.apply(current); - } else { - current = null; + private void updateChanged(Object entity) { + var current = entity; + while (current != null) { + var anyChanged = false; + for (var updater : sortedVariableUpdaterInfos) { + var oldValue = updater.memberAccessor().executeGetter(current); + var newValue = updater.calculator().apply(current); + if (!Objects.equals(oldValue, newValue)) { + anyChanged = true; + changedVariableNotifier.beforeVariableChanged().accept(updater.variableDescriptor(), current); + updater.memberAccessor().executeSetter(current, newValue); + changedVariableNotifier.afterVariableChanged().accept(updater.variableDescriptor(), current); } } + if (anyChanged) { + current = successorFunction.apply(current); + } else { + current = null; + } } - changedSet.clear(); } @Override @@ -143,7 +123,12 @@ public void beforeVariableChanged(VariableMetaModel variableReference, @Override public void afterVariableChanged(VariableMetaModel variableReference, Object entity) { if (monitoredSourceVariableSet.contains(variableReference)) { - changedSet.add(entity); + updateChanged(entity); } } + + @Override + public boolean shouldQueueAfterEvents() { + return true; + } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java index fdd88c4e46c..cc50dd3be2a 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java @@ -24,4 +24,13 @@ public sealed interface VariableReferenceGraph void afterVariableChanged(VariableMetaModel variableReference, Object entity); + /** + * If afterVariableChanged events should be queued instead of executed immediately. + * Needed if variables are updated in afterVariableChanged instead of updateChanged + * to guarantee the input variables are consistent with one another. + * + * @return true iff the afterVariableChanged events should be queued. + */ + boolean shouldQueueAfterEvents(); + } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java index f29b7c87a8f..de3be10a84e 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java @@ -1,11 +1,20 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; +import static ai.timefold.solver.core.impl.domain.variable.declarative.GraphStructure.ARBITRARY; +import static ai.timefold.solver.core.impl.domain.variable.declarative.GraphStructure.EMPTY; +import static ai.timefold.solver.core.impl.domain.variable.declarative.GraphStructure.NO_DYNAMIC_EDGES; +import static ai.timefold.solver.core.impl.domain.variable.declarative.GraphStructure.SINGLE_DIRECTIONAL_PARENT; import static org.assertj.core.api.Assertions.assertThat; +import java.util.Collections; +import java.util.List; + import ai.timefold.solver.core.testdomain.TestdataSolution; import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentSolution; +import ai.timefold.solver.core.testdomain.declarative.concurrent.TestdataConcurrentValue; import ai.timefold.solver.core.testdomain.declarative.extended.TestdataDeclarativeExtendedSolution; import ai.timefold.solver.core.testdomain.declarative.follower.TestdataFollowerSolution; +import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarSolution; import ai.timefold.solver.core.testdomain.declarative.simple_list.TestdataDeclarativeSimpleListSolution; import org.junit.jupiter.api.Test; @@ -15,34 +24,61 @@ public class GraphStructureTest { public void simpleListStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataDeclarativeSimpleListSolution.buildSolutionDescriptor())) - .isEqualTo(GraphStructure.SINGLE_DIRECTIONAL_PARENT); + .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) + .hasFieldOrPropertyWithValue("direction", ParentVariableType.PREVIOUS); + } + + @Test + public void simpleChainedStructure() { + assertThat(GraphStructure.determineGraphStructure( + TestdataChainedSimpleVarSolution.buildSolutionDescriptor())) + .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) + .hasFieldOrPropertyWithValue("direction", ParentVariableType.CHAINED_NEXT); } @Test public void extendedSimpleListStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataDeclarativeExtendedSolution.buildSolutionDescriptor())) - .isEqualTo(GraphStructure.SINGLE_DIRECTIONAL_PARENT); + .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) + .hasFieldOrPropertyWithValue("direction", ParentVariableType.PREVIOUS); + } + + @Test + public void concurrentValuesStructureWithoutGroups() { + var value1 = new TestdataConcurrentValue("v1"); + var value2 = new TestdataConcurrentValue("v2"); + value2.setConcurrentValueGroup(Collections.emptyList()); + assertThat(GraphStructure.determineGraphStructure( + TestdataConcurrentSolution.buildSolutionDescriptor(), + value1, value2)) + .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) + .hasFieldOrPropertyWithValue("direction", ParentVariableType.PREVIOUS); } @Test - public void concurrentValuesStructure() { + public void concurrentValuesStructureWithGroups() { + var value1 = new TestdataConcurrentValue("v1"); + var value2 = new TestdataConcurrentValue("v2"); + var group = List.of(value1, value2); + value2.setConcurrentValueGroup(group); assertThat(GraphStructure.determineGraphStructure( - TestdataConcurrentSolution.buildSolutionDescriptor())) - .isEqualTo(GraphStructure.ARBITRARY); + TestdataConcurrentSolution.buildSolutionDescriptor(), + value1, value2)) + .hasFieldOrPropertyWithValue("structure", ARBITRARY); } @Test public void followerStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataFollowerSolution.buildSolutionDescriptor())) - .isEqualTo(GraphStructure.NO_DYNAMIC_EDGES); + .hasFieldOrPropertyWithValue("structure", NO_DYNAMIC_EDGES); } @Test public void emptyStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataSolution.buildSolutionDescriptor())) - .isEqualTo(GraphStructure.EMPTY); + .hasFieldOrPropertyWithValue("structure", EMPTY); } } diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java new file mode 100644 index 00000000000..c301ecb39ac --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java @@ -0,0 +1,50 @@ +package ai.timefold.solver.core.preview.api.variable.declarative.simple_chained; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import java.util.List; + +import ai.timefold.solver.core.api.solver.SolverFactory; +import ai.timefold.solver.core.config.solver.EnvironmentMode; +import ai.timefold.solver.core.config.solver.PreviewFeature; +import ai.timefold.solver.core.config.solver.SolverConfig; +import ai.timefold.solver.core.config.solver.termination.TerminationConfig; +import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarConstraintProvider; +import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarEntity; +import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarSolution; +import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarValue; + +import org.junit.jupiter.api.Test; + +public class SimpleChainedTest { + @Test + public void simpleChained() { + var solverConfig = new SolverConfig() + .withSolutionClass(TestdataChainedSimpleVarSolution.class) + .withEntityClasses(TestdataChainedSimpleVarEntity.class, TestdataChainedSimpleVarValue.class) + .withConstraintProviderClass(TestdataChainedSimpleVarConstraintProvider.class) + .withPreviewFeature(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES) + .withEnvironmentMode(EnvironmentMode.FULL_ASSERT) + .withTerminationConfig(new TerminationConfig().withBestScoreLimit("-48")); + + var entityList = List.of(new TestdataChainedSimpleVarEntity("e1", Duration.ofDays(1)), + new TestdataChainedSimpleVarEntity("e2", Duration.ofDays(2)), + new TestdataChainedSimpleVarEntity("e3", Duration.ofDays(3))); + var valueList = List.of( + new TestdataChainedSimpleVarValue("a1", Duration.ofDays(1)), + new TestdataChainedSimpleVarValue("a2", Duration.ofDays(2)), + new TestdataChainedSimpleVarValue("a3", Duration.ofDays(3))); + var problem = new TestdataChainedSimpleVarSolution(entityList, valueList); + var solverFactory = SolverFactory. create(solverConfig); + var solver = solverFactory.buildSolver(); + var solution = solver.solve(problem); + + // In the optimal solution, each value is paired with the opposite entity + // i.e. v1 -> e3, v2 -> e2, and v3 -> e1. + var values = solution.getValues(); + for (var value : values) { + assertThat(value.getCumulativeDurationInDays()).isEqualTo(4); + } + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarConstraintProvider.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarConstraintProvider.java new file mode 100644 index 00000000000..5bbafb72194 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarConstraintProvider.java @@ -0,0 +1,20 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_chained; + +import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; +import ai.timefold.solver.core.api.score.stream.Constraint; +import ai.timefold.solver.core.api.score.stream.ConstraintFactory; +import ai.timefold.solver.core.api.score.stream.ConstraintProvider; + +import org.jspecify.annotations.NonNull; + +public class TestdataChainedSimpleVarConstraintProvider implements ConstraintProvider { + @Override + public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory constraintFactory) { + return new Constraint[] { + constraintFactory.forEach(TestdataChainedSimpleVarValue.class) + .filter(entity -> !(entity instanceof TestdataChainedSimpleVarEntity)) + .penalize(SimpleScore.ONE, value -> value.cumulativeDurationInDays * value.cumulativeDurationInDays) + .asConstraint("Minimize cumulative duration in days product") + }; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarEntity.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarEntity.java new file mode 100644 index 00000000000..3f00cf8dea9 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarEntity.java @@ -0,0 +1,46 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_chained; + +import java.time.Duration; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.PlanningVariable; +import ai.timefold.solver.core.api.domain.variable.PlanningVariableGraphType; + +@PlanningEntity +public class TestdataChainedSimpleVarEntity extends TestdataChainedSimpleVarValue { + String id; + + @PlanningVariable(graphType = PlanningVariableGraphType.CHAINED) + TestdataChainedSimpleVarValue previous; + + public TestdataChainedSimpleVarEntity() { + } + + public TestdataChainedSimpleVarEntity(String id, Duration duration) { + super(id, duration); + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public TestdataChainedSimpleVarValue getPrevious() { + return previous; + } + + public void setPrevious(TestdataChainedSimpleVarValue previous) { + this.previous = previous; + } + + @Override + public String toString() { + return "TestdataChainedSimpleVarEntity{" + + "id=" + id + + ", previous=" + previous + + '}'; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarSolution.java new file mode 100644 index 00000000000..a0d048b7baf --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarSolution.java @@ -0,0 +1,75 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_chained; + +import java.util.EnumSet; +import java.util.List; + +import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty; +import ai.timefold.solver.core.api.domain.solution.PlanningScore; +import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; +import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore; +import ai.timefold.solver.core.config.solver.PreviewFeature; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; + +@PlanningSolution +public class TestdataChainedSimpleVarSolution { + + public static SolutionDescriptor buildSolutionDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataChainedSimpleVarSolution.class, TestdataChainedSimpleVarEntity.class, + TestdataChainedSimpleVarValue.class); + } + + @PlanningEntityCollectionProperty + List entities; + + @PlanningEntityCollectionProperty + @ValueRangeProvider + List values; + + @PlanningScore + SimpleScore score; + + public TestdataChainedSimpleVarSolution() { + } + + public TestdataChainedSimpleVarSolution(List entities, + List values) { + this.values = values; + this.entities = entities; + } + + public List getValues() { + return values; + } + + public void setValues(List values) { + this.values = values; + } + + public List getEntities() { + return entities; + } + + public void setEntities( + List entities) { + this.entities = entities; + } + + public SimpleScore getScore() { + return score; + } + + public void setScore(SimpleScore score) { + this.score = score; + } + + @Override + public String toString() { + return "TestdataChainedSimpleVarSolution{" + + "entities=" + entities + + ", values=" + values + + ", score=" + score + + '}'; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarValue.java new file mode 100644 index 00000000000..cdc8d2c6630 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/simple_chained/TestdataChainedSimpleVarValue.java @@ -0,0 +1,59 @@ +package ai.timefold.solver.core.testdomain.declarative.simple_chained; + +import java.time.Duration; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.InverseRelationShadowVariable; +import ai.timefold.solver.core.api.domain.variable.ShadowVariable; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; + +@PlanningEntity +public class TestdataChainedSimpleVarValue { + String id; + + @InverseRelationShadowVariable(sourceVariableName = "previous") + TestdataChainedSimpleVarEntity next; + + Duration duration; + + @ShadowVariable(supplierName = "updateCumulativeDurationInDays") + int cumulativeDurationInDays; + + public TestdataChainedSimpleVarValue() { + } + + public TestdataChainedSimpleVarValue(String id, Duration duration) { + this.id = id; + this.duration = duration; + this.cumulativeDurationInDays = (int) duration.toDays(); + } + + public TestdataChainedSimpleVarEntity getNext() { + return next; + } + + public void setNext(TestdataChainedSimpleVarEntity next) { + this.next = next; + } + + public Duration getDuration() { + return duration; + } + + public void setDuration(Duration duration) { + this.duration = duration; + } + + public int getCumulativeDurationInDays() { + return cumulativeDurationInDays; + } + + @ShadowSources("next.cumulativeDurationInDays") + public int updateCumulativeDurationInDays() { + if (next == null) { + return (int) duration.toDays(); + } else { + return next.getCumulativeDurationInDays() + (int) duration.toDays(); + } + } +} From 3d26361b602311202f96a1a46c21f3cffd08212d Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Thu, 26 Jun 2025 09:10:34 -0400 Subject: [PATCH 4/7] chore: review comments, refactoring, do not update entities that are not instances of the declarative class --- .../AbstractVariableReferenceGraph.java | 18 +++---- .../DefaultShadowVariableSession.java | 37 +------------ .../DefaultShadowVariableSessionFactory.java | 8 --- .../EmptyVariableReferenceGraph.java | 29 ----------- .../variable/declarative/GraphStructure.java | 10 ++-- .../declarative/ParentVariableType.java | 26 ++++++---- .../declarative/RootVariableSource.java | 6 +-- ...rectionalParentVariableReferenceGraph.java | 52 ++++++------------- .../declarative/VariableReferenceGraph.java | 21 -------- .../VariableReferenceGraphBuilder.java | 8 +-- 10 files changed, 51 insertions(+), 164 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java index 769b656a8fc..993340928e9 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java @@ -21,8 +21,8 @@ public sealed abstract class AbstractVariableReferenceGraph> instanceList; protected final Map, Map>> variableReferenceToInstanceMap; - protected final Map, List, Object>>> variableReferenceToBeforeProcessor; - protected final Map, List, Object>>> variableReferenceToAfterProcessor; + protected final Map, List, Object>>> variableReferenceToBeforeProcessor; + protected final Map, List, Object>>> variableReferenceToAfterProcessor; // These structures are mutable. protected final DynamicIntArray[] edgeCount; @@ -63,7 +63,6 @@ public AbstractVariableReferenceGraph(VariableReferenceGraphBuilder o abstract protected ChangeSet_ createChangeSet(int instanceCount); - @Override public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { var map = variableReferenceToInstanceMap.get(variableId); if (map == null) { @@ -72,7 +71,6 @@ public AbstractVariableReferenceGraph(VariableReferenceGraphBuilder o return map.get(entity); } - @Override public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { var fromNodeId = from.graphNodeId(); var toNodeId = to.graphNodeId(); @@ -88,7 +86,6 @@ public void addEdge(@NonNull EntityVariablePair from, @NonNull Entity markChanged(to); } - @Override public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { var fromNodeId = from.graphNodeId(); var toNodeId = to.graphNodeId(); @@ -104,6 +101,8 @@ public void removeEdge(@NonNull EntityVariablePair from, @NonNull Ent markChanged(to); } + abstract void markChanged(EntityVariablePair changed); + @Override public void beforeVariableChanged(VariableMetaModel variableReference, Object entity) { if (variableReference.entity().type().isInstance(entity)) { @@ -111,10 +110,12 @@ public void beforeVariableChanged(VariableMetaModel variableReference, } } - private void processEntity(List, Object>> processorList, Object entity) { + private void processEntity(List, Object>> processorList, + Object entity) { var processorCount = processorList.size(); // Avoid creation of iterators on the hot path. // The short-lived instances were observed to cause considerable GC pressure. + //noinspection ForLoopReplaceableByForEach for (int i = 0; i < processorCount; i++) { processorList.get(i).accept(this, entity); } @@ -131,11 +132,6 @@ public void afterVariableChanged(VariableMetaModel variableReference, O } } - @Override - public boolean shouldQueueAfterEvents() { - return false; - } - @Override public String toString() { var edgeList = new LinkedHashMap, List>>(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java index 3c0565d22ed..b49b7523227 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java @@ -1,38 +1,17 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; -import java.util.ArrayList; -import java.util.List; - import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.supply.Supply; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.Nullable; @NullMarked public final class DefaultShadowVariableSession implements Supply { final VariableReferenceGraph graph; - final boolean shouldQueueAfterEvents; - - record VariableChangedEvent(VariableMetaModel variableMetaModel, Object entity) { - } - - @Nullable - final List> variableChangedList; - - boolean isUpdating; public DefaultShadowVariableSession(VariableReferenceGraph graph) { this.graph = graph; - this.isUpdating = false; - if (graph.shouldQueueAfterEvents()) { - this.shouldQueueAfterEvents = true; - this.variableChangedList = new ArrayList<>(); - } else { - this.shouldQueueAfterEvents = false; - this.variableChangedList = null; - } } public void beforeVariableChanged(VariableDescriptor variableDescriptor, Object entity) { @@ -51,23 +30,11 @@ public void beforeVariableChanged(VariableMetaModel variableMet } public void afterVariableChanged(VariableMetaModel variableMetaModel, Object entity) { - if (shouldQueueAfterEvents && !isUpdating) { - variableChangedList.add(new VariableChangedEvent<>(variableMetaModel, entity)); - } else { - graph.afterVariableChanged(variableMetaModel, - entity); - } + graph.afterVariableChanged(variableMetaModel, + entity); } public void updateVariables() { - if (shouldQueueAfterEvents) { - isUpdating = true; - for (var variableChanged : variableChangedList) { - graph.afterVariableChanged(variableChanged.variableMetaModel, variableChanged.entity); - } - variableChangedList.clear(); - isUpdating = false; - } graph.updateChanged(); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index 5cffb1f562f..1be0a11de14 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -19,7 +19,6 @@ import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor; -import ai.timefold.solver.core.impl.domain.variable.inverserelation.SingletonInverseVariableDemand; import ai.timefold.solver.core.impl.score.director.InnerScoreDirector; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; @@ -129,13 +128,6 @@ private static VariableReferenceGraph buildSingleDirectio scoreDirector.getListVariableStateSupply(solutionDescriptor.getListVariableDescriptor())::getNextElement; case NEXT -> scoreDirector.getListVariableStateSupply(solutionDescriptor.getListVariableDescriptor())::getPreviousElement; - case CHAINED_PREVIOUS -> { - var entityDescriptor = solutionDescriptor.getEntityDescriptorStrict(parentMetaModel.entity().type()); - var variableDescriptor = entityDescriptor.getVariableDescriptor(parentMetaModel.name()); - var inverseSupply = - scoreDirector.getSupplyManager().demand(new SingletonInverseVariableDemand<>(variableDescriptor)); - yield inverseSupply::getInverseSingleton; - } case CHAINED_NEXT -> { var entityDescriptor = solutionDescriptor.getEntityDescriptorStrict(parentMetaModel.entity().type()); var inverseVariable = (InverseRelationShadowVariableDescriptor) entityDescriptor diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java index 870388c13ba..23791b736c5 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java @@ -2,34 +2,11 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -import org.jspecify.annotations.NonNull; -import org.jspecify.annotations.Nullable; - final class EmptyVariableReferenceGraph implements VariableReferenceGraph { @SuppressWarnings("rawtypes") public static final EmptyVariableReferenceGraph INSTANCE = new EmptyVariableReferenceGraph<>(); - @Override - public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { - return null; - } - - @Override - public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - throw new IllegalStateException("Impossible state: cannot modify an empty graph."); - } - - @Override - public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - throw new IllegalStateException("Impossible state: cannot modify an empty graph."); - } - - @Override - public void markChanged(@NonNull EntityVariablePair node) { - throw new IllegalStateException("Impossible state: cannot modify an empty graph."); - } - @Override public void updateChanged() { // No need to do anything. @@ -45,12 +22,6 @@ public void afterVariableChanged(VariableMetaModel variableReference, O // No need to do anything. } - @Override - public boolean shouldQueueAfterEvents() { - // Queuing involves more work, so don't - return false; - } - @Override public String toString() { return "{}"; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java index bcf9c0527fc..6e3d0eae6d8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructure.java @@ -6,10 +6,12 @@ import ai.timefold.solver.core.impl.util.MutableInt; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; +import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@NullMarked public enum GraphStructure { /** * A graph structure that only accepts the empty graph. @@ -70,9 +72,7 @@ public static GraphStructureAndDirection determineGraphStructure( var groupMemberCount = new MutableInt(0); for (var entity : entities) { if (variableSource.rootEntity().isInstance(entity)) { - variableSource.valueEntityFunction().accept(entity, fromEntity -> { - groupMemberCount.increment(); - }); + variableSource.valueEntityFunction().accept(entity, fromEntity -> groupMemberCount.increment()); } } if (groupMemberCount.intValue() != 0) { @@ -80,10 +80,10 @@ public static GraphStructureAndDirection determineGraphStructure( } // The group variable is unused/always empty } - case INDIRECT, INVERSE -> { + case INDIRECT, INVERSE, VARIABLE -> { return new GraphStructureAndDirection(ARBITRARY, null, null); } - case NEXT, PREVIOUS, CHAINED_NEXT, CHAINED_PREVIOUS -> { + case NEXT, PREVIOUS, CHAINED_NEXT -> { if (parentMetaModel == null) { parentMetaModel = variableSource.variableSourceReferences().get(0).variableMetaModel(); directionalType = parentVariableType; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java index 10274c99e02..61e6472bc66 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/ParentVariableType.java @@ -7,30 +7,36 @@ public enum ParentVariableType { NO_PARENT(false, false), /** - * A variable accessed from the inverse. + * A variable accessed from another variable. + */ + VARIABLE(false, false), + + /** + * Variable on the inverse accessed from the root object. */ INVERSE(false, false), /** - * Next element variable accessed from the root object. + * Variable on a next element variable accessed from the root object. */ NEXT(true, false), /** - * Previous element variable accessed from the root object. + * Variable on a previous element variable accessed from the root object. */ PREVIOUS(true, false), - /** - * PREVIOUS element variable accessed from the root object in a chained model. - * Note: it might be impossible, since a chained model uses separate classes - * for the anchor and values, and the anchor does not have a planning - * variable on it. + /* + * Previous element variable accessed from the root object in a chained model + * (i.e. PlanningVariable(graphType = PlanningVariableGraphType.CHAINED)) + * is not included, since it would require a source path to accept properties + * that are only included on subclasses of the property's type (since the + * value of a chained value is either an entity (which has the property) or + * an anchor (which does not have the property)). */ - CHAINED_PREVIOUS(true, false), /** - * Next element variable accessed from the root object in a chained model. + * Variable on a next element variable accessed from the root object in a chained model. */ CHAINED_NEXT(true, false), diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java index fe578069ae6..cfbad371459 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/RootVariableSource.java @@ -374,11 +374,7 @@ private static ParentVariableType determineParentVariableType(List implements VariableReferenceGraph { private final Set> monitoredSourceVariableSet; private final VariableUpdaterInfo[] sortedVariableUpdaterInfos; private final Function successorFunction; private final ChangedVariableNotifier changedVariableNotifier; + private final List changedEntities; + private final Class monitoredEntityClass; + private boolean isUpdating; public SingleDirectionalParentVariableReferenceGraph( List> sortedDeclarativeShadowVariableDescriptors, Function successorFunction, ChangedVariableNotifier changedVariableNotifier, Object[] entities) { - var entityClass = sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getEntityClass(); + monitoredEntityClass = sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getEntityClass(); // noinspection unchecked sortedVariableUpdaterInfos = new VariableUpdaterInfo[sortedDeclarativeShadowVariableDescriptors.size()]; monitoredSourceVariableSet = new HashSet<>(); + changedEntities = new ArrayList<>(); + isUpdating = false; this.successorFunction = successorFunction; this.changedVariableNotifier = changedVariableNotifier; - var shadowEntities = Arrays.stream(entities).filter(entityClass::isInstance).toArray(); + var shadowEntities = Arrays.stream(entities).filter(monitoredEntityClass::isInstance).toArray(); var loopedDescriptor = sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getShadowVariableLoopedDescriptor(); @@ -64,33 +67,14 @@ public SingleDirectionalParentVariableReferenceGraph( } } - @Override - public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { - throw new IllegalStateException("Impossible state: cannot lookup in a %s graph." - .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); - } - - @Override - public void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - throw new IllegalStateException("Impossible state: cannot modify an %s graph." - .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); - } - - @Override - public void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to) { - throw new IllegalStateException("Impossible state: cannot modify an %s graph." - .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); - } - - @Override - public void markChanged(@NonNull EntityVariablePair node) { - throw new IllegalStateException("Impossible state: cannot mark changed an %s graph." - .formatted(SingleDirectionalParentVariableReferenceGraph.class.getSimpleName())); - } - @Override public void updateChanged() { - // Do nothing; afterVariableChanged do the update + isUpdating = true; + for (var changedEntity : changedEntities) { + updateChanged(changedEntity); + } + isUpdating = false; + changedEntities.clear(); } private void updateChanged(Object entity) { @@ -122,13 +106,9 @@ public void beforeVariableChanged(VariableMetaModel variableReference, @Override public void afterVariableChanged(VariableMetaModel variableReference, Object entity) { - if (monitoredSourceVariableSet.contains(variableReference)) { - updateChanged(entity); + if (!isUpdating && monitoredSourceVariableSet.contains(variableReference) && monitoredEntityClass.isInstance(entity)) { + changedEntities.add(entity); } } - @Override - public boolean shouldQueueAfterEvents() { - return true; - } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java index cc50dd3be2a..a2446de8dfa 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java @@ -2,35 +2,14 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -import org.jspecify.annotations.NonNull; -import org.jspecify.annotations.Nullable; - public sealed interface VariableReferenceGraph permits AbstractVariableReferenceGraph, DefaultVariableReferenceGraph, EmptyVariableReferenceGraph, FixedVariableReferenceGraph, SingleDirectionalParentVariableReferenceGraph { - @Nullable - EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity); - - void addEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to); - - void removeEdge(@NonNull EntityVariablePair from, @NonNull EntityVariablePair to); - - void markChanged(@NonNull EntityVariablePair node); - void updateChanged(); void beforeVariableChanged(VariableMetaModel variableReference, Object entity); void afterVariableChanged(VariableMetaModel variableReference, Object entity); - /** - * If afterVariableChanged events should be queued instead of executed immediately. - * Needed if variables are updated in afterVariableChanged instead of updateChanged - * to guarantee the input variables are consistent with one another. - * - * @return true iff the afterVariableChanged events should be queued. - */ - boolean shouldQueueAfterEvents(); - } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java index 8ec2ed73df7..2eccc71c382 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java @@ -16,8 +16,8 @@ public final class VariableReferenceGraphBuilder { final ChangedVariableNotifier changedVariableNotifier; - final Map, List, Object>>> variableReferenceToBeforeProcessor; - final Map, List, Object>>> variableReferenceToAfterProcessor; + final Map, List, Object>>> variableReferenceToBeforeProcessor; + final Map, List, Object>>> variableReferenceToAfterProcessor; final List> instanceList; final Map, List>> fixedEdges; final Map, Map>> variableReferenceToInstanceMap; @@ -57,14 +57,14 @@ public void addFixedEdge(@NonNull EntityVariablePair from, @NonNull E } public void addBeforeProcessor(GraphChangeType graphChangeType, VariableMetaModel variableId, - BiConsumer, Object> consumer) { + BiConsumer, Object> consumer) { isGraphFixed &= !graphChangeType.affectsGraph(); variableReferenceToBeforeProcessor.computeIfAbsent(variableId, k -> new ArrayList<>()) .add(consumer); } public void addAfterProcessor(GraphChangeType graphChangeType, VariableMetaModel variableId, - BiConsumer, Object> consumer) { + BiConsumer, Object> consumer) { isGraphFixed &= !graphChangeType.affectsGraph(); variableReferenceToAfterProcessor.computeIfAbsent(variableId, k -> new ArrayList<>()) .add(consumer); From 685575e8e3e52f18e954d84d6a7dff98e577e18e Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Thu, 26 Jun 2025 09:55:01 -0400 Subject: [PATCH 5/7] chore: Sonar --- .../variable/ShadowVariableUpdateHelper.java | 2 +- .../AbstractVariableReferenceGraph.java | 8 +++---- .../DefaultShadowVariableSession.java | 4 ++-- .../DefaultShadowVariableSessionFactory.java | 22 +++++++++---------- .../DefaultVariableReferenceGraph.java | 2 +- .../EmptyVariableReferenceGraph.java | 2 +- .../FixedVariableReferenceGraph.java | 2 +- ...rectionalParentVariableReferenceGraph.java | 10 ++++----- .../declarative/VariableReferenceGraph.java | 2 +- .../VariableReferenceGraphBuilder.java | 2 +- .../declarative/GraphStructureTest.java | 16 +++++++------- .../simple_chained/SimpleChainedTest.java | 4 ++-- .../simple_list/SimpleListTest.java | 4 ++-- 13 files changed, 39 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java index f2c8d6e72e6..61aebc85623 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java @@ -133,7 +133,7 @@ public void updateShadowVariables(Class solutionClass, } private record InternalShadowVariableSession(SolutionDescriptor solutionDescriptor, - VariableReferenceGraph graph) { + VariableReferenceGraph graph) { public static InternalShadowVariableSession build( SolutionDescriptor solutionDescriptor, VariableReferenceGraphBuilder graph, diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java index 993340928e9..e604c2b38ef 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AbstractVariableReferenceGraph.java @@ -16,7 +16,7 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; -public sealed abstract class AbstractVariableReferenceGraph implements VariableReferenceGraph +public abstract sealed class AbstractVariableReferenceGraph implements VariableReferenceGraph permits DefaultVariableReferenceGraph, FixedVariableReferenceGraph { // These structures are immutable. protected final List> instanceList; @@ -29,7 +29,7 @@ public sealed abstract class AbstractVariableReferenceGraph outerGraph, + AbstractVariableReferenceGraph(VariableReferenceGraphBuilder outerGraph, IntFunction graphCreator) { instanceList = List.copyOf(outerGraph.instanceList); var instanceCount = instanceList.size(); @@ -61,7 +61,7 @@ public AbstractVariableReferenceGraph(VariableReferenceGraphBuilder o } } - abstract protected ChangeSet_ createChangeSet(int instanceCount); + protected abstract ChangeSet_ createChangeSet(int instanceCount); public @Nullable EntityVariablePair lookupOrNull(VariableMetaModel variableId, Object entity) { var map = variableReferenceToInstanceMap.get(variableId); @@ -110,12 +110,12 @@ public void beforeVariableChanged(VariableMetaModel variableReference, } } + @SuppressWarnings("ForLoopReplaceableByForEach") private void processEntity(List, Object>> processorList, Object entity) { var processorCount = processorList.size(); // Avoid creation of iterators on the hot path. // The short-lived instances were observed to cause considerable GC pressure. - //noinspection ForLoopReplaceableByForEach for (int i = 0; i < processorCount; i++) { processorList.get(i).accept(this, entity); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java index b49b7523227..32961759be4 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java @@ -8,9 +8,9 @@ @NullMarked public final class DefaultShadowVariableSession implements Supply { - final VariableReferenceGraph graph; + final VariableReferenceGraph graph; - public DefaultShadowVariableSession(VariableReferenceGraph graph) { + public DefaultShadowVariableSession(VariableReferenceGraph graph) { this.graph = graph; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index 1be0a11de14..2f0d25c97c8 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -13,8 +13,8 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.Function; import java.util.function.IntFunction; +import java.util.function.UnaryOperator; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; @@ -30,7 +30,7 @@ @NullMarked public class DefaultShadowVariableSessionFactory { - private final static Logger LOGGER = LoggerFactory.getLogger(DefaultShadowVariableSessionFactory.class); + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultShadowVariableSessionFactory.class); private final SolutionDescriptor solutionDescriptor; private final InnerScoreDirector scoreDirector; private final IntFunction graphCreator; @@ -45,7 +45,7 @@ public DefaultShadowVariableSessionFactory( } @SuppressWarnings("unchecked") - public static VariableReferenceGraph buildGraph( + public static VariableReferenceGraph buildGraph( SolutionDescriptor solutionDescriptor, VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { @@ -68,7 +68,7 @@ yield buildSingleDirectionalParentGraph(solutionDescriptor, }; } - private static VariableReferenceGraph buildSingleDirectionalParentGraph( + private static VariableReferenceGraph buildSingleDirectionalParentGraph( SolutionDescriptor solutionDescriptor, ChangedVariableNotifier changedVariableNotifier, GraphStructure.GraphStructureAndDirection graphStructureAndDirection, @@ -76,7 +76,7 @@ private static VariableReferenceGraph buildSingleDirectio var declarativeShadowVariables = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); var sortedDeclarativeVariables = topologicallySortedDeclarativeShadowVariables(declarativeShadowVariables); - Function successorFunction = + var successorFunction = getSuccessorFunction(solutionDescriptor, Objects.requireNonNull(changedVariableNotifier.innerScoreDirector()), Objects.requireNonNull(graphStructureAndDirection.parentMetaModel()), Objects.requireNonNull(graphStructureAndDirection.direction())); @@ -120,7 +120,7 @@ private static VariableReferenceGraph buildSingleDirectio return sortedDeclarativeVariables; } - private static @NonNull Function getSuccessorFunction( + private static @NonNull UnaryOperator<@Nullable Object> getSuccessorFunction( SolutionDescriptor solutionDescriptor, InnerScoreDirector scoreDirector, VariableMetaModel parentMetaModel, ParentVariableType parentVariableType) { return switch (parentVariableType) { @@ -136,15 +136,13 @@ private static VariableReferenceGraph buildSingleDirectio var entityType = sourceVariable.getEntityDescriptor().getEntityClass(); yield old -> entityType.isInstance(old) ? sourceVariable.getValue(old) : null; } - default -> { - throw new IllegalStateException( - "Impossible state: expected parentVariableType to be previous or next but was %s." - .formatted(parentVariableType)); - } + default -> throw new IllegalStateException( + "Impossible state: expected parentVariableType to be previous or next but was %s." + .formatted(parentVariableType)); }; } - private static VariableReferenceGraph buildArbitraryGraph( + private static VariableReferenceGraph buildArbitraryGraph( SolutionDescriptor solutionDescriptor, VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java index b726c634cf4..a8bcb1084e0 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java @@ -10,7 +10,7 @@ import org.jspecify.annotations.NonNull; final class DefaultVariableReferenceGraph extends AbstractVariableReferenceGraph - implements VariableReferenceGraph { + implements VariableReferenceGraph { // These structures are mutable. private final Consumer affectedEntitiesUpdater; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java index 23791b736c5..106b334eb11 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java @@ -2,7 +2,7 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -final class EmptyVariableReferenceGraph implements VariableReferenceGraph { +final class EmptyVariableReferenceGraph implements VariableReferenceGraph { @SuppressWarnings("rawtypes") public static final EmptyVariableReferenceGraph INSTANCE = new EmptyVariableReferenceGraph<>(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java index 3765d3a8610..bfc06e180dd 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/FixedVariableReferenceGraph.java @@ -9,7 +9,7 @@ public final class FixedVariableReferenceGraph extends AbstractVariableReferenceGraph> - implements VariableReferenceGraph { + implements VariableReferenceGraph { // These are immutable private final ChangedVariableNotifier changedVariableNotifier; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java index 914404ed30e..fe5c137496f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraph.java @@ -6,26 +6,26 @@ import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.function.Function; +import java.util.function.UnaryOperator; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -public final class SingleDirectionalParentVariableReferenceGraph implements VariableReferenceGraph { +public final class SingleDirectionalParentVariableReferenceGraph implements VariableReferenceGraph { private final Set> monitoredSourceVariableSet; private final VariableUpdaterInfo[] sortedVariableUpdaterInfos; - private final Function successorFunction; + private final UnaryOperator successorFunction; private final ChangedVariableNotifier changedVariableNotifier; private final List changedEntities; private final Class monitoredEntityClass; private boolean isUpdating; + @SuppressWarnings("unchecked") public SingleDirectionalParentVariableReferenceGraph( List> sortedDeclarativeShadowVariableDescriptors, - Function successorFunction, + UnaryOperator successorFunction, ChangedVariableNotifier changedVariableNotifier, Object[] entities) { monitoredEntityClass = sortedDeclarativeShadowVariableDescriptors.get(0).getEntityDescriptor().getEntityClass(); - // noinspection unchecked sortedVariableUpdaterInfos = new VariableUpdaterInfo[sortedDeclarativeShadowVariableDescriptors.size()]; monitoredSourceVariableSet = new HashSet<>(); changedEntities = new ArrayList<>(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java index a2446de8dfa..5350c159554 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraph.java @@ -2,7 +2,7 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -public sealed interface VariableReferenceGraph +public sealed interface VariableReferenceGraph permits AbstractVariableReferenceGraph, DefaultVariableReferenceGraph, EmptyVariableReferenceGraph, FixedVariableReferenceGraph, SingleDirectionalParentVariableReferenceGraph { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java index 2eccc71c382..53dce5d2a53 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableReferenceGraphBuilder.java @@ -71,7 +71,7 @@ public void addAfterProcessor(GraphChangeType graphChangeType, VariableMetaModel } @SuppressWarnings("unchecked") - public VariableReferenceGraph build(IntFunction graphCreator) { + public VariableReferenceGraph build(IntFunction graphCreator) { // TODO empty shows up in VRP example when using it as CVRP, not CVRPTW // In that case, TimeWindowedCustomer does not exist // and therefore Customer has no shadow variable. diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java index de3be10a84e..5435aa870a7 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/GraphStructureTest.java @@ -19,9 +19,9 @@ import org.junit.jupiter.api.Test; -public class GraphStructureTest { +class GraphStructureTest { @Test - public void simpleListStructure() { + void simpleListStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataDeclarativeSimpleListSolution.buildSolutionDescriptor())) .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) @@ -29,7 +29,7 @@ public void simpleListStructure() { } @Test - public void simpleChainedStructure() { + void simpleChainedStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataChainedSimpleVarSolution.buildSolutionDescriptor())) .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) @@ -37,7 +37,7 @@ public void simpleChainedStructure() { } @Test - public void extendedSimpleListStructure() { + void extendedSimpleListStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataDeclarativeExtendedSolution.buildSolutionDescriptor())) .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) @@ -45,7 +45,7 @@ public void extendedSimpleListStructure() { } @Test - public void concurrentValuesStructureWithoutGroups() { + void concurrentValuesStructureWithoutGroups() { var value1 = new TestdataConcurrentValue("v1"); var value2 = new TestdataConcurrentValue("v2"); value2.setConcurrentValueGroup(Collections.emptyList()); @@ -57,7 +57,7 @@ public void concurrentValuesStructureWithoutGroups() { } @Test - public void concurrentValuesStructureWithGroups() { + void concurrentValuesStructureWithGroups() { var value1 = new TestdataConcurrentValue("v1"); var value2 = new TestdataConcurrentValue("v2"); var group = List.of(value1, value2); @@ -69,14 +69,14 @@ public void concurrentValuesStructureWithGroups() { } @Test - public void followerStructure() { + void followerStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataFollowerSolution.buildSolutionDescriptor())) .hasFieldOrPropertyWithValue("structure", NO_DYNAMIC_EDGES); } @Test - public void emptyStructure() { + void emptyStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataSolution.buildSolutionDescriptor())) .hasFieldOrPropertyWithValue("structure", EMPTY); diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java index c301ecb39ac..33a72976711 100644 --- a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_chained/SimpleChainedTest.java @@ -17,9 +17,9 @@ import org.junit.jupiter.api.Test; -public class SimpleChainedTest { +class SimpleChainedTest { @Test - public void simpleChained() { + void simpleChained() { var solverConfig = new SolverConfig() .withSolutionClass(TestdataChainedSimpleVarSolution.class) .withEntityClasses(TestdataChainedSimpleVarEntity.class, TestdataChainedSimpleVarValue.class) diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java index a06aa5cfee6..49d1dd12390 100644 --- a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/simple_list/SimpleListTest.java @@ -17,9 +17,9 @@ import org.junit.jupiter.api.Test; -public class SimpleListTest { +class SimpleListTest { @Test - public void simpleList() { + void simpleList() { var solverConfig = new SolverConfig() .withSolutionClass(TestdataDeclarativeSimpleListSolution.class) .withEntityClasses(TestdataDeclarativeSimpleListEntity.class, TestdataDeclarativeSimpleListValue.class) From bdbd3dfb101d583387e6060e966950833d3586ed Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Thu, 26 Jun 2025 09:56:00 -0400 Subject: [PATCH 6/7] chore: logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lukáš Petrovický --- .../declarative/DefaultShadowVariableSessionFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java index 2f0d25c97c8..fba6889e5c5 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java @@ -50,7 +50,7 @@ public static VariableReferenceGraph buildGraph( VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { var graphStructureAndDirection = GraphStructure.determineGraphStructure(solutionDescriptor, entities); - LOGGER.trace("Graph structure: {}", graphStructureAndDirection); + LOGGER.trace("Shadow variable graph structure: {}", graphStructureAndDirection); return switch (graphStructureAndDirection.structure()) { case EMPTY -> EmptyVariableReferenceGraph.INSTANCE; case SINGLE_DIRECTIONAL_PARENT -> { From 3d0411dd721216aa79dc5a930e3a8dc7c2eeac1d Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Thu, 26 Jun 2025 10:24:27 -0400 Subject: [PATCH 7/7] chore: remove unused generics --- .../variable/declarative/EmptyVariableReferenceGraph.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java index 106b334eb11..8a6bae7576f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EmptyVariableReferenceGraph.java @@ -2,10 +2,9 @@ import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -final class EmptyVariableReferenceGraph implements VariableReferenceGraph { +final class EmptyVariableReferenceGraph implements VariableReferenceGraph { - @SuppressWarnings("rawtypes") - public static final EmptyVariableReferenceGraph INSTANCE = new EmptyVariableReferenceGraph<>(); + public static final EmptyVariableReferenceGraph INSTANCE = new EmptyVariableReferenceGraph(); @Override public void updateChanged() {