javanna commented on code in PR #16452:
URL: https://github.com/apache/lucene/pull/16452#discussion_r3768185011
##########
lucene/misc/src/java/org/apache/lucene/misc/search/MemoryAccountingBitsetCollectorManager.java:
##########
@@ -47,21 +55,25 @@ public MemoryAccountingBitsetCollector newCollector() {
@Override
public Result reduce(Collection<MemoryAccountingBitsetCollector> collectors)
{
- int globalMaxDocEnd = 0;
+ // Size the result to just cover the highest matched doc across all
collectors. Each
+ // collector's maxDocEnd is inflated by doSetNextReader to the full leaf
regardless of what
+ // actually matches, so keying off it can significantly over-allocate on
selective queries or
+ // narrow intra-segment slices; use the actual high-water mark tracked at
collect time.
+ int resultSize = 0;
for (MemoryAccountingBitsetCollector collector : collectors) {
- globalMaxDocEnd = Math.max(globalMaxDocEnd, collector.getMaxDocEnd());
+ int last = collector.getHighestSetBit();
+ if (last >= 0) {
+ resultSize = Math.max(resultSize, collector.getMinDocBase() + last +
1);
+ }
}
- // TODO: with intra-segment concurrency enabled, globalMaxDocEnd equals
the full index maxDoc
- // even when only a portion of the index was searched, causing
over-allocation of the result
- // bitset.
- FixedBitSet result = new FixedBitSet(globalMaxDocEnd);
+ FixedBitSet result = new FixedBitSet(resultSize);
Review Comment:
When a query matches no documents, reduce() now returns new FixedBitSet(0).
The following `result.bitSet().nextSetBit(0)` then hits nextSetBitInRange(0,
0) on an empty long[] and throws ArrayIndexOutOfBoundsException.
The previous code was safe here: doSetNextReader() fired for every visited
leaf, so globalMaxDocEnd was always > 0 even when nothing matched.
nextSetBit(0) safely returned -1.
The new test testResultBitSetEmptyOnNoMatches only asserts cardinality() and
length() — it doesn't catch this.
##########
lucene/CHANGES.txt:
##########
@@ -434,6 +434,9 @@ Bug Fixes
GlobalOrdinalsWithScoreCollector, fixing an intermittent failure caused by
non-associative float addition that #16378 missed. (Luca Cavanna)
+* GITHUB#16452: Fix over-allocation in
MemoryAccountingBitsetCollectorManager.Result#bitSet, which
Review Comment:
can you fix the merge conflicts here please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
