[DiskBBQ] Clean up of DocIdsWriter (#134539)

Author: iverase

server/src/main/java/org/elasticsearch/index/codec/vectors/diskbbq/DocIdsWriter.java

@@ -18,10 +18,8 @@
  */
 package org.elasticsearch.index.codec.vectors.diskbbq;
 
-import org.apache.lucene.index.PointValues.IntersectVisitor;
 import org.apache.lucene.store.DataOutput;
 import org.apache.lucene.store.IndexInput;
-import org.apache.lucene.util.IntsRef;
 import org.apache.lucene.util.hnsw.IntToIntFunction;
 
 import java.io.IOException;
@@ -33,7 +31,6 @@
  * <p>It is copied from the BKD implementation.
  */
 final class DocIdsWriter {
-    public static final int DEFAULT_MAX_POINTS_IN_LEAF_NODE = 512;
 
     private static final byte CONTINUOUS_IDS = (byte) -2;
     private static final byte DELTA_BPV_16 = (byte) 16;
@@ -43,22 +40,6 @@ final class DocIdsWriter {
 
     private int[] scratch = new int[0];
 
-    /**
-     * IntsRef to be used to iterate over the scratch buffer. A single instance is reused to avoid
-     * re-allocating the object. The ints and length fields need to be reset each use.
-     *
-     * <p>The main reason for existing is to be able to call the {@link
-     * IntersectVisitor#visit(IntsRef)} method rather than the {@link IntersectVisitor#visit(int)}
-     * method. This seems to make a difference in performance, probably due to fewer virtual calls
-     * then happening (once per read call rather than once per doc).
-     */
-    private final IntsRef scratchIntsRef = new IntsRef();
-
-    {
-        // This is here to not rely on the default constructor of IntsRef to set offset to 0
-        scratchIntsRef.offset = 0;
-    }
-
     DocIdsWriter() {}
 
     /**
@@ -334,13 +315,7 @@ private static void readDelta16(IndexInput in, int count, int[] docIds) throws I
         final int min = in.readVInt();
         final int half = count >> 1;
         in.readInts(docIds, 0, half);
-        if (count == DEFAULT_MAX_POINTS_IN_LEAF_NODE) {
-            // Same format, but enabling the JVM to specialize the decoding logic for the default number
-            // of points per node proved to help on benchmarks
-            decode16(docIds, DEFAULT_MAX_POINTS_IN_LEAF_NODE / 2, min);
-        } else {
-            decode16(docIds, half, min);
-        }
+        decode16(docIds, half, min);
         // read the remaining doc if count is odd.
         for (int i = half << 1; i < count; i++) {
             docIds[i] = Short.toUnsignedInt(in.readShort()) + min;
@@ -364,18 +339,7 @@ private void readInts21(IndexInput in, int count, int[] docIDs) throws IOExcepti
         int oneThird = floorToMultipleOf16(count / 3);
         int numInts = oneThird << 1;
         in.readInts(scratch, 0, numInts);
-        if (count == DEFAULT_MAX_POINTS_IN_LEAF_NODE) {
-            // Same format, but enabling the JVM to specialize the decoding logic for the default number
-            // of points per node proved to help on benchmarks
-            decode21(
-                docIDs,
-                scratch,
-                floorToMultipleOf16(DEFAULT_MAX_POINTS_IN_LEAF_NODE / 3),
-                floorToMultipleOf16(DEFAULT_MAX_POINTS_IN_LEAF_NODE / 3) * 2
-            );
-        } else {
-            decode21(docIDs, scratch, oneThird, numInts);
-        }
+        decode21(docIDs, scratch, oneThird, numInts);
         int i = oneThird * 3;
         for (; i < count - 2; i += 3) {
             long l = in.readLong();
@@ -401,17 +365,7 @@ private void readInts24(IndexInput in, int count, int[] docIDs) throws IOExcepti
         int quarter = count >> 2;
         int numInts = quarter * 3;
         in.readInts(scratch, 0, numInts);
-        if (count == DEFAULT_MAX_POINTS_IN_LEAF_NODE) {
-            // Same format, but enabling the JVM to specialize the decoding logic for the default number
-            // of points per node proved to help on benchmarks
-            assert floorToMultipleOf16(quarter) == quarter
-                : "We are relying on the fact that quarter of DEFAULT_MAX_POINTS_IN_LEAF_NODE"
-                    + " is a multiple of 16 to vectorize the decoding loop,"
-                    + " please check performance issue if you want to break this assumption.";
-            decode24(docIDs, scratch, DEFAULT_MAX_POINTS_IN_LEAF_NODE / 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE / 4 * 3);
-        } else {
-            decode24(docIDs, scratch, quarter, numInts);
-        }
+        decode24(docIDs, scratch, quarter, numInts);
         // Now read the remaining 0, 1, 2 or 3 values
         for (int i = quarter << 2; i < count; ++i) {
             docIDs[i] = (in.readShort() & 0xFFFF) | (in.readByte() & 0xFF) << 16;

server/src/test/java/org/elasticsearch/index/codec/vectors/diskbbq/DocIdsWriterTests.java

@@ -36,8 +36,6 @@
 import java.util.List;
 import java.util.Set;
 
-import static org.elasticsearch.index.codec.vectors.diskbbq.DocIdsWriter.DEFAULT_MAX_POINTS_IN_LEAF_NODE;
-
 public class DocIdsWriterTests extends LuceneTestCase {
 
     public void testNoDocs() throws Exception {
@@ -50,7 +48,7 @@ public void testRandom() throws Exception {
         int numIters = atLeast(100);
         try (Directory dir = newDirectory()) {
             for (int iter = 0; iter < numIters; ++iter) {
-                int count = random().nextBoolean() ? 1 + random().nextInt(5000) : DEFAULT_MAX_POINTS_IN_LEAF_NODE;
+                int count = 1 + random().nextInt(5000);
                 int[] docIDs = new int[count];
                 final int bpv = TestUtil.nextInt(random(), 1, 32);
                 for (int i = 0; i < docIDs.length; ++i) {
@@ -80,7 +78,7 @@ public void testCluster() throws Exception {
         int numIters = atLeast(100);
         try (Directory dir = newDirectory()) {
             for (int iter = 0; iter < numIters; ++iter) {
-                int count = random().nextBoolean() ? 1 + random().nextInt(5000) : DEFAULT_MAX_POINTS_IN_LEAF_NODE;
+                int count = 1 + random().nextInt(5000);
                 int[] docIDs = new int[count];
                 int min = random().nextInt(1000);
                 final int bpv = TestUtil.nextInt(random(), 1, 16);