chore: address comments

Author: zepfred

core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/move/decorator/FilteringMoveSelector.java

@@ -100,6 +100,7 @@ private long determineBailOutSize() {
 
     private class JustInTimeFilteringMoveIterator extends UpcomingSelectionIterator<Move<Solution_>> {
 
+        private final long TERMINATION_BAIL_OUT_SIZE = 1000L;
         private final Iterator<Move<Solution_>> childMoveIterator;
         private final long bailOutSize;
         private final AbstractPhaseScope<Solution_> phaseScope;
@@ -118,9 +119,8 @@ protected Move<Solution_> createUpcomingSelection() {
             Move<Solution_> next;
             long attemptsBeforeBailOut = bailOutSize;
             // To reduce the impact of checking for termination on each move,
-            // we only check for termination
-            // after filtering out a total number of moves equal to size(childMoveSelector).
-            long attemptsBeforeCheckTermination = bailOutSize / BAIL_OUT_MULTIPLIER;
+            // we only check for termination after filtering out 1000 moves.
+            long attemptsBeforeCheckTermination = TERMINATION_BAIL_OUT_SIZE;
             do {
                 if (!childMoveIterator.hasNext()) {
                     return noUpcomingSelection();
@@ -133,7 +133,7 @@ protected Move<Solution_> createUpcomingSelection() {
                         return noUpcomingSelection();
                     } else if (termination != null && attemptsBeforeCheckTermination <= 0L) {
                         // Reset the counter
-                        attemptsBeforeCheckTermination = bailOutSize / BAIL_OUT_MULTIPLIER;
+                        attemptsBeforeCheckTermination = TERMINATION_BAIL_OUT_SIZE;
                         if (termination.isPhaseTerminated(phaseScope)) {
                             logger.trace(
                                     "Bailing out of neverEnding selector ({}) because the termination setting has been triggered.",

core/src/main/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhase.java

@@ -125,7 +125,6 @@ public void phaseStarted(LocalSearchPhaseScope<Solution_> phaseScope) {
         super.phaseStarted(phaseScope);
         decider.phaseStarted(phaseScope);
         assertWorkingSolutionInitialized(phaseScope);
-        phaseScope.setTermination(phaseTermination);
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/phase/AbstractPhase.java

@@ -95,6 +95,7 @@ public void phaseStarted(AbstractPhaseScope<Solution_> phaseScope) {
             solver.phaseStarted(phaseScope);
         }
         phaseTermination.phaseStarted(phaseScope);
+        phaseScope.setTermination(phaseTermination);
         phaseLifecycleSupport.firePhaseStarted(phaseScope);
     }
 

core/src/main/java/ai/timefold/solver/core/impl/phase/scope/AbstractPhaseScope.java

@@ -38,7 +38,7 @@ public abstract class AbstractPhaseScope<Solution_> {
     protected int bestSolutionStepIndex;
 
     /**
-     * The solver termination configuration
+     * The phase termination configuration
      */
     private PhaseTermination<Solution_> termination;
 

core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/decorator/FilteringMoveSelectorTest.java

@@ -52,7 +52,7 @@ void bailOutByTermination() {
         var moveSelector = mock(MoveSelector.class);
         var termination = mock(BasicPlumbingTermination.class);
         var iterator = mock(Iterator.class);
-        when(moveSelector.getSize()).thenReturn(100L);
+        when(moveSelector.getSize()).thenReturn(1000L);
         when(moveSelector.isNeverEnding()).thenReturn(true);
         when(moveSelector.iterator()).thenReturn(iterator);
         when(iterator.hasNext()).thenReturn(true);
@@ -61,8 +61,8 @@ void bailOutByTermination() {
         var filteredMoveSelector = FilteringMoveSelector.of(moveSelector, (scoreDirector, selection) -> false);
         filteredMoveSelector.phaseStarted(phaseScope);
         assertThat(filteredMoveSelector.iterator().hasNext()).isFalse();
-        // The termination returns true at the second call, and only (100 * 10) * 30% calls are executed per check
-        verify(iterator, times(200)).next();
+        // The termination returns true at the second call, and 2000 calls are executed in total
+        verify(iterator, times(2000)).next();
     }
 
     public void filter(SelectionCacheType cacheType, int timesCalled) {