Skip to content

Commit 29e6a54

Browse files
triceotriceo
committed
Experiment with a different approach
1 parent a6e619e commit 29e6a54

File tree

2 files changed

+77
-83
lines changed

2 files changed

+77
-83
lines changed

‎core/src/main/java/ai/timefold/solver/core/impl/bavet/common/index/IndexedSet.java‎

Lines changed: 66 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
package ai.timefold.solver.core.impl.bavet.common.index;
22

3+
import ai.timefold.solver.core.impl.util.ElementAwareList;
4+
import org.jspecify.annotations.NullMarked;
5+
import org.jspecify.annotations.Nullable;
6+
37
import java.util.ArrayList;
8+
import java.util.BitSet;
49
import java.util.Collections;
510
import java.util.List;
611
import java.util.Objects;
712
import java.util.function.Consumer;
813

9-
import ai.timefold.solver.core.impl.util.ElementAwareList;
10-
11-
import org.jspecify.annotations.NullMarked;
12-
import org.jspecify.annotations.Nullable;
13-
1414
/**
1515
* {@link ArrayList}-backed set which allows to {@link #remove(Object)} an element
1616
* without knowing its position and without an expensive lookup.
@@ -33,6 +33,9 @@
3333
* <p>
3434
* For performance reasons, this class does not check if an element was already added;
3535
* duplicates must be avoided by the caller and will cause undefined behavior.
36+
* <p>
37+
* This class is not thread-safe.
38+
* It is in fact very thread-unsafe.
3639
*
3740
* @param <T>
3841
*/
@@ -41,7 +44,8 @@ public final class IndexedSet<T> {
4144

4245
private final ElementPositionTracker<T> elementPositionTracker;
4346
private @Nullable ArrayList<T> elementList; // Lazily initialized, so that empty indexes use no memory.
44-
private int removalCount = 0;
47+
private final BitSet gaps = new BitSet(0);
48+
private int gapCount = 0;
4549

4650
public IndexedSet(ElementPositionTracker<T> elementPositionTracker) {
4751
this.elementPositionTracker = Objects.requireNonNull(elementPositionTracker);
@@ -66,8 +70,16 @@ private List<T> getElementList() {
6670
*/
6771
public void add(T element) {
6872
var actualElementList = getElementList();
69-
actualElementList.add(element);
70-
elementPositionTracker.setPosition(element, actualElementList.size() - 1);
73+
if (gapCount > 0) {
74+
var gapIndex = gaps.nextSetBit(0);
75+
actualElementList.set(gapIndex, element);
76+
elementPositionTracker.setPosition(element, gapIndex);
77+
gaps.clear(gapIndex);
78+
gapCount--;
79+
} else {
80+
actualElementList.add(element);
81+
elementPositionTracker.setPosition(element, actualElementList.size() - 1);
82+
}
7183
}
7284

7385
/**
@@ -94,46 +106,19 @@ private boolean innerRemove(T element) {
94106
return false;
95107
}
96108
var actualElementList = getElementList();
97-
var upperBound = Math.min(insertionPosition, actualElementList.size() - 1);
98-
var lowerBound = Math.max(0, insertionPosition - removalCount);
99-
var actualPosition = findElement(actualElementList, element, lowerBound, upperBound);
100-
if (actualPosition < 0) {
101-
return false;
102-
}
103-
actualElementList.remove(actualPosition);
104-
if (isEmpty()) {
105-
removalCount = 0;
106-
} else if (actualElementList.size() > actualPosition) {
107-
// We only mark removals that actually affect later elements.
108-
// Removing the last element does not affect any other element.
109-
removalCount++;
109+
if (insertionPosition == actualElementList.size() - 1) {
110+
// The element was the last one added; we can simply remove it.
111+
actualElementList.remove(insertionPosition);
112+
} else {
113+
actualElementList.set(insertionPosition, null);
114+
gaps.set(insertionPosition);
115+
gapCount++;
110116
}
111117
return true;
112118
}
113119

114-
/**
115-
* Search for the element in the given range.
116-
*
117-
* @param actualElementList the list to search in
118-
* @param element the element to search for
119-
* @param startIndex start of the range we are currently considering (inclusive)
120-
* @param endIndex end of the range we are currently considering (inclusive)
121-
* @return the index of the element if found, -1 otherwise
122-
*/
123-
private static <T> int findElement(List<T> actualElementList, T element, int startIndex, int endIndex) {
124-
for (var i = endIndex; i >= startIndex; i--) {
125-
// Iterating backwards as the element is more likely to be closer to the end of the range,
126-
// which is where it was originally inserted.
127-
var maybeElement = actualElementList.get(i);
128-
if (maybeElement == element) {
129-
return i;
130-
}
131-
}
132-
return -1;
133-
}
134-
135120
public int size() {
136-
return elementList == null ? 0 : elementList.size();
121+
return elementList == null ? 0 : elementList.size() - gapCount;
137122
}
138123

139124
/**
@@ -146,19 +131,16 @@ public void forEach(Consumer<T> tupleConsumer) {
146131
if (elementList == null) {
147132
return;
148133
}
149-
var i = 0;
150-
while (i < elementList.size()) {
151-
var oldRemovalCount = removalCount; // The consumer may remove some elements, shifting others forward.
152-
tupleConsumer.accept(elementList.get(i));
153-
var elementDrift = removalCount - oldRemovalCount;
154-
// Move to the next element, adjusting for any shifts due to removals.
155-
// If no elements were removed by the consumer, we simply move to the next index.
156-
i -= elementDrift - 1;
134+
for (var i = 0; i < elementList.size(); i++) {
135+
var element = elementList.get(i);
136+
if (element != null) {
137+
tupleConsumer.accept(element);
138+
}
157139
}
158140
}
159141

160142
public boolean isEmpty() {
161-
return elementList == null || elementList.isEmpty();
143+
return size() == 0;
162144
}
163145

164146
/**
@@ -168,11 +150,40 @@ public boolean isEmpty() {
168150
* @return a standard list view of this element-aware list
169151
*/
170152
public List<T> asList() {
171-
return elementList == null ? Collections.emptyList() : elementList;
153+
if (elementList == null) {
154+
return Collections.emptyList();
155+
}
156+
var actualElementList = getElementList();
157+
defrag(actualElementList);
158+
return actualElementList;
159+
}
160+
161+
private void defrag(List<T> actualElementList) {
162+
if (gapCount == 0) {
163+
return;
164+
}
165+
var gap = gaps.nextSetBit(0);
166+
while (gap >= 0) {
167+
var lastNonGapIndex = findNonGapFromEnd(actualElementList);
168+
if (lastNonGapIndex < 0 || gap >= lastNonGapIndex) {
169+
break;
170+
}
171+
var lastElement = actualElementList.remove(lastNonGapIndex);
172+
actualElementList.set(gap, lastElement);
173+
elementPositionTracker.setPosition(lastElement, gap);
174+
gap = gaps.nextSetBit(gap + 1);
175+
}
176+
gaps.clear();
177+
gapCount = 0;
172178
}
173179

174-
public String toString() {
175-
return elementList == null ? "[]" : elementList.toString();
180+
private int findNonGapFromEnd(List<T> actualElementList) {
181+
var end = actualElementList.size() - 1;
182+
var lastNonGap = gaps.previousClearBit(end);
183+
for (var i = end; i > lastNonGap; i--) {
184+
actualElementList.remove(i);
185+
}
186+
return lastNonGap;
176187
}
177188

178189
}

‎core/src/test/java/ai/timefold/solver/core/impl/bavet/common/index/IndexedSetTest.java‎

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package ai.timefold.solver.core.impl.bavet.common.index;
22

3-
import static org.assertj.core.api.Assertions.assertThat;
4-
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3+
import org.junit.jupiter.api.Test;
54

65
import java.util.ArrayList;
76
import java.util.HashMap;
87
import java.util.Map;
98

10-
import org.junit.jupiter.api.Test;
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1111

1212
class IndexedSetTest {
1313

@@ -22,7 +22,7 @@ void addMultipleElements() {
2222
set.add("C");
2323

2424
assertThat(set.size()).isEqualTo(3);
25-
assertThat(set.asList()).containsExactly("A", "B", "C");
25+
assertThat(set.asList()).containsExactlyInAnyOrder("A", "B", "C");
2626
}
2727

2828
@Test
@@ -65,7 +65,7 @@ void removeMiddleElement() {
6565
set.remove("B");
6666

6767
assertThat(set.size()).isEqualTo(2);
68-
assertThat(set.asList()).containsExactly("A", "C");
68+
assertThat(set.asList()).containsExactlyInAnyOrder("A", "C");
6969
}
7070

7171
@Test
@@ -78,7 +78,7 @@ void removeLastElement() {
7878
set.remove("C");
7979

8080
assertThat(set.size()).isEqualTo(2);
81-
assertThat(set.asList()).containsExactly("A", "B");
81+
assertThat(set.asList()).containsExactlyInAnyOrder("A", "B");
8282
}
8383

8484
@Test
@@ -91,7 +91,7 @@ void removeFirstElement() {
9191
set.remove("A");
9292

9393
assertThat(set.size()).isEqualTo(2);
94-
assertThat(set.asList()).containsExactly("B", "C");
94+
assertThat(set.asList()).containsExactlyInAnyOrder("B", "C");
9595
}
9696

9797
@Test
@@ -107,7 +107,7 @@ void multipleRemovalsAndAdditions() {
107107
set.add("E");
108108

109109
assertThat(set.size()).isEqualTo(3);
110-
assertThat(set.asList()).containsExactly("C", "D", "E");
110+
assertThat(set.asList()).containsExactlyInAnyOrder("C", "D", "E");
111111
}
112112

113113
@Test
@@ -121,7 +121,7 @@ void forEach() {
121121
var result = new ArrayList<String>();
122122
set.forEach(result::add);
123123

124-
assertThat(result).containsExactly("A", "B", "C");
124+
assertThat(result).containsExactlyInAnyOrder("A", "B", "C");
125125
}
126126

127127
@Test
@@ -151,8 +151,8 @@ void forEachWithRemoval() {
151151
}
152152
});
153153

154-
assertThat(result).containsExactly("A", "B", "C", "D");
155-
assertThat(set.asList()).containsExactly("A", "C", "D");
154+
assertThat(result).containsExactlyInAnyOrder("A", "B", "C", "D");
155+
assertThat(set.asList()).containsExactlyInAnyOrder("A", "C", "D");
156156
}
157157

158158
@Test
@@ -175,23 +175,6 @@ void asListOnEmptySet() {
175175
assertThat(set.asList()).isEmpty();
176176
}
177177

178-
@Test
179-
void toStringOnEmptySet() {
180-
var set = new IndexedSet<>(stringTracker);
181-
182-
assertThat(set.toString()).isEqualTo("[]");
183-
}
184-
185-
@Test
186-
void toStringWithElements() {
187-
var set = new IndexedSet<>(stringTracker);
188-
189-
set.add("A");
190-
set.add("B");
191-
192-
assertThat(set.toString()).isEqualTo("[A, B]");
193-
}
194-
195178
@Test
196179
void largeSetWithManyRemovals() {
197180
var intTracker = new SimpleTracker<Integer>();

0 commit comments

Comments
 (0)