Skip to content

Commit 3aee054

Browse files
committed
revert: min/max collector perf improvement was causing score corruptions
1 parent 5443347 commit 3aee054

File tree

1 file changed

+20
-18
lines changed

1 file changed

+20
-18
lines changed
Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,22 @@
11
package ai.timefold.solver.core.impl.score.stream.collector;
22

33
import java.util.Comparator;
4+
import java.util.LinkedHashMap;
5+
import java.util.Map;
46
import java.util.NavigableMap;
57
import java.util.TreeMap;
68
import java.util.function.Function;
79

810
import ai.timefold.solver.core.impl.util.ConstantLambdaUtils;
911
import ai.timefold.solver.core.impl.util.MutableInt;
1012

11-
public final class MinMaxUndoableActionable<Result_, Property_>
12-
implements UndoableActionable<Result_, Result_> {
13-
13+
public final class MinMaxUndoableActionable<Result_, Property_> implements UndoableActionable<Result_, Result_> {
1414
private final boolean isMin;
15-
private final NavigableMap<Property_, ItemCount<Result_>> propertyToItemCountMap;
15+
private final NavigableMap<Property_, Map<Result_, MutableInt>> propertyToItemCountMap;
1616
private final Function<? super Result_, ? extends Property_> propertyFunction;
1717

18-
private MinMaxUndoableActionable(boolean isMin, NavigableMap<Property_, ItemCount<Result_>> propertyToItemCountMap,
18+
private MinMaxUndoableActionable(boolean isMin,
19+
NavigableMap<Property_, Map<Result_, MutableInt>> propertyToItemCountMap,
1920
Function<? super Result_, ? extends Property_> propertyFunction) {
2021
this.isMin = isMin;
2122
this.propertyToItemCountMap = propertyToItemCountMap;
@@ -39,29 +40,30 @@ public static <Result> MinMaxUndoableActionable<Result, Result> maxCalculator(Co
3940
}
4041

4142
public static <Result, Property extends Comparable<? super Property>> MinMaxUndoableActionable<Result, Property>
42-
minCalculator(Function<? super Result, ? extends Property> propertyMapper) {
43+
minCalculator(
44+
Function<? super Result, ? extends Property> propertyMapper) {
4345
return new MinMaxUndoableActionable<>(true, new TreeMap<>(), propertyMapper);
4446
}
4547

4648
public static <Result, Property extends Comparable<? super Property>> MinMaxUndoableActionable<Result, Property>
47-
maxCalculator(Function<? super Result, ? extends Property> propertyMapper) {
49+
maxCalculator(
50+
Function<? super Result, ? extends Property> propertyMapper) {
4851
return new MinMaxUndoableActionable<>(false, new TreeMap<>(), propertyMapper);
4952
}
5053

5154
@Override
5255
public Runnable insert(Result_ item) {
5356
Property_ key = propertyFunction.apply(item);
54-
var value = propertyToItemCountMap.get(key);
55-
if (value == null) {
56-
value = new ItemCount<>(item, new MutableInt());
57-
propertyToItemCountMap.put(key, value);
58-
}
59-
var count = value.count;
57+
Map<Result_, MutableInt> itemCountMap = propertyToItemCountMap.computeIfAbsent(key, ignored -> new LinkedHashMap<>());
58+
MutableInt count = itemCountMap.computeIfAbsent(item, ignored -> new MutableInt());
6059
count.increment();
6160

6261
return () -> {
6362
if (count.decrement() == 0) {
64-
propertyToItemCountMap.remove(key);
63+
itemCountMap.remove(item);
64+
if (itemCountMap.isEmpty()) {
65+
propertyToItemCountMap.remove(key);
66+
}
6567
}
6668
};
6769
}
@@ -71,11 +73,11 @@ public Result_ result() {
7173
if (propertyToItemCountMap.isEmpty()) {
7274
return null;
7375
}
74-
var itemCount = isMin ? propertyToItemCountMap.firstEntry().getValue() : propertyToItemCountMap.lastEntry().getValue();
75-
return itemCount.item;
76+
return isMin ? getFirstKey(propertyToItemCountMap.firstEntry().getValue())
77+
: getFirstKey(propertyToItemCountMap.lastEntry().getValue());
7678
}
7779

78-
private record ItemCount<Item_>(Item_ item, MutableInt count) {
80+
private static <Key_> Key_ getFirstKey(Map<Key_, ?> map) {
81+
return map.keySet().iterator().next();
7982
}
80-
8183
}

0 commit comments

Comments
 (0)