From ae6381d435adc1953637d2ad891d572051f20f18 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Thu, 3 Jul 2025 14:45:53 -0400 Subject: [PATCH 1/3] perf: Use a single node to represent multiple shadow variables in graphs When there is only a single declarative entity and all non-declarative parents go in the same direction (ex: previous), then some variables can share the same node in the graph. In particular, there is a static topological order when only the variables on a single entity is considered, and as long as the next declarative shadow variable does not have any new non-declarative parents (i.e. group), it can be grouped into the same node as the previous one. --- .../declarative/AffectedEntitiesUpdater.java | 41 +++-- .../DefaultShadowVariableSessionFactory.java | 116 +++++++++++- .../declarative/EntityVariablePair.java | 7 +- .../FixedVariableReferenceGraph.java | 32 ++-- .../variable/declarative/GraphStructure.java | 60 ++++++- .../declarative/RootVariableSource.java | 13 +- ...rectionalParentVariableReferenceGraph.java | 1 + .../VariableReferenceGraphBuilder.java | 18 +- .../declarative/VariableUpdaterInfo.java | 1 + .../declarative/GraphStructureTest.java | 72 +++++++- ...ionalParentVariableReferenceGraphTest.java | 6 +- .../support/VariableListenerSupportTest.java | 19 +- ...estdataMultiDirectionConcurrentEntity.java | 60 +++++++ ...tdataMultiDirectionConcurrentSolution.java | 55 ++++++ ...TestdataMultiDirectionConcurrentValue.java | 167 ++++++++++++++++++ .../TestdataMultiEntityDependencyDelay.java | 4 + .../TestdataMultiEntityDependencyEntity.java | 75 ++++++++ ...TestdataMultiEntityDependencySolution.java | 94 ++++++++++ .../TestdataMultiEntityDependencyValue.java | 140 +++++++++++++++ 19 files changed, 910 insertions(+), 71 deletions(-) create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentEntity.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentSolution.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyDelay.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyEntity.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencySolution.java create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java index e6682206b4f..0cd5d812ece 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java @@ -96,7 +96,7 @@ private void updateLoopedStatusOfAffectedEntity(Object affectedEntity) { for (var node : entityVariablePairFunction.apply(affectedEntity)) { // All variables come from the same entity, // therefore all have the same looped marker. - shadowVariableLoopedDescriptor = node.variableReference().shadowVariableLoopedDescriptor(); + shadowVariableLoopedDescriptor = node.variableReferences()[0].shadowVariableLoopedDescriptor(); if (graph.isLooped(loopedTracker, node.graphNodeId())) { isEntityLooped = true; break; @@ -116,9 +116,9 @@ private void updateLoopedStatusOfAffectedEntity(Object affectedEntity) { private boolean updateShadowVariable(EntityVariablePair entityVariable, boolean isLooped) { var entity = entityVariable.entity(); - var shadowVariableReference = entityVariable.variableReference(); - var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); - var loopDescriptor = shadowVariableReference.shadowVariableLoopedDescriptor(); + var shadowVariableReferences = entityVariable.variableReferences(); + var loopDescriptor = shadowVariableReferences[0].shadowVariableLoopedDescriptor(); + if (loopDescriptor != null) { var oldLooped = (boolean) loopDescriptor.getValue(entity); if (oldLooped != isLooped) { @@ -127,21 +127,26 @@ private boolean updateShadowVariable(EntityVariablePair entityVariabl } } - if (isLooped) { - if (oldValue != null) { - affectedEntities.add(entityVariable); - changeShadowVariableAndNotify(shadowVariableReference, entity, null); - } - return true; - } else { - var newValue = shadowVariableReference.calculator().apply(entity); - if (!Objects.equals(oldValue, newValue)) { - affectedEntities.add(entityVariable); - changeShadowVariableAndNotify(shadowVariableReference, entity, newValue); - return true; + var anyChanged = false; + for (var shadowVariableReference : shadowVariableReferences) { + var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); + if (isLooped) { + if (oldValue != null) { + affectedEntities.add(entityVariable); + changeShadowVariableAndNotify(shadowVariableReference, entity, null); + } + anyChanged = true; + } else { + var newValue = shadowVariableReference.calculator().apply(entity); + if (!Objects.equals(oldValue, newValue)) { + affectedEntities.add(entityVariable); + changeShadowVariableAndNotify(shadowVariableReference, entity, newValue); + anyChanged = true; + } } } - return false; + + return anyChanged; } private void changeShadowVariableAndNotify(VariableUpdaterInfo shadowVariableReference, Object entity, @@ -168,7 +173,7 @@ public AffectedEntities(Consumer consumer) { } public void add(EntityVariablePair shadowVariable) { - var shadowVariableLoopedDescriptor = shadowVariable.variableReference().shadowVariableLoopedDescriptor(); + var shadowVariableLoopedDescriptor = shadowVariable.variableReferences()[0].shadowVariableLoopedDescriptor(); if (shadowVariableLoopedDescriptor == null) { return; } 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 ba28768a010..9a75765173b 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,6 +1,7 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; import java.util.ArrayList; +import java.util.Arrays; import java.util.BitSet; import java.util.Collections; import java.util.Comparator; @@ -13,6 +14,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.BiFunction; import java.util.function.IntFunction; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; @@ -59,6 +61,8 @@ yield buildSingleDirectionalParentGraph(solutionDescriptor, graphStructureAndDirection, entities); } + case ARBITRARY_SINGLE_ENTITY_SINGLE_DIRECTIONAL_PARENT_TYPE -> + buildArbitrarySingleEntityGraph(solutionDescriptor, variableReferenceGraphBuilder, entities, graphCreator); case NO_DYNAMIC_EDGES, ARBITRARY -> buildArbitraryGraph(solutionDescriptor, variableReferenceGraphBuilder, entities, graphCreator); }; @@ -143,7 +147,7 @@ private static VariableReferenceGraph buildArbitraryGraph( VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); - var variableIdToUpdater = new HashMap, VariableUpdaterInfo>(); + var variableIdToUpdater = new HashMap, VariableUpdaterInfo[]>(); // Create graph node for each entity/declarative shadow variable pair. // Maps a variable id to its source aliases; @@ -168,23 +172,117 @@ private static VariableReferenceGraph buildArbitraryGraph( return variableReferenceGraphBuilder.build(graphCreator); } + record GroupVariableUpdaterInfo( + VariableUpdaterInfo[] updaters) { + + } + + private static Map, GroupVariableUpdaterInfo> + getGroupVariableUpdaterInfoMap( + List> declarativeShadowVariableDescriptors) { + var sortedDeclarativeVariableDescriptors = + topologicallySortedDeclarativeShadowVariables(declarativeShadowVariableDescriptors); + var groupIndexToVariables = new HashMap>>(); + var groupVariables = new ArrayList>(); + groupIndexToVariables.put(0, groupVariables); + for (var declarativeShadowVariableDescriptor : sortedDeclarativeVariableDescriptors) { + if (Arrays.stream(declarativeShadowVariableDescriptor.getSources()) + .anyMatch(rootVariableSource -> rootVariableSource.parentVariableType() == ParentVariableType.GROUP)) { + // Create a new variable group, since the group might reference prior variables + groupVariables = new ArrayList<>(); + groupIndexToVariables.put(groupIndexToVariables.size(), groupVariables); + } + groupVariables.add(declarativeShadowVariableDescriptor); + } + + var out = new HashMap, GroupVariableUpdaterInfo>(); + for (var i = 0; i < groupIndexToVariables.size(); i++) { + var entryKey = i; + var entryGroupVariables = groupIndexToVariables.get(i); + var updaters = new ArrayList>(); + for (var declarativeShadowVariableDescriptor : entryGroupVariables) { + updaters.add(new VariableUpdaterInfo<>( + declarativeShadowVariableDescriptor.getVariableMetaModel(), + entryKey, + declarativeShadowVariableDescriptor, + declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), + declarativeShadowVariableDescriptor.getMemberAccessor(), + declarativeShadowVariableDescriptor.getCalculator()::executeGetter)); + } + var updaterArray = updaters.toArray(VariableUpdaterInfo[]::new); + @SuppressWarnings("unchecked") + var groupVariableUpdaterInfo = new GroupVariableUpdaterInfo(updaterArray); + for (var declarativeShadowVariableDescriptor : entryGroupVariables) { + out.put(declarativeShadowVariableDescriptor.getVariableMetaModel(), groupVariableUpdaterInfo); + } + } + return out; + } + + private static VariableReferenceGraph buildArbitrarySingleEntityGraph( + 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 group pair. + // Maps a variable id to the source aliases of all variables in its group; + // If the variables are (in topological order) + // arrivalTime, readyTime, serviceStartTime, serviceFinishTime, + // where serviceStartTime depends on a group of readyTime, then + // the groups are [arrivalTime, readyTime] and [serviceStartTime, serviceFinishTime] + // this is because from arrivalTime, you can compute readyTime without knowing either + // serviceStartTime or serviceFinishTime. + var variableIdToGroupedUpdater = getGroupVariableUpdaterInfoMap(declarativeShadowVariableDescriptors); + var declarativeShadowVariableToAliasMap = createGraphNodes(variableReferenceGraphBuilder, entities, + declarativeShadowVariableDescriptors, variableIdToUpdater, + (declarativeShadowVariable, variableId) -> variableIdToGroupedUpdater.get(variableId).updaters()); + + // Create variable processors for each declarative shadow variable descriptor + for (var declarativeShadowVariable : declarativeShadowVariableDescriptors) { + var fromVariableId = declarativeShadowVariable.getVariableMetaModel(); + createSourceChangeProcessors(entities, variableReferenceGraphBuilder, declarativeShadowVariable, fromVariableId); + var aliasSet = declarativeShadowVariableToAliasMap.get(fromVariableId); + if (aliasSet != null) { + createAliasToVariableChangeProcessors(variableReferenceGraphBuilder, aliasSet, fromVariableId); + } + } + + // Create the fixed edges in the graph + createFixedVariableRelationEdges(variableReferenceGraphBuilder, entities, declarativeShadowVariableDescriptors); + return variableReferenceGraphBuilder.build(graphCreator); + } + + private static Map, Set> createGraphNodes( + VariableReferenceGraphBuilder graph, Object[] entities, + List> declarativeShadowVariableDescriptors, + Map, VariableUpdaterInfo[]> variableIdToUpdaters) { + return createGraphNodes(graph, entities, declarativeShadowVariableDescriptors, variableIdToUpdaters, + (declarativeShadowVariableDescriptor, variableId) -> new VariableUpdaterInfo[] { new VariableUpdaterInfo<>( + variableId, + variableIdToUpdaters.size(), + declarativeShadowVariableDescriptor, + declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), + declarativeShadowVariableDescriptor.getMemberAccessor(), + declarativeShadowVariableDescriptor.getCalculator()::executeGetter) }); + } + private static Map, Set> createGraphNodes( VariableReferenceGraphBuilder graph, Object[] entities, List> declarativeShadowVariableDescriptors, - Map, VariableUpdaterInfo> variableIdToUpdater) { + Map, VariableUpdaterInfo[]> variableIdToUpdaters, + BiFunction, VariableMetaModel, VariableUpdaterInfo[]> variableToUpdatersMapper) { var result = new HashMap, Set>(); for (var entity : entities) { for (var declarativeShadowVariableDescriptor : declarativeShadowVariableDescriptors) { var entityClass = declarativeShadowVariableDescriptor.getEntityDescriptor().getEntityClass(); if (entityClass.isInstance(entity)) { var variableId = declarativeShadowVariableDescriptor.getVariableMetaModel(); - var updater = variableIdToUpdater.computeIfAbsent(variableId, ignored -> new VariableUpdaterInfo<>( - variableId, - declarativeShadowVariableDescriptor, - declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), - declarativeShadowVariableDescriptor.getMemberAccessor(), - declarativeShadowVariableDescriptor.getCalculator()::executeGetter)); - graph.addVariableReferenceEntity(entity, updater); + var updaters = variableIdToUpdaters.computeIfAbsent(variableId, + variableMetaModel -> variableToUpdatersMapper.apply(declarativeShadowVariableDescriptor, + (VariableMetaModel) variableMetaModel)); + graph.addVariableReferenceEntity(entity, updaters); for (var sourceRoot : declarativeShadowVariableDescriptor.getSources()) { for (var source : sourceRoot.variableSourceReferences()) { if (source.downstreamDeclarativeVariableMetamodel() != null) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java index da94bcd609c..1d1293d6f72 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java @@ -1,9 +1,12 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; +import java.util.Arrays; + import org.jspecify.annotations.NullMarked; @NullMarked -public record EntityVariablePair(Object entity, VariableUpdaterInfo variableReference, int graphNodeId) { +public record EntityVariablePair(Object entity, VariableUpdaterInfo[] variableReferences, + int graphNodeId) { @Override public boolean equals(Object object) { if (!(object instanceof EntityVariablePair that)) @@ -18,6 +21,6 @@ public int hashCode() { @Override public String toString() { - return entity + ":" + variableReference.id(); + return entity + ":" + Arrays.stream(variableReferences).map(VariableUpdaterInfo::id).toList(); } } 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 74e113fdf45..b297046f9d8 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 @@ -60,23 +60,25 @@ public void updateChanged() { 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); + var shadowVariableReferences = entityVariable.variableReferences(); + for (var shadowVariableReference : shadowVariableReferences) { + 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; + 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)); } - visited.set(nextNode); - changeSet.add(graph.getTopologicalOrder(nextNode)); } } } 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 b54ed93b50d..f63ad5ef0c4 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 @@ -1,6 +1,7 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; import java.util.Arrays; +import java.util.List; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; import ai.timefold.solver.core.impl.util.MutableInt; @@ -36,6 +37,13 @@ public enum GraphStructure { */ SINGLE_DIRECTIONAL_PARENT, + /** + * A graph structure that accepts all graphs that only have a single + * entity that uses declarative shadow variables with all directional + * parents being the same type. + */ + ARBITRARY_SINGLE_ENTITY_SINGLE_DIRECTIONAL_PARENT_TYPE, + /** * A graph structure that accepts all graphs. */ @@ -55,15 +63,25 @@ public static GraphStructureAndDirection determineGraphStructure( if (declarativeShadowVariableDescriptors.isEmpty()) { return new GraphStructureAndDirection(EMPTY, null, null); } + + if (!doesEntitiesUseDeclarativeShadowVariables(entities, declarativeShadowVariableDescriptors)) { + return new GraphStructureAndDirection(EMPTY, null, null); + } + var multipleDeclarativeEntityClasses = declarativeShadowVariableDescriptors.stream() .map(variable -> variable.getEntityDescriptor().getEntityClass()) .distinct().count() > 1; + final var arbitraryGraphStructure = new GraphStructureAndDirection( + multipleDeclarativeEntityClasses ? ARBITRARY : ARBITRARY_SINGLE_ENTITY_SINGLE_DIRECTIONAL_PARENT_TYPE, + null, null); + var rootVariableSources = declarativeShadowVariableDescriptors.stream() .flatMap(descriptor -> Arrays.stream(descriptor.getSources())) .toList(); ParentVariableType directionalType = null; VariableMetaModel parentMetaModel = null; + var isArbitrary = multipleDeclarativeEntityClasses; for (var variableSource : rootVariableSources) { var parentVariableType = variableSource.parentVariableType(); LOGGER.trace("{} has parentVariableType {}", variableSource, parentVariableType); @@ -76,21 +94,32 @@ public static GraphStructureAndDirection determineGraphStructure( } } if (groupMemberCount.intValue() != 0) { - return new GraphStructureAndDirection(ARBITRARY, null, null); + isArbitrary = true; + var groupParentVariableType = variableSource.groupParentVariableType(); + if (groupParentVariableType != null && groupParentVariableType.isDirectional()) { + var groupParentVariableMetamodel = + variableSource.variableSourceReferences().get(0).variableMetaModel(); + if (parentMetaModel == null) { + parentMetaModel = groupParentVariableMetamodel; + } else if (!parentMetaModel + .equals(variableSource.variableSourceReferences().get(0).variableMetaModel())) { + return new GraphStructureAndDirection(GraphStructure.ARBITRARY, null, null); + } + } } // The group variable is unused/always empty } case INDIRECT, INVERSE, VARIABLE, CHAINED_NEXT -> { // CHAINED_NEXT has a complex comparator function; // so use ARBITRARY despite the fact it can be represented using SINGLE_DIRECTIONAL_PARENT - return new GraphStructureAndDirection(ARBITRARY, null, null); + isArbitrary = true; } case NEXT, PREVIOUS -> { if (parentMetaModel == null) { parentMetaModel = variableSource.variableSourceReferences().get(0).variableMetaModel(); directionalType = parentVariableType; } else if (!parentMetaModel.equals(variableSource.variableSourceReferences().get(0).variableMetaModel())) { - return new GraphStructureAndDirection(ARBITRARY, null, null); + return new GraphStructureAndDirection(GraphStructure.ARBITRARY, null, null); } } case NO_PARENT -> { @@ -99,12 +128,33 @@ public static GraphStructureAndDirection determineGraphStructure( } } + if (isArbitrary) { + return arbitraryGraphStructure; + } + if (directionalType == null) { return new GraphStructureAndDirection(NO_DYNAMIC_EDGES, null, null); } else { // 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); + return new GraphStructureAndDirection(SINGLE_DIRECTIONAL_PARENT, parentMetaModel, directionalType); + } + } + + private static boolean doesEntitiesUseDeclarativeShadowVariables(Object[] entities, + List> declarativeShadowVariableDescriptors) { + boolean anyDeclarativeEntities = false; + for (var declarativeShadowVariable : declarativeShadowVariableDescriptors) { + var entityClass = declarativeShadowVariable.getEntityDescriptor().getEntityClass(); + for (var entity : entities) { + if (entityClass.isInstance(entity)) { + anyDeclarativeEntities = true; + break; + } + if (anyDeclarativeEntities) { + break; + } + } } + return anyDeclarativeEntities; } } 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 cfbad371459..009f5e3a520 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 @@ -25,15 +25,18 @@ import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +@NullMarked public record RootVariableSource( Class rootEntity, List listMemberAccessors, BiConsumer> valueEntityFunction, List variableSourceReferences, String variablePath, - ParentVariableType parentVariableType) { + ParentVariableType parentVariableType, + @Nullable ParentVariableType groupParentVariableType) { public static final String COLLECTION_REFERENCE_SUFFIX = "[]"; public static final String MEMBER_SEPERATOR_REGEX = "\\."; @@ -75,6 +78,7 @@ public static RootVariableSource from( Class currentEntity = rootEntityClass; var factCountSinceLastVariable = 0; ParentVariableType parentVariableType = null; + ParentVariableType groupParentVariableType = null; for (var iterator = pathIterator(rootEntityClass, variablePath); iterator.hasNext();) { var pathPart = iterator.next(); @@ -130,6 +134,9 @@ public static RootVariableSource from( if (parentVariableType == null) { parentVariableType = determineParentVariableType(chainToVariable, memberAccessor); } + if (hasListMemberAccessor && groupParentVariableType == null) { + groupParentVariableType = determineParentVariableType(chainToVariable, memberAccessor); + } } else { factCountSinceLastVariable++; if (factCountSinceLastVariable == 2) { @@ -199,7 +206,8 @@ public static RootVariableSource from( valueEntityFunction, variableSourceReferences, variablePath, - parentVariableType); + parentVariableType, + groupParentVariableType); } public @NonNull BiConsumer> getEntityVisitor(List chainToEntity) { @@ -379,6 +387,7 @@ private static ParentVariableType determineParentVariableType(List T getAnnotation(Class declaringClass, String memberName, Class annotationClass) { var currentClass = declaringClass; 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 f49290a6e62..9e053545c6a 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 @@ -50,6 +50,7 @@ public SingleDirectionalParentVariableReferenceGraph( var variableMetaModel = variableDescriptor.getVariableMetaModel(); var variableUpdaterInfo = new VariableUpdaterInfo<>( variableMetaModel, + updaterIndex, variableDescriptor, loopedDescriptor, variableDescriptor.getMemberAccessor(), 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 adb6a441a68..1a356121e35 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,30 +21,38 @@ public final class VariableReferenceGraphBuilder { final List> instanceList; final Map, List>> fixedEdges; final Map, Map>> variableReferenceToInstanceMap; + final Map>> variableGroupIdToInstanceMap; boolean isGraphFixed; public VariableReferenceGraphBuilder(ChangedVariableNotifier changedVariableNotifier) { this.changedVariableNotifier = changedVariableNotifier; instanceList = new ArrayList<>(); variableReferenceToInstanceMap = new HashMap<>(); + variableGroupIdToInstanceMap = new HashMap<>(); variableReferenceToBeforeProcessor = new HashMap<>(); variableReferenceToAfterProcessor = new HashMap<>(); fixedEdges = new HashMap<>(); isGraphFixed = true; } - public void addVariableReferenceEntity(Entity_ entity, VariableUpdaterInfo variableReference) { - var variableId = variableReference.id(); - var instanceMap = variableReferenceToInstanceMap.get(variableId); + public void addVariableReferenceEntity(Entity_ entity, VariableUpdaterInfo[] variableReferences) { + var groupId = variableReferences[0].groupId(); + var instanceMap = variableGroupIdToInstanceMap.get(groupId); var instance = instanceMap == null ? null : instanceMap.get(entity); if (instance != null) { return; } if (instanceMap == null) { instanceMap = new IdentityHashMap<>(); - variableReferenceToInstanceMap.put(variableId, instanceMap); + variableGroupIdToInstanceMap.put(groupId, instanceMap); } - var node = new EntityVariablePair<>(entity, variableReference, instanceList.size()); + + for (var variable : variableReferences) { + if (!variableReferenceToInstanceMap.containsKey(variable.id())) { + variableReferenceToInstanceMap.put(variable.id(), instanceMap); + } + } + var node = new EntityVariablePair<>(entity, variableReferences, instanceList.size()); instanceMap.put(entity, node); instanceList.add(node); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java index b0447948ef7..33ff3861113 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java @@ -11,6 +11,7 @@ @NullMarked public record VariableUpdaterInfo( VariableMetaModel id, + int groupId, DeclarativeShadowVariableDescriptor variableDescriptor, @Nullable ShadowVariableLoopedVariableDescriptor shadowVariableLoopedDescriptor, MemberAccessor memberAccessor, 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 d58faa39ba1..0ad5504fa83 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,6 +1,7 @@ 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.ARBITRARY_SINGLE_ENTITY_SINGLE_DIRECTIONAL_PARENT_TYPE; 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; @@ -12,37 +13,66 @@ 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.TestdataDeclarativeExtendedBaseValue; import ai.timefold.solver.core.testdomain.declarative.extended.TestdataDeclarativeExtendedSolution; +import ai.timefold.solver.core.testdomain.declarative.extended.TestdataDeclarativeExtendedSubclassValue; +import ai.timefold.solver.core.testdomain.declarative.follower.TestdataFollowerEntity; import ai.timefold.solver.core.testdomain.declarative.follower.TestdataFollowerSolution; +import ai.timefold.solver.core.testdomain.declarative.multi_directional_parent.TestdataMultiDirectionConcurrentEntity; +import ai.timefold.solver.core.testdomain.declarative.multi_directional_parent.TestdataMultiDirectionConcurrentSolution; +import ai.timefold.solver.core.testdomain.declarative.multi_directional_parent.TestdataMultiDirectionConcurrentValue; +import ai.timefold.solver.core.testdomain.declarative.multi_entity.TestdataMultiEntityDependencyEntity; +import ai.timefold.solver.core.testdomain.declarative.multi_entity.TestdataMultiEntityDependencySolution; +import ai.timefold.solver.core.testdomain.declarative.multi_entity.TestdataMultiEntityDependencyValue; import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarSolution; +import ai.timefold.solver.core.testdomain.declarative.simple_chained.TestdataChainedSimpleVarValue; 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; class GraphStructureTest { @Test - void simpleListStructure() { + void emptySimpleListStructure() { assertThat(GraphStructure.determineGraphStructure( TestdataDeclarativeSimpleListSolution.buildSolutionDescriptor())) + .hasFieldOrPropertyWithValue("structure", EMPTY); + } + + @Test + void simpleListStructure() { + var entity = new TestdataDeclarativeSimpleListValue(); + assertThat(GraphStructure.determineGraphStructure( + TestdataDeclarativeSimpleListSolution.buildSolutionDescriptor(), entity)) .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) .hasFieldOrPropertyWithValue("direction", ParentVariableType.PREVIOUS); } @Test void simpleChainedStructure() { + var entity = new TestdataChainedSimpleVarValue(); assertThat(GraphStructure.determineGraphStructure( - TestdataChainedSimpleVarSolution.buildSolutionDescriptor())) - .hasFieldOrPropertyWithValue("structure", ARBITRARY); + TestdataChainedSimpleVarSolution.buildSolutionDescriptor(), entity)) + .hasFieldOrPropertyWithValue("structure", ARBITRARY_SINGLE_ENTITY_SINGLE_DIRECTIONAL_PARENT_TYPE); } @Test void extendedSimpleListStructure() { + var entity = new TestdataDeclarativeExtendedSubclassValue(); assertThat(GraphStructure.determineGraphStructure( - TestdataDeclarativeExtendedSolution.buildSolutionDescriptor())) + TestdataDeclarativeExtendedSolution.buildSolutionDescriptor(), entity)) .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) .hasFieldOrPropertyWithValue("direction", ParentVariableType.PREVIOUS); } + @Test + void extendedSimpleListStructureWithoutDeclarativeEntities() { + var entity = new TestdataDeclarativeExtendedBaseValue(); + assertThat(GraphStructure.determineGraphStructure( + TestdataDeclarativeExtendedSolution.buildSolutionDescriptor(), entity)) + .hasFieldOrPropertyWithValue("structure", EMPTY); + } + @Test void concurrentValuesStructureWithoutGroups() { var value1 = new TestdataConcurrentValue("v1"); @@ -64,16 +94,46 @@ void concurrentValuesStructureWithGroups() { assertThat(GraphStructure.determineGraphStructure( TestdataConcurrentSolution.buildSolutionDescriptor(), value1, value2)) - .hasFieldOrPropertyWithValue("structure", ARBITRARY); + .hasFieldOrPropertyWithValue("structure", ARBITRARY_SINGLE_ENTITY_SINGLE_DIRECTIONAL_PARENT_TYPE); } @Test void followerStructure() { + var entity = new TestdataFollowerEntity(); assertThat(GraphStructure.determineGraphStructure( - TestdataFollowerSolution.buildSolutionDescriptor())) + TestdataFollowerSolution.buildSolutionDescriptor(), entity)) .hasFieldOrPropertyWithValue("structure", NO_DYNAMIC_EDGES); } + @Test + void multiEntity() { + var entity = new TestdataMultiEntityDependencyEntity(); + var value = new TestdataMultiEntityDependencyValue(); + assertThat(GraphStructure.determineGraphStructure( + TestdataMultiEntityDependencySolution.buildSolutionDescriptor(), entity, value)) + .hasFieldOrPropertyWithValue("structure", ARBITRARY); + } + + @Test + void multiDirectionalParents() { + var entity = new TestdataMultiDirectionConcurrentEntity(); + var value = new TestdataMultiDirectionConcurrentValue(); + value.setConcurrentValueGroup(List.of(value)); + assertThat(GraphStructure.determineGraphStructure( + TestdataMultiDirectionConcurrentSolution.buildSolutionDescriptor(), entity, value)) + .hasFieldOrPropertyWithValue("structure", ARBITRARY); + } + + @Test + void multiDirectionalParentsEmptyGroups() { + var entity = new TestdataMultiDirectionConcurrentEntity(); + var value = new TestdataMultiDirectionConcurrentValue(); + assertThat(GraphStructure.determineGraphStructure( + TestdataMultiDirectionConcurrentSolution.buildSolutionDescriptor(), entity, value)) + .hasFieldOrPropertyWithValue("structure", SINGLE_DIRECTIONAL_PARENT) + .hasFieldOrPropertyWithValue("direction", ParentVariableType.PREVIOUS); + } + @Test void emptyStructure() { assertThat(GraphStructure.determineGraphStructure( diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraphTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraphTest.java index 06970ac431f..17383946a70 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraphTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/declarative/SingleDirectionalParentVariableReferenceGraphTest.java @@ -19,8 +19,6 @@ class SingleDirectionalParentVariableReferenceGraphTest { @Test void supplierMethodsAreOnlyCalledOnce() { var solutionDescriptor = TestdataCountingSolution.buildSolutionDescriptor(); - var graphStructureAndDirection = GraphStructure.determineGraphStructure(solutionDescriptor); - var entity1 = new TestdataCountingEntity("e1"); var entity2 = new TestdataCountingEntity("e2"); @@ -30,6 +28,10 @@ void supplierMethodsAreOnlyCalledOnce() { var value4 = new TestdataCountingValue("v4"); var value5 = new TestdataCountingValue("v5"); + var graphStructureAndDirection = GraphStructure.determineGraphStructure(solutionDescriptor, + entity1, entity2, value1, value2, value3, value4, value5); + assertThat(graphStructureAndDirection.structure()).isEqualTo(GraphStructure.SINGLE_DIRECTIONAL_PARENT); + var scoreDirector = Mockito.mock(InnerScoreDirector.class); var listStateSupply = Mockito.mock(ListVariableStateSupply.class); Mockito.when(scoreDirector.getListVariableStateSupply(Mockito.any())) diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java index 2af87cb6775..9429c3d90e3 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java @@ -10,6 +10,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -20,6 +21,7 @@ import ai.timefold.solver.core.impl.domain.variable.declarative.DefaultTopologicalOrderGraph; import ai.timefold.solver.core.impl.domain.variable.declarative.EntityVariablePair; import ai.timefold.solver.core.impl.domain.variable.declarative.TopologicalOrderGraph; +import ai.timefold.solver.core.impl.domain.variable.declarative.VariableUpdaterInfo; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.inverserelation.ExternalizedSingletonInverseVariableSupply; import ai.timefold.solver.core.impl.domain.variable.inverserelation.SingletonInverseVariableDemand; @@ -164,20 +166,21 @@ void shadowVariableListenerOrder() { private static class MockTopologicalOrderGraph extends DefaultTopologicalOrderGraph implements TopologicalOrderGraph { Object[] nodeToEntities; - VariableMetaModel[] nodeToVariableMetamodel; + VariableMetaModel[][] nodeToVariableMetamodel; public MockTopologicalOrderGraph(int size) { super(size); nodeToEntities = new Object[size]; - nodeToVariableMetamodel = new VariableMetaModel[size]; + nodeToVariableMetamodel = new VariableMetaModel[size][]; } @Override public void withNodeData(List> nodes) { nodeToEntities = nodes.stream().map(EntityVariablePair::entity).toArray(Object[]::new); nodeToVariableMetamodel = nodes.stream() - .map(e -> e.variableReference().id()) - .toArray(VariableMetaModel[]::new); + .map(e -> Arrays.stream(e.variableReferences()).map(VariableUpdaterInfo::id) + .toArray(VariableMetaModel[]::new)) + .toArray(VariableMetaModel[][]::new); } public void addEdge(VariableMetaModel fromId, Object fromEntity, VariableMetaModel toId, @@ -193,14 +196,16 @@ public void removeEdge(VariableMetaModel fromId, Object fromEntity, Var @Override public void addEdge(int fromNode, int toNode) { super.addEdge(fromNode, toNode); - addEdge(nodeToVariableMetamodel[fromNode], nodeToEntities[fromNode], nodeToVariableMetamodel[toNode], + addEdge(nodeToVariableMetamodel[fromNode][nodeToVariableMetamodel[fromNode].length - 1], nodeToEntities[fromNode], + nodeToVariableMetamodel[toNode][0], nodeToEntities[toNode]); } @Override public void removeEdge(int fromNode, int toNode) { super.removeEdge(fromNode, toNode); - removeEdge(nodeToVariableMetamodel[fromNode], nodeToEntities[fromNode], nodeToVariableMetamodel[toNode], + removeEdge(nodeToVariableMetamodel[fromNode][nodeToVariableMetamodel[fromNode].length - 1], + nodeToEntities[fromNode], nodeToVariableMetamodel[toNode][0], nodeToEntities[toNode]); } } @@ -285,7 +290,7 @@ void shadowVariableListGraphEvents() { for (var visit : solution.getValues()) { verifyAddEdge.accept(serviceReadyTime, visit, serviceStartTime, visit); - verifyAddEdge.accept(serviceStartTime, visit, serviceFinishTime, visit); + // serviceStartTime and serviceFinishTime are in the same group, so no edge between them! if (visit.getPreviousValue() != null) { verifyAddEdge.accept(serviceFinishTime, visit.getPreviousValue(), serviceReadyTime, visit); diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentEntity.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentEntity.java new file mode 100644 index 00000000000..23238da49ad --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentEntity.java @@ -0,0 +1,60 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_directional_parent; + +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; + +@PlanningEntity +public class TestdataMultiDirectionConcurrentEntity { + String id; + + @PlanningListVariable + List values; + + public TestdataMultiDirectionConcurrentEntity() { + values = new ArrayList<>(); + } + + public TestdataMultiDirectionConcurrentEntity(String id) { + this.id = id; + values = new ArrayList<>(); + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public List getValues() { + return values; + } + + public void setValues(List values) { + this.values = values; + } + + public void updateValueShadows() { + TestdataMultiDirectionConcurrentValue previousVisit = null; + for (var visit : values) { + visit.setEntity(this); + visit.setPreviousValue(previousVisit); + if (previousVisit != null) { + previousVisit.setNextValue(visit); + } + previousVisit = visit; + } + if (previousVisit != null) { + previousVisit.setNextValue(null); + } + } + + @Override + public String toString() { + return id; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentSolution.java new file mode 100644 index 00000000000..308231509ab --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentSolution.java @@ -0,0 +1,55 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_directional_parent; + +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.hardsoft.HardSoftScore; +import ai.timefold.solver.core.config.solver.PreviewFeature; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; + +@PlanningSolution +public class TestdataMultiDirectionConcurrentSolution { + public static SolutionDescriptor buildSolutionDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataMultiDirectionConcurrentSolution.class, + TestdataMultiDirectionConcurrentEntity.class, TestdataMultiDirectionConcurrentValue.class); + } + + @PlanningEntityCollectionProperty + List entities; + + @PlanningEntityCollectionProperty + @ValueRangeProvider + List values; + + @PlanningScore + HardSoftScore score; + + public List getEntities() { + return entities; + } + + public void setEntities(List entities) { + this.entities = entities; + } + + public List getValues() { + return values; + } + + public void setValues(List values) { + this.values = values; + } + + public HardSoftScore getScore() { + return score; + } + + public void setScore(HardSoftScore score) { + this.score = score; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java new file mode 100644 index 00000000000..32ddc8e252c --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_directional_parent/TestdataMultiDirectionConcurrentValue.java @@ -0,0 +1,167 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_directional_parent; + +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.util.List; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.IndexShadowVariable; +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.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.preview.api.domain.variable.declarative.ShadowVariableLooped; + +@PlanningEntity +public class TestdataMultiDirectionConcurrentValue { + public static final LocalDateTime BASE_START_TIME = LocalDate.of(2025, 1, 1).atTime(LocalTime.of(9, 0)); + String id; + + @InverseRelationShadowVariable(sourceVariableName = "values") + TestdataMultiDirectionConcurrentEntity entity; + + @ShadowVariable(supplierName = "serviceReadyTimeUpdater") + LocalDateTime serviceReadyTime; + + @ShadowVariable(supplierName = "serviceStartTimeUpdater") + LocalDateTime serviceStartTime; + + @ShadowVariable(supplierName = "serviceFinishTimeUpdater") + LocalDateTime serviceFinishTime; + + @PreviousElementShadowVariable(sourceVariableName = "values") + TestdataMultiDirectionConcurrentValue previousValue; + + @NextElementShadowVariable(sourceVariableName = "values") + TestdataMultiDirectionConcurrentValue nextValue; + + @IndexShadowVariable(sourceVariableName = "values") + Integer index; + + List concurrentValueGroup; + + @ShadowVariableLooped + boolean isInvalid; + + public TestdataMultiDirectionConcurrentValue() { + } + + public TestdataMultiDirectionConcurrentValue(String id) { + this.id = id; + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public TestdataMultiDirectionConcurrentEntity getEntity() { + return entity; + } + + public void setEntity(TestdataMultiDirectionConcurrentEntity entity) { + this.entity = entity; + } + + public TestdataMultiDirectionConcurrentValue getPreviousValue() { + return previousValue; + } + + public void setPreviousValue(TestdataMultiDirectionConcurrentValue previousValue) { + this.previousValue = previousValue; + } + + public TestdataMultiDirectionConcurrentValue getNextValue() { + return nextValue; + } + + public void setNextValue(TestdataMultiDirectionConcurrentValue nextValue) { + this.nextValue = nextValue; + } + + public Integer getIndex() { + return index; + } + + public void setIndex(Integer index) { + this.index = index; + } + + public LocalDateTime getServiceStartTime() { + return serviceStartTime; + } + + public void setServiceStartTime(LocalDateTime serviceStartTime) { + this.serviceStartTime = serviceStartTime; + } + + @ShadowSources({ "previousValue.serviceFinishTime", "entity" }) + public LocalDateTime serviceReadyTimeUpdater() { + if (previousValue != null) { + return previousValue.serviceFinishTime.plusMinutes(30L); + } + if (entity != null) { + return BASE_START_TIME; + } + return null; + } + + @ShadowSources({ "serviceReadyTime", "concurrentValueGroup[].nextValue" }) + public LocalDateTime serviceStartTimeUpdater() { + if (serviceReadyTime == null) { + return null; + } + var startTime = serviceReadyTime; + if (concurrentValueGroup != null) { + for (var visit : concurrentValueGroup) { + if (visit.serviceReadyTime != null && startTime.isBefore(visit.serviceReadyTime)) { + startTime = visit.serviceReadyTime; + } + } + } + return startTime; + } + + @ShadowSources("serviceStartTime") + public LocalDateTime serviceFinishTimeUpdater() { + if (serviceStartTime == null) { + return null; + } + return serviceStartTime.plusMinutes(30L); + } + + public LocalDateTime getServiceFinishTime() { + return serviceFinishTime; + } + + public void setServiceFinishTime(LocalDateTime serviceFinishTime) { + this.serviceFinishTime = serviceFinishTime; + } + + public List getConcurrentValueGroup() { + return concurrentValueGroup; + } + + public void setConcurrentValueGroup(List concurrentValueGroup) { + this.concurrentValueGroup = concurrentValueGroup; + } + + public boolean isInvalid() { + return isInvalid; + } + + public void setInvalid(boolean invalid) { + isInvalid = invalid; + } + + @Override + public String toString() { + return (previousValue != null) ? previousValue + " -> " + id + : (entity != null) ? entity.id + " -> " + id : "null -> " + id; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyDelay.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyDelay.java new file mode 100644 index 00000000000..f27cb2b52a7 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyDelay.java @@ -0,0 +1,4 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_entity; + +public record TestdataMultiEntityDependencyDelay(int delay) { +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyEntity.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyEntity.java new file mode 100644 index 00000000000..c663686ecd1 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyEntity.java @@ -0,0 +1,75 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_entity; + +import java.time.LocalDateTime; +import java.time.ZoneOffset; +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.api.domain.variable.PlanningVariable; +import ai.timefold.solver.core.api.domain.variable.ShadowVariable; +import ai.timefold.solver.core.preview.api.domain.variable.declarative.ShadowSources; + +@PlanningEntity +public class TestdataMultiEntityDependencyEntity { + @PlanningListVariable + List values; + + @PlanningVariable + TestdataMultiEntityDependencyDelay delay; + + @ShadowVariable(supplierName = "readyTimeSupplier") + LocalDateTime readyTime; + + LocalDateTime startTime; + + public TestdataMultiEntityDependencyEntity() { + this(LocalDateTime.ofEpochSecond(0L, 0, ZoneOffset.UTC)); + } + + public TestdataMultiEntityDependencyEntity(LocalDateTime startTime) { + this.startTime = startTime; + this.values = new ArrayList<>(); + } + + public List getValues() { + return values; + } + + public void setValues(List values) { + this.values = values; + } + + public LocalDateTime getStartTime() { + return startTime; + } + + public void setStartTime(LocalDateTime startTime) { + this.startTime = startTime; + } + + public LocalDateTime getReadyTime() { + return readyTime; + } + + public void setReadyTime(LocalDateTime readyTime) { + this.readyTime = readyTime; + } + + @ShadowSources("delay") + public LocalDateTime readyTimeSupplier() { + if (delay == null) { + return null; + } + return startTime.plusHours(delay.delay()); + } + + @Override + public String toString() { + return "TestdataPredecessorEntity{" + + "values=" + values + + ", startTime=" + startTime + + '}'; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencySolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencySolution.java new file mode 100644 index 00000000000..804d830f3d5 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencySolution.java @@ -0,0 +1,94 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_entity; + +import java.util.EnumSet; +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; + +@PlanningSolution +public class TestdataMultiEntityDependencySolution { + public static SolutionDescriptor buildDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(Set.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataMultiEntityDependencySolution.class, + TestdataMultiEntityDependencyEntity.class, + TestdataMultiEntityDependencyValue.class); + } + + @PlanningEntityCollectionProperty + List entities; + + @PlanningEntityCollectionProperty + @ValueRangeProvider + List values; + + @ValueRangeProvider + List delays; + + @PlanningScore + HardSoftScore score; + + public TestdataMultiEntityDependencySolution() { + } + + public TestdataMultiEntityDependencySolution(List entities, + List values) { + this.values = values; + this.entities = entities; + } + + public static SolutionDescriptor buildSolutionDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataMultiEntityDependencySolution.class, TestdataMultiEntityDependencyEntity.class, + TestdataMultiEntityDependencyValue.class); + } + + 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 List getDelays() { + return delays; + } + + public void setDelays( + List delays) { + this.delays = delays; + } + + public HardSoftScore getScore() { + return score; + } + + public void setScore(HardSoftScore score) { + this.score = score; + } + + @Override + public String toString() { + return "TestdataPredecessorSolution{" + + "entities=" + entities + + ", values=" + values + + ", score=" + score + + '}'; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java new file mode 100644 index 00000000000..702e98c825b --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/declarative/multi_entity/TestdataMultiEntityDependencyValue.java @@ -0,0 +1,140 @@ +package ai.timefold.solver.core.testdomain.declarative.multi_entity; + +import java.time.Duration; +import java.time.LocalDateTime; +import java.util.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.preview.api.domain.variable.declarative.ShadowVariableLooped; + +import org.apache.commons.lang3.ObjectUtils; + +@PlanningEntity +public class TestdataMultiEntityDependencyValue { + String id; + List dependencies; + + @PreviousElementShadowVariable(sourceVariableName = "values") + TestdataMultiEntityDependencyValue previousValue; + + @ShadowVariable(supplierName = "calculateStartTime") + LocalDateTime startTime; + + @ShadowVariable(supplierName = "calculateEndTime") + LocalDateTime endTime; + + @ShadowVariableLooped + boolean isInvalid; + + @InverseRelationShadowVariable(sourceVariableName = "values") + TestdataMultiEntityDependencyEntity entity; + + Duration duration; + + public TestdataMultiEntityDependencyValue() { + } + + public TestdataMultiEntityDependencyValue(String id, Duration duration) { + this(id, duration, null); + } + + public TestdataMultiEntityDependencyValue(String id, Duration duration, + List dependencies) { + this.id = id; + this.duration = duration; + this.dependencies = dependencies; + } + + public List getDependencies() { + return dependencies; + } + + public void setDependencies( + List dependencies) { + this.dependencies = dependencies; + } + + public LocalDateTime getStartTime() { + return startTime; + } + + public void setStartTime(LocalDateTime startTime) { + this.startTime = startTime; + } + + @ShadowSources({ "dependencies[].endTime", "previousValue.endTime", "entity.readyTime" }) + public LocalDateTime calculateStartTime() { + LocalDateTime readyTime; + if (previousValue != null) { + readyTime = previousValue.endTime; + } else if (entity != null) { + readyTime = entity.readyTime; + if (readyTime == null) { + return null; + } + } else { + return null; + } + + if (dependencies != null) { + for (var dependency : dependencies) { + if (dependency.endTime == null) { + return null; + } + readyTime = ObjectUtils.max(readyTime, dependency.endTime); + } + } + return readyTime; + } + + public LocalDateTime getEndTime() { + return endTime; + } + + public void setEndTime(LocalDateTime endTime) { + this.endTime = endTime; + } + + @ShadowSources({ "startTime" }) + public LocalDateTime calculateEndTime() { + if (startTime == null) { + return null; + } + return startTime.plus(duration); + } + + public Duration getDuration() { + return duration; + } + + public void setDuration(Duration duration) { + this.duration = duration; + } + + public boolean isInvalid() { + return isInvalid; + } + + public void setInvalid(boolean invalid) { + isInvalid = invalid; + } + + public TestdataMultiEntityDependencyEntity getEntity() { + return entity; + } + + public void setEntity(TestdataMultiEntityDependencyEntity entity) { + this.entity = entity; + } + + @Override + public String toString() { + return id + "{" + + "endTime=" + endTime + + '}'; + } +} From d6cfa973d4ef2fda5960067ca20facd5c1b3b84d Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Fri, 4 Jul 2025 16:44:57 -0400 Subject: [PATCH 2/3] perf: review comments, all nodes share the same node if no groups are used - Also fixed a bug in the topological sort done to determine variable trigger order when groups are used (previously it added group variables, which caused it to create a cycle and make all topological orders valid). --- .../declarative/AffectedEntitiesUpdater.java | 43 +++++----- .../DefaultShadowVariableSessionFactory.java | 81 +++++++++++++------ .../EntityVariableUpdaterLookup.java | 74 +++++++++++++++++ .../variable/declarative/GraphStructure.java | 14 ++-- .../VariableReferenceGraphBuilder.java | 10 +-- .../declarative/VariableUpdaterInfo.java | 5 ++ .../support/VariableListenerSupportTest.java | 8 +- 7 files changed, 176 insertions(+), 59 deletions(-) create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java index 0cd5d812ece..cb8d507af57 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java @@ -51,7 +51,7 @@ public void accept(BitSet changed) { } visited.set(nextNode); var shadowVariable = instanceList.get(nextNode); - var isChanged = updateShadowVariable(shadowVariable, graph.isLooped(loopedTracker, nextNode)); + var isChanged = updateEntityShadowVariables(shadowVariable, graph.isLooped(loopedTracker, nextNode)); if (isChanged) { var iterator = graph.nodeForwardEdges(nextNode); @@ -114,41 +114,48 @@ private void updateLoopedStatusOfAffectedEntity(Object affectedEntity) { } - private boolean updateShadowVariable(EntityVariablePair entityVariable, boolean isLooped) { + private boolean updateEntityShadowVariables(EntityVariablePair entityVariable, boolean isLooped) { var entity = entityVariable.entity(); var shadowVariableReferences = entityVariable.variableReferences(); var loopDescriptor = shadowVariableReferences[0].shadowVariableLoopedDescriptor(); + var anyChanged = false; if (loopDescriptor != null) { var oldLooped = (boolean) loopDescriptor.getValue(entity); if (oldLooped != isLooped) { // Loop status change; add to affected entities affectedEntities.add(entityVariable); + anyChanged = true; } } - var anyChanged = false; for (var shadowVariableReference : shadowVariableReferences) { - var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); - if (isLooped) { - if (oldValue != null) { - affectedEntities.add(entityVariable); - changeShadowVariableAndNotify(shadowVariableReference, entity, null); - } - anyChanged = true; - } else { - var newValue = shadowVariableReference.calculator().apply(entity); - if (!Objects.equals(oldValue, newValue)) { - affectedEntities.add(entityVariable); - changeShadowVariableAndNotify(shadowVariableReference, entity, newValue); - anyChanged = true; - } - } + anyChanged |= updateShadowVariable(entityVariable, isLooped, shadowVariableReference, entity, anyChanged); } return anyChanged; } + private boolean updateShadowVariable(EntityVariablePair entityVariable, boolean isLooped, + VariableUpdaterInfo shadowVariableReference, Object entity, boolean anyChanged) { + var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); + if (isLooped) { + if (oldValue != null) { + affectedEntities.add(entityVariable); + changeShadowVariableAndNotify(shadowVariableReference, entity, null); + } + return true; + } else { + var newValue = shadowVariableReference.calculator().apply(entity); + if (!Objects.equals(oldValue, newValue)) { + affectedEntities.add(entityVariable); + changeShadowVariableAndNotify(shadowVariableReference, entity, newValue); + return true; + } + } + return false; + } + private void changeShadowVariableAndNotify(VariableUpdaterInfo shadowVariableReference, Object entity, Object newValue) { var variableDescriptor = shadowVariableReference.variableDescriptor(); 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 9a75765173b..a3330029c95 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 @@ -14,12 +14,13 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.BiFunction; import java.util.function.IntFunction; +import ai.timefold.solver.core.api.function.TriFunction; 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.score.director.InnerScoreDirector; +import ai.timefold.solver.core.impl.util.MutableInt; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NonNull; @@ -98,8 +99,10 @@ static VariableReferenceGraph buildSingleDirectionalParentGraph( var visited = new HashSet(); for (var source : declarativeShadowVariable.getSources()) { var variableReferences = source.variableSourceReferences(); - if (variableReferences.size() != 1) { - // variableReferences is from directional variable + if (source.parentVariableType() != ParentVariableType.NO_PARENT) { + // We only look at direct usage; if we also added + // edges for groups/directional, we will end up creating a cycle + // which makes all topological orders valid continue; } var variableReference = variableReferences.get(0); @@ -147,7 +150,7 @@ private static VariableReferenceGraph buildArbitraryGraph( VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); - var variableIdToUpdater = new HashMap, VariableUpdaterInfo[]>(); + var variableIdToUpdater = EntityVariableUpdaterLookup. entityIndependentLookup(); // Create graph node for each entity/declarative shadow variable pair. // Maps a variable id to its source aliases; @@ -172,9 +175,24 @@ private static VariableReferenceGraph buildArbitraryGraph( return variableReferenceGraphBuilder.build(graphCreator); } - record GroupVariableUpdaterInfo( - VariableUpdaterInfo[] updaters) { - + private record GroupVariableUpdaterInfo( + List> sortedDeclarativeVariableDescriptors, + List> allUpdaters, + VariableUpdaterInfo[] groupedUpdaters) { + public VariableUpdaterInfo[] getUpdatersForEntity(Object entity) { + for (var shadowVariableDescriptor : sortedDeclarativeVariableDescriptors) { + for (var rootSource : shadowVariableDescriptor.getSources()) { + if (rootSource.parentVariableType() == ParentVariableType.GROUP) { + var visitedCount = new MutableInt(); + rootSource.valueEntityFunction().accept(entity, ignored -> visitedCount.increment()); + if (visitedCount.intValue() > 0) { + return groupedUpdaters; + } + } + } + } + return allUpdaters.toArray(VariableUpdaterInfo[]::new); + } } private static Map, GroupVariableUpdaterInfo> @@ -186,7 +204,7 @@ record GroupVariableUpdaterInfo( var groupVariables = new ArrayList>(); groupIndexToVariables.put(0, groupVariables); for (var declarativeShadowVariableDescriptor : sortedDeclarativeVariableDescriptors) { - if (Arrays.stream(declarativeShadowVariableDescriptor.getSources()) + if (!groupVariables.isEmpty() && Arrays.stream(declarativeShadowVariableDescriptor.getSources()) .anyMatch(rootVariableSource -> rootVariableSource.parentVariableType() == ParentVariableType.GROUP)) { // Create a new variable group, since the group might reference prior variables groupVariables = new ArrayList<>(); @@ -196,26 +214,32 @@ record GroupVariableUpdaterInfo( } var out = new HashMap, GroupVariableUpdaterInfo>(); + var allUpdaters = new ArrayList>(); for (var i = 0; i < groupIndexToVariables.size(); i++) { var entryKey = i; var entryGroupVariables = groupIndexToVariables.get(i); var updaters = new ArrayList>(); for (var declarativeShadowVariableDescriptor : entryGroupVariables) { - updaters.add(new VariableUpdaterInfo<>( + var updater = new VariableUpdaterInfo<>( declarativeShadowVariableDescriptor.getVariableMetaModel(), entryKey, declarativeShadowVariableDescriptor, declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), declarativeShadowVariableDescriptor.getMemberAccessor(), - declarativeShadowVariableDescriptor.getCalculator()::executeGetter)); + declarativeShadowVariableDescriptor.getCalculator()::executeGetter); + updaters.add(updater); + allUpdaters.add(updater); } var updaterArray = updaters.toArray(VariableUpdaterInfo[]::new); @SuppressWarnings("unchecked") - var groupVariableUpdaterInfo = new GroupVariableUpdaterInfo(updaterArray); + var groupVariableUpdaterInfo = new GroupVariableUpdaterInfo(sortedDeclarativeVariableDescriptors, + allUpdaters, + updaterArray); for (var declarativeShadowVariableDescriptor : entryGroupVariables) { out.put(declarativeShadowVariableDescriptor.getVariableMetaModel(), groupVariableUpdaterInfo); } } + allUpdaters.replaceAll(updater -> updater.withGroupId(groupIndexToVariables.size())); return out; } @@ -224,7 +248,9 @@ private static VariableReferenceGraph buildArbitrarySingleEntityGrap VariableReferenceGraphBuilder variableReferenceGraphBuilder, Object[] entities, IntFunction graphCreator) { var declarativeShadowVariableDescriptors = solutionDescriptor.getDeclarativeShadowVariableDescriptors(); - var variableIdToUpdater = new HashMap, VariableUpdaterInfo[]>(); + // Use a dependent lookup; if an entity does not use groups, then all variables can share the same node. + // If the entity use groups, then variables must be grouped into their own nodes. + var variableIdToUpdater = EntityVariableUpdaterLookup. entityDependentLookup(); // Create graph node for each entity/declarative shadow variable group pair. // Maps a variable id to the source aliases of all variables in its group; @@ -237,7 +263,8 @@ private static VariableReferenceGraph buildArbitrarySingleEntityGrap var variableIdToGroupedUpdater = getGroupVariableUpdaterInfoMap(declarativeShadowVariableDescriptors); var declarativeShadowVariableToAliasMap = createGraphNodes(variableReferenceGraphBuilder, entities, declarativeShadowVariableDescriptors, variableIdToUpdater, - (declarativeShadowVariable, variableId) -> variableIdToGroupedUpdater.get(variableId).updaters()); + (entity, declarativeShadowVariable, variableId) -> variableIdToGroupedUpdater.get(variableId) + .getUpdatersForEntity(entity)); // Create variable processors for each declarative shadow variable descriptor for (var declarativeShadowVariable : declarativeShadowVariableDescriptors) { @@ -257,31 +284,33 @@ private static VariableReferenceGraph buildArbitrarySingleEntityGrap private static Map, Set> createGraphNodes( VariableReferenceGraphBuilder graph, Object[] entities, List> declarativeShadowVariableDescriptors, - Map, VariableUpdaterInfo[]> variableIdToUpdaters) { + EntityVariableUpdaterLookup variableIdToUpdaters) { return createGraphNodes(graph, entities, declarativeShadowVariableDescriptors, variableIdToUpdaters, - (declarativeShadowVariableDescriptor, variableId) -> new VariableUpdaterInfo[] { new VariableUpdaterInfo<>( - variableId, - variableIdToUpdaters.size(), - declarativeShadowVariableDescriptor, - declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), - declarativeShadowVariableDescriptor.getMemberAccessor(), - declarativeShadowVariableDescriptor.getCalculator()::executeGetter) }); + (entity, declarativeShadowVariableDescriptor, + variableId) -> new VariableUpdaterInfo[] { new VariableUpdaterInfo<>( + variableId, + variableIdToUpdaters.getNextId(), + declarativeShadowVariableDescriptor, + declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), + declarativeShadowVariableDescriptor.getMemberAccessor(), + declarativeShadowVariableDescriptor.getCalculator()::executeGetter) }); } private static Map, Set> createGraphNodes( VariableReferenceGraphBuilder graph, Object[] entities, List> declarativeShadowVariableDescriptors, - Map, VariableUpdaterInfo[]> variableIdToUpdaters, - BiFunction, VariableMetaModel, VariableUpdaterInfo[]> variableToUpdatersMapper) { + EntityVariableUpdaterLookup variableIdToUpdaters, + TriFunction, VariableMetaModel, VariableUpdaterInfo[]> entityVariableToUpdatersMapper) { var result = new HashMap, Set>(); for (var entity : entities) { for (var declarativeShadowVariableDescriptor : declarativeShadowVariableDescriptors) { var entityClass = declarativeShadowVariableDescriptor.getEntityDescriptor().getEntityClass(); if (entityClass.isInstance(entity)) { var variableId = declarativeShadowVariableDescriptor.getVariableMetaModel(); - var updaters = variableIdToUpdaters.computeIfAbsent(variableId, - variableMetaModel -> variableToUpdatersMapper.apply(declarativeShadowVariableDescriptor, - (VariableMetaModel) variableMetaModel)); + var updaters = variableIdToUpdaters.computeUpdatersForVariableOnEntity(variableId, + entity, + () -> entityVariableToUpdatersMapper.apply(entity, declarativeShadowVariableDescriptor, + variableId)); graph.addVariableReferenceEntity(entity, updaters); for (var sourceRoot : declarativeShadowVariableDescriptor.getSources()) { for (var source : sourceRoot.variableSourceReferences()) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java new file mode 100644 index 00000000000..e3c035682ae --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java @@ -0,0 +1,74 @@ +package ai.timefold.solver.core.impl.domain.variable.declarative; + +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.function.Supplier; + +import ai.timefold.solver.core.impl.util.MutableReference; +import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; + +public class EntityVariableUpdaterLookup { + private final Map, Lookup> variableToEntityLookup; + private final Supplier> entityLookupSupplier; + private int nextId = 0; + + private record Lookup(Function[]> getter, + BiConsumer[]>> setter) { + VariableUpdaterInfo[] getUpdaters(Object entity) { + return getter.apply(entity); + } + + void setUpdaters(Object entity, Supplier[]> updatersSupplier) { + setter.accept(entity, updatersSupplier); + } + } + + private EntityVariableUpdaterLookup(Supplier> entityLookupSupplier) { + this.variableToEntityLookup = new LinkedHashMap<>(); + this.entityLookupSupplier = entityLookupSupplier; + } + + public static EntityVariableUpdaterLookup entityIndependentLookup() { + Supplier> lookupSupplier = () -> { + var sharedValue = new MutableReference[]>(null); + return new Lookup<>(ignored -> sharedValue.getValue(), + (ignored, valueSupplier) -> { + if (sharedValue.getValue() == null) { + sharedValue.setValue(valueSupplier.get()); + } + }); + }; + return new EntityVariableUpdaterLookup<>(lookupSupplier); + } + + public static EntityVariableUpdaterLookup entityDependentLookup() { + Supplier> lookupSupplier = () -> { + var valueMap = new IdentityHashMap[]>(); + return new Lookup<>(valueMap::get, + (entity, valueSupplier) -> valueMap.computeIfAbsent(entity, ignored -> valueSupplier.get())); + }; + return new EntityVariableUpdaterLookup<>(lookupSupplier); + } + + public VariableUpdaterInfo[] getUpdatersForVariableOnEntity(VariableMetaModel variableMetaModel, + Object entity) { + var entityLookup = variableToEntityLookup.get(variableMetaModel); + return entityLookup.getUpdaters(entity); + } + + public VariableUpdaterInfo[] computeUpdatersForVariableOnEntity(VariableMetaModel variableMetaModel, + Object entity, + Supplier[]> updatersSupplier) { + var entityLookup = variableToEntityLookup.computeIfAbsent(variableMetaModel, ignored -> entityLookupSupplier.get()); + entityLookup.setUpdaters(entity, updatersSupplier); + return entityLookup.getUpdaters(entity); + } + + public int getNextId() { + return nextId++; + } + +} 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 f63ad5ef0c4..d25afeecbed 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 @@ -64,7 +64,7 @@ public static GraphStructureAndDirection determineGraphStructure( return new GraphStructureAndDirection(EMPTY, null, null); } - if (!doesEntitiesUseDeclarativeShadowVariables(entities, declarativeShadowVariableDescriptors)) { + if (!doEntitiesUseDeclarativeShadowVariables(declarativeShadowVariableDescriptors, entities)) { return new GraphStructureAndDirection(EMPTY, null, null); } @@ -109,11 +109,9 @@ public static GraphStructureAndDirection determineGraphStructure( } // The group variable is unused/always empty } - case INDIRECT, INVERSE, VARIABLE, CHAINED_NEXT -> { - // CHAINED_NEXT has a complex comparator function; - // so use ARBITRARY despite the fact it can be represented using SINGLE_DIRECTIONAL_PARENT - isArbitrary = true; - } + // CHAINED_NEXT has a complex comparator function; + // so use ARBITRARY despite the fact it can be represented using SINGLE_DIRECTIONAL_PARENT + case INDIRECT, INVERSE, VARIABLE, CHAINED_NEXT -> isArbitrary = true; case NEXT, PREVIOUS -> { if (parentMetaModel == null) { parentMetaModel = variableSource.variableSourceReferences().get(0).variableMetaModel(); @@ -140,8 +138,8 @@ public static GraphStructureAndDirection determineGraphStructure( } } - private static boolean doesEntitiesUseDeclarativeShadowVariables(Object[] entities, - List> declarativeShadowVariableDescriptors) { + private static boolean doEntitiesUseDeclarativeShadowVariables( + List> declarativeShadowVariableDescriptors, Object... entities) { boolean anyDeclarativeEntities = false; for (var declarativeShadowVariable : declarativeShadowVariableDescriptors) { var entityClass = declarativeShadowVariable.getEntityDescriptor().getEntityClass(); 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 1a356121e35..f1a20845b4f 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 @@ -47,13 +47,13 @@ public void addVariableReferenceEntity(Entity_ entity, VariableUpdater variableGroupIdToInstanceMap.put(groupId, instanceMap); } - for (var variable : variableReferences) { - if (!variableReferenceToInstanceMap.containsKey(variable.id())) { - variableReferenceToInstanceMap.put(variable.id(), instanceMap); - } - } var node = new EntityVariablePair<>(entity, variableReferences, instanceList.size()); instanceMap.put(entity, node); + for (var variable : variableReferences) { + var variableInstanceMap = + variableReferenceToInstanceMap.computeIfAbsent(variable.id(), ignored -> new IdentityHashMap<>()); + variableInstanceMap.put(entity, node); + } instanceList.add(node); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java index 33ff3861113..01b1d504ace 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/VariableUpdaterInfo.java @@ -16,4 +16,9 @@ public record VariableUpdaterInfo( @Nullable ShadowVariableLoopedVariableDescriptor shadowVariableLoopedDescriptor, MemberAccessor memberAccessor, Function calculator) { + + public VariableUpdaterInfo withGroupId(int groupId) { + return new VariableUpdaterInfo<>(id, groupId, variableDescriptor, shadowVariableLoopedDescriptor, memberAccessor, + calculator); + } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java index 9429c3d90e3..14fca76d61e 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java @@ -289,8 +289,12 @@ void shadowVariableListGraphEvents() { }; for (var visit : solution.getValues()) { - verifyAddEdge.accept(serviceReadyTime, visit, serviceStartTime, visit); - // serviceStartTime and serviceFinishTime are in the same group, so no edge between them! + + // If a visit does not have a concurrent group, all variables of that visit share the same node. + if (visit.getConcurrentValueGroup() != null) { + // serviceStartTime and serviceFinishTime are in the same group, so no edge between them! + verifyAddEdge.accept(serviceReadyTime, visit, serviceStartTime, visit); + } if (visit.getPreviousValue() != null) { verifyAddEdge.accept(serviceFinishTime, visit.getPreviousValue(), serviceReadyTime, visit); From c124adbcbf5648b81ba758c63016ab3ceb5e4fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Sat, 5 Jul 2025 10:33:11 +0200 Subject: [PATCH 3/3] Sonar and small cleanup --- .../declarative/AffectedEntitiesUpdater.java | 14 ++--- .../DefaultShadowVariableSessionFactory.java | 55 +++++++++---------- .../declarative/EntityVariablePair.java | 6 +- .../EntityVariableUpdaterLookup.java | 26 ++++----- .../VariableReferenceGraphBuilder.java | 4 +- .../support/VariableListenerSupportTest.java | 4 +- 6 files changed, 50 insertions(+), 59 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java index cb8d507af57..a1cb67280a7 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java @@ -96,7 +96,7 @@ private void updateLoopedStatusOfAffectedEntity(Object affectedEntity) { for (var node : entityVariablePairFunction.apply(affectedEntity)) { // All variables come from the same entity, // therefore all have the same looped marker. - shadowVariableLoopedDescriptor = node.variableReferences()[0].shadowVariableLoopedDescriptor(); + shadowVariableLoopedDescriptor = node.variableReferences().get(0).shadowVariableLoopedDescriptor(); if (graph.isLooped(loopedTracker, node.graphNodeId())) { isEntityLooped = true; break; @@ -117,12 +117,12 @@ private void updateLoopedStatusOfAffectedEntity(Object affectedEntity) { private boolean updateEntityShadowVariables(EntityVariablePair entityVariable, boolean isLooped) { var entity = entityVariable.entity(); var shadowVariableReferences = entityVariable.variableReferences(); - var loopDescriptor = shadowVariableReferences[0].shadowVariableLoopedDescriptor(); + var loopDescriptor = shadowVariableReferences.get(0).shadowVariableLoopedDescriptor(); var anyChanged = false; if (loopDescriptor != null) { - var oldLooped = (boolean) loopDescriptor.getValue(entity); - if (oldLooped != isLooped) { + var oldLooped = loopDescriptor.getValue(entity); + if (!Objects.equals(oldLooped, isLooped)) { // Loop status change; add to affected entities affectedEntities.add(entityVariable); anyChanged = true; @@ -130,14 +130,14 @@ private boolean updateEntityShadowVariables(EntityVariablePair entity } for (var shadowVariableReference : shadowVariableReferences) { - anyChanged |= updateShadowVariable(entityVariable, isLooped, shadowVariableReference, entity, anyChanged); + anyChanged |= updateShadowVariable(entityVariable, isLooped, shadowVariableReference, entity); } return anyChanged; } private boolean updateShadowVariable(EntityVariablePair entityVariable, boolean isLooped, - VariableUpdaterInfo shadowVariableReference, Object entity, boolean anyChanged) { + VariableUpdaterInfo shadowVariableReference, Object entity) { var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); if (isLooped) { if (oldValue != null) { @@ -180,7 +180,7 @@ public AffectedEntities(Consumer consumer) { } public void add(EntityVariablePair shadowVariable) { - var shadowVariableLoopedDescriptor = shadowVariable.variableReferences()[0].shadowVariableLoopedDescriptor(); + var shadowVariableLoopedDescriptor = shadowVariable.variableReferences().get(0).shadowVariableLoopedDescriptor(); if (shadowVariableLoopedDescriptor == null) { return; } 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 a3330029c95..b9b010c004d 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 @@ -23,7 +23,6 @@ import ai.timefold.solver.core.impl.util.MutableInt; import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; -import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -86,7 +85,7 @@ static VariableReferenceGraph buildSingleDirectionalParentGraph( topologicalSorter, changedVariableNotifier, entities); } - private static @NonNull List> + private static List> topologicallySortedDeclarativeShadowVariables( List> declarativeShadowVariables) { Map nameToIndex = new LinkedHashMap<>(); @@ -159,7 +158,16 @@ private static VariableReferenceGraph buildArbitraryGraph( // to "startTime" of some visit, and thus alias it. var declarativeShadowVariableToAliasMap = createGraphNodes(variableReferenceGraphBuilder, entities, declarativeShadowVariableDescriptors, variableIdToUpdater); + return buildVariableReferenceGraph(declarativeShadowVariableDescriptors, variableReferenceGraphBuilder, + declarativeShadowVariableToAliasMap, + graphCreator, entities); + } + private static VariableReferenceGraph buildVariableReferenceGraph( + List> declarativeShadowVariableDescriptors, + VariableReferenceGraphBuilder variableReferenceGraphBuilder, + Map, Set> declarativeShadowVariableToAliasMap, + IntFunction graphCreator, Object... entities) { // Create variable processors for each declarative shadow variable descriptor for (var declarativeShadowVariable : declarativeShadowVariableDescriptors) { var fromVariableId = declarativeShadowVariable.getVariableMetaModel(); @@ -178,8 +186,9 @@ private static VariableReferenceGraph buildArbitraryGraph( private record GroupVariableUpdaterInfo( List> sortedDeclarativeVariableDescriptors, List> allUpdaters, - VariableUpdaterInfo[] groupedUpdaters) { - public VariableUpdaterInfo[] getUpdatersForEntity(Object entity) { + List> groupedUpdaters) { + + public List> getUpdatersForEntity(Object entity) { for (var shadowVariableDescriptor : sortedDeclarativeVariableDescriptors) { for (var rootSource : shadowVariableDescriptor.getSources()) { if (rootSource.parentVariableType() == ParentVariableType.GROUP) { @@ -191,8 +200,9 @@ public VariableUpdaterInfo[] getUpdatersForEntity(Object entity) { } } } - return allUpdaters.toArray(VariableUpdaterInfo[]::new); + return allUpdaters; } + } private static Map, GroupVariableUpdaterInfo> @@ -215,9 +225,8 @@ public VariableUpdaterInfo[] getUpdatersForEntity(Object entity) { var out = new HashMap, GroupVariableUpdaterInfo>(); var allUpdaters = new ArrayList>(); - for (var i = 0; i < groupIndexToVariables.size(); i++) { - var entryKey = i; - var entryGroupVariables = groupIndexToVariables.get(i); + for (var entryKey = 0; entryKey < groupIndexToVariables.size(); entryKey++) { + var entryGroupVariables = groupIndexToVariables.get(entryKey); var updaters = new ArrayList>(); for (var declarativeShadowVariableDescriptor : entryGroupVariables) { var updater = new VariableUpdaterInfo<>( @@ -230,11 +239,8 @@ public VariableUpdaterInfo[] getUpdatersForEntity(Object entity) { updaters.add(updater); allUpdaters.add(updater); } - var updaterArray = updaters.toArray(VariableUpdaterInfo[]::new); - @SuppressWarnings("unchecked") - var groupVariableUpdaterInfo = new GroupVariableUpdaterInfo(sortedDeclarativeVariableDescriptors, - allUpdaters, - updaterArray); + var groupVariableUpdaterInfo = + new GroupVariableUpdaterInfo(sortedDeclarativeVariableDescriptors, allUpdaters, updaters); for (var declarativeShadowVariableDescriptor : entryGroupVariables) { out.put(declarativeShadowVariableDescriptor.getVariableMetaModel(), groupVariableUpdaterInfo); } @@ -265,20 +271,9 @@ private static VariableReferenceGraph buildArbitrarySingleEntityGrap declarativeShadowVariableDescriptors, variableIdToUpdater, (entity, declarativeShadowVariable, variableId) -> variableIdToGroupedUpdater.get(variableId) .getUpdatersForEntity(entity)); - - // Create variable processors for each declarative shadow variable descriptor - for (var declarativeShadowVariable : declarativeShadowVariableDescriptors) { - var fromVariableId = declarativeShadowVariable.getVariableMetaModel(); - createSourceChangeProcessors(entities, variableReferenceGraphBuilder, declarativeShadowVariable, fromVariableId); - var aliasSet = declarativeShadowVariableToAliasMap.get(fromVariableId); - if (aliasSet != null) { - createAliasToVariableChangeProcessors(variableReferenceGraphBuilder, aliasSet, fromVariableId); - } - } - - // Create the fixed edges in the graph - createFixedVariableRelationEdges(variableReferenceGraphBuilder, entities, declarativeShadowVariableDescriptors); - return variableReferenceGraphBuilder.build(graphCreator); + return buildVariableReferenceGraph(declarativeShadowVariableDescriptors, variableReferenceGraphBuilder, + declarativeShadowVariableToAliasMap, + graphCreator, entities); } private static Map, Set> createGraphNodes( @@ -287,20 +282,20 @@ private static VariableReferenceGraph buildArbitrarySingleEntityGrap EntityVariableUpdaterLookup variableIdToUpdaters) { return createGraphNodes(graph, entities, declarativeShadowVariableDescriptors, variableIdToUpdaters, (entity, declarativeShadowVariableDescriptor, - variableId) -> new VariableUpdaterInfo[] { new VariableUpdaterInfo<>( + variableId) -> Collections.singletonList(new VariableUpdaterInfo<>( variableId, variableIdToUpdaters.getNextId(), declarativeShadowVariableDescriptor, declarativeShadowVariableDescriptor.getEntityDescriptor().getShadowVariableLoopedDescriptor(), declarativeShadowVariableDescriptor.getMemberAccessor(), - declarativeShadowVariableDescriptor.getCalculator()::executeGetter) }); + declarativeShadowVariableDescriptor.getCalculator()::executeGetter))); } private static Map, Set> createGraphNodes( VariableReferenceGraphBuilder graph, Object[] entities, List> declarativeShadowVariableDescriptors, EntityVariableUpdaterLookup variableIdToUpdaters, - TriFunction, VariableMetaModel, VariableUpdaterInfo[]> entityVariableToUpdatersMapper) { + TriFunction, VariableMetaModel, List>> entityVariableToUpdatersMapper) { var result = new HashMap, Set>(); for (var entity : entities) { for (var declarativeShadowVariableDescriptor : declarativeShadowVariableDescriptors) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java index 1d1293d6f72..3c9726f39f7 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java @@ -1,11 +1,11 @@ package ai.timefold.solver.core.impl.domain.variable.declarative; -import java.util.Arrays; +import java.util.List; import org.jspecify.annotations.NullMarked; @NullMarked -public record EntityVariablePair(Object entity, VariableUpdaterInfo[] variableReferences, +public record EntityVariablePair(Object entity, List> variableReferences, int graphNodeId) { @Override public boolean equals(Object object) { @@ -21,6 +21,6 @@ public int hashCode() { @Override public String toString() { - return entity + ":" + Arrays.stream(variableReferences).map(VariableUpdaterInfo::id).toList(); + return entity + ":" + variableReferences.stream().map(VariableUpdaterInfo::id).toList(); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java index e3c035682ae..5a011f50102 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariableUpdaterLookup.java @@ -2,6 +2,7 @@ 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.Function; @@ -15,15 +16,17 @@ public class EntityVariableUpdaterLookup { private final Supplier> entityLookupSupplier; private int nextId = 0; - private record Lookup(Function[]> getter, - BiConsumer[]>> setter) { - VariableUpdaterInfo[] getUpdaters(Object entity) { + private record Lookup(Function>> getter, + BiConsumer>>> setter) { + + List> getUpdaters(Object entity) { return getter.apply(entity); } - void setUpdaters(Object entity, Supplier[]> updatersSupplier) { + void setUpdaters(Object entity, Supplier>> updatersSupplier) { setter.accept(entity, updatersSupplier); } + } private EntityVariableUpdaterLookup(Supplier> entityLookupSupplier) { @@ -33,7 +36,7 @@ private EntityVariableUpdaterLookup(Supplier> entityLookupSupp public static EntityVariableUpdaterLookup entityIndependentLookup() { Supplier> lookupSupplier = () -> { - var sharedValue = new MutableReference[]>(null); + var sharedValue = new MutableReference>>(null); return new Lookup<>(ignored -> sharedValue.getValue(), (ignored, valueSupplier) -> { if (sharedValue.getValue() == null) { @@ -46,22 +49,15 @@ public static EntityVariableUpdaterLookup entityIndepende public static EntityVariableUpdaterLookup entityDependentLookup() { Supplier> lookupSupplier = () -> { - var valueMap = new IdentityHashMap[]>(); + var valueMap = new IdentityHashMap>>(); return new Lookup<>(valueMap::get, (entity, valueSupplier) -> valueMap.computeIfAbsent(entity, ignored -> valueSupplier.get())); }; return new EntityVariableUpdaterLookup<>(lookupSupplier); } - public VariableUpdaterInfo[] getUpdatersForVariableOnEntity(VariableMetaModel variableMetaModel, - Object entity) { - var entityLookup = variableToEntityLookup.get(variableMetaModel); - return entityLookup.getUpdaters(entity); - } - - public VariableUpdaterInfo[] computeUpdatersForVariableOnEntity(VariableMetaModel variableMetaModel, - Object entity, - Supplier[]> updatersSupplier) { + public List> computeUpdatersForVariableOnEntity(VariableMetaModel variableMetaModel, + Object entity, Supplier>> updatersSupplier) { var entityLookup = variableToEntityLookup.computeIfAbsent(variableMetaModel, ignored -> entityLookupSupplier.get()); entityLookup.setUpdaters(entity, updatersSupplier); return entityLookup.getUpdaters(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 f1a20845b4f..85136a03824 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 @@ -35,8 +35,8 @@ public VariableReferenceGraphBuilder(ChangedVariableNotifier changedV isGraphFixed = true; } - public void addVariableReferenceEntity(Entity_ entity, VariableUpdaterInfo[] variableReferences) { - var groupId = variableReferences[0].groupId(); + public void addVariableReferenceEntity(Entity_ entity, List> variableReferences) { + var groupId = variableReferences.get(0).groupId(); var instanceMap = variableGroupIdToInstanceMap.get(groupId); var instance = instanceMap == null ? null : instanceMap.get(entity); if (instance != null) { diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java index 14fca76d61e..d9d97c168f6 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupportTest.java @@ -10,7 +10,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -178,7 +177,8 @@ public MockTopologicalOrderGraph(int size) { public void withNodeData(List> nodes) { nodeToEntities = nodes.stream().map(EntityVariablePair::entity).toArray(Object[]::new); nodeToVariableMetamodel = nodes.stream() - .map(e -> Arrays.stream(e.variableReferences()).map(VariableUpdaterInfo::id) + .map(e -> e.variableReferences().stream() + .map(VariableUpdaterInfo::id) .toArray(VariableMetaModel[]::new)) .toArray(VariableMetaModel[][]::new); }