perf(fts): bulk MAXSCORE search path for top-k disjunctions#7603
Conversation
0781681 to
acac9d8
Compare
0ec1da3 to
96b172d
Compare
|
Rebased on the updated train: the maxscore paths fetch scoring doc lengths through DocSet::scoring_num_tokens (quantized on V3 partitions per #7466), and the ImpactScoreCache machinery this rework makes dead is now removed here (it previously tripped clippy -D warnings downstream). No behavior change beyond the rebase. |
## Feature Linear: [OSS-1344](https://linear.app/lancedb/issue/OSS-1344/make-fts-index-block-size-configurable) ### What is the new feature? FTS inverted index creation now accepts a `block_size` parameter for compressed posting blocks. Supported values are `128` and `256`. ### Why do we need this feature? The posting block size was previously fixed at `128`, which made the block-max granularity impossible to tune for different datasets and query profiles. ### How does it work? - Adds `block_size` to `InvertedIndexParams`, protobuf details, posting-list schema metadata, and cache headers. - Uses `128` as the default for newly created indexes. - Treats older serialized params, schema metadata, and cache entries that omit `block_size` as legacy `128`. - Rejects unsupported values, including `512`, with a clear validation error. - Uses Lance-owned `BitPacker4x` for physical 128-value posting blocks and `BitPacker8x` for physical 256-value posting blocks. - Marks `block_size=256` as experimental in public API docs because it may introduce breaking changes. - Keeps position-stream packing on the legacy 128-value block format. - Keeps downgrade compatibility tests on explicit legacy `block_size=128`, since older wheels cannot read current-created physical 256 FTS posting blocks. - Threads the configured block size through FTS build, read, iterator, WAND, cache, and MemWAL flush paths. - Exposes the parameter in Python and Java FTS index creation APIs, with docs and focused tests. ## Validation - `cargo fmt --all` - `cargo fmt --all --check` - `git diff --check` - `CARGO_TARGET_DIR=/tmp/lance-target-a479-no512 cargo test -p lance-index block_size -- --nocapture` - `CARGO_TARGET_DIR=/tmp/lance-target-a479-no512 cargo clippy -p lance-index --tests -- -D warnings` - `uv run make build` from `python/` - `uv run pytest python/tests/test_scalar_index.py::test_create_scalar_index_fts_block_size` from `python/` - `uv run ruff format --check python/tests/test_scalar_index.py python/lance/dataset.py` from `python/` - `uv run ruff check python/tests/test_scalar_index.py python/lance/dataset.py` from `python/` - `CARGO_TARGET_DIR=/tmp/lance-target-a479-merge-main cargo test -p lance-index block_size -- --nocapture` - `CARGO_TARGET_DIR=/tmp/lance-target-a479-merge-main cargo test -p lance-index test_256_posting_block_uses_single_physical_bitpack_chunk -- --nocapture` - `CARGO_TARGET_DIR=/tmp/lance-target-a479-merge-main cargo test -p lance-bitpacking` - `CARGO_TARGET_DIR=/tmp/lance-target-a479-merge-main cargo clippy -p lance-bitpacking -p lance-index --tests -- -D warnings` - `uv run ruff format --check python/tests/compat/test_scalar_indices.py` from `python/` - `uv run ruff check python/tests/compat/test_scalar_indices.py` from `python/` - `uv run pytest --run-compat -vvv -s python/tests/compat/test_scalar_indices.py::test_FtsIndex_downgrade --durations=30` from `python/` - `CARGO_TARGET_DIR=/tmp/lance-a479-target cargo test -p lance-index test_new_training_request_defaults_missing_block_size_to_128` - `CARGO_TARGET_DIR=/tmp/lance-a479-target cargo test -p lance-index block_size` - `uv run ruff format --check python/lance/dataset.py` from `python/` - `uv run ruff check python/lance/dataset.py` from `python/` Not run locally: Java focused test / spotless check, because this machine has no Java Runtime installed (`Unable to locate a Java Runtime`). --- ## Update: all V3 breaking changes consolidated here Per review direction, every breaking change for the 256-doc block format now lands in this single PR (the follow-up stack #7602/#7603/#7604/#7624/#7625/#7629 carries none). On top of the configurable block size and PFOR frequency encoding, this PR now also includes: - **Quantized doc-length scoring (Lucene norm semantics), 256-doc blocks only.** BM25 doc lengths are quantized to a SmallFloat-style byte code (4 mantissa bits: 0-7 exact, <= 6.25% relative error, decode = bucket floor). The byte-norm slab bakes lazily per loaded DocSet and quarters the doc-length bytes scoring pulls through the cache (200M docs: 800MB -> 200MB). 128-block indexes keep exact-length scoring bit-for-bit. Measured top-k overlap vs exact scoring on the (score-clustered, synthetic) mmlb corpus: 98.1% mean for phrase, 89.7% for 3-word AND; corpora with more score spread shift less. - **256-doc posting blocks drop the leading block-max-score f32** (~1.5G on a 200M-doc index; 131G -> 130G). Block layout: `[first_doc u32][doc num_bits u8][docs][pfor freqs]`; `posting_block_score_prefix_len(block_size)` keys every reader/writer. The impact skip data from the stacked #7602 supplies a tighter per-block bound; until it lands, 256-block block-max pruning falls back to the (valid, looser) list-level max score. **BREAKING:** 256-doc-block (v3) indexes must be rebuilt; v3 is unreleased so no migration is provided. BM25 scores on v3 differ from exact-length BM25 by the norm quantization, matching Lucene's norm semantics. The format discussion #7606 documents the final layout and scoring semantics. Additional validation for this update: bulk-vs-classic A/B under quantized scoring is score-identical (both paths quantize identically); the full stack's warm benchmarks vs Lucene 10.4 on mmlb-200m: OR k10 0.0249s/318qps and OR k100 0.0467s/170qps (both ahead of Lucene sliced), AND k10 0.0443s (1.29x), AND k100 0.0883s (1.94x). --- ## Standalone results vs main (per-branch-tip wheels) **Legacy (128) read-path parity.** Threading a runtime `block_size` through `PostingIterator` initially replaced the compile-time `BLOCK_SIZE` division (a shift) with real `div` instructions in the `doc()`/`next()` hot loops, measured as +11-14% on 3-word OR against the 200M legacy index (`PostingIterator::next` grew from 16.5% to 23.6% of the profile). Block sizes are validated powers of two, so the iterator now derives block indices with `trailing_zeros` shifts and masks; after that fix the legacy path is at parity with main: 3-word OR k10 0.131s (main 0.132-0.134s), k100 0.255-0.256s (main 0.256-0.257s), single-term 0.025s (main 0.027s) across 3 warm passes on the 200M legacy index, 400G cache. **block_size=256 index size** (5M-doc controlled build, same wheel, `with_position=false`): postings shrink **3.33 GiB → 2.62 GiB (−21%)** from PFOR frequencies + no per-block max-score prefix + half the block headers. Index build 192s → 210s (+10%, PFOR encode cost). Top-10 overlap vs the 128 exact-length scoring on 10 3-word OR queries: 95% mean (5/10 identical sets; the rest differ by 1-2 near-tie docs, from the quantized-norm scoring). **Query wins for 256 land in the stacked PRs.** A 256 index without impact skip data prunes on the (valid, looser) list-level max and is *slower* than 128 — e.g. classic AND k10 0.547s until #7602's impacts restore block-granular bounds (0.115s), and #7603/#7604/#7624/#7625/#7629 take the same index to 0.025s OR k10 / 0.045s AND k10. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable FTS posting `block_size` (128/256) to scalar index creation, including updated examples and APIs. * Enabled FTS format version 3 (`v3`) for `block_size=256`, with quantized doc-length scoring for v3. * **Bug Fixes** * Enforced `block_size`/`format_version` compatibility (invalid combinations now error). * Persisted and restored FTS metadata for format version and posting block size, with legacy indexes defaulting to `128`. * **Documentation** * Updated full-text-search and quickstart guides and parameter docs for `block_size`, defaults, accepted values, and the experimental `256`/`v3` behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Yang Cen <yang@lancedb.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
4db6b8c to
4d334d8
Compare
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
Port of Lucene's MaxScoreBulkScorer, opt-in via LANCE_FTS_MAXSCORE=1: per outer window (bounded by the essential clauses' blocks with adaptive growth), clauses split into a non-essential prefix and essential rest by window max score vs the running threshold. Essential clauses bulk-stream decompressed blocks (single-essential windows stream with no accumulator); non-essential clauses are only probed for candidates that can still beat the threshold. Dead ranges with one live clause skip by scanning the baked per-block bound slab. Candidate emission matches the classic path (must beat the running threshold), so results are score-identical. Measured on a 200M-doc warm benchmark at 24 partitions: 3-word OR match k10 0.137s -> 0.035s, k100 0.250s -> 0.064s; hot single-term 250ms -> 3ms.
acac9d8 to
38a6670
Compare
📝 WalkthroughWalkthroughImpact skip-data accessors and anchored caches now support impact-aware WAND bounds. Compressed OR queries can optionally use bulk MAXSCORE execution, while level-1 impact groups provide additional pruning and skipping. ChangesImpact-aware WAND execution
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant WandSearch as Wand::search
participant MaxScore as maxscore_search
participant Posting as PostingIterator
participant Accumulator as WindowAccumulator
participant TopK as TopKHeap
WandSearch->>MaxScore: dispatch eligible OR query
MaxScore->>Posting: collect essential clause scores
Posting->>Accumulator: accumulate window scores and frequencies
MaxScore->>Posting: probe remaining clauses
MaxScore->>TopK: update candidates and threshold
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance-index/src/scalar/inverted/impact.rs`:
- Around line 295-308: Restrict the visibility of
ImpactScoreCache::level1_score_cached to crate scope by changing its declaration
from pub to pub(crate), since its consumer is internal to the crate. Keep the
method’s behavior and signature otherwise unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f65e25ab-0008-48c9-a6a3-3ab017316247
📒 Files selected for processing (2)
rust/lance-index/src/scalar/inverted/impact.rsrust/lance-index/src/scalar/inverted/wand.rs
| let doc_length = self.docs.scoring_num_tokens(doc as u32); | ||
| let score = essential_weight * self.scorer.doc_weight(freq, doc_length); | ||
| if !(self.threshold > 0.0 | ||
| && score + total_non_essential_bound <= self.threshold) |
There was a problem hiding this comment.
prefix_bound is accumulated with raw f32 addition but is treated as a conservative upper bound here. This can round down and reject a competitive document: with non-essential bounds 6.2868384e-7 and 0.015441144, essential score 2.7624962, and threshold 2.7779378891, this comparison rejects on equality while the path's actual accumulation yields 2.7779381275 > threshold. Since this path is now enabled by default, exact top-k results can differ from the classic WAND loop.
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/lance-index/src/scalar/inverted/wand.rs (1)
1798-1892: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a
Wand::search()case for a non-defaultRowAddrMask. The current tests inrust/lance-index/src/scalar/inverted/wand.rsonly hit this masking logic throughflat_search(...)orsearch(..., RowAddrMask::default()); add a compressed OR case that exercisesmaxscore_search()with masked and tombstoned rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-index/src/scalar/inverted/wand.rs` around lines 1798 - 1892, Add a Wand::search() test case using a non-default RowAddrMask that routes through maxscore_search(), with compressed OR postings containing both masked rows and tombstoned rows. Assert the results match the expected unmasked live rows, covering the docs_has_row_ids masking logic in consider_candidate without relying on flat_search() or RowAddrMask::default().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@rust/lance-index/src/scalar/inverted/wand.rs`:
- Around line 1798-1892: Add a Wand::search() test case using a non-default
RowAddrMask that routes through maxscore_search(), with compressed OR postings
containing both masked rows and tombstoned rows. Assert the results match the
expected unmasked live rows, covering the docs_has_row_ids masking logic in
consider_candidate without relying on flat_search() or RowAddrMask::default().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d9d8dc2-3875-4748-8f26-02eda0c7f3c3
📒 Files selected for processing (2)
rust/lance-index/src/scalar/inverted/impact.rsrust/lance-index/src/scalar/inverted/wand.rs
|
The failed CI is caused by rust toolchain bug, not related to this PR, merge it |
) Built on the MAXSCORE work merged in #7603; this is the next PR in the Lucene-parity series. This PR now includes the 2/3-clause SIMD and frequency-bound work previously stacked as #7625. ## Performance issue Top-k AND and phrase queries previously leapfrogged doc-at-a-time through boxed `PostingIterator::next` calls. Phrase checks additionally decoded a whole 256-doc position block per candidate and allocated cursor vectors per candidate. ## What changed - Add a bulk conjunction path for compressed top-k AND and phrase queries. It keeps the classic block-max window pruning semantics while intersecting decompressed posting slices in batches. - Specialize the 2- and 3-clause merge kernels. x86_64 uses runtime-dispatched AVX2 catch-up scans, with scalar kernels on other CPUs. - Add a frequency-bucketed score-bound LUT so lead docs that cannot beat the threshold are rejected before follower advances. - Keep a generic merge kernel for 4+ clauses when bulk mode is explicitly forced. - Score candidates in two passes so document-length loads are issued back-to-back. - Decode only the PackedDelta position groups needed by the current phrase candidate instead of the whole position block. - Use an allocation-free exact-phrase check and recycled owned position buffers. - Return contextual errors for malformed packed-position data instead of panicking. ## Runtime selection `LANCE_FTS_BULK_AND` accepts: - `auto` (default): use bulk for 2/3 effective posting clauses and classic for all other widths. - `on` or legacy `1`: force bulk for every eligible compressed AND/phrase query; 4+ clauses use the generic kernel. - `off` or legacy `0`: always use the classic conjunction loop. The clause count is measured after tokenization, deduplication, and expansion. Invalid values warn and fall back to `auto`. The 4/5-clause experiments did not show a consistent specialized-kernel win, and generic bulk regressed against classic in several tiers. Therefore no 4/5-clause specialized kernels are retained and `auto` remains limited to 2/3 clauses. ## Measured performance Per-branch-tip wheels, 1000 queries × 8 concurrent. AND used the warm, fully prewarmed 200M-doc V3/256 index; phrase used the 50M-doc V3/256 positions index. | query | previous stack base | combined PR | |---|---:|---:| | AND 3w k10 @200M | 70 qps | **172 qps (2.46×)** | | AND 3w k100 @200M | 33 qps | **86 qps (2.61×)** | | phrase 3w k10 @50m | 19 qps | **35 qps (1.84×)** | | phrase 2w k10 @50m | 59 qps | **102 qps (1.73×)** | The AWS 4/5-clause follow-up used an `r7i.24xlarge`, the 200M English-only MMLB dataset, a V3/256 index with 338 partitions, k=100, c32, a 600 GB cache, and synchronous full prewarm. ## Compatibility This is a query-time implementation change. It does not change the FTS index format or index version. ## Verification - Bulk/classic/auto parity and dispatch coverage for 2 through 6 clauses, including nonzero phrase slop. - PackedDelta per-doc seek parity across group boundaries and tails. - PackedDelta and VarintDocDelta cursor-recycling coverage. - Malformed packed-position data returns a recoverable search error. - Final WAND suite: 80 passed. - `cargo test -p lance-index`, `cargo clippy --all --tests --benches -- -D warnings`, and `cargo fmt --all -- --check` passed across the reviewed stack. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Performance Improvements** * Added a faster bulk AND/phrase execution path for compressed postings with block-level skipping and slice intersection; availability controlled via `LANCE_FTS_BULK_AND` (auto by default). * Improved phrase verification to decode only required positions per matching document. * Optimized packed-delta position seeking using cached group state and efficient tail handling. * **Bug Fixes** * Ensured bulk AND/phrase results match classic behavior, including filtering and pruning semantics. * Improved rejection of malformed packed-position data. * **Tests** * Added seek accuracy coverage across group boundaries and tail cases, plus malformed-group rejection and cursor independence checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Yang Cen <yang@lancedb.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Port of Lucene's MaxScoreBulkScorer, enabled by default for compatible top-k OR queries. Set
LANCE_FTS_MAXSCORE=0to fall back to the classic WAND loop:per outer window (bounded by the essential clauses' blocks with adaptive
growth), clauses split into a non-essential prefix and essential rest by
window max score vs the running threshold. Essential clauses bulk-stream
decompressed blocks (single-essential windows stream with no accumulator);
non-essential clauses are only probed for candidates that can still beat
the threshold. Dead ranges with one live clause skip by scanning the baked
per-block bound slab. Candidate emission matches the classic path, so
results are score-identical (verified over a 40-case A/B snapshot).
Measured vs #7602 (its base)
Per-branch-tip wheels, 200M-doc v3-256 index, 1000 3-word OR queries × 8
concurrent, warm, default settings:
🤖 Generated with Claude Code
Summary by CodeRabbit
LANCE_FTS_MAXSCORE(with the prior approach as fallback).