perf(fts): remove OR hot path bookkeeping#7416
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
One minor suggestion from Claude:
rust/lance-index/src/scalar/inverted/index.rs ~line 884 — Scoring loop body is duplicated ~25 lines between the two branches. The fast path and full path share identical logic for addr resolution, score accumulation over freqs, and heap push/pop. Only the grouped_positions.contains guard and the grouped-expansion inner loop differ. Extracting the shared scaffolding (addr resolution + heap update) into a closure or small helper would halve the duplication and reduce maintenance risk when either scoring formula changes in the future. This is a cleanup concern, not a correctness bug.
7a8cb25 to
94c6e2a
Compare
2 participants
Performance Improvement
What is the performance issue or bottleneck?
Recent FTS changes added AND/fuzzy bookkeeping to shared OR query paths. Ordinary non-fuzzy OR searches were still allocating and checking grouped expansion state, maintaining matched-position tracking in posting-list loading, and recording AND-only WAND metrics even though those values are only meaningful for conjunction/grouped expansion paths.
How does this PR improve performance?
This PR keeps ordinary OR on the lightweight path:
HashSetcreation and per-frequency grouped checks unless grouped fuzzy expansions are presentOperator::AndGrouped fuzzy AND rescoring remains on its existing grouped path so matched-expansion scoring stays correct.
Benchmark or measurement results
No benchmark was run in this PR. The change is intentionally scoped to hot-path bookkeeping removal; benchmark validation can run later on the existing benchmark branch/VM if needed.
Bug Fix
What is the bug?
The ordinary OR path was paying for bookkeeping intended for AND/fuzzy/grouped expansion correctness.
What issues or incorrect behavior does the bug cause?
It caused avoidable overhead in OR query execution and showed up as a small regression in OR benchmark configurations after the recent FTS changes.
How does this PR fix the problem?
The OR path now bypasses the AND/fuzzy-only bookkeeping while preserving the grouped expansion branch used by fuzzy AND.
Validation
cargo fmt --allcargo fmt --all --checkgit diff --checkCARGO_TARGET_DIR=/tmp/lance-target-fts-or-hotpath cargo clippy -p lance-index --tests -- -D warningsCARGO_TARGET_DIR=/tmp/lance-target-fts-or-hotpath cargo test -p lance-index scalar::inverted::wand::tests -- --nocaptureCARGO_TARGET_DIR=/tmp/lance-target-fts-or-hotpath cargo test -p lance-index scalar::inverted::index::tests::test_bm25_search_uses_global_idf -- --nocaptureCARGO_TARGET_DIR=/tmp/lance-target-fts-or-hotpath cargo test -p lance-index scalar::inverted::index::tests::test_and_query_returns_empty_when_exact_term_missing -- --nocaptureCARGO_TARGET_DIR=/tmp/lance-target-fts-or-hotpath cargo test -p lance-index scalar::inverted::index::tests::test_fuzzy_and -- --nocapture