perf(rowids): reuse cursors across sequential batches - #8713
Conversation
0399a48 to
c4e1c70
Compare
c4e1c70 to
149026f
Compare
149026f to
e9edbfc
Compare
| let _rowids = tracing::span!(tracing::Level::DEBUG, "fetch_row_ids").entered(); | ||
| if let Some(row_id_sequence) = &config.row_id_sequence { | ||
| if let Some(row_ids) = precomputed_row_ids { | ||
| debug_assert_eq!(row_ids.len(), num_rows as usize); |
There was a problem hiding this comment.
This needs to return a CorruptFile error instead of relying on a debug-only assertion. select_range_with_cursor can return fewer values when the stable row-ID metadata is shorter than the batch, and normal fragment open does not validate that sequence length. This makes debug builds panic, while a zero-column row-ID-only batch can silently shrink in release. The underfill check currently added in #8717 should move into this root PR so it is safe to merge independently.
There was a problem hiding this comment.
Fixed in eda877f. The root PR now checks that cursor selection produced exactly the requested batch length and returns a contextual CorruptFile error on underfill. The truncated-metadata regression lives here, and the full lance-table library suite passes (336 tests).
## Summary Reduce fixed per-batch work in the system-column scan pipeline after the row-ID and version cursors have removed decoder prefix scans. This change: - reads at most 64K stable row IDs ahead and serves adjacent batches as zero-copy Arrow slices; - assembles all requested system columns into one `RecordBatch` instead of repeatedly extending schema/batch objects; - caches structurally equivalent schemas, not only pointer-identical `Arc<Schema>` values; - reuses a full-size zero-column batch template; - skips no-op projections for system-only reads only when schemas are strictly equal; - bounds read-ahead by the actual range selection, including empty and tail reads. The 64K read-ahead bound is about 512 KiB of `u64` values per active fragment. Zero-copy slices can keep that chunk alive until adjacent batches are released. ## Performance 100K-row synthetic scan on top of #8713, #8715, and #8716: - batch size 64: row-ID-only cases improve 29-45%; all-system-column cases improve 53-57%; - batch size 1024, stable isolated all-system cases: | Shape | Payload | Before | After | Reduction | |---|---:|---:|---:|---:| | 50% bitmap | u64 | 544.23 us | 377.30 us | 30.7% | | ~94% bitmap | none | 449.06 us | 291.73 us | 35.0% | | ~94% bitmap | u64 | 481.21 us | 296.43 us | 38.4% | Full-suite batch-size-1024 runs showed pod outliers, so the table reports paired one-case reruns that were stable. The batch-size-64 matrix was stable as a full run. CPU profiling before this change identified `RecordBatch::try_with_column`, `SchemaExt::try_with_column`, schema `Arc` drops, and allocator consolidation as material hotspots. Those repeated per-column/schema-extension hotspots disappear afterward; remaining time is one system-column assembly, row-ID decode, array ownership, and allocator work. ## Validation - `cargo test -p lance-table --lib` (344 passed) - `cargo test -p lance dataset::fragment::tests` (82 passed) - `cargo check -p lance-table --tests --benches` - `cargo clippy -p lance-table --all-targets --no-deps -- -D warnings` - `cargo clippy -p lance --all-targets --no-deps -- -D warnings` - `cargo fmt --all -- --check` - `git diff --check` - 32/32 read-only correctness matrix spanning stable-row-ID encodings, boundary offsets, batch sizes 1 through 65536, and row-ID / row-address / combined projections; every array matched an independent oracle Targeted regressions cover mixed payload + all-system projection schema/order, structurally equal schemas with different `Arc`s, stable-row-ID unsorted indices, read-ahead tail/chunk boundaries, empty tasks, and system-only `read_all` / `read_ranges`. Stack dependencies: #8713, #8715, #8716. The PR base is a temporary upstream integration ref containing those three dependencies and will be retargeted to `main` after they merge.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The cursor optimization continues to preserve repeated and unsorted selection semantics. This revision additionally rejects truncated stable row-ID metadata as CorruptFile before column attachment while retaining the cross-segment rewind behavior.
## Summary Add dense and near-dense bitmap decode paths. This follows #8713 and targets `RangeWithBitmap` segments after the sequential cursor has removed repeated prefix scans. The decoder now: - emits a full `0xff` byte as one contiguous eight-value range; - expands full bytes with at least six set bits as contiguous runs; - selects the specialized dense stream once for a single bitmap segment; - seeds the cursor with the cardinality used for that decision, avoiding a second bitmap scan; - keeps multi-segment sequences on the existing sparse path because one stream-wide decoder cannot safely assume that every segment has the same density. The sparse cursor and bitmap loop remain separate and source-equivalent to `main`. This avoids the measurable fallback regression caused by performing adaptive dispatch in every batch. `Bitmap.data` and `Bitmap.len` remain publicly accessible for source compatibility. A proposed popcount cache was removed because direct mutation of the public byte vector could otherwise make the cached cardinality stale. The on-disk encoding remains byte-for-byte unchanged. ## Performance Measured on Linux x86_64 with `release-with-debug`, 1,000,000 output rows, batch size 1,024, 10 Criterion samples, 1 second warm-up, and 3 seconds measurement. Both binaries used the same benchmark source. Baseline was current `main` at `d57d0fb42`; candidate was `fbde7d600`. | Shape | Payload | `main` | This PR | Change | |---|---:|---:|---:|---:| | 50% density (`holes_2`) | no | 2.243 ms | 2.274 ms | +0.96% (within Criterion noise threshold) | | 50% density (`holes_2`) | yes | 2.600 ms | 2.594 ms | -0.22% (no significant change) | | ~94% density (`holes_17`) | no | 2.484 ms | 1.600 ms | **-35.58%** | | ~94% density (`holes_17`) | yes | 2.592 ms | 1.677 ms | **-35.14%** | Linux `perf` attributes the dense-shape improvement to the intended decoder change: on `main`, `SegmentCursorState::extend_range` accounts for 68.17% of CPU samples; this PR moves that work to `SegmentCursorState::extend_dense_range` (48.61% of samples) while reducing end-to-end time by 35.58%. For the 50%-density fallback, both `main` and this PR remain in `SegmentCursorState::extend_range` (66.83% and 70.13% respectively); no adaptive-dispatch helper appears in the hot path. ## Validation - `cargo test -p lance-table --lib` (365 passed) - `cargo clippy --all --tests --benches -- -D warnings` - `cargo fmt --all -- --check` - `git diff --check` - full-width-byte regression coverage, including the previous shift-by-8 panic - dense sequential cursor coverage across batch and byte boundaries, tail reads, past-end reads, and rewind - sparse and multi-segment fallback coverage - concurrent multi-batch stable row-ID coverage with deletions - unsorted stable row-ID index coverage - direct public-byte mutation cardinality coverage - public-field source-compatibility coverage through `U64Segment::RangeWithBitmap` - byte-exact serde coverage Dependency #8713 is merged. This PR targets `main` directly and contains only the bitmap follow-up. --------- Co-authored-by: Xuanwo <github@xuanwo.io>
## Summary Stream dataset-version RLE metadata with a persistent cursor instead of rebuilding and searching run offsets for every batch. The change: - adds `RowDatasetVersionCursor` for adjacent, monotonic batch traversal; - caches the active run length and bulk-expands range values; - lazily builds a run-offset index on the first backward seek, then uses it for every later non-adjacent seek in either direction; - precomputes created-at and last-updated-at arrays while polling the ordered task stream; - preserves the direct-call fallback and the single-run fast path. #8713 and #8715 are merged. This branch is rebased on `774e32d67`, and its diff now contains only the dataset-version work. ## Performance Linux Criterion A/B, 100,000 rows, batch size 1,024, `release-with-debug`, pinned to the same CPU. Baseline is `774e32d67`; this PR is `6f22e566c`. ### Ordered stream | Version runs | main | This PR | Result | |---:|---:|---:|---:| | 1 | 98.325 us | 98.855 us | +0.54% (neutral) | | 98 | 930.69 us | 112.30 us | 8.29x faster | | 3,125 | 3.2053 ms | 128.27 us | 24.99x faster | | 100,000 | 56.033 ms | 517.17 us | 108.34x faster | ### Direct-call fallback | Version runs | main | This PR | Result | |---:|---:|---:|---:| | 1 | 631.81 ns | 634.07 ns | +0.36% (neutral) | | 98 | 8.983 us | 762.10 ns | 11.79x faster | | 3,125 | 32.532 us | 5.108 us | 6.37x faster | | 100,000 | 552.08 us | 142.84 us | 3.87x faster | This PR makes no single-run speedup claim. It improves robustness for updated and multi-run fragments. For attribution, Linux `perf` sampled the 3,125-run ordered-stream case for five seconds at 997 Hz. On main, 49.72% of samples were in `version_values_for_selection`, 39.39% in `U64Segment::len`, and 4.96% in run-offset `Vec` construction (94.07% combined). With this PR, `U64Segment::len` and offset construction were each below the 0.5% report threshold; the remaining profile was distributed across cursor expansion, allocation, Arrow column assembly, and stream scheduling. ## Correctness The tests cover adjacent ranges, gaps, rewinds, empty runs, non-range spans, out-of-bounds selections, descending seeks, alternating far-forward/backward seeks, direct calls, deletions, both version columns, concurrent batches, and unsorted indices split across multiple batches. ## Validation - `cargo test -p lance-table --lib` (372 passed) - `cargo clippy --all --tests --benches -- -D warnings` - `cargo fmt --all -- --check` - `git diff --check` - Linux `release-with-debug` Criterion A/B and `perf` profiles described above --------- Co-authored-by: Xuanwo <github@xuanwo.io>
## Summary Reduce fixed per-batch work in the system-column scan pipeline after the row-ID and version cursors have removed decoder prefix scans. This supersedes #8717, which was merged into its temporary integration base while the dependency stack was being refreshed. No version of #8717 was merged into `main`. This change: - reads stable row IDs ahead in batch-aligned chunks and serves uniform adjacent batches as zero-copy Arrow slices; - validates the complete prefetched selection before exposing or caching it; - drains the current row-ID chunk and falls back to exact per-task decoding when non-final task sizes vary, avoiding repeated boundary copies or cursor rewinds; - assembles all requested system columns into one `RecordBatch` for uniform task streams, while variable task streams retain the prior incremental assembly; - caches structurally equivalent schemas for uniform task streams, not only pointer-identical `Arc<Schema>` values, and disables that cache after task sizes vary; - reuses a full-size zero-column batch template; - skips no-op projections for system-only reads only when schemas are strictly equal; - bounds read-ahead by the actual range selection, including empty and tail reads. For batches up to 64K rows, each cached row-ID chunk is at most 64K `u64` values (512 KiB). A larger batch is decoded as one batch without additional read-ahead. Zero-copy slices can keep a chunk alive until adjacent batches are released; an irregular stream can make one boundary copy while draining the existing chunk. ## Performance Benchmarks compare `main` at `a8bec27d5` (including #8716) with this branch, use `release-with-debug`, pin both revisions to the same CPU, and run each case in a fresh process. The 100K-row uniform-task matrix improved all 16 combinations of batch size (64/1024), bitmap density (holes every 2/17 values), payload (absent/present), and system columns (`_rowid`/all): - batch size 64: `_rowid` improved 30.3%-33.7%; all system columns improved 51.4%-56.8%; - batch size 1024: `_rowid` improved 13.1%-26.5%; all system columns improved 29.5%-36.7%. A standard Criterion run for the representative batch-size-64, holes-17, zero-payload, all-system-columns case measured 3.7000 ms on `main` and 1.7455 ms here (-52.8%). CPU profiles attribute the change: `RecordBatchExt::try_with_column` (7.47% self time on `main`) and `SchemaExt::try_with_column` (4.59%) both fell below the 0.1% reporting threshold. The review reproducer uses 10M rows, holes-17, one payload column, `_rowid`, and alternating 32,768/32,769-row tasks. With isolated base/head target directories, standard Criterion measured 10.387-10.468 ms on `main` and 10.449-10.499 ms here (+0.51% by point estimate, within run noise). A second pair under `perf record` measured 10.699-10.746 ms and 10.652-10.724 ms, respectively. Profiles show the same dominant workload (`SegmentCursorState::extend_dense_range`, 42.17% / 42.22% self time); `RecordBatchExt::try_with_column` was only 0.16% / 0.10%, and schema reconstruction and boundary copying were not hotspots. ## Validation - `cargo test -p lance-table --lib` (384 passed) - `cargo test -p lance --lib dataset::fragment` (108 passed) - `cargo check -p lance-table --tests --benches` - `cargo clippy --all --tests --benches -- -D warnings` - `cargo fmt --all -- --check` - `git diff --check` - Python integration: `test_to_batches_with_partial_last_batch`, `test_to_batches`, `test_scan_no_columns`, and `test_roundtrip_reader` (4 passed) - read-only production-scale matrix: 32/32 cases passed against an independent metadata oracle, with the dataset version asserted unchanged before and after Targeted regressions cover mixed payload + all-system projection schema/order, uniform structurally equal schemas with different `Arc`s, variable-task fallback, stable-row-ID unsorted indices and metadata underfill, read-ahead tail/chunk boundaries, empty tasks, and system-only `read_all` / `read_ranges`. Dependencies #8713, #8715, and #8716 are merged into `main`. This PR's diff is limited to the system-column benchmark and the two implementation files. --------- Co-authored-by: Xuanwo <github@xuanwo.io>
Summary
Reuse a stable row-ID cursor across ordered record-batch tasks and bulk-decode range-backed segments.
Stable row IDs represented as
RangeWithBitmap/RangeWithHolespreviously rebuilt selection state for every batch. Sequential scans therefore rescanned an ever-growing prefix, approaching quadratic work as batch count increased.This change:
RowIdSequenceCursoracross ordered tasks and caches segment lengths;SegmentCursorState::extend_rangefor bulk expansion of range and bitmap segments;CorruptFileinstead of panicking or returning a short batch.Performance
100K-row synthetic sequential scan, identical Criterion harness:
mainCPU profiling on
mainattributed 51.73% toRowIdSequence::selectand 33.45% toU64Segment::len, matching repeated prefix traversal. After this change those hotspots are replaced bySegmentCursorState::extend_range; remaining time is fixed allocation/schema work.Validation
cargo test -p lance-table --lib(336 passed)cargo check -p lance-table --tests --benchescargo clippy -p lance-table --all-targets --no-deps -- -D warningscargo fmt --all -- --checkgit diff --checkThis is the root of the row-ID optimization stack and has no dependency on the bitmap or version-cursor follow-ups.
Follow-up PRs: