Skip to content

perf(rowids): compact and reuse prepared row id lookups - #10799

Merged
BubbleCal merged 5 commits into
mainfrom
yang/rowid-bounded-memory
Nov 9, 2026
Merged

BubbleCal merged 5 commits into
mainfrom
yang/rowid-bounded-memory

Conversation

@BubbleCal

@BubbleCal BubbleCal commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

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.

  • Use bitmap/rank offsets for dense IDs, packed relative-ID/offset words for narrower sparse IDs, and 12-byte entries for wide IDs. Release redundant source arrays and skip inverse preparation when the existing cost model selects merging.
  • Share immutable prepared fragments and merged chunks through new_reusing and try_reuse_fragments. The caller verifies content, deletion and physical-address identity; partial merged groups or representation changes retain an explicit rebuild fallback.
  • Add try_combine to 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 return None for the caller to rebuild. It never retains the input indices themselves.
  • Keep FragmentRowIdIndex::get_many available 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:

  • 64 RowIdIndex tests passed, including independent physical-offset expectations, deletion and duplicate cases, released original arrays, input-index lifetimes, shared accounting and the 64-to-65 overlap fallback.
  • All four row-ID API doctests passed.
  • cargo fmt --all and git diff --check passed.
  • Full workspace cargo clippy --profile ci --all --tests --benches -- -D warnings passed with pinned Rust 1.98.1.

Combined acceptance with #10800 at 23aceccd1fe83a44ba48b3d52028b264ef2803e8 is complete; its five core files match this PR at 7786af74faca4103f801cce0d929b336c04b6ebe. 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_combine deliberately rejects merged/mixed or cost-rejected unions; the loader then rereads/rebuilds. Unknown overlapping spilled candidates still require inspection. Earlier core-only measurements apply to e3d3a2869, 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.

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
BubbleCal marked this pull request as ready for review November 6, 2026 17:29

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 6, 2026
@BubbleCal
BubbleCal merged commit 6e9be07 into main Nov 9, 2026
48 checks passed
@BubbleCal
BubbleCal deleted the yang/rowid-bounded-memory branch November 9, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

K-approved Latest Gatekeeper recommendation permits acceptance. performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants