Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CountedBitSet doesn't need to extend BitSet. #28239

Merged
merged 1 commit into from
Jan 22, 2018
Merged

CountedBitSet doesn't need to extend BitSet. #28239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 4 additions & 32 deletions server/src/main/java/org/elasticsearch/index/seqno/CountedBitSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -28,7 +27,7 @@
* 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;
Expand All @@ -41,14 +40,12 @@ public CountedBitSet(short numBits) {
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";
Expand All @@ -67,41 +64,16 @@ public void set(int index) {
}
}

@Override
public void clear(int startIndex, int endIndex) {
throw new UnsupportedOperationException();
}

@Override
public void clear(int index) {
throw new UnsupportedOperationException();
}
// Below methods are pkg-private for testing

@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;
}
Expand Down
13 changes: 6 additions & 7 deletions server/src/main/java/org/elasticsearch/index/seqno/LocalCheckpointTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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.
Expand Down Expand Up @@ -96,7 +95,7 @@ public synchronized void markSeqNoAsCompleted(final long seqNo) {
// 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) {
Expand Down Expand Up @@ -170,7 +169,7 @@ assert getBitSetForSeqNo(checkpoint + 1).get(seqNoToBitSetOffset(checkpoint + 1)
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;
Expand All @@ -184,7 +183,7 @@ assert getBitSetForSeqNo(checkpoint + 1).get(seqNoToBitSetOffset(checkpoint + 1)
*/
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);
}
Expand All @@ -210,11 +209,11 @@ private long getBitSetKey(final long seqNo) {
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 {
Expand Down
5 changes: 2 additions & 3 deletions server/src/main/java/org/elasticsearch/index/translog/MultiSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -85,15 +84,15 @@ public void close() throws IOException {

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.
*/
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);
Expand Down
5 changes: 0 additions & 5 deletions server/src/test/java/org/elasticsearch/index/seqno/CountedBitSetTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@
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 {

Expand All @@ -55,7 +53,6 @@ public void testReleaseInternalBitSet() {
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);
Expand All @@ -68,7 +65,6 @@ public void testReleaseInternalBitSet() {
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.
Expand All @@ -83,7 +79,6 @@ public void testReleaseInternalBitSet() {
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.
Expand Down
3 changes: 1 addition & 2 deletions server/src/test/java/org/elasticsearch/index/seqno/LocalCheckpointTrackerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -260,7 +259,7 @@ public void testResetCheckpoint() {
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());
Expand Down