Skip to content

Commit 7d614cf

Browse files
triceotriceo
committed
Improve performance of the system
1 parent 107254f commit 7d614cf

9 files changed

Lines changed: 248 additions & 48 deletions

File tree

‎core/src/main/java/ai/timefold/solver/core/api/domain/solution/PlanningEntityCollectionProperty.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import java.lang.annotation.Retention;
88
import java.lang.annotation.Target;
99
import java.util.Collection;
10+
import java.util.LinkedHashSet;
11+
import java.util.List;
12+
import java.util.SortedSet;
1013

1114
import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
1215
import ai.timefold.solver.core.api.score.director.ScoreDirector;
@@ -16,6 +19,9 @@
1619
* <p>
1720
* Every element in the planning entity collection should have the {@link PlanningEntity} annotation.
1821
* Every element in the planning entity collection will be added to the {@link ScoreDirector}.
22+
* <p>
23+
* For solver reproducibility, the collection must have a deterministic, stable iteration order.
24+
* It is recommended to use a {@link List}, {@link LinkedHashSet} or {@link SortedSet}.
1925
*/
2026
@Target({ METHOD, FIELD })
2127
@Retention(RUNTIME)

‎core/src/main/java/ai/timefold/solver/core/api/domain/solution/ProblemFactCollectionProperty.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import java.lang.annotation.Retention;
88
import java.lang.annotation.Target;
99
import java.util.Collection;
10+
import java.util.LinkedHashSet;
11+
import java.util.List;
12+
import java.util.SortedSet;
1013

1114
import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
1215
import ai.timefold.solver.core.api.score.stream.ConstraintFactory;
@@ -21,6 +24,9 @@
2124
* <p>
2225
* Do not annotate {@link PlanningEntity planning entities} as problem facts:
2326
* they are automatically available as facts for {@link ConstraintFactory#forEach(Class)}.
27+
* <p>
28+
* For solver reproducibility, the collection must have a deterministic, stable iteration order.
29+
* It is recommended to use a {@link List}, {@link LinkedHashSet} or {@link SortedSet}.
2430
*
2531
* @see ProblemFactProperty
2632
*/

‎core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeProvider.java‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@
77
import java.lang.annotation.Retention;
88
import java.lang.annotation.Target;
99
import java.util.Collection;
10+
import java.util.LinkedHashSet;
11+
import java.util.List;
12+
import java.util.SortedSet;
1013

1114
import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
15+
import ai.timefold.solver.core.api.domain.variable.PlanningListVariable;
1216
import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
1317
import ai.timefold.solver.core.api.solver.SolverFactory;
1418
import ai.timefold.solver.core.api.solver.change.ProblemChange;
@@ -17,23 +21,32 @@
1721

1822
/**
1923
* Provides the planning values that can be used for a {@link PlanningVariable}.
24+
*
2025
* <p>
2126
* This is specified on a getter of a java bean property (or directly on a field)
2227
* which returns a {@link Collection} or {@link ValueRange}.
2328
* A {@link Collection} is implicitly converted to a {@link ValueRange}.
29+
* For solver reproducibility, the collection must have a deterministic, stable iteration order.
30+
* It is recommended to use a {@link List}, {@link LinkedHashSet} or {@link SortedSet}.
2431
*
2532
* <p>
26-
* If two values in a value range are equal according to {@link Object#equals(Object)},
27-
* then they are considered the same value
28-
* and will only be present once in the value range,
29-
* regardless of how many times they are present originally.
33+
* Value ranges are not allowed to contain {@code null} values.
34+
* The solver will automatically add a null to any range
35+
* when {@link PlanningVariable#allowsUnassigned()} or {@link PlanningListVariable#allowsUnassignedValues()} is true.
36+
*
37+
* <p>
38+
* Value ranges are not allowed to contain multiple copies of the same object,
39+
* as defined by {@code ==}.
40+
* It is recommended that the value range never contains two objects
41+
* that are equal according to {@link Object#equals(Object)},
42+
* but this is not enforced to not depend on user-defined {@link Object#equals(Object)} implementations.
43+
* Having duplicates in a value range can lead to unexpected behavior,
44+
* and skews selection probabilities in random selection algorithms.
3045
*
3146
* <p>
3247
* Value ranges are not allowed to change during solving.
3348
* This is especially important for value ranges defined on {@link PlanningEntity}-annotated classes;
3449
* these must never depend on any of that entity's variables, or on any other entity's variables.
35-
*
36-
* <p>
3750
* If you need to change a value range defined on an entity,
3851
* trigger a {@link ProblemChange} for that entity or restart the solver with an updated planning solution.
3952
* If you need to change a value range defined on a planning solution,

‎core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/ListValueRange.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package ai.timefold.solver.core.impl.domain.valuerange.buildin.collection;
22

3+
import java.util.Collections;
34
import java.util.Iterator;
45
import java.util.List;
56
import java.util.Random;
67
import java.util.Set;
78

89
import ai.timefold.solver.core.impl.domain.valuerange.AbstractCountableValueRange;
910
import ai.timefold.solver.core.impl.heuristic.selector.common.iterator.CachedListRandomIterator;
11+
import ai.timefold.solver.core.impl.util.CollectionUtils;
1012

1113
import org.jspecify.annotations.NonNull;
1214
import org.jspecify.annotations.NullMarked;
@@ -15,7 +17,7 @@
1517
@NullMarked
1618
public final class ListValueRange<T> extends AbstractCountableValueRange<T> {
1719

18-
private static final int LIST_SIZE_LOOKUP_LIMIT = 10;
20+
private static final int LIST_SIZE_LOOKUP_LIMIT = 10; // Selected arbitrarily.
1921

2022
private final List<T> list;
2123
private @Nullable Set<T> lookupSet; // Initialized lazily for large lists.
@@ -32,7 +34,8 @@ public long getSize() {
3234
@Override
3335
public T get(long index) {
3436
if (index > Integer.MAX_VALUE) {
35-
throw new IndexOutOfBoundsException("The index (" + index + ") must fit in an int.");
37+
throw new IndexOutOfBoundsException("The index (%d) must fit in an int."
38+
.formatted(index));
3639
}
3740
return list.get((int) index);
3841
}
@@ -41,7 +44,11 @@ public T get(long index) {
4144
public boolean contains(@Nullable T value) {
4245
if (list.size() > LIST_SIZE_LOOKUP_LIMIT) {
4346
if (lookupSet == null) {
44-
lookupSet = Set.copyOf(list);
47+
lookupSet = Collections.newSetFromMap(CollectionUtils.newIdentityHashMap(list.size()));
48+
lookupSet.addAll(list);
49+
if (lookupSet.size() != list.size()) {
50+
throw new IllegalStateException("The value range contains duplicate values: " + list);
51+
}
4552
}
4653
return lookupSet.contains(value);
4754
} else { // For small lists, sequential scanning is not a performance issue.
@@ -60,9 +67,8 @@ public boolean contains(@Nullable T value) {
6067
}
6168

6269
@Override
63-
public String toString() {
64-
// Formatting: interval (mathematics) ISO 31-11
65-
return list.isEmpty() ? "[]" : "[" + list.get(0) + "-" + list.get(list.size() - 1) + "]";
70+
public String toString() { // Formatting: interval (mathematics) ISO 31-11
71+
return list.isEmpty() ? "[]" : "[%s-%s]".formatted(list.get(0), list.get(list.size() - 1));
6672
}
6773

6874
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/SetValueRange.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package ai.timefold.solver.core.impl.domain.valuerange.buildin.collection;
2+
3+
import java.util.Iterator;
4+
import java.util.List;
5+
import java.util.Random;
6+
import java.util.Set;
7+
import java.util.stream.Collectors;
8+
9+
import ai.timefold.solver.core.impl.domain.valuerange.AbstractCountableValueRange;
10+
import ai.timefold.solver.core.impl.heuristic.selector.common.iterator.CachedListRandomIterator;
11+
12+
import org.jspecify.annotations.NonNull;
13+
import org.jspecify.annotations.NullMarked;
14+
import org.jspecify.annotations.Nullable;
15+
16+
@NullMarked
17+
public final class SetValueRange<T> extends AbstractCountableValueRange<T> {
18+
19+
private static final int VALUES_TO_LIST_IN_TO_STRING = 3;
20+
private static final String VALUE_DELIMITER = ", ";
21+
22+
private final Set<T> set;
23+
private @Nullable List<T> list; // To allow for random access; null until first access.
24+
25+
public SetValueRange(Set<T> set) {
26+
this.set = set;
27+
}
28+
29+
@Override
30+
public long getSize() {
31+
return set.size();
32+
}
33+
34+
@Override
35+
public T get(long index) {
36+
if (index > Integer.MAX_VALUE) {
37+
throw new IndexOutOfBoundsException("The index (%d) must fit in an int."
38+
.formatted(index));
39+
}
40+
return getList().get((int) index);
41+
}
42+
43+
private List<T> getList() {
44+
if (list == null) {
45+
list = List.copyOf(set);
46+
}
47+
return list;
48+
}
49+
50+
@Override
51+
public boolean contains(@Nullable T value) {
52+
return set.contains(value);
53+
}
54+
55+
@Override
56+
public @NonNull Iterator<T> createOriginalIterator() {
57+
return set.iterator();
58+
}
59+
60+
@Override
61+
public @NonNull Iterator<T> createRandomIterator(@NonNull Random workingRandom) {
62+
return new CachedListRandomIterator<>(getList(), workingRandom);
63+
}
64+
65+
@Override
66+
public String toString() { // Formatting: interval (mathematics) ISO 31-11, shorten long sets
67+
var suffix = set.size() > VALUES_TO_LIST_IN_TO_STRING ? VALUE_DELIMITER + "...}" : "}";
68+
return set.isEmpty() ? "{}"
69+
: set.stream()
70+
.limit(VALUES_TO_LIST_IN_TO_STRING)
71+
.map(Object::toString)
72+
.collect(Collectors.joining(VALUE_DELIMITER, "{", suffix));
73+
}
74+
75+
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java‎

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,22 @@
44
import java.util.Arrays;
55
import java.util.Collection;
66
import java.util.Collections;
7+
import java.util.LinkedHashSet;
78
import java.util.List;
9+
import java.util.Set;
10+
import java.util.SortedSet;
811

912
import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
1013
import ai.timefold.solver.core.api.domain.valuerange.CountableValueRange;
1114
import ai.timefold.solver.core.api.domain.valuerange.ValueRange;
1215
import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider;
16+
import ai.timefold.solver.core.api.domain.variable.PlanningListVariable;
1317
import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
1418
import ai.timefold.solver.core.config.util.ConfigUtils;
1519
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
1620
import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor;
1721
import ai.timefold.solver.core.impl.domain.valuerange.buildin.collection.ListValueRange;
22+
import ai.timefold.solver.core.impl.domain.valuerange.buildin.collection.SetValueRange;
1823
import ai.timefold.solver.core.impl.domain.variable.descriptor.GenuineVariableDescriptor;
1924

2025
/**
@@ -103,23 +108,35 @@ protected <Value_> ValueRange<Value_> readValueRange(Object bean) {
103108
.formatted(ValueRangeProvider.class.getSimpleName(), memberAccessor, bean, valueRangeObject));
104109
}
105110
ValueRange<Value_> valueRange;
106-
if (collectionWrapping || arrayWrapping) {
107-
List<Value_> list = collectionWrapping ? transformCollectionToUniqueList((Collection<Value_>) valueRangeObject)
108-
: transformArrayToUniqueList(valueRangeObject);
109-
// Don't check the entire list for performance reasons, but do check common pitfalls
110-
if (!list.isEmpty() && (list.get(0) == null || list.get(list.size() - 1) == null)) {
111-
throw new IllegalStateException(
112-
"""
113-
The @%s-annotated member (%s) called on bean (%s) must not return a %s (%s) with an element that is null.
114-
Maybe remove that null element from the dataset.
115-
Maybe use @%s(allowsUnassigned = true) instead."""
116-
.formatted(ValueRangeProvider.class.getSimpleName(),
117-
memberAccessor, bean,
118-
collectionWrapping ? Collection.class.getSimpleName() : "array",
119-
list,
120-
PlanningVariable.class.getSimpleName()));
121-
}
111+
if (arrayWrapping) {
112+
List<Value_> list = transformArrayToList(valueRangeObject);
113+
assertNullNotPresent(list, bean);
122114
valueRange = new ListValueRange<>(list);
115+
} else if (collectionWrapping) {
116+
var collection = (Collection<Value_>) valueRangeObject;
117+
if (collection instanceof Set<Value_> set) {
118+
if (set.contains(null)) {
119+
throw new IllegalStateException("""
120+
The @%s-annotated member (%s) called on bean (%s) returns a Set (%s) with a null element.
121+
Maybe remove that null element from the dataset.
122+
Maybe use @%s(allowsUnassigned = true) or @%s(allowsUnassignedValues = true) instead."""
123+
.formatted(ValueRangeProvider.class.getSimpleName(), memberAccessor, bean, set,
124+
PlanningVariable.class.getSimpleName(), PlanningListVariable.class.getSimpleName()));
125+
}
126+
if (collection instanceof SortedSet<Value_> || collection instanceof LinkedHashSet<Value_>) {
127+
valueRange = new SetValueRange<>(set);
128+
} else {
129+
throw new IllegalStateException("""
130+
The @%s-annotated member (%s) called on bean (%s) returns a Set (%s) with undefined iteration order.
131+
Maybe use SortedSet or LinkedHashSet to ensure solver reproducibility.
132+
"""
133+
.formatted(ValueRangeProvider.class.getSimpleName(), memberAccessor, bean, set.getClass()));
134+
}
135+
} else {
136+
List<Value_> list = transformCollectionToList(collection);
137+
assertNullNotPresent(list, bean);
138+
valueRange = new ListValueRange<>(list);
139+
}
123140
} else {
124141
valueRange = (ValueRange<Value_>) valueRangeObject;
125142
}
@@ -133,6 +150,20 @@ protected <Value_> ValueRange<Value_> readValueRange(Object bean) {
133150
return valueRange;
134151
}
135152

153+
private void assertNullNotPresent(List<?> list, Object bean) {
154+
// Don't check the entire list for performance reasons, but do check common pitfalls
155+
if (!list.isEmpty() && (list.get(0) == null || list.get(list.size() - 1) == null)) {
156+
throw new IllegalStateException("""
157+
The @%s-annotated member (%s) called on bean (%s) must not return a %s (%s) with an element that is null.
158+
Maybe remove that null element from the dataset.
159+
Maybe remove that null element from the dataset.
160+
Maybe use @%s(allowsUnassigned = true) or @%s(allowsUnassignedValues = true) instead."""
161+
.formatted(ValueRangeProvider.class.getSimpleName(), memberAccessor, bean,
162+
collectionWrapping ? Collection.class.getSimpleName() : "array", list,
163+
PlanningVariable.class.getSimpleName(), PlanningListVariable.class.getSimpleName()));
164+
}
165+
}
166+
136167
@SuppressWarnings("unchecked")
137168
protected long readValueRangeSize(Object bean) {
138169
var valueRangeObject = memberAccessor.executeGetter(bean);
@@ -162,27 +193,26 @@ protected long readValueRangeSize(Object bean) {
162193
}
163194
}
164195

165-
private <T> List<T> transformCollectionToUniqueList(Collection<T> collection) {
196+
private <T> List<T> transformCollectionToList(Collection<T> collection) {
166197
if (collection.isEmpty()) {
167198
return Collections.emptyList();
199+
} else if (collection instanceof List<T> list) {
200+
return list;
201+
} else {
202+
return List.copyOf(collection);
168203
}
169-
return collection.stream()
170-
.distinct()
171-
.toList();
172204
}
173205

174206
@SuppressWarnings("unchecked")
175-
public static <Value_> List<Value_> transformArrayToUniqueList(Object arrayObject) {
207+
public static <Value_> List<Value_> transformArrayToList(Object arrayObject) {
176208
if (arrayObject == null) {
177209
return Collections.emptyList();
178210
}
179211
var array = (Value_[]) arrayObject;
180212
if (array.length == 0) {
181213
return Collections.emptyList();
182214
}
183-
return Arrays.stream(array)
184-
.distinct()
185-
.toList();
215+
return Arrays.asList(array);
186216
}
187217

188218
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractValueRangeDescriptor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public boolean mightContainEntity() {
4444

4545
protected <T> ValueRange<T> doNullInValueRangeWrapping(ValueRange<T> valueRange) {
4646
if (addNullInValueRange) {
47-
valueRange = new NullAllowingCountableValueRange<>((CountableValueRange) valueRange);
47+
valueRange = new NullAllowingCountableValueRange<>((CountableValueRange<T>) valueRange);
4848
}
4949
return valueRange;
5050
}

0 commit comments

Comments
 (0)