CountedBitSet doesn't need to extend BitSet. (#28239)

This commit is contained in:
Adrien Grand 2018-01-22 12:43:34 +01:00 committed by GitHub
parent 452c36c552
commit 8d195c86de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 13 additions and 49 deletions

View File

@ -19,7 +19,6 @@
package org.elasticsearch.index.seqno;
import org.apache.lucene.util.BitSet;
import org.apache.lucene.util.FixedBitSet;
import org.apache.lucene.util.RamUsageEstimator;
@ -28,7 +27,7 @@ import org.apache.lucene.util.RamUsageEstimator;
* when all bits are set to reduce memory usage. This structure can work well for sequence numbers as
* these numbers are likely to form contiguous ranges (eg. filling all bits).
*/
public final class CountedBitSet extends BitSet {
public final class CountedBitSet {
static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(CountedBitSet.class);
private short onBits; // Number of bits are set.
private FixedBitSet bitset;
@ -41,14 +40,12 @@ public final class CountedBitSet extends BitSet {
this.bitset = new FixedBitSet(numBits);
}
@Override
public boolean get(int index) {
assert 0 <= index && index < this.length();
assert bitset == null || onBits < bitset.length() : "Bitset should be released when all bits are set";
return bitset == null ? true : bitset.get(index);
}
@Override
public void set(int index) {
assert 0 <= index && index < this.length();
assert bitset == null || onBits < bitset.length() : "Bitset should be released when all bits are set";
@ -67,41 +64,16 @@ public final class CountedBitSet extends BitSet {
}
}
@Override
public void clear(int startIndex, int endIndex) {
throw new UnsupportedOperationException();
}
// Below methods are pkg-private for testing
@Override
public void clear(int index) {
throw new UnsupportedOperationException();
}
@Override
public int cardinality() {
int cardinality() {
return onBits;
}
@Override
public int length() {
int length() {
return bitset == null ? onBits : bitset.length();
}
@Override
public int prevSetBit(int index) {
throw new UnsupportedOperationException();
}
@Override
public int nextSetBit(int index) {
throw new UnsupportedOperationException();
}
@Override
public long ramBytesUsed() {
return BASE_RAM_BYTES_USED + (bitset == null ? 0 : bitset.ramBytesUsed());
}
boolean isInternalBitsetReleased() {
return bitset == null;
}

View File

@ -20,7 +20,6 @@
package org.elasticsearch.index.seqno;
import com.carrotsearch.hppc.LongObjectHashMap;
import org.apache.lucene.util.BitSet;
import org.elasticsearch.common.SuppressForbidden;
/**
@ -39,7 +38,7 @@ public class LocalCheckpointTracker {
* A collection of bit sets representing pending sequence numbers. Each sequence number is mapped to a bit set by dividing by the
* bit set size.
*/
final LongObjectHashMap<BitSet> processedSeqNo = new LongObjectHashMap<>();
final LongObjectHashMap<CountedBitSet> processedSeqNo = new LongObjectHashMap<>();
/**
* The current local checkpoint, i.e., all sequence numbers no more than this number have been completed.
@ -96,7 +95,7 @@ public class LocalCheckpointTracker {
// this is possible during recovery where we might replay an operation that was also replicated
return;
}
final BitSet bitSet = getBitSetForSeqNo(seqNo);
final CountedBitSet bitSet = getBitSetForSeqNo(seqNo);
final int offset = seqNoToBitSetOffset(seqNo);
bitSet.set(offset);
if (seqNo == checkpoint + 1) {
@ -170,7 +169,7 @@ public class LocalCheckpointTracker {
try {
// keep it simple for now, get the checkpoint one by one; in the future we can optimize and read words
long bitSetKey = getBitSetKey(checkpoint);
BitSet current = processedSeqNo.get(bitSetKey);
CountedBitSet current = processedSeqNo.get(bitSetKey);
if (current == null) {
// the bit set corresponding to the checkpoint has already been removed, set ourselves up for the next bit set
assert checkpoint % BIT_SET_SIZE == BIT_SET_SIZE - 1;
@ -184,7 +183,7 @@ public class LocalCheckpointTracker {
*/
if (checkpoint == lastSeqNoInBitSet(bitSetKey)) {
assert current != null;
final BitSet removed = processedSeqNo.remove(bitSetKey);
final CountedBitSet removed = processedSeqNo.remove(bitSetKey);
assert removed == current;
current = processedSeqNo.get(++bitSetKey);
}
@ -210,11 +209,11 @@ public class LocalCheckpointTracker {
return seqNo / BIT_SET_SIZE;
}
private BitSet getBitSetForSeqNo(final long seqNo) {
private CountedBitSet getBitSetForSeqNo(final long seqNo) {
assert Thread.holdsLock(this);
final long bitSetKey = getBitSetKey(seqNo);
final int index = processedSeqNo.indexOf(bitSetKey);
final BitSet bitSet;
final CountedBitSet bitSet;
if (processedSeqNo.indexExists(index)) {
bitSet = processedSeqNo.indexGet(index);
} else {

View File

@ -20,7 +20,6 @@
package org.elasticsearch.index.translog;
import com.carrotsearch.hppc.LongObjectHashMap;
import org.apache.lucene.util.BitSet;
import org.elasticsearch.index.seqno.CountedBitSet;
import org.elasticsearch.index.seqno.SequenceNumbers;
@ -85,7 +84,7 @@ final class MultiSnapshot implements Translog.Snapshot {
static final class SeqNoSet {
static final short BIT_SET_SIZE = 1024;
private final LongObjectHashMap<BitSet> bitSets = new LongObjectHashMap<>();
private final LongObjectHashMap<CountedBitSet> bitSets = new LongObjectHashMap<>();
/**
* Marks this sequence number and returns <tt>true</tt> if it is seen before.
@ -93,7 +92,7 @@ final class MultiSnapshot implements Translog.Snapshot {
boolean getAndSet(long value) {
assert value >= 0;
final long key = value / BIT_SET_SIZE;
BitSet bitset = bitSets.get(key);
CountedBitSet bitset = bitSets.get(key);
if (bitset == null) {
bitset = new CountedBitSet(BIT_SET_SIZE);
bitSets.put(key, bitset);

View File

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

View File

@ -20,7 +20,6 @@
package org.elasticsearch.index.seqno;
import com.carrotsearch.hppc.LongObjectHashMap;
import org.apache.lucene.util.BitSet;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
@ -260,7 +259,7 @@ public class LocalCheckpointTrackerTests extends ESTestCase {
tracker.resetCheckpoint(localCheckpoint);
assertThat(tracker.getCheckpoint(), equalTo((long) localCheckpoint));
assertThat(tracker.getMaxSeqNo(), equalTo((long) maxSeqNo));
assertThat(tracker.processedSeqNo, new BaseMatcher<LongObjectHashMap<BitSet>>() {
assertThat(tracker.processedSeqNo, new BaseMatcher<LongObjectHashMap<CountedBitSet>>() {
@Override
public boolean matches(Object item) {
return (item instanceof LongObjectHashMap && ((LongObjectHashMap) item).isEmpty());