Enable simd numeric range usage for not-equals queries - #16568
Draft
parkertimmins wants to merge 5 commits into
Draft
Enable simd numeric range usage for not-equals queries#16568parkertimmins wants to merge 5 commits into
parkertimmins wants to merge 5 commits into
Conversation
Introduces NegationIterator, a DocIdSetIterator that inverts any excluded clause, enabling DenseConjunctionBulkScorer to handle MUST_NOT via bitset intersection rather than ReqExclBulkScorer's per-document exclusion loop. Key optimizations exposed through NegationIterator: - docIDRunEnd() returns the next excluded document position, so large gaps between excluded docs (NO blocks) become collectRange calls — O(1) for TotalHitCountCollector instead of traversing every hit. - intoBitSet() sets all window bits then AND-NOTs the excluded clause's intoBitSet result. For numeric range queries this calls rangeIntoBitSet, enabling SIMD acceleration via the Panama vector API on MAYBE blocks. BooleanScorerSupplier.negatedRequiredBulkScorer() applies this path when the required side is dense FILTER-only (no MUST clauses, no scoring needed, and lead cost meets DenseConjunctionBulkScorer's density threshold). Falls back to ReqExclBulkScorer otherwise. Benchmark results (1M docs, numeric range MUST_NOT, default provider): selectivity=0.01: 32.6 → 175.9 ops/s (+5.4x) selectivity=0.1: 33.0 → 154.6 ops/s (+4.7x) selectivity=0.5: 46.6 → 174.4 ops/s (+3.7x) selectivity=0.9: ~33 → 474.5 ops/s selectivity=0.99: ~33 → 605.2 ops/s With Panama provider, notEquals now matches filterRange performance at all selectivities (e.g. 32.7 → 412.8 ops/s at selectivity=0.01).
NegationTwoPhaseIterator uses DocIdSetIterator.all as its approximation, making advance() O(1) regardless of what the excluded clause looks like. This is slicing-safe: each partition's advance to its start position never iterates through excluded documents. DenseConjunctionBulkScorer calls applyMask() for TwoPhaseIterator clauses (rather than intoBitSet), which removes excluded docs directly from the surviving candidate set. The SIMD subtraction via exclTwoPhase.intoBitSet() / rangeIntoBitSet is preserved. The sorting rule in DenseConjunctionBulkScorer places TwoPhaseIterators after plain DISIs, so the lead clause is always a plain DISI (e.g. the dense term filter), and NegationTwoPhaseIterator never races it for lead position. docIDRunEnd() still exposes NO blocks (gaps before the excluded range) to enable the collectRange shortcut for large unexcluded spans.
Two bugs caused a 2-5x slowdown vs the deleted NegationIterator: 1. Missing intoBitSet() override. When NegationTwoPhaseIterator is the lead clause in DenseConjunctionBulkScorer (which happens when the dense term query's docIDRunEnd() spans the whole window), the scorer calls lead.intoBitSet() on it. Without an override this fell back to TwoPhaseIterator's default per-doc matches() loop, bypassing SIMD entirely and explaining the flat Panama vs default numbers. 2. docIDRunEnd() never advanced exclApprox. The excluded approximation was only positioned inside applyMask(), so docIDRunEnd() always saw a stale position and returned doc+1, preventing the scorer from detecting NO-block spans and using the collectRange shortcut at high selectivity. After the fix notEquals throughput matches filterRange at all selectivities.
Extract shared scratch-fill/andNot/clear logic from intoBitSet() and applyMask() into a private subtractExcluded() helper, eliminating the duplication between the two methods. Rename TestNegationIterator to TestNegationTwoPhaseIterator and expand coverage: randomised correctness test for the TwoPhaseIterator excluded path (including approximation false positives), direct tests for intoBitSet() and applyMask(), docIDRunEnd() edge cases (NO-block with fresh iterator, excl at current doc), all-excluded and none-excluded edge cases, and a test verifying the scoring fallback to ReqExclBulkScorer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#16050 added numeric range queries to make use of SIMD. #16307 added usage of
docIDRunEnd()toReqExclBulkScorer, allowingmust_notqueries to make use of the bulk logic indocIDRunEnd(). This is called on the excluded TwoPhaseIterator, allowing it to quickly skip a large group of excluded docs. Because of this, queries with lots of excluded docs are efficient.But queries with few excluded docs cannot use
docIDRunEnd()bulk logic and fall back to per-element checking. This can be seen in this benchmark. It tests a numeric range query with either afilteror amust_not. The selectivity is how many of the filter queries matched. For the must_not query it's now the percent that didn't match. (So techniquely it's1-selectivity).The importantly thing to note are that:
The reason for 1. is because as the exclude filter matches more documents,
docIDRunEnd()can be used more. (At least I think that's why.)Which brings us to this PR. We want to be able to take advantage of bulk logic for must_not queries. Here are the results from the change in this PR: