Tighten the CountedBitSet class

This commit addresses the missed comments from https://github.com/elastic/elasticsearch/pull/27547.
This commit is contained in:
Nhat Nguyen 2017-12-04 09:51:34 -05:00 committed by GitHub
parent 2900e3f345
commit e213fa033d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 9 deletions

View File

@ -21,6 +21,7 @@ package org.elasticsearch.index.translog;
import org.apache.lucene.util.BitSet; import org.apache.lucene.util.BitSet;
import org.apache.lucene.util.FixedBitSet; import org.apache.lucene.util.FixedBitSet;
import org.apache.lucene.util.RamUsageEstimator;
/** /**
* A {@link CountedBitSet} wraps a {@link FixedBitSet} but automatically releases the internal bitset * A {@link CountedBitSet} wraps a {@link FixedBitSet} but automatically releases the internal bitset
@ -28,11 +29,14 @@ import org.apache.lucene.util.FixedBitSet;
* from translog as these numbers are likely to form contiguous ranges (eg. filling all bits). * from translog as these numbers are likely to form contiguous ranges (eg. filling all bits).
*/ */
final class CountedBitSet extends BitSet { final class CountedBitSet extends BitSet {
static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(CountedBitSet.class);
private short onBits; // Number of bits are set. private short onBits; // Number of bits are set.
private FixedBitSet bitset; private FixedBitSet bitset;
CountedBitSet(short numBits) { CountedBitSet(short numBits) {
assert numBits > 0; if (numBits <= 0) {
throw new IllegalArgumentException("Number of bits must be positive. Given [" + numBits + "]");
}
this.onBits = 0; this.onBits = 0;
this.bitset = new FixedBitSet(numBits); this.bitset = new FixedBitSet(numBits);
} }
@ -41,7 +45,6 @@ final class CountedBitSet extends BitSet {
public boolean get(int index) { public boolean get(int index) {
assert 0 <= index && index < this.length(); assert 0 <= index && index < this.length();
assert bitset == null || onBits < bitset.length() : "Bitset should be released when all bits are set"; assert bitset == null || onBits < bitset.length() : "Bitset should be released when all bits are set";
return bitset == null ? true : bitset.get(index); return bitset == null ? true : bitset.get(index);
} }
@ -52,7 +55,7 @@ final class CountedBitSet extends BitSet {
// Ignore set when bitset is full. // Ignore set when bitset is full.
if (bitset != null) { if (bitset != null) {
boolean wasOn = bitset.getAndSet(index); final boolean wasOn = bitset.getAndSet(index);
if (wasOn == false) { if (wasOn == false) {
onBits++; onBits++;
// Once all bits are set, we can simply just return YES for all indexes. // Once all bits are set, we can simply just return YES for all indexes.
@ -66,12 +69,12 @@ final class CountedBitSet extends BitSet {
@Override @Override
public void clear(int startIndex, int endIndex) { public void clear(int startIndex, int endIndex) {
throw new UnsupportedOperationException("Not implemented yet"); throw new UnsupportedOperationException();
} }
@Override @Override
public void clear(int index) { public void clear(int index) {
throw new UnsupportedOperationException("Not implemented yet"); throw new UnsupportedOperationException();
} }
@Override @Override
@ -86,20 +89,19 @@ final class CountedBitSet extends BitSet {
@Override @Override
public int prevSetBit(int index) { public int prevSetBit(int index) {
throw new UnsupportedOperationException("Not implemented yet"); throw new UnsupportedOperationException();
} }
@Override @Override
public int nextSetBit(int index) { public int nextSetBit(int index) {
throw new UnsupportedOperationException("Not implemented yet"); throw new UnsupportedOperationException();
} }
@Override @Override
public long ramBytesUsed() { public long ramBytesUsed() {
throw new UnsupportedOperationException("Not implemented yet"); return BASE_RAM_BYTES_USED + (bitset == null ? 0 : bitset.ramBytesUsed());
} }
// Exposed for testing
boolean isInternalBitsetReleased() { boolean isInternalBitsetReleased() {
return bitset == null; return bitset == null;
} }

View File

@ -26,7 +26,9 @@ import java.util.List;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import java.util.stream.IntStream; import java.util.stream.IntStream;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThan;
public class CountedBitSetTests extends ESTestCase { public class CountedBitSetTests extends ESTestCase {
@ -53,6 +55,7 @@ public class CountedBitSetTests extends ESTestCase {
int numBits = (short) randomIntBetween(8, 4096); int numBits = (short) randomIntBetween(8, 4096);
final CountedBitSet countedBitSet = new CountedBitSet((short) numBits); final CountedBitSet countedBitSet = new CountedBitSet((short) numBits);
final List<Integer> values = IntStream.range(0, numBits).boxed().collect(Collectors.toList()); final List<Integer> values = IntStream.range(0, numBits).boxed().collect(Collectors.toList());
final long ramBytesUsedWithBitSet = countedBitSet.ramBytesUsed();
for (int i = 1; i < numBits; i++) { for (int i = 1; i < numBits; i++) {
final int value = values.get(i); final int value = values.get(i);
@ -65,6 +68,7 @@ public class CountedBitSetTests extends ESTestCase {
assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(false)); assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(false));
assertThat(countedBitSet.length(), equalTo(numBits)); assertThat(countedBitSet.length(), equalTo(numBits));
assertThat(countedBitSet.cardinality(), equalTo(i)); assertThat(countedBitSet.cardinality(), equalTo(i));
assertThat(countedBitSet.ramBytesUsed(), equalTo(ramBytesUsedWithBitSet));
} }
// The missing piece to fill all bits. // The missing piece to fill all bits.
@ -79,6 +83,7 @@ public class CountedBitSetTests extends ESTestCase {
assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(true)); assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(true));
assertThat(countedBitSet.length(), equalTo(numBits)); assertThat(countedBitSet.length(), equalTo(numBits));
assertThat(countedBitSet.cardinality(), equalTo(numBits)); assertThat(countedBitSet.cardinality(), equalTo(numBits));
assertThat(countedBitSet.ramBytesUsed(), allOf(equalTo(CountedBitSet.BASE_RAM_BYTES_USED), lessThan(ramBytesUsedWithBitSet)));
} }
// Tests with released internal bitset. // Tests with released internal bitset.