Add TwoPhaseIterator.applyMask() method

[romseygeek]

TwoPhaseIterator.intoBitSet() can vectorize two-phase matching across a window of documents, but it does not take into account any previous conjunction clauses that could have excluded documents in the window from consideration. This can cause inefficiencies in DenseConjunctionBulkScorer, where we use intoBitSet for windows that we know have more than 25% matches - for example, a window with 26% matching documents when combined with a two-phase iterator whose approximation matches everything will call match() on the 74% of documents in the window that have already been excluded.

We add a complementary method applyMask() to TwoPhaseIterator that will call matches() on all documents marked as selected in a passed-in BitSet, and clear them if they do not pass. DocValuesRangeIterator can use a bulk implementation, while the default implementation still avoids matching documents already excluded by earlier clauses. DenseConjunctionBulkScorer always calls applyMask() when presented with a TwoPhaseIterator as a trailing clause.

[jimczi]

Reviewed the applyMask change. The design is clean and the default-impl tests via RandomTwoPhaseView are nice. Two things worth a look before merge — a correctness bug in the DocValuesRangeIterator override for conjunctions of 3+ clauses, and a spot where the MAYBE branch drops the vectorized path this PR is trying to protect — plus a couple of minor notes inline.

One more nit that doesn't land on a changed line: the comment at DenseConjunctionBulkScorer.java:228 ("the bit set's own WINDOW_SIZE/4 bulk-confirm cutoff") now references the bulkConfirmThreshold = windowSize / 4 that this PR removes — stale after this change.

[jimczi]

Correctness: leading NO-gap candidates are never cleared.

This loop starts at blockStart = blockIterator.docID(), i.e. the first matching block >= windowBase, and only clears NO-gaps between and after processed blocks. It never clears [offset, firstBlockStart), and if the block iterator starts >= upTo the loop body doesn't run and nothing is cleared — even though no doc in the window matches this clause. The removed dense path got this for free (intoBitSet(...) then windowMatches.and(...) zeroes the leading gap); the default applyMask also handles it by walking every candidate.

Not reachable with a simple two-clause conjunction (the sole trailing two-phase clause always sits exactly at windowBase after scoreWindow's alignment loop). It needs 3+ driving iterators so the offending range is a middle clause — e.g. +term +field1:[a TO b] +field2:[c TO d] (all skip-indexed), three range filters, or a range + collector competitiveIterator(). A later clause raises min past the range's block, then scoreWindowUsingBitSet re-advances it to windowBase; if the block covering windowBase is a NO block, advance lands past windowBase and the leading candidate bits survive → docs that pass the lead clause but violate the range are collected as false matches.

Suggest clearing ahead of each block, not just after it: track a cursor from offset, clear [cursor, blockStart) before each block and [cursor, upTo) after the loop. Worth a regression test with a non-zero offset, a leading NO-gap, and the approximation->= upTo case — the current tests all position the approximation at the first candidate, so this path is uncovered.

[jimczi]

The MAYBE branch drops the vectorized path for dense candidate windows.

This confirms candidates one at a time (scalar advance + matches()), whereas intoBitSet's MAYBE case delegates to intoMaybeBlock → numericValues.rangeIntoBitSet(...) (SIMD). A single range query keeps SIMD because the range is the leading clause (intoBitSet), but in a conjunction the range sorts after plain approximations and becomes a trailing clause → applyMask. Since WINDOW_SIZE == 4096 == the skip-block size, a dense candidate window is typically one MAYBE block — exactly where the old bulkConfirmThreshold split used the vectorized decode. So this reintroduces the doc-values range regression the PR is trying to avoid, just relocated to the conjunction case.

The YES_IF_PRESENT branch just above already shows the fix shape: bulk-scan into a scratch bitset, then FixedBitSet.andRange(scratch, ...). The MAYBE branch could do the same with rangeIntoBitSet into scratch — keeping SIMD while still never confirming a doc another clause already excluded.

[jimczi]

Minor: this loop is bounded by bitSet.length() rather than upTo - offset, so the default applyMask will confirm/clear candidate bits for docs >= upTo and can advance the approximation past upTo. The DocValuesRangeIterator override strictly stops at upTo, so the two implementations disagree on bits beyond upTo. Latent today (the sole caller's windowMatches has no set bits >= windowMax - windowBase), but for a newly public method it'd be safer to bound by upTo - offset to match the override and intoBitSet semantics.

[romseygeek]

Thanks for the review @jimczi! I added tests for the cases you picked up and pushed fixes.

[jimczi]

LGTM, thanks for the quick turnaround on the review comments.

I did a final pass on the updated applyMask and everything checks out:

• Correctness — the cursor-based clearing now covers the leading NO-gap, inter-block gaps, and the whole-window-is-a-NO-gap case (approximation already >= upTo), so the false-positive path for a middle range clause in a 3+-clause conjunction is closed. Bounds on the nextSetBit/andRange/clear calls all hold, and disi positioning stays forward-only (YES_IF_PRESENT re-syncs, MAYBE's from = max(blockStart, disi.docID()) handles skipped/empty blocks; docs without a value are correctly excluded from the range).
• Performance — no regressions, wins both ways: generic two-phase clauses (the script-query case) confirm only surviving candidates via the default applyMask instead of decoding the whole approximation, while doc-values ranges keep the vectorized rangeIntoBitSet for MAYBE blocks and now skip blocks with no candidates. applyMask only runs on dense windows, so the whole-block SIMD decode is the right tradeoff.
• The default TwoPhaseIterator.applyMask bound (min(bitSet.length(), upTo - offset)) keeps it from touching bits at/beyond upTo, matching the override.

The added regression tests cover exactly the previously-uncovered paths (leading NO-gap, approximation past upTo, MAYBE using the vectorized path, and the >= upTo bound). Nice work.