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

Author: jpountz

server/src/main/java/org/elasticsearch/index/seqno/CountedBitSet.java

@@ -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 @@
  * 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 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";
@@ -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;
     }

server/src/main/java/org/elasticsearch/index/seqno/LocalCheckpointTracker.java

@@ -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 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) {
@@ -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;
@@ -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);
                 }
@@ -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 {

server/src/main/java/org/elasticsearch/index/translog/MultiSnapshot.java

@@ -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,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);

server/src/test/java/org/elasticsearch/index/seqno/CountedBitSetTests.java

@@ -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 {
 
@@ -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);
@@ -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.
@@ -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.

server/src/test/java/org/elasticsearch/index/seqno/LocalCheckpointTrackerTests.java

@@ -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 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());