From c3687525149d2d9b0ed26dc156d49c5a6ecc7a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Thu, 29 May 2025 13:43:16 +0200 Subject: [PATCH] chore: simplify DSV's affected entity update --- .../declarative/AffectedEntitiesUpdater.java | 66 +------------------ .../DefaultVariableReferenceGraph.java | 8 +-- 2 files changed, 3 insertions(+), 71 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 e6682206b4f..ddd366875aa 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 @@ -4,12 +4,9 @@ import java.util.List; import java.util.Objects; import java.util.PriorityQueue; -import java.util.Set; import java.util.function.Consumer; -import java.util.function.Function; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; -import ai.timefold.solver.core.impl.util.LinkedIdentityHashSet; final class AffectedEntitiesUpdater implements Consumer { @@ -17,24 +14,19 @@ final class AffectedEntitiesUpdater // From WorkingReferenceGraph. private final BaseTopologicalOrderGraph graph; private final List> instanceList; // Immutable. - private final Function>> entityVariablePairFunction; private final ChangedVariableNotifier changedVariableNotifier; // Internal state; expensive to create, therefore we reuse. - private final AffectedEntities affectedEntities; private final LoopedTracker loopedTracker; private final BitSet visited; private final PriorityQueue changeQueue; AffectedEntitiesUpdater(BaseTopologicalOrderGraph graph, List> instanceList, - Function>> entityVariablePairFunction, ChangedVariableNotifier changedVariableNotifier) { this.graph = graph; this.instanceList = instanceList; - this.entityVariablePairFunction = entityVariablePairFunction; this.changedVariableNotifier = changedVariableNotifier; var instanceCount = instanceList.size(); - this.affectedEntities = new AffectedEntities<>(this::updateLoopedStatusOfAffectedEntity); this.loopedTracker = new LoopedTracker(instanceCount); this.visited = new BitSet(instanceCount); this.changeQueue = new PriorityQueue<>(instanceCount); @@ -64,7 +56,6 @@ public void accept(BitSet changed) { } } - affectedEntities.processAndClear(); // Prepare for the next time updateChanged() is called. // No need to clear changeQueue, as that already finishes empty. loopedTracker.clear(); @@ -90,30 +81,6 @@ private void initializeChangeQueue(BitSet changed) { changed.clear(); } - private void updateLoopedStatusOfAffectedEntity(Object affectedEntity) { - ShadowVariableLoopedVariableDescriptor shadowVariableLoopedDescriptor = null; - var isEntityLooped = false; - for (var node : entityVariablePairFunction.apply(affectedEntity)) { - // All variables come from the same entity, - // therefore all have the same looped marker. - shadowVariableLoopedDescriptor = node.variableReference().shadowVariableLoopedDescriptor(); - if (graph.isLooped(loopedTracker, node.graphNodeId())) { - isEntityLooped = true; - break; - } - } - if (shadowVariableLoopedDescriptor == null) { - // At this point, affectedEntity is guaranteed to have looped marker. - // Otherwise AffectedEntities would not have sent it here. - throw new IllegalStateException("Impossible state: loop marker descriptor does not exist."); - } - var oldValue = shadowVariableLoopedDescriptor.getValue(affectedEntity); - if (!Objects.equals(oldValue, isEntityLooped)) { - changeShadowVariableAndNotify(shadowVariableLoopedDescriptor, affectedEntity, isEntityLooped); - } - - } - private boolean updateShadowVariable(EntityVariablePair entityVariable, boolean isLooped) { var entity = entityVariable.entity(); var shadowVariableReference = entityVariable.variableReference(); @@ -122,21 +89,19 @@ private boolean updateShadowVariable(EntityVariablePair entityVariabl if (loopDescriptor != null) { var oldLooped = (boolean) loopDescriptor.getValue(entity); if (oldLooped != isLooped) { - // Loop status change; add to affected entities - affectedEntities.add(entityVariable); + // Loop status change; mark it + changeShadowVariableAndNotify(loopDescriptor, entity, isLooped); } } 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; } @@ -157,31 +122,4 @@ private void changeShadowVariableAndNotify(VariableDescriptor variabl changedVariableNotifier.afterVariableChanged().accept(variableDescriptor, entity); } - private static final class AffectedEntities { - - private final Consumer consumer; - private final Set entitiesForLoopedVarUpdateSet; - - public AffectedEntities(Consumer consumer) { - this.consumer = consumer; - this.entitiesForLoopedVarUpdateSet = new LinkedIdentityHashSet<>(); - } - - public void add(EntityVariablePair shadowVariable) { - var shadowVariableLoopedDescriptor = shadowVariable.variableReference().shadowVariableLoopedDescriptor(); - if (shadowVariableLoopedDescriptor == null) { - return; - } - entitiesForLoopedVarUpdateSet.add(shadowVariable.entity()); - } - - public void processAndClear() { - for (var entity : entitiesForLoopedVarUpdateSet) { - consumer.accept(entity); - } - entitiesForLoopedVarUpdateSet.clear(); - } - - } - } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java index 2b5fb00034c..38c78eb7f34 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultVariableReferenceGraph.java @@ -49,7 +49,6 @@ public DefaultVariableReferenceGraph(VariableReferenceGraphBuilder ou graph.withNodeData(instanceList); changed = new BitSet(instanceCount); - var entityToVariableReferenceMap = new IdentityHashMap>>(); var visited = Collections.newSetFromMap(new IdentityHashMap<>()); for (var instance : instanceList) { var entity = instance.entity(); @@ -58,21 +57,16 @@ public DefaultVariableReferenceGraph(VariableReferenceGraphBuilder ou afterVariableChanged(variableId, entity); } } - entityToVariableReferenceMap.computeIfAbsent(entity, ignored -> new ArrayList<>()) - .add(instance); } for (var fixedEdgeEntry : outerGraph.fixedEdges.entrySet()) { for (var toEdge : fixedEdgeEntry.getValue()) { addEdge(fixedEdgeEntry.getKey(), toEdge); } } - // Immutable optimized version of the map, now that it won't be updated anymore. - var immutableEntityToVariableReferenceMap = mapOfListsDeepCopyOf(entityToVariableReferenceMap); // This mutable structure is created once, and reused from there on. // Otherwise its internal collections were observed being re-created so often // that the allocation of arrays would become a major bottleneck. - affectedEntitiesUpdater = new AffectedEntitiesUpdater<>(graph, instanceList, immutableEntityToVariableReferenceMap::get, - outerGraph.changedVariableNotifier); + affectedEntitiesUpdater = new AffectedEntitiesUpdater<>(graph, instanceList, outerGraph.changedVariableNotifier); } @Override