Repository navigation
perf(rowids): compact and reuse prepared row id lookups - #10799
Merged
Merged
Conversation
Add direct fragment lookup for small requests, preserve deletion and duplicate semantics, and cover physical offset bounds. Sort full-width inverse entries directly to avoid indirect comparison overhead and account for expired source identity storage.
BubbleCal
marked this pull request as ready for review
November 6, 2026 17:29
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The compact representation preserves physical offsets, deletion filtering, and duplicate-live-ID errors. The reuse and composition APIs retain explicit reconstruction fallbacks; dataset cache integration can follow in dependent #10800.
Xuanwo
approved these changes
Nov 9, 2026
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
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.
Shuffled stable row IDs retain both their source arrays and a padded inverse mapping. When the resulting RowIdIndex cannot enter the cache, small requests reconstruct the complete map. This change compacts the prepared representation and allows verified unchanged fragments to share lookup data across snapshots and independently cached units.
new_reusingandtry_reuse_fragments. The caller verifies content, deletion and physical-address identity; partial merged groups or representation changes retain an explicit rebuild fallback.try_combineto compose independently prepared probe indices after original arrays are released. It shares prepared allocations, rebuilds routing metadata, rejects duplicate fragment namespaces and preserves the global probe/merge cost gate. Merged or mixed inputs returnNonefor the caller to rebuild. It never retains the input indices themselves.FragmentRowIdIndex::get_manyavailable for explicit direct lookup of native encodings. Dataset loading and prepared-fragment cache admission are integrated in perf(rowids): reuse prepared fragments across snapshots #10800; this core API does not provide routing metadata for unknown spilled candidates.Shared allocations are counted once per DeepSizeOf context. No on-disk format change.
Validation on this source:
cargo fmt --allandgit diff --checkpassed.cargo clippy --profile ci --all --tests --benches -- -D warningspassed with pinned Rust 1.98.1.Combined acceptance with #10800 at
23aceccd1fe83a44ba48b3d52028b264ef2803e8is complete; its five core files match this PR at7786af74faca4103f801cce0d929b336c04b6ebe. The integration passed 116 row-ID tests (including 11 resolver cases), 56 take tests, 6 compaction tests, the cache-key test and full workspace Clippy. Real-S3 validation covered 34 matched jobs with equal cache and process-memory limits, verified executable/source identities and independent row-address/payload oracles. Every current arm completed; the sole OOM was the core-only parent while retaining an old reader during 300M-row append under a 4 GiB cap. The current arm completed once with a cgroup peak near that cap, so this is a feasibility observation, not an arbitrary-input memory guarantee.The loader now caches prepared fragment indices rather than individual ID answers. #10800 reports the current child-versus-this-core-parent measurements, including improved cold/distinct-ID and append behavior, cache-pressure behavior, and regressions in warm high-fragment-count and point-to-bulk workloads.
try_combinedeliberately rejects merged/mixed or cost-rejected unions; the loader then rereads/rebuilds. Unknown overlapping spilled candidates still require inspection. Earlier core-only measurements apply toe3d3a2869, not this complete head; no current-head core-only speedup is claimed here.Separate S3 mutation checks validated update, deleted IDs, missing IDs and retained snapshots on the same product source. The fixture's hidden persistent-row-ID index metadata prevented UpdateBuilder discovery on both parent and current; the mutation helper removed that metadata only on an owned shallow clone. This does not validate coexistence with that persistent index or modify the retained input dataset. No BTree performance claim is made.