diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java index 7ea9ad0ab2e..879323bb542 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/LoopedTracker.java +++ b/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(); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/util/DynamicIntArray.java b/core/src/main/java/ai/timefold/solver/core/impl/util/DynamicIntArray.java index ca89231a9c6..7cd8d29da8b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/util/DynamicIntArray.java +++ b/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); } } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/util/DynamicIntArrayTest.java b/core/src/test/java/ai/timefold/solver/core/impl/util/DynamicIntArrayTest.java index 47cf49b05b7..35683d2e48d 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/util/DynamicIntArrayTest.java +++ b/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 - } } }