perf(index): norm-addend cache and slim top-k heap for FTS scoring#7629
perf(index): norm-addend cache and slim top-k heap for FTS scoring#7629BubbleCal wants to merge 2 commits into
Conversation
## 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>
62bd294 to
458a96e
Compare
a1aea58 to
081c3c0
Compare
📝 WalkthroughWalkthroughThe PR adds BM25 document-norm access and cached scoring across WAND search paths. It also changes candidate heaps to store frequency-log indices, materializing term frequencies only when results are emitted. ChangesWAND scoring and candidate representation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Wand
participant Scorer
participant norm_k_cache
participant bm25_doc_weight_with_norm
Wand->>Scorer: request doc_norm for quantized lengths
Scorer->>norm_k_cache: provide denominator addends
Wand->>norm_k_cache: retrieve cached addend
Wand->>bm25_doc_weight_with_norm: score frequency with cached addend
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
458a96e to
5844cfe
Compare
081c3c0 to
a030f02
Compare
5844cfe to
02a7752
Compare
a030f02 to
135c15b
Compare
02a7752 to
110540f
Compare
135c15b to
d3fb0e8
Compare
Two hot-loop changes, both result-identical (scores are bit-equal; only tie ordering among equal scores can shift with the slimmer heap tuple): - Scorer::doc_norm exposes the BM25 doc-length denominator addend, and quantized (v3) searches bake a per-norm-code addend cache once per partition search (Lucene's norm cache). The OR streaming/drain loops and the bulk conjunction pass score with one byte-norm load plus the cached addend instead of recomputing the denominator per doc — the factored expressions are evaluated identically, so scores don't move. - Top-k heap entries shrink to a Copy tuple; the per-candidate (term, freq) pairs go to an append-only side log and only the final top-k materialize a Vec, removing the per-insert allocation. Warm mmlb, 8 concurrent: OR@200M k10 0.0343->0.0249s (318qps), k100 0.0628->0.0467s (170qps) — both now ahead of Lucene 10.4's sliced searcher; AND@200M k10 0.0456->0.0443s, k100 0.0916->0.0883s; phrase@50M 2w 0.0392->0.0370s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d3fb0e8 to
aa084bd
Compare
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.
Actionable comments posted: 3
🤖 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/scorer.rs`:
- Around line 137-144: The BM25 norm-addend calculation is duplicated across
both scoring paths. In MemBM25Scorer, extract the shared formula into a private
norm_addend/doc_norm-backed helper and have doc_weight reuse it; apply the same
change to IndexBM25Scorer at rust/lance-index/src/scalar/inverted/scorer.rs
lines 137-144 and 213-220, preserving identical scoring behavior.
In `@rust/lance-index/src/scalar/inverted/wand.rs`:
- Around line 2129-2143: Extract the duplicated per-document norm-addend lookup
into a shared norm_addend_for(norm_k_ref, doc) helper, then replace the inline
lookup in rust/lance-index/src/scalar/inverted/wand.rs lines 2129-2143 and lines
2373-2378 with calls to that helper. Preserve the existing optional-result
behavior and scoring logic at both sites.
- Around line 1547-1566: Update norm_k_cache to remove the separate doc_norm(0)
probe and build the 256-entry cache by propagating None from each
doc_norm(dequantize_doc_length(...)) call, returning Some only when every entry
succeeds and eliminating the expect panic.
🪄 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: f6b0ce14-b426-45a1-8b80-c846a558d2c7
📒 Files selected for processing (2)
rust/lance-index/src/scalar/inverted/scorer.rsrust/lance-index/src/scalar/inverted/wand.rs
| // One byte-norm load + cached addend when available; | ||
| // the exact doc length is only needed at insert time. | ||
| let norm_addend = | ||
| norm_k_ref.map(|(norms, cache)| cache[norms[doc as usize] as usize]); | ||
| let score = match norm_addend { | ||
| Some(addend) => { | ||
| essential_weight * bm25_doc_weight_with_norm(freq, addend) | ||
| } | ||
| None => { | ||
| essential_weight | ||
| * self | ||
| .scorer | ||
| .doc_weight(freq, self.docs.scoring_num_tokens(doc as u32)) | ||
| } | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Per-doc norm-addend lookup duplicated verbatim in maxscore_search. The same one-liner norm_k_ref.map(|(norms, cache)| cache[norms[doc as usize] as usize]) appears in both the single-essential-clause fast path and the drain-loop completion path.
rust/lance-index/src/scalar/inverted/wand.rs#L2129-L2143: replace with a call to a sharednorm_addend_for(norm_k_ref, doc)helper.rust/lance-index/src/scalar/inverted/wand.rs#L2373-L2378: replace with the same helper call.
As per coding guidelines, "Extract logic repeated in two or more places into a shared helper."
📍 Affects 1 file
rust/lance-index/src/scalar/inverted/wand.rs#L2129-L2143(this comment)rust/lance-index/src/scalar/inverted/wand.rs#L2373-L2378
🤖 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 2129 - 2143,
Extract the duplicated per-document norm-addend lookup into a shared
norm_addend_for(norm_k_ref, doc) helper, then replace the inline lookup in
rust/lance-index/src/scalar/inverted/wand.rs lines 2129-2143 and lines 2373-2378
with calls to that helper. Preserve the existing optional-result behavior and
scoring logic at both sites.
| /// top-k get their pairs materialized. | ||
| type TopKHeap = BinaryHeap<Reverse<(ScoredDoc, u32, u64, u32)>>; | ||
|
|
||
| /// Append-only (term, freq) log for heap candidates: flat storage plus |
There was a problem hiding this comment.
The append-only FreqLog makes retained memory proportional to historical heap admissions rather than the live top-k. A valid corpus with BM25 scores increasing in doc-id order can replace the kth entry for every scored document, changing the collector from O(k × clauses) to O(scored_docs × clauses) retained memory per partition and amplifying peak memory under concurrent partition searches.
A bounded alternative could keep the slim Copy heap while backing it with at most k reusable frequency slots. Each heap entry could carry a slot ID; on replacement, the slot from the evicted entry could be cleared and refilled while retaining its Vec capacity. Final candidates could then mem::take their surviving slots. This would preserve the allocation and movement benefits while restoring O(k × clauses) memory and removing the append-only offset overflow path. An N >> k monotonic-score test could verify that the slot count remains bounded by k.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Built on #7624, now merged into main; this is the last PR in the series and contains only the norm-cache and slim-heap changes.
What
Two hot-loop changes, both score-identical (only tie ordering among equal scores can shift with the slimmer heap tuple):
Scorer::doc_normexposes the BM25 doc-length denominator addend, and quantized V3 searches bake the 256 possible addends once per partition-search. The OR streaming loop, OR drain/single-essential completion paths, and bulk conjunction scoring pass then use one byte-norm load plus a cached addend instead of recomputingk1*(1-b+b*dl/avgdl)per doc. The factored expressions are evaluated identically, so scores are bit-equal to the uncached path.(term, freq)pairs live in at mostkreusable side slots. Replacements clear and refill the evicted entry's slot while retaining itsVeccapacity, and final candidates take their surviving slots without copying. This keeps frequency storage bounded atO(k × clauses)while avoiding Vec-carrying heap entries.Measured vs #7624
Measured at
aa084bdbefore the bounded-slot review follow-up in5ce91ee: per-branch-tip wheels, 1000 queries × 8 concurrent. OR and AND used the warm, fully prewarmed 200M-doc V3/256 index; phrase used the 50M-doc V3/256 positions index. The scoring path is unchanged, but the bounded collector still needs a full 200M rerun.The OR win comes from the streaming loop, where recomputing the BM25 denominator per
clause × docwas the largest single cost.Compatibility
The norm cache consumes the existing V3 quantized byte norms and falls back to the existing scorer path otherwise. It does not change the FTS index format or index version.
Verification
score_diff=0across AND (bulk and classic) and phrase query sets.cargo test -p lance-index: 817 passed, 2 ignored.cargo clippy --all --tests --benches -- -D warningsandcargo fmt --all -- --checkpassed across the reviewed stack.Summary by CodeRabbit
Performance
Search Quality