From 93ba91ecbf369bbe4327d92733da8d908bc2f649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Mon, 14 Aug 2023 08:27:47 +0200 Subject: [PATCH] perf: read value range size instead of the value range To read an entire value range to get its size, the value range needs to be first converted to a collection. This is costly and we therefore implement a shortcut that allows to read the size without creating a collection. --- .../domain/valuerange/ValueRangeFactory.java | 2 + .../descriptor/SolutionDescriptor.java | 4 +- .../AbstractUncountableValueRange.java | 4 ++ .../buildin/primdouble/DoubleValueRange.java | 4 ++ ...tractFromPropertyValueRangeDescriptor.java | 49 +++++++++++++++++-- .../CompositeValueRangeDescriptor.java | 20 ++++++++ ...EntityIndependentValueRangeDescriptor.java | 9 ++++ ...romEntityPropertyValueRangeDescriptor.java | 5 ++ ...mSolutionPropertyValueRangeDescriptor.java | 10 ++++ .../descriptor/ValueRangeDescriptor.java | 9 ++++ .../descriptor/GenuineVariableDescriptor.java | 7 +-- .../FromEntityPropertyValueSelector.java | 3 +- 12 files changed, 112 insertions(+), 14 deletions(-) diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeFactory.java b/core/core-impl/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeFactory.java index 33c36243cf..fd5c958acc 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeFactory.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeFactory.java @@ -82,7 +82,9 @@ public static CountableValueRange createLongValueRange(long from, long to, * @param from inclusive minimum * @param to exclusive maximum, {@code >= from} * @return never null + * @deprecated Prefer {@link #createBigDecimalValueRange(BigDecimal, BigDecimal)}. */ + @Deprecated(forRemoval = true, since = "1.1.0") public static ValueRange createDoubleValueRange(double from, double to) { return new DoubleValueRange(from, to); } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java index 9bd1584c52..bc2b155a27 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java @@ -1010,7 +1010,9 @@ public long getProblemScale(Solution_ solution) { * @return {@code >= 0} */ public int countUninitialized(Solution_ solution) { - return countUninitializedVariables(solution) + countUnassignedListVariableValues(solution); + int uninitializedVariableCount = countUninitializedVariables(solution); + int uninitializedValueCount = countUnassignedListVariableValues(solution); + return uninitializedValueCount + uninitializedVariableCount; } /** diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/AbstractUncountableValueRange.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/AbstractUncountableValueRange.java index 2e1a18bcd2..d71ec9380c 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/AbstractUncountableValueRange.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/AbstractUncountableValueRange.java @@ -9,7 +9,11 @@ * * @see ValueRange * @see ValueRangeFactory + * @deprecated Uncountable value ranges were never fully supported in many places throughout the solver + * and therefore never gained traction. + * Use {@link CountableValueRange} instead, and configure a step. */ +@Deprecated(forRemoval = true, since = "1.1.0") public abstract class AbstractUncountableValueRange implements ValueRange { } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primdouble/DoubleValueRange.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primdouble/DoubleValueRange.java index 6c0b3f3836..037ce4d093 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primdouble/DoubleValueRange.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primdouble/DoubleValueRange.java @@ -5,13 +5,17 @@ import java.util.Random; import ai.timefold.solver.core.impl.domain.valuerange.AbstractUncountableValueRange; +import ai.timefold.solver.core.impl.domain.valuerange.buildin.bigdecimal.BigDecimalValueRange; import ai.timefold.solver.core.impl.domain.valuerange.util.ValueRangeIterator; /** * Note: Floating point numbers (float, double) cannot represent a decimal number correctly. * If floating point numbers leak into the scoring function, they are likely to cause score corruptions. * To avoid that, use either {@link java.math.BigDecimal} or fixed-point arithmetic. + * + * @deprecated Prefer {@link BigDecimalValueRange}. */ +@Deprecated(forRemoval = true, since = "1.1.0") public class DoubleValueRange extends AbstractUncountableValueRange { private final double from; diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java index 0b7a9e8513..f9c4bda2f8 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java @@ -1,7 +1,9 @@ package ai.timefold.solver.core.impl.domain.valuerange.descriptor; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedList; import java.util.List; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; @@ -138,12 +140,49 @@ protected ValueRange readValueRange(Object bean) { return valueRange; } + protected long readValueRangeSize(Object bean) { + Object valueRangeObject = memberAccessor.executeGetter(bean); + if (valueRangeObject == null) { + throw new IllegalStateException("The @" + ValueRangeProvider.class.getSimpleName() + + " annotated member (" + memberAccessor + + ") called on bean (" + bean + + ") must not return a null valueRangeObject (" + valueRangeObject + ")."); + } + int size = addNullInValueRange ? 1 : 0; + if (collectionWrapping) { + return size + ((Collection) valueRangeObject).size(); + } else if (arrayWrapping) { + return size + Array.getLength(valueRangeObject); + } + ValueRange valueRange = (ValueRange) valueRangeObject; + if (valueRange.isEmpty()) { + throw new IllegalStateException("The @" + ValueRangeProvider.class.getSimpleName() + + " annotated member (" + memberAccessor + + ") called on bean (" + bean + + ") must not return an empty valueRange (" + valueRangeObject + ").\n" + + "Maybe apply overconstrained planning as described in the documentation."); + } else if (valueRange instanceof CountableValueRange countableValueRange) { + return size + countableValueRange.getSize(); + } else { + throw new UnsupportedOperationException("The @" + ValueRangeProvider.class.getSimpleName() + + " annotated member (" + memberAccessor + + ") called on bean (" + bean + + ") is not countable and therefore does not support getSize()."); + } + } + private List transformCollectionToList(Collection collection) { - // TODO The user might not be aware of these performance pitfalls with Set and LinkedList: - // - If only ValueRange.createOriginalIterator() is used, cloning a Set to a List is a waste of time. - // - If the List is a LinkedList, ValueRange.createRandomIterator(Random) - // and ValueRange.get(int) are not efficient. - return (collection instanceof List ? (List) collection : new ArrayList<>(collection)); + if (collection instanceof List list) { + if (collection instanceof LinkedList linkedList) { + // ValueRange.createRandomIterator(Random) and ValueRange.get(int) wouldn't be efficient. + return new ArrayList<>(linkedList); + } else { + return list; + } + } else { + // TODO If only ValueRange.createOriginalIterator() is used, cloning a Set to a List is a waste of time. + return new ArrayList<>(collection); + } } } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/CompositeValueRangeDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/CompositeValueRangeDescriptor.java index 5cf2a48403..4c4cd77ff7 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/CompositeValueRangeDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/CompositeValueRangeDescriptor.java @@ -70,4 +70,24 @@ public ValueRange extractValueRange(Solution_ solution) { return doNullInValueRangeWrapping(new CompositeCountableValueRange(childValueRangeList)); } + @Override + public long extractValueRangeSize(Solution_ solution, Object entity) { + int size = addNullInValueRange ? 1 : 0; + for (ValueRangeDescriptor valueRangeDescriptor : childValueRangeDescriptorList) { + size += ((CountableValueRange) valueRangeDescriptor.extractValueRange(solution, entity)).getSize(); + } + return size; + } + + @Override + public long extractValueRangeSize(Solution_ solution) { + int size = addNullInValueRange ? 1 : 0; + for (ValueRangeDescriptor valueRangeDescriptor : childValueRangeDescriptorList) { + EntityIndependentValueRangeDescriptor entityIndependentValueRangeDescriptor = + (EntityIndependentValueRangeDescriptor) valueRangeDescriptor; + size += ((CountableValueRange) entityIndependentValueRangeDescriptor.extractValueRange(solution)).getSize(); + } + return size; + } + } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/EntityIndependentValueRangeDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/EntityIndependentValueRangeDescriptor.java index 99787a572b..b12bd2b18f 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/EntityIndependentValueRangeDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/EntityIndependentValueRangeDescriptor.java @@ -17,4 +17,13 @@ public interface EntityIndependentValueRangeDescriptor extends ValueR */ ValueRange extractValueRange(Solution_ solution); + /** + * As specified by {@link ValueRangeDescriptor#extractValueRangeSize}. + * + * @param solution never null + * @return never null + * @see ValueRangeDescriptor#extractValueRangeSize + */ + long extractValueRangeSize(Solution_ solution); + } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromEntityPropertyValueRangeDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromEntityPropertyValueRangeDescriptor.java index b18908d282..0a87c65afa 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromEntityPropertyValueRangeDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromEntityPropertyValueRangeDescriptor.java @@ -30,4 +30,9 @@ public ValueRange extractValueRange(Solution_ solution, Object entity) { return readValueRange(entity); } + @Override + public long extractValueRangeSize(Solution_ solution, Object entity) { + return readValueRangeSize(entity); + } + } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromSolutionPropertyValueRangeDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromSolutionPropertyValueRangeDescriptor.java index 86a26c6ace..235e51a7e8 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromSolutionPropertyValueRangeDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/FromSolutionPropertyValueRangeDescriptor.java @@ -32,9 +32,19 @@ public ValueRange extractValueRange(Solution_ solution, Object entity) { return readValueRange(solution); } + @Override + public long extractValueRangeSize(Solution_ solution, Object entity) { + return readValueRangeSize(solution); + } + @Override public ValueRange extractValueRange(Solution_ solution) { return readValueRange(solution); } + @Override + public long extractValueRangeSize(Solution_ solution) { + return readValueRangeSize(solution); + } + } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/ValueRangeDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/ValueRangeDescriptor.java index fa81e41bcd..b14f9991d2 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/ValueRangeDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/ValueRangeDescriptor.java @@ -42,4 +42,13 @@ public interface ValueRangeDescriptor { */ ValueRange extractValueRange(Solution_ solution, Object entity); + /** + * @param solution never null + * @param entity never null. To avoid this parameter, + * use {@link EntityIndependentValueRangeDescriptor#extractValueRangeSize} instead. + * @return never null + * @throws UnsupportedOperationException if {@link #isCountable()} returns false + */ + long extractValueRangeSize(Solution_ solution, Object entity); + } diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/variable/descriptor/GenuineVariableDescriptor.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/variable/descriptor/GenuineVariableDescriptor.java index 09da460d99..87f42f74ca 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/variable/descriptor/GenuineVariableDescriptor.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/variable/descriptor/GenuineVariableDescriptor.java @@ -10,7 +10,6 @@ import java.util.stream.Stream; import ai.timefold.solver.core.api.domain.solution.PlanningSolution; -import ai.timefold.solver.core.api.domain.valuerange.CountableValueRange; import ai.timefold.solver.core.api.domain.valuerange.ValueRange; import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; import ai.timefold.solver.core.api.domain.variable.PlanningListVariable; @@ -275,11 +274,7 @@ public SelectionSorter getDecreasingStrengthSorter() { } public long getValueCount(Solution_ solution, Object entity) { - if (!valueRangeDescriptor.isCountable()) { - // TODO report this better than just ignoring it - return 0L; - } - return ((CountableValueRange) valueRangeDescriptor.extractValueRange(solution, entity)).getSize(); + return valueRangeDescriptor.extractValueRangeSize(solution, entity); } @Override diff --git a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/FromEntityPropertyValueSelector.java b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/FromEntityPropertyValueSelector.java index c3fca31193..48d6ea2ade 100644 --- a/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/FromEntityPropertyValueSelector.java +++ b/core/core-impl/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/FromEntityPropertyValueSelector.java @@ -64,8 +64,7 @@ public boolean isNeverEnding() { @Override public long getSize(Object entity) { - ValueRange valueRange = valueRangeDescriptor.extractValueRange(workingSolution, entity); - return ((CountableValueRange) valueRange).getSize(); + return valueRangeDescriptor.extractValueRangeSize(workingSolution, entity); } @Override