Reduce long[] array allocation for bitset in readBitSetIterator

[easyice]

I got a memory allocation flamegraph from my cluster, It shows the DocIdsWriter#readBitSetIterator allocating about ~14GB RAM in 30 seconds, this proposal uses a scratchLongs[] array instead of allocating a new array.

mem.html.zip

When using some mock data that let DocIdsWriter#readBitSetIterator allocate a long[] array of size 96, the JMH output shows:

main:

Benchmark                                Mode  Cnt        Score     Error   Units
BKDWithBitMap.search                     avgt    5       65.001 ±   0.579   us/op
BKDWithBitMap.search:gc.alloc.rate       avgt    5    18626.467 ± 166.049  MB/sec
BKDWithBitMap.search:gc.alloc.rate.norm  avgt    5  1269568.090 ±   0.001    B/op
BKDWithBitMap.search:gc.count            avgt    5      423.000            counts
BKDWithBitMap.search:gc.time             avgt    5      218.000                ms

pr:

Benchmark                                Mode  Cnt       Score     Error   Units
BKDWithBitMap.search                     avgt    5      63.622 ±   0.784   us/op
BKDWithBitMap.search:gc.alloc.rate       avgt    5    9582.419 ± 118.030  MB/sec
BKDWithBitMap.search:gc.alloc.rate.norm  avgt    5  639272.088 ±   0.001    B/op
BKDWithBitMap.search:gc.count            avgt    5     209.000            counts
BKDWithBitMap.search:gc.time             avgt    5     111.000                ms

JMH Code

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@State(Scope.Benchmark)
@Warmup(iterations = 3, time = 3)
@Measurement(iterations = 5, time = 5)
@Fork(
    value = 1,
    jvmArgsPrepend = {"--add-modules=jdk.unsupported"})
public class BKDWithBitMap {
  IndexReader reader;
  IndexSearcher searcher;
  Query q = LongPoint.newSetQuery("f", 0);

  @Setup
  public void setup() throws IOException {
    Path path = Files.createTempDirectory("points");
    Directory dir = MMapDirectory.open(path);

    try (IndexWriter w =
        new IndexWriter(
            dir,
            new IndexWriterConfig()
                .setMaxBufferedDocs(IndexWriterConfig.DISABLE_AUTO_FLUSH)
                .setRAMBufferSizeMB(256.0))) {
      int actualIndexed = 0;
      for (int i = 0; i < 5_000_000; ++i) {
        Document doc = new Document();
        doc.add(new StringField("id", Long.toString(i), Field.Store.NO));
        if (i % 12 == 0) {
          doc.add(new LongPoint("f", 0));
        }
        w.addDocument(doc);
        if (actualIndexed++ % 1_000_000 == 0) {
          System.out.println("Indexed: " + actualIndexed);
        }
      }
      w.commit();
      w.forceMerge(1);
      w.commit();
    }

    reader = DirectoryReader.open(dir);
    searcher = new IndexSearcher(reader);
    searcher.setQueryCache(null);
  }

  @Benchmark
  public void search(Blackhole bh) throws IOException {
    Weight weight = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1f);
    ScorerSupplier ss = weight.scorerSupplier(reader.leaves().get(0));
    if (ss != null) {
      Scorer scorer = ss.get(Long.MAX_VALUE);
      bh.consume(scorer);
    }
  }
}

[gf2121]

I left some minor comments, otherwise LGTM. Thanks @easyice !

[gf2121]

You can use LongsRef.EMPTY_LONGS here.

[gf2121]

Do we need this comparison when growNoCopy check this as well?

[easyice]

It’s an interesting question, currently we have some places that perform the comparison before growNoCopy and some that don't. In #13171, I found a slight performance impact regarding the extra assignment like scratchLongs = scratchLongs. But indeed, keeping things simple is more important.

[gf2121]

Can we make scratchLongs a LongsRef to know how many words we used last time, so that we can avoid this clear when unnecessary?

[easyice]

Good idea. If the current longLen is greater than the previous value, we don’t need to perform the clear.

[gf2121]

LGTM, Thanks @easyice !