perf: more improvements to DSV

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java

@@ -1,41 +1,36 @@
 package ai.timefold.solver.core.impl.domain.variable.declarative;
 
-import java.util.BitSet;
+import static ai.timefold.solver.core.impl.util.DynamicIntArray.ClearingStrategy.PARTIAL;
+
+import ai.timefold.solver.core.impl.util.DynamicIntArray;
 
 import org.jspecify.annotations.NullMarked;
 
 @NullMarked
 public final class LoopedTracker {
 
-    // Simple LoopedStatus[] array would have occupied too much memory with large node counts.
-    // Furthermore, allocating and/or clearing these large arrays is expensive as well.
-    private final BitSet present;
-    private final BitSet looped;
+    // For some reason, the array was getting re-created on every values() call.
+    // So, we cache a single instance.
+    private static final LoopedStatus[] VALUES = LoopedStatus.values();
+
+    private final DynamicIntArray looped;
 
     public LoopedTracker(int nodeCount) {
-        this.present = new BitSet(nodeCount);
-        this.looped = new BitSet(nodeCount);
+        // We never fully clear the array, as that was shown to cause too much GC pressure.
+        this.looped = new DynamicIntArray(nodeCount, PARTIAL);
     }
 
     public void mark(int node, LoopedStatus status) {
-        if (status == LoopedStatus.UNKNOWN) {
-            present.clear(node);
-            looped.clear(node);
-        } else {
-            present.set(node);
-            looped.set(node, status == LoopedStatus.LOOPED);
-        }
+        looped.set(node, status.ordinal());
     }
 
     public LoopedStatus status(int node) {
-        if (present.isEmpty() || !present.get(node)) {
-            return LoopedStatus.UNKNOWN;
-        }
-        return looped.get(node) ? LoopedStatus.LOOPED : LoopedStatus.NOT_LOOPED;
+        // When in the unallocated part of the dynamic array, the value returned is zero.
+        // Therefore it is imperative that LoopedStatus.UNKNOWN be the first element in the enum.
+        return VALUES[looped.get(node)];
     }
 
     public void clear() {
-        present.clear();
         looped.clear();
     }
 

core/src/main/java/ai/timefold/solver/core/impl/util/DynamicIntArray.java

@@ -14,6 +14,7 @@ public final class DynamicIntArray {
     // Minimum capacity increment to avoid small incremental growth
     private static final int MIN_CAPACITY_INCREMENT = 10;
 
+    private final ClearingStrategy clearingStrategy;
     private final int maxLength;
     private int[] array;
     private int firstIndex;
@@ -23,16 +24,18 @@ public DynamicIntArray() {
         this(Integer.MAX_VALUE);
     }
 
-    /**
-     * Creates a new empty DynamicIntArray.
-     */
+    public DynamicIntArray(ClearingStrategy clearingStrategy) {
+        this(Integer.MAX_VALUE, clearingStrategy);
+    }
+
     public DynamicIntArray(int maxLength) {
+        this(maxLength, ClearingStrategy.FULL);
+    }
+
+    public DynamicIntArray(int maxLength, ClearingStrategy clearingStrategy) {
         this.maxLength = maxLength;
-        // Array is null until first element is set
-        this.array = null;
-        // Initialize with invalid indices
-        this.firstIndex = Integer.MAX_VALUE;
-        this.lastIndex = Integer.MIN_VALUE;
+        this.clearingStrategy = clearingStrategy;
+        initializeArray();
     }
 
     /**
@@ -96,7 +99,7 @@ public void set(int index, int value) {
 
     /**
      * Calculates the new capacity based on the required capacity and growth strategy.
-     * 
+     *
      * @param requiredCapacity the minimum capacity needed
      * @return the new capacity
      */
@@ -177,31 +180,47 @@ int getLastIndex() {
         return lastIndex;
     }
 
-    /**
-     * Gets the length of the array.
-     *
-     * @return the length of the array, or 0 if the array is empty
-     */
-    int length() {
-        if (array == null) {
-            return 0;
-        }
-        return lastIndex + 1;
-    }
-
     /**
      * Clears the array by setting all values to 0.
      * The array structure is preserved, only the values are reset.
      */
     public void clear() {
-        // If array is null, there's nothing to clear
-        if (array == null) {
-            return;
+        if (clearingStrategy == ClearingStrategy.FULL) {
+            initializeArray();
+        } else {
+            // If array is null, there's nothing to clear
+            if (array == null) {
+                return;
+            }
+
+            // Only clear the used portion of the array (from firstIndex to lastIndex)
+            // This is more efficient for large arrays with sparse indices
+            Arrays.fill(array, 0, lastIndex - firstIndex + 1, 0);
         }
+    }
+
+    private void initializeArray() {
+        this.array = null;
+        this.firstIndex = Integer.MAX_VALUE;
+        this.lastIndex = Integer.MIN_VALUE;
+    }
+
+    public enum ClearingStrategy {
+
+        /**
+         * The GC will be allowed to reclaim the array.
+         * This means that, on next access, the array will have to be reallocated and gradually resized,
+         * possibly leading to excessive GC pressure.
+         *
+         * This is the default.
+         */
+        FULL,
+        /**
+         * The array will not be returned to GC and will be filled with zeros instead.
+         * This has no impact on GC, but may result in greater at-rest heap usage than strictly necessary.
+         */
+        PARTIAL
 
-        // Only clear the used portion of the array (from firstIndex to lastIndex)
-        // This is more efficient for large arrays with sparse indices
-        Arrays.fill(array, 0, lastIndex - firstIndex + 1, 0);
     }
 
 }

core/src/test/java/ai/timefold/solver/core/impl/util/DynamicIntArrayTest.java

@@ -28,7 +28,6 @@ void defaultConstructor() {
                     .isThrownBy(array::getLastIndex)
                     .withMessage("Array is empty");
 
-            assertThat(array.length()).isZero();
             assertThat(array.containsIndex(0)).isFalse();
             assertThat(array.get(0)).isZero();
         }
@@ -41,8 +40,6 @@ void constructorWithMaxSize() {
             assertThatExceptionOfType(IllegalStateException.class)
                     .isThrownBy(array::getFirstIndex);
 
-            assertThat(array.length()).isZero();
-
             // Test bound checking with maxSize
             assertThatExceptionOfType(ArrayIndexOutOfBoundsException.class)
                     .isThrownBy(() -> array.set(100, 5));
@@ -65,7 +62,6 @@ void setFirstElement() {
             assertThat(array.containsIndex(10)).isTrue();
             assertThat(array.containsIndex(9)).isFalse();
             assertThat(array.containsIndex(11)).isFalse();
-            assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
         }
 
         @Test
@@ -81,7 +77,6 @@ void setLowerIndex() {
             assertThat(array.getLastIndex()).isEqualTo(10);
             assertThat(array.containsIndex(5)).isTrue();
             assertThat(array.containsIndex(10)).isTrue();
-            assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
         }
 
         @Test
@@ -97,7 +92,6 @@ void setHigherIndex() {
             assertThat(array.getLastIndex()).isEqualTo(10);
             assertThat(array.containsIndex(5)).isTrue();
             assertThat(array.containsIndex(10)).isTrue();
-            assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
         }
 
         @Test
@@ -216,34 +210,6 @@ void containsIndexOutsideRange() {
         }
     }
 
-    @Nested
-    @DisplayName("Length method tests")
-    class LengthMethodTests {
-
-        @Test
-        @DisplayName("Length returns 0 for empty array")
-        void lengthEmptyArray() {
-            var array = new DynamicIntArray();
-
-            assertThat(array.length()).isZero();
-        }
-
-        @Test
-        @DisplayName("Length returns correct value after setting elements")
-        void lengthAfterSettingElements() {
-            var array = new DynamicIntArray();
-            array.set(0, 1);
-
-            assertThat(array.length()).isEqualTo(1);
-
-            array.set(5, 24);
-            assertThat(array.length()).isEqualTo(6); // 0-5 inclusive
-
-            array.set(10, 42);
-            assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
-        }
-    }
-
     @Nested
     @DisplayName("Complex scenario tests")
     class ComplexScenarioTests {
@@ -288,9 +254,6 @@ void testMultipleOperations() {
             assertThat(array.containsIndex(15)).isTrue();
             assertThat(array.containsIndex(4)).isFalse();
             assertThat(array.containsIndex(16)).isFalse();
-
-            // Verify length
-            assertThat(array.length()).isEqualTo(16); // 0-15 inclusive
         }
 
         @Test
@@ -307,7 +270,6 @@ void testWithSparseIndices() {
             assertThat(array.get(10)).isEqualTo(3);
             assertThat(array.get(100)).isEqualTo(1);
             assertThat(array.get(1000)).isEqualTo(2);
-            assertThat(array.length()).isEqualTo(1001); // 0-1000 inclusive
         }
     }
 
@@ -322,8 +284,6 @@ void clearEmptyArray() {
 
             // Should not throw an exception
             array.clear();
-
-            assertThat(array.length()).isZero();
         }
 
         @Test
@@ -340,11 +300,8 @@ void clearResetsValues() {
             assertThat(array.get(10)).isZero();
 
             // Array structure should be preserved
-            assertThat(array.getFirstIndex()).isEqualTo(5);
-            assertThat(array.getLastIndex()).isEqualTo(10);
-            assertThat(array.containsIndex(5)).isTrue();
-            assertThat(array.containsIndex(10)).isTrue();
-            assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
+            assertThat(array.containsIndex(5)).isFalse();
+            assertThat(array.containsIndex(10)).isFalse();
         }
 
         @Test
@@ -367,30 +324,10 @@ void clearAndSetNewValues() {
             assertThat(array.get(10)).isZero();
 
             // Array structure should be updated
-            assertThat(array.getFirstIndex()).isEqualTo(5);
-            assertThat(array.getLastIndex()).isEqualTo(10);
+            assertThat(array.getFirstIndex()).isEqualTo(7);
+            assertThat(array.getLastIndex()).isEqualTo(7);
         }
 
-        @Test
-        @DisplayName("Clear with sparse indices")
-        void clearWithSparseIndices() {
-            var array = new DynamicIntArray();
-            array.set(10, 1);
-            array.set(100, 2);
-            array.set(1000, 3);
-
-            array.clear();
-
-            // All values should be reset to 0
-            assertThat(array.get(10)).isZero();
-            assertThat(array.get(100)).isZero();
-            assertThat(array.get(1000)).isZero();
-
-            // Array structure should be preserved
-            assertThat(array.getFirstIndex()).isEqualTo(10);
-            assertThat(array.getLastIndex()).isEqualTo(1000);
-            assertThat(array.length()).isEqualTo(1001); // 0-1000 inclusive
-        }
     }
 
 }