Skip to content

Commit e213fa0

Browse files
authored
Tighten the CountedBitSet class
This commit addresses the missed comments from #27547.
1 parent 2900e3f commit e213fa0

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

core/src/main/java/org/elasticsearch/index/translog/CountedBitSet.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,22 @@
2121

2222
import org.apache.lucene.util.BitSet;
2323
import org.apache.lucene.util.FixedBitSet;
24+
import org.apache.lucene.util.RamUsageEstimator;
2425

2526
/**
2627
* A {@link CountedBitSet} wraps a {@link FixedBitSet} but automatically releases the internal bitset
2728
* when all bits are set to reduce memory usage. This structure can work well for sequence numbers
2829
* from translog as these numbers are likely to form contiguous ranges (eg. filling all bits).
2930
*/
3031
final class CountedBitSet extends BitSet {
32+
static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(CountedBitSet.class);
3133
private short onBits; // Number of bits are set.
3234
private FixedBitSet bitset;
3335

3436
CountedBitSet(short numBits) {
35-
assert numBits > 0;
37+
if (numBits <= 0) {
38+
throw new IllegalArgumentException("Number of bits must be positive. Given [" + numBits + "]");
39+
}
3640
this.onBits = 0;
3741
this.bitset = new FixedBitSet(numBits);
3842
}
@@ -41,7 +45,6 @@ final class CountedBitSet extends BitSet {
4145
public boolean get(int index) {
4246
assert 0 <= index && index < this.length();
4347
assert bitset == null || onBits < bitset.length() : "Bitset should be released when all bits are set";
44-
4548
return bitset == null ? true : bitset.get(index);
4649
}
4750

@@ -52,7 +55,7 @@ public void set(int index) {
5255

5356
// Ignore set when bitset is full.
5457
if (bitset != null) {
55-
boolean wasOn = bitset.getAndSet(index);
58+
final boolean wasOn = bitset.getAndSet(index);
5659
if (wasOn == false) {
5760
onBits++;
5861
// Once all bits are set, we can simply just return YES for all indexes.
@@ -66,12 +69,12 @@ public void set(int index) {
6669

6770
@Override
6871
public void clear(int startIndex, int endIndex) {
69-
throw new UnsupportedOperationException("Not implemented yet");
72+
throw new UnsupportedOperationException();
7073
}
7174

7275
@Override
7376
public void clear(int index) {
74-
throw new UnsupportedOperationException("Not implemented yet");
77+
throw new UnsupportedOperationException();
7578
}
7679

7780
@Override
@@ -86,20 +89,19 @@ public int length() {
8689

8790
@Override
8891
public int prevSetBit(int index) {
89-
throw new UnsupportedOperationException("Not implemented yet");
92+
throw new UnsupportedOperationException();
9093
}
9194

9295
@Override
9396
public int nextSetBit(int index) {
94-
throw new UnsupportedOperationException("Not implemented yet");
97+
throw new UnsupportedOperationException();
9598
}
9699

97100
@Override
98101
public long ramBytesUsed() {
99-
throw new UnsupportedOperationException("Not implemented yet");
102+
return BASE_RAM_BYTES_USED + (bitset == null ? 0 : bitset.ramBytesUsed());
100103
}
101104

102-
// Exposed for testing
103105
boolean isInternalBitsetReleased() {
104106
return bitset == null;
105107
}

core/src/test/java/org/elasticsearch/index/translog/CountedBitSetTests.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
import java.util.stream.Collectors;
2727
import java.util.stream.IntStream;
2828

29+
import static org.hamcrest.Matchers.allOf;
2930
import static org.hamcrest.Matchers.equalTo;
31+
import static org.hamcrest.Matchers.lessThan;
3032

3133
public class CountedBitSetTests extends ESTestCase {
3234

@@ -53,6 +55,7 @@ public void testReleaseInternalBitSet() {
5355
int numBits = (short) randomIntBetween(8, 4096);
5456
final CountedBitSet countedBitSet = new CountedBitSet((short) numBits);
5557
final List<Integer> values = IntStream.range(0, numBits).boxed().collect(Collectors.toList());
58+
final long ramBytesUsedWithBitSet = countedBitSet.ramBytesUsed();
5659

5760
for (int i = 1; i < numBits; i++) {
5861
final int value = values.get(i);
@@ -65,6 +68,7 @@ public void testReleaseInternalBitSet() {
6568
assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(false));
6669
assertThat(countedBitSet.length(), equalTo(numBits));
6770
assertThat(countedBitSet.cardinality(), equalTo(i));
71+
assertThat(countedBitSet.ramBytesUsed(), equalTo(ramBytesUsedWithBitSet));
6872
}
6973

7074
// The missing piece to fill all bits.
@@ -79,6 +83,7 @@ public void testReleaseInternalBitSet() {
7983
assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(true));
8084
assertThat(countedBitSet.length(), equalTo(numBits));
8185
assertThat(countedBitSet.cardinality(), equalTo(numBits));
86+
assertThat(countedBitSet.ramBytesUsed(), allOf(equalTo(CountedBitSet.BASE_RAM_BYTES_USED), lessThan(ramBytesUsedWithBitSet)));
8287
}
8388

8489
// Tests with released internal bitset.

0 commit comments

Comments
 (0)