Skip to content

Commit 2a1fe87

Browse files
triceotriceo
authored
perf: more improvements to DSV (#1615)
1 parent 3a1c4cb commit 2a1fe87

3 files changed

Lines changed: 64 additions & 113 deletions

File tree

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

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,36 @@
11
package ai.timefold.solver.core.impl.domain.variable.declarative;
22

3-
import java.util.BitSet;
3+
import static ai.timefold.solver.core.impl.util.DynamicIntArray.ClearingStrategy.PARTIAL;
4+
5+
import ai.timefold.solver.core.impl.util.DynamicIntArray;
46

57
import org.jspecify.annotations.NullMarked;
68

79
@NullMarked
810
public final class LoopedTracker {
911

10-
// Simple LoopedStatus[] array would have occupied too much memory with large node counts.
11-
// Furthermore, allocating and/or clearing these large arrays is expensive as well.
12-
private final BitSet present;
13-
private final BitSet looped;
12+
// For some reason, the array was getting re-created on every values() call.
13+
// So, we cache a single instance.
14+
private static final LoopedStatus[] VALUES = LoopedStatus.values();
15+
16+
private final DynamicIntArray looped;
1417

1518
public LoopedTracker(int nodeCount) {
16-
this.present = new BitSet(nodeCount);
17-
this.looped = new BitSet(nodeCount);
19+
// We never fully clear the array, as that was shown to cause too much GC pressure.
20+
this.looped = new DynamicIntArray(nodeCount, PARTIAL);
1821
}
1922

2023
public void mark(int node, LoopedStatus status) {
21-
if (status == LoopedStatus.UNKNOWN) {
22-
present.clear(node);
23-
looped.clear(node);
24-
} else {
25-
present.set(node);
26-
looped.set(node, status == LoopedStatus.LOOPED);
27-
}
24+
looped.set(node, status.ordinal());
2825
}
2926

3027
public LoopedStatus status(int node) {
31-
if (present.isEmpty() || !present.get(node)) {
32-
return LoopedStatus.UNKNOWN;
33-
}
34-
return looped.get(node) ? LoopedStatus.LOOPED : LoopedStatus.NOT_LOOPED;
28+
// When in the unallocated part of the dynamic array, the value returned is zero.
29+
// Therefore it is imperative that LoopedStatus.UNKNOWN be the first element in the enum.
30+
return VALUES[looped.get(node)];
3531
}
3632

3733
public void clear() {
38-
present.clear();
3934
looped.clear();
4035
}
4136

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

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ public final class DynamicIntArray {
1414
// Minimum capacity increment to avoid small incremental growth
1515
private static final int MIN_CAPACITY_INCREMENT = 10;
1616

17+
private final ClearingStrategy clearingStrategy;
1718
private final int maxLength;
1819
private int[] array;
1920
private int firstIndex;
@@ -23,16 +24,18 @@ public DynamicIntArray() {
2324
this(Integer.MAX_VALUE);
2425
}
2526

26-
/**
27-
* Creates a new empty DynamicIntArray.
28-
*/
27+
public DynamicIntArray(ClearingStrategy clearingStrategy) {
28+
this(Integer.MAX_VALUE, clearingStrategy);
29+
}
30+
2931
public DynamicIntArray(int maxLength) {
32+
this(maxLength, ClearingStrategy.FULL);
33+
}
34+
35+
public DynamicIntArray(int maxLength, ClearingStrategy clearingStrategy) {
3036
this.maxLength = maxLength;
31-
// Array is null until first element is set
32-
this.array = null;
33-
// Initialize with invalid indices
34-
this.firstIndex = Integer.MAX_VALUE;
35-
this.lastIndex = Integer.MIN_VALUE;
37+
this.clearingStrategy = clearingStrategy;
38+
initializeArray();
3639
}
3740

3841
/**
@@ -96,7 +99,7 @@ public void set(int index, int value) {
9699

97100
/**
98101
* Calculates the new capacity based on the required capacity and growth strategy.
99-
*
102+
*
100103
* @param requiredCapacity the minimum capacity needed
101104
* @return the new capacity
102105
*/
@@ -177,31 +180,47 @@ int getLastIndex() {
177180
return lastIndex;
178181
}
179182

180-
/**
181-
* Gets the length of the array.
182-
*
183-
* @return the length of the array, or 0 if the array is empty
184-
*/
185-
int length() {
186-
if (array == null) {
187-
return 0;
188-
}
189-
return lastIndex + 1;
190-
}
191-
192183
/**
193184
* Clears the array by setting all values to 0.
194185
* The array structure is preserved, only the values are reset.
195186
*/
196187
public void clear() {
197-
// If array is null, there's nothing to clear
198-
if (array == null) {
199-
return;
188+
if (clearingStrategy == ClearingStrategy.FULL) {
189+
initializeArray();
190+
} else {
191+
// If array is null, there's nothing to clear
192+
if (array == null) {
193+
return;
194+
}
195+
196+
// Only clear the used portion of the array (from firstIndex to lastIndex)
197+
// This is more efficient for large arrays with sparse indices
198+
Arrays.fill(array, 0, lastIndex - firstIndex + 1, 0);
200199
}
200+
}
201+
202+
private void initializeArray() {
203+
this.array = null;
204+
this.firstIndex = Integer.MAX_VALUE;
205+
this.lastIndex = Integer.MIN_VALUE;
206+
}
207+
208+
public enum ClearingStrategy {
209+
210+
/**
211+
* The GC will be allowed to reclaim the array.
212+
* This means that, on next access, the array will have to be reallocated and gradually resized,
213+
* possibly leading to excessive GC pressure.
214+
*
215+
* This is the default.
216+
*/
217+
FULL,
218+
/**
219+
* The array will not be returned to GC and will be filled with zeros instead.
220+
* This has no impact on GC, but may result in greater at-rest heap usage than strictly necessary.
221+
*/
222+
PARTIAL
201223

202-
// Only clear the used portion of the array (from firstIndex to lastIndex)
203-
// This is more efficient for large arrays with sparse indices
204-
Arrays.fill(array, 0, lastIndex - firstIndex + 1, 0);
205224
}
206225

207226
}

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

Lines changed: 4 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ void defaultConstructor() {
2828
.isThrownBy(array::getLastIndex)
2929
.withMessage("Array is empty");
3030

31-
assertThat(array.length()).isZero();
3231
assertThat(array.containsIndex(0)).isFalse();
3332
assertThat(array.get(0)).isZero();
3433
}
@@ -41,8 +40,6 @@ void constructorWithMaxSize() {
4140
assertThatExceptionOfType(IllegalStateException.class)
4241
.isThrownBy(array::getFirstIndex);
4342

44-
assertThat(array.length()).isZero();
45-
4643
// Test bound checking with maxSize
4744
assertThatExceptionOfType(ArrayIndexOutOfBoundsException.class)
4845
.isThrownBy(() -> array.set(100, 5));
@@ -65,7 +62,6 @@ void setFirstElement() {
6562
assertThat(array.containsIndex(10)).isTrue();
6663
assertThat(array.containsIndex(9)).isFalse();
6764
assertThat(array.containsIndex(11)).isFalse();
68-
assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
6965
}
7066

7167
@Test
@@ -81,7 +77,6 @@ void setLowerIndex() {
8177
assertThat(array.getLastIndex()).isEqualTo(10);
8278
assertThat(array.containsIndex(5)).isTrue();
8379
assertThat(array.containsIndex(10)).isTrue();
84-
assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
8580
}
8681

8782
@Test
@@ -97,7 +92,6 @@ void setHigherIndex() {
9792
assertThat(array.getLastIndex()).isEqualTo(10);
9893
assertThat(array.containsIndex(5)).isTrue();
9994
assertThat(array.containsIndex(10)).isTrue();
100-
assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
10195
}
10296

10397
@Test
@@ -216,34 +210,6 @@ void containsIndexOutsideRange() {
216210
}
217211
}
218212

219-
@Nested
220-
@DisplayName("Length method tests")
221-
class LengthMethodTests {
222-
223-
@Test
224-
@DisplayName("Length returns 0 for empty array")
225-
void lengthEmptyArray() {
226-
var array = new DynamicIntArray();
227-
228-
assertThat(array.length()).isZero();
229-
}
230-
231-
@Test
232-
@DisplayName("Length returns correct value after setting elements")
233-
void lengthAfterSettingElements() {
234-
var array = new DynamicIntArray();
235-
array.set(0, 1);
236-
237-
assertThat(array.length()).isEqualTo(1);
238-
239-
array.set(5, 24);
240-
assertThat(array.length()).isEqualTo(6); // 0-5 inclusive
241-
242-
array.set(10, 42);
243-
assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
244-
}
245-
}
246-
247213
@Nested
248214
@DisplayName("Complex scenario tests")
249215
class ComplexScenarioTests {
@@ -288,9 +254,6 @@ void testMultipleOperations() {
288254
assertThat(array.containsIndex(15)).isTrue();
289255
assertThat(array.containsIndex(4)).isFalse();
290256
assertThat(array.containsIndex(16)).isFalse();
291-
292-
// Verify length
293-
assertThat(array.length()).isEqualTo(16); // 0-15 inclusive
294257
}
295258

296259
@Test
@@ -307,7 +270,6 @@ void testWithSparseIndices() {
307270
assertThat(array.get(10)).isEqualTo(3);
308271
assertThat(array.get(100)).isEqualTo(1);
309272
assertThat(array.get(1000)).isEqualTo(2);
310-
assertThat(array.length()).isEqualTo(1001); // 0-1000 inclusive
311273
}
312274
}
313275

@@ -322,8 +284,6 @@ void clearEmptyArray() {
322284

323285
// Should not throw an exception
324286
array.clear();
325-
326-
assertThat(array.length()).isZero();
327287
}
328288

329289
@Test
@@ -340,11 +300,8 @@ void clearResetsValues() {
340300
assertThat(array.get(10)).isZero();
341301

342302
// Array structure should be preserved
343-
assertThat(array.getFirstIndex()).isEqualTo(5);
344-
assertThat(array.getLastIndex()).isEqualTo(10);
345-
assertThat(array.containsIndex(5)).isTrue();
346-
assertThat(array.containsIndex(10)).isTrue();
347-
assertThat(array.length()).isEqualTo(11); // 0-10 inclusive
303+
assertThat(array.containsIndex(5)).isFalse();
304+
assertThat(array.containsIndex(10)).isFalse();
348305
}
349306

350307
@Test
@@ -367,30 +324,10 @@ void clearAndSetNewValues() {
367324
assertThat(array.get(10)).isZero();
368325

369326
// Array structure should be updated
370-
assertThat(array.getFirstIndex()).isEqualTo(5);
371-
assertThat(array.getLastIndex()).isEqualTo(10);
327+
assertThat(array.getFirstIndex()).isEqualTo(7);
328+
assertThat(array.getLastIndex()).isEqualTo(7);
372329
}
373330

374-
@Test
375-
@DisplayName("Clear with sparse indices")
376-
void clearWithSparseIndices() {
377-
var array = new DynamicIntArray();
378-
array.set(10, 1);
379-
array.set(100, 2);
380-
array.set(1000, 3);
381-
382-
array.clear();
383-
384-
// All values should be reset to 0
385-
assertThat(array.get(10)).isZero();
386-
assertThat(array.get(100)).isZero();
387-
assertThat(array.get(1000)).isZero();
388-
389-
// Array structure should be preserved
390-
assertThat(array.getFirstIndex()).isEqualTo(10);
391-
assertThat(array.getLastIndex()).isEqualTo(1000);
392-
assertThat(array.length()).isEqualTo(1001); // 0-1000 inclusive
393-
}
394331
}
395332

396333
}

0 commit comments

Comments
 (0)