perf: optimize value range handling of neighborhoods by triceo · Pull Request #2817 · TimefoldAI/timefold-solver

triceo · 2026-11-11T16:30:31Z

The basic and list variable move providers draw only moves that fit the value ranges.

A list variable value can be pinned through its entity (@PlanningPin, @PlanningPinToIndex) even when its class is a problem fact, not a planning entity. MoveStreamFactory.forEach treated every non-entity class as not pinnable, so the list move providers moved pinned values and inserted values into pinned positions.

Much of this was tested on ESS, and led to significant performance improvements.
Isolated benchmarks show improvements from 2x to 100x.

The basic and list variable move providers draw only moves that fit the value ranges. A list variable value can be pinned through its entity (@PlanningPin, @PlanningPinToIndex) even when its class is a problem fact, not a planning entity. MoveStreamFactory.forEach treated every non-entity class as not pinnable, so the list move providers moved pinned values and inserted values into pinned positions. The new provider tests run every drawn move and check value ranges and pinning.

Copilot

🟡 Changes recommended

Unresolved defects can crash pillar iteration or retire the wrong pillar, cause gigabyte-scale unnecessary allocations, and stall list move searches on sparse datasets.

3 open issues

What changed in this PR

This PR optimizes candidate selection in the preview Neighborhoods API and fixes pin handling for list values represented as problem facts.

Changes:

Adds range-based entity and value lookups, with cached movability filtering.
Refactors built-in move providers to use specialized iterators and candidate pools.
Adds tests for move validity, sampling distribution, pinning, and iterator termination, plus user documentation.

File	Description
docs/src/modules/ROOT/pages/optimization-algorithms/neighborhoods.adoc	Documents range-based candidate selection.
core/src/test/java/ai/timefold/solver/core/testutil/MoveProviderAssertions.java	Adds shared validity and distribution assertions.
core/src/test/java/ai/timefold/solver/core/testdomain/valuerange/entityproviding/pinned/TestdataPinnedEntityProvidingSolution.java	Adds a pinned entity-range solution fixture.
core/src/test/java/ai/timefold/solver/core/testdomain/valuerange/entityproviding/pinned/TestdataPinnedEntityProvidingEntity.java	Adds a pinned entity-range entity fixture.
core/src/test/java/ai/timefold/solver/core/testdomain/list/valuerange/unassignedvar/pinned/TestdataListUnassignedPinnedEntityProvidingSolution.java	Exposes list-range values as problem facts.
core/src/test/java/ai/timefold/solver/core/testdomain/list/valuerange/pinned/TestdataListPinnedEntityProvidingSolution.java	Exposes list-range values as problem facts.
core/src/test/java/ai/timefold/solver/core/testdomain/inheritance/entity/single/baseannotated/classes/pinned/TestdataExtendedPinnedSolution.java	Adds an inherited-variable solution fixture.
core/src/test/java/ai/timefold/solver/core/testdomain/inheritance/entity/single/baseannotated/classes/pinned/TestdataExtendedPinnedEntity.java	Adds subclass pinning and variable fixtures.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/SwapMoveProviderTest.java	Tests valid partners, inheritance, and termination.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/SubPillarSwapMoveProviderTest.java	Tests subpillar validity and sampling distribution.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/SubListSwapMoveProviderTest.java	Tests pin safety and sparse valid moves.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/SubListChangeMoveProviderTest.java	Tests ranges, pinning, and sparse moves.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/PillarSwapMoveProviderTest.java	Tests pillar validity and size-independent sampling.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/MassListChangeMoveProviderTest.java	Tests mass-change range and pin safety.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/MassListAssignMoveProviderTest.java	Tests mass-assignment range and pin safety.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/ListTailSwapMoveProviderTest.java	Tests tail-swap validity and partner distribution.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/ListSwapMoveProviderTest.java	Tests list-swap validity and partner distribution.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/ListChangeMoveProviderTest.java	Tests list-change validity and destination distribution.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/ListAssignMoveProviderTest.java	Tests assignment validity and destination distribution.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/ChangeMoveProviderTest.java	Tests range-based changes and termination.
core/src/test/java/ai/timefold/solver/core/preview/api/move/builtin/AssignMoveProviderTest.java	Tests non-null assignments and value distribution.
core/src/test/java/ai/timefold/solver/core/impl/score/director/ValueRangeManagerTest.java	Tests working-solution requirements for movable lists.
core/src/test/java/ai/timefold/solver/core/impl/move/builtin/ListCandidatePoolTest.java	Tests sampling, rejection, and iterator slot safety.
core/src/test/java/ai/timefold/solver/core/impl/move/builtin/AbstractSingleDrawMoveIteratorTest.java	Tests single-draw retention and distribution.
core/src/test/java/ai/timefold/solver/core/impl/move/builtin/AbstractRepeatedDrawMoveIteratorTest.java	Tests retry budgets and retirement.
core/src/test/java/ai/timefold/solver/core/impl/move/builtin/AbstractLeftDrivenMoveIteratorTest.java	Tests budget overflow protection.
core/src/main/java/ai/timefold/solver/core/preview/api/move/SolutionView.java	Adds entity and value candidate lookup methods.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/SwapMoveProvider.java	Delegates swaps to a range-aware iterator.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/SubPillarSwapMoveProvider.java	Narrows candidate pillar keys.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/SubListSwapMoveProvider.java	Delegates sublist swaps to a dedicated iterator.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/SubListChangeMoveProvider.java	Selects destination handling by range type.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/PillarSwapMoveProvider.java	Caches keyed pillars and delegates iteration.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/MassListChangeMoveProvider.java	Uses candidate pools for entity-provided ranges.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/MassListAssignMoveProvider.java	Uses range-aware mass-assignment destinations.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/ListTailSwapMoveProvider.java	Delegates tail swaps to a dedicated iterator.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/ListSwapMoveProvider.java	Uses candidate pools for entity-range swaps.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/ListChangeMoveProvider.java	Delegates range-aware list changes.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/ListAssignMoveProvider.java	Draws destinations from accepting entities.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/ChangeMoveProvider.java	Draws values directly from entity ranges.
core/src/main/java/ai/timefold/solver/core/preview/api/move/builtin/AssignMoveProvider.java	Reuses direct range-based value selection.
core/src/main/java/ai/timefold/solver/core/preview/api/domain/metamodel/VariableMetaModel.java	Clarifies inherited metamodel equality in Javadoc.
core/src/main/java/ai/timefold/solver/core/impl/score/director/ValueRangeState.java	Supplies movability filters to reachability data.
core/src/main/java/ai/timefold/solver/core/impl/score/director/ValueRangeManager.java	Caches movable entities and list values.
core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/EnumeratingStreamFactory.java	Applies pin filtering to problem-fact list values.
core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/DefaultMoveStreamFactory.java	Handles problem-fact pinning and simplifies helpers.
core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/dataset/DefaultUniDatasetInstance.java	Updates retiring-iterator documentation.
core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/dataset/CachedBiDatasetInstance.java	Exposes retiring iteration over row values.
core/src/main/java/ai/timefold/solver/core/impl/move/MoveDirector.java	Implements candidate queries and subclass pin checks.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/ValueRangeChangeMoveIterator.java	Draws changed values from entity ranges.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/SwapMoveIterator.java	Searches narrowed swap partner pools.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/SubListSwapMoveIterator.java	Adds single-draw sublist swap selection.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/SubListChangeMoveIterator.java	Adds single-draw sublist destination selection.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/PillarWithRanges.java	Extracts the cached pillar-and-ranges record.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/PillarSwapMoveIterator.java	Adds range-aware pillar partner selection.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/MoveProviderUtil.java	Adds candidate narrowing and bulk range checks.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/MassListDestinationMoveIterator.java	Moves implementation and adds candidate-pool searches.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/ListTailSwapMoveIterator.java	Combines sampled and direct tail-partner searches.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/ListChangeMoveIterator.java	Implements range-aware list destination searches.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/ListCandidatePool.java	Adds shared unpinned position and partner sampling.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/AbstractSingleDrawMoveIterator.java	Retains candidates known to have valid moves.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/AbstractRepeatedDrawMoveIterator.java	Retries searches before retiring candidates.
core/src/main/java/ai/timefold/solver/core/impl/move/builtin/AbstractLeftDrivenMoveIterator.java	Centralizes retirement and bounded candidate searches.
core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/common/ReachableValues.java	Caches movable and class-filtered candidate lists.

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Candidate selection and probabilistic retirement changes span most built-in neighborhoods, requiring end-to-end solver regression checks and representative performance benchmarks beyond source review and sampling tests.

No open issues 🎉

1 resolved since last review

Track left elements when move generation succeeds

_{Review effort: Balanced}

zepfred · 2026-11-12T11:18:15Z

+        var valueList = onDemandRandomAccessMovableValue[itemValue.ordinal];
+        if (valueList == null) {
+            // Built in one pass from the index; the list of all values is only built if asked for.
+            valueList = Collections.unmodifiableList(movableValueFilter == null ? extractValuesAsList(value)


This logic makes sense since it avoids duplicating the list. Can the filter for entities be null? If so, I suggest applying the same idea there.

Never mind, I understand why we cannot apply this when filtering by class.

zepfred · 2026-11-12T11:44:55Z

+     * The slots of one kind over a list of entities, numbered in the order of the entities.
+     * Either through prefix sums of the counts (no padding), or padded: each entity gets the same number of slots.
+     */
+    private final class SlotLayout {


Some classes include internal classes at the end, but it seems we don't have a clear rule regarding this. Personally, I prefer to place these internal classes at the very bottom. As for enums, we can add them either at the beginning or at the end of the class. For some reason, I prefer not to mix them with the main logic :-)

zepfred · 2026-11-12T11:55:16Z

+     */
+    private SlotLayout searchLayout(List<Entity_> entityList, SlotKind kind) {
+        var layout = new SlotLayout(entityList, kind);
+        var paddedCount = (long) entityList.size() * layout.maximumCount;


It would be better for SlotLayout to compute the correct search layout. Therefore, we would allow it to check the condition and adjust its internal state instead of returning a new instance for a padded layout, for example.

zepfred · 2026-11-12T11:57:09Z

+        private @Nullable PositionInList paddedPosition(long slot) {
+            var entity = entityList.get((int) (slot / maximumCount));
+            var offset = (int) (slot % maximumCount);
+            return offset < slotCount(entity, kind) ? positionAt(entity, offset) : null;


We chose to return null instead of checking the next slots. Is that intended?

zepfred · 2026-11-12T12:02:39Z

+         */
+        private @Nullable PositionInList position(long slot) {
+            var counts = cumulativeCounts;
+            return counts == null ? paddedPosition(slot) : prefixPosition(counts, slot);


Do we need counts, or can we simply pass cumulativeCounts instead?

zepfred · 2026-11-12T12:37:54Z

+ * Create one instance per move iterator: the solution does not change while the iterator runs.
+ */
+@NullMarked
+public final class ListCandidatePool<Solution_, Entity_, Value_>


The logic looks better. Will this pool be recreated each time a move is accepted? If so, I wonder about the performance impact of recalculating the layouts at every step, but this can only be answered with a proper benchmark. In the past, we observed that generating only valid moves when using entity ranges was beneficial, even at the cost of slower move speed.

Should we call it as ListCandidatePoolSource?

zepfred · 2026-11-12T12:46:51Z

+import org.jspecify.annotations.Nullable;
+
+@NullMarked
+public final class ListChangeMoveIterator<Solution_, Entity_, Value_>


It's much easier to understand it now!

zepfred · 2026-11-12T13:05:42Z

+            return null;
+        }
+        var pick = random.nextInt(partnerCount);
+        for (var i = 0; i < partnerEntityList.size(); i++) {


Arguably, we could use a binary search or a NavigableMap here.

zepfred · 2026-11-12T13:12:34Z

                continue;
            }
-            var destination = destinationIterator.next();
            nextMove = (destination instanceof UnassignedElement)


nextMove = (destination instanceof PositionInList positionInList) ? Moves.massChange(variableMetaModel, sample, positionInList) : Moves.massChange(variableMetaModel, sample, null);

zepfred · 2026-11-12T13:14:06Z

@@ -104,12 +100,7 @@ private boolean isValidDestination(Sample<Value_> sample, ElementPosition destin
            // We can move freely between entities, no per-entity value range to violate.


Let's move this check to the first if-block.

zepfred · 2026-11-12T15:20:22Z

+
+        @Override
+        public boolean hasNext() {
+            return true;


Is it possible for the size to be zero?

zepfred · 2026-11-12T15:27:45Z

+            }
+            // Only the entities which accept the narrowest value can accept the span; the budget fits that pool.
+            var narrowestValue = candidateSource.narrowestValue(range.entity(), range.fromIndex(), range.toIndex());
+            if (candidateSource.drawDestination(narrowestValue, position -> isValidChange(range, position)) != null) {


Why not cache the move and return it instead of recalculating it in tryBuildMove?

zepfred · 2026-11-12T16:22:25Z

+        if (leftRange == null) {
+            return null;
+        }
+        var rightValue = candidateSource.drawPartnerOnce(value -> true);


Can we use the entity from the left range to iterate only over reachable values?

zepfred · 2026-11-12T16:25:22Z

+                return false;
+            }
+            // Only the entities which accept the narrowest value can accept the left span; the budget fits that pool.
+            var narrowestValue = candidateSource.narrowestValue(leftRange.entity(), leftRange.fromIndex(), leftRange.toIndex());


This is the second iterator, where we generate a valid move without caching it. Why are we using this strategy?

zepfred · 2026-11-12T16:34:41Z

+        var valueIterator = valueRange.createRandomIterator(random);
+        for (var attempt = 0L; attempt < budget && valueIterator.hasNext(); attempt++) {
+            var value = valueIterator.next();
+            if (value == null ? nullAllowed && currentValue != null : !value.equals(currentValue)) {


Please extract this conditions to improve the readability

zepfred · 2026-11-12T16:43:36Z

        assertValidForEachType(sourceClass);
-        if (!solutionDescriptor.getMetaModel().hasEntity(sourceClass)) {
-            // The sourceClass is not a planning entity, therefore it cannot be pinned.
+        var listVariableDescriptor = solutionDescriptor.getListVariableDescriptor();


Perhaps we should consider moving the pinning check logic to the solution descriptor.

zepfred · 2026-11-12T16:48:05Z

-                        }
-                        return solutionView.isValueInRange(variableMetaModel, entity, value);
-                    },
+                    // Only for a solution value range; the solution can hold values of the same class outside it.


I'm confused by the comment. I would expect that a solution range always returns true. The function makes sense for the entity range.

zepfred · 2026-11-12T16:54:42Z

    private final @Nullable ValueRangeState<Solution_, ?, ?> @Nullable [] multipleValueRangeState;
    private @Nullable Solution_ cachedWorkingSolution = null;
    private @Nullable ValueRangeStatistics<Solution_> statistics;
+    private @Nullable Map<EntityDescriptor<Solution_>, List<Object>> movableEntityListMap;


This is internal state information, and I prefer to keep all related information in one place. Can we add this data to singleValueRangeState and/or multipleValueRangeState?

zepfred · 2026-11-12T16:56:13Z

+     *
+     * @return null if no entity of the given descriptor's class or of its subclasses can be pinned
+     */
+    static <Solution_, Entity_> @Nullable Predicate<Entity_> movableEntityFilter(EntityDescriptor<Solution_> entityDescriptor,


I'm not sure if this is the appropriate location for this function.

zepfred · 2026-11-12T16:56:28Z

+     *
+     * @return null if no value of the variable can be pinned
+     */
+    static <Solution_, Value_> @Nullable Predicate<Value_> movableValueFilter(


Same as before

zepfred · 2026-11-12T16:59:50Z

+     * Collects the pinned values on the first test, as most users of the filter never test a value.
+     */
+    @NullMarked
+    private static final class MovableValueFilter<Solution_, Value_> implements Predicate<Value_> {


Same as before. We have some similar logic in the descriptor classes, and it seems to me that this is a better place for the filter logic.

zepfred · 2026-11-12T17:12:34Z

+            var valueInstance = (DefaultUniDatasetInstance<Solution_, Value_>) session.getInstance(valueDataset);
+            var candidatePool = ListCandidatePool.of(session.getSolutionView(), variableMetaModel, random,
+                    session.getInstance(entityDataset), false);
+            return new AbstractRepeatedDrawMoveIterator<>(valueInstance.retiringRandomIterator(random)) {


The logic seems more complex than in the previous instance, or I'm not used to it yet.

zepfred · 2026-11-12T17:19:19Z

+    /**
+     * @return false if both positions are the same, or both are unassigned
+     */
+    private static boolean isSwap(ElementPosition left, ElementPosition right) {


isSwapValid

zepfred · 2026-11-12T17:26:57Z

                MoveProviderUtil.flag("crossingNull", crossingNull));
    }

+    private UniDataset<Solution_, ElementPosition> destinationDataset(MoveStreamFactory<Solution_> moveStreamFactory) {


Most providers are using ternary operations to compute the dataset. Personally, I find this method easier to read and prefer it. Please take a look at the other providers and evaluate whether adopting the same approach would be beneficial.

zepfred

The logic looks much better. I have a few more comments and want to do another review before approving it.

triceo requested a review from a team November 11, 2026 16:30

triceo requested review from TomCools, rsynek and zepfred as code owners November 11, 2026 16:30

Copilot AI balanced review requested due to automatic review settings November 11, 2026 16:30

triceo temporarily deployed to internal November 11, 2026 16:30 — with GitHub Actions Inactive

triceo deployed to documentation (preview) November 11, 2026 16:30 — with GitHub Actions

View deployment

Copilot started reviewing on behalf of triceo November 11, 2026 16:31

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes