Skip to content

perf(encoding): auto-select sparse structural pages#7756

Merged
Xuanwo merged 3 commits into
mainfrom
xuanwo/sparse-stack-5-auto-sparse
Jul 22, 2026
Merged

perf(encoding): auto-select sparse structural pages#7756
Xuanwo merged 3 commits into
mainfrom
xuanwo/sparse-stack-5-auto-sparse

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Part of #7750

Summary

  • In Lance 2.3+, automatically choose sparse structural pages only when the structural encoding is implicit, the dense mini-block rep/def plan requires top-level page splitting or has a single row over budget, and the sparse writer supports the value path.
  • Keep within-budget pages on the dense path without constructing a sparse value block or sparse plan.
  • Preserve every explicit structural mode, Lance 2.2 behavior, and dense fallback/error behavior for dictionary, variable packed-struct, and other sparse-ineligible value paths.
  • Keep the policy decision separate from sparse serialization; this adds no wire-format fields.

This is the policy/performance layer that turns the already-readable and explicitly-writable sparse format on automatically only for over-budget eligible pages.

Integration

  • PR3 #7754 absorbed the readable and explicitly writable sparse format from the closed PR4 #7755. Its squash patch is replayed directly on the current main base before the policy commits in this PR.
  • PR1 #7751 is merged; the rebase resolves the stack onto its MiniBlockRepDefBudget naming.

100M-row S3 benchmark

Environment: r7i.8xlarge in us-east-2b, Lance V2.3, 100 files of 1,000,000 rows, 65,536-row write batches, and 1,024-row takes. The final committed harness is at 66c936391aa46bfcd14f7ac1c820878da68fcc0b on xuanwo/sparse-stack-5-auto-sparse-bench-final, based on pre-rebase PR5 head ed0ce68d7f4cceafe6a249c912f7f1da8c4fecd9. The rebase consolidates #7754 and resolves #7751 naming without changing the auto-selection or dense-fallback policy; these measurements remain attributed to that exact benchmarked head and are not relabeled as current-head results.

Cases:

  • hnsw: List<UInt32>; the first 1/7 of rows (14,285,714 of 100M) contain 32 consecutive values, followed by an empty tail.
  • uniform: List<UInt32>; one singleton non-empty row every 10,000 rows.
  • deep: Struct<events: List<Struct<id: Int32, tags: List<Int32>, pair: FixedSizeList<Int32; 2>>>>; non-empty every 4,096 rows with nullable top struct, event list, event struct, id, tags, pair, and pair items.

cold is the first operation on a newly opened Dataset; warm is the immediate repeat on the same Dataset. Times are milliseconds. Bytes are total dataset object bytes. Every dataset has 102 objects, including 100 Lance data files.

Case Mode Bytes Pages Cold full scan Cold random Warm full scan Warm random Warm empty Warm non-empty
hnsw sparse 1,600,850,504 317 1,627.830 466.940 1,472.636 272.931 0.484 29.238
hnsw mini-block 1,663,361,256 328 1,720.362 484.619 1,655.289 273.812 0.466 20.664
hnsw full-zip 2,342,961,549 328 3,239.070 478.839 3,104.055 273.603 0.659 25.735
uniform sparse 117,091 100 99.990 77.346 76.518 3.408 0.548 28.299
uniform mini-block 689,601 1,600 111.844 137.503 606.852 123.301 93.404 94.822
uniform full-zip 496,906,995 1,600 1,589.645 1,205.110 650.542 359.496 72.292 58.997
deep sparse 1,144,900 300 159.361 77.122 135.065 9.309 0.884 25.580
deep mini-block 376,199,150 10,100 6,735.485 4,673.904 2,560.397 582.959 174.014 189.929
deep full-zip 994,111,217 10,100 failed failed failed failed failed failed

deep/full-zip wrote all 100M rows and produced the reported size/page/object counts, then its first full scan panicked at primitive.rs:2588 by unwrapping a missing fixed full-zip decode task. Sparse and mini-block completed all reads. A separate 200K-row check at the exact PR4 base SHA da06b79a7f5eda4f33e2d80990c46766095ba383 independently confirmed that this nested-null deep/full-zip read was already broken before this PR, returning Structural validity has 6 entries for an array with 7 values. The precise failure site varies with this decoder path, but explicit full-zip never enters PR5's automatic sparse candidate closure, so this is not an auto-sparse regression. The policy deliberately does not disguise that behavior as sparse support.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Lance 2.3+ sparse structural page writing with automatic selection, prepared value serialization, value-path validation, rep/def plan borrowing, tests, and format documentation.

Changes

Sparse structural page layout

Layer / File(s) Summary
Structural planning and layout selection
rust/lance-encoding/src/repdef.rs, rust/lance-encoding/src/encodings/logical/primitive.rs, rust/lance-encoding/src/encodings/logical/primitive/layout.rs
Structural plans are borrowed during serialization; automatic sparse selection is gated by version, explicit encoding, and dense-page budget; primitive pages route prepared or unprepared sparse values.
Sparse value preparation and page writing
rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs, rust/lance-encoding/src/encodings/logical/primitive.rs
Sparse value blocks are validated, compressed into serialized value pages, checked against visible-item counts, and covered by eligibility, fallback, and explicit-rejection tests.
Sparse page format documentation
docs/src/format/file/encoding.md
Documents sparse structural layers, position and count buffers, writer selection, page layout, read behavior, caching, and reader validation requirements.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

  • lance-format/lance issue 7750 — Directly tracks the sparse structural layout implemented by this change.

Possibly related PRs

  • lance-format/lance#7889 — Related Lance 2.3+ sparse structural page implementation across primitive encoding and format integration.

Suggested labels: enhancement

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: automatic sparse structural page selection for encoding.
Description check ✅ Passed The description is directly related to the changeset and accurately explains the auto-sparse policy and fallback behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch xuanwo/sparse-stack-5-auto-sparse

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Important

This PR touches the Lance format specification.

Substantive changes to the format specification — the .proto definitions
and the spec docs under docs/src/format/ — require a PMC vote before merge.
Minor edits such as typo fixes, wording, or formatting are excluded; use your
judgment.

If this is a meaningful format change:

  • Start a vote following the Lance community voting process.
    Format specification modifications need 3 binding +1 votes (excluding the
    proposer), held on GitHub Discussions, with a minimum voting period of 1 week.
  • Once the vote passes, link the completed vote in this PR. It should not be
    merged until the vote is linked.

@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer A-format On-disk format: protos and format spec docs performance labels Jul 13, 2026
@Xuanwo
Xuanwo force-pushed the xuanwo/sparse-stack-5-auto-sparse branch from ed0ce68 to d7cab0f Compare July 21, 2026 17:18
@Xuanwo
Xuanwo changed the base branch from xuanwo/sparse-stack-4-sparse-writer to main July 21, 2026 17:18
@Xuanwo
Xuanwo marked this pull request as ready for review July 21, 2026 17:20
@Xuanwo
Xuanwo force-pushed the xuanwo/sparse-stack-5-auto-sparse branch from d7cab0f to 4c1eb4f Compare July 21, 2026 20:07

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs (1)

225-226: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Pre-size non_empty_positions and counts.

Both vectors grow across the 0..num_slots loop, so their capacity is estimable. Prefer over-estimating with num_slots to avoid repeated reallocations.

♻️ Proposed change
-                let mut non_empty_positions = Vec::new();
-                let mut counts = Vec::new();
+                let mut non_empty_positions = Vec::with_capacity(num_slots);
+                let mut counts = Vec::with_capacity(num_slots);
As per coding guidelines: "Use `Vec::with_capacity()` when size is known or estimable, and prefer over-estimating capacity to multiple reallocations."
🤖 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-encoding/src/encodings/logical/primitive/sparse/writer.rs` around
lines 225 - 226, Initialize non_empty_positions and counts with
Vec::with_capacity(num_slots) before the 0..num_slots loop, preserving their
existing element types and subsequent behavior.

Source: Coding guidelines

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

Nitpick comments:
In `@rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs`:
- Around line 225-226: Initialize non_empty_positions and counts with
Vec::with_capacity(num_slots) before the 0..num_slots loop, preserving their
existing element types and subsequent behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 20b59d15-985b-42d7-bf23-8a5944e1606a

📥 Commits

Reviewing files that changed from the base of the PR and between d7cab0f and 4c1eb4f.

📒 Files selected for processing (5)
  • docs/src/format/file/encoding.md
  • rust/lance-encoding/src/encodings/logical/primitive.rs
  • rust/lance-encoding/src/encodings/logical/primitive/layout.rs
  • rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs
  • rust/lance-encoding/src/repdef.rs

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

@westonpace westonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These rules make sense to me

Comment thread rust/lance-encoding/src/encodings/logical/primitive.rs Outdated
Comment thread rust/lance-encoding/src/repdef.rs Outdated
Comment thread rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs Outdated
Comment thread rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs Outdated
Comment thread rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs Outdated

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
rust/lance-encoding/src/encodings/logical/primitive.rs (1)

5754-5765: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use one sparse eligibility contract for every selection path.

Automatic selection, explicit sparse requests, and value preparation currently validate different subsets of DataBlock variants. Centralize the recursive predicate and invoke it for explicit requests and inside prepare_values.

  • rust/lance-encoding/src/encodings/logical/primitive.rs#L5754-L5765: validate explicit sparse values before constructing the sparse page.
  • rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs#L359-L363: recurse through struct children instead of checking only variable-width descendants.
  • rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs#L539-L562: enforce the same predicate and return caller-facing errors for rejected inputs.
🤖 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-encoding/src/encodings/logical/primitive.rs` around lines 5754 -
5765, Unify sparse eligibility around the shared predicate used by sparse
planning: in rust/lance-encoding/src/encodings/logical/primitive.rs#L5754-L5765,
validate explicit sparse values before constructing the sparse page; in
rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs#L359-L363,
update the predicate to recurse through struct children; and in
rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs#L539-L562,
invoke that predicate during value preparation and return caller-facing errors
for rejected inputs.
🧹 Nitpick comments (1)
rust/lance-encoding/src/encodings/logical/primitive.rs (1)

5781-5803: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Build the structural plan before compressing candidate values.

This closure fully prepares and serializes values before checking whether the plan uses constant layout. That work is discarded when sparse is rejected, and dense encoding may compress the same values again. Move plan construction and the constant-layout gate before prepare_values.

🤖 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-encoding/src/encodings/logical/primitive.rs` around lines 5781 -
5803, In the sparse encoding closure, move sparse::writer::plan(&normalized,
num_values)? and the sparse::writer::uses_constant_layout(&plan, &self.field)
early-return before constructing DataBlock or calling
sparse::writer::prepare_values. Keep the supports_value_block check and
preparation/error handling after this gate so candidate values are only
compressed when the structural plan is eligible.
🤖 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-encoding/src/encodings/logical/primitive.rs`:
- Around line 5754-5765: Unify sparse eligibility around the shared predicate
used by sparse planning: in
rust/lance-encoding/src/encodings/logical/primitive.rs#L5754-L5765, validate
explicit sparse values before constructing the sparse page; in
rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs#L359-L363,
update the predicate to recurse through struct children; and in
rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs#L539-L562,
invoke that predicate during value preparation and return caller-facing errors
for rejected inputs.

---

Nitpick comments:
In `@rust/lance-encoding/src/encodings/logical/primitive.rs`:
- Around line 5781-5803: In the sparse encoding closure, move
sparse::writer::plan(&normalized, num_values)? and the
sparse::writer::uses_constant_layout(&plan, &self.field) early-return before
constructing DataBlock or calling sparse::writer::prepare_values. Keep the
supports_value_block check and preparation/error handling after this gate so
candidate values are only compressed when the structural plan is eligible.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 4f94d639-da08-414d-a8ce-4f466070132d

📥 Commits

Reviewing files that changed from the base of the PR and between 4c1eb4f and 461a33c.

📒 Files selected for processing (3)
  • rust/lance-encoding/src/encodings/logical/primitive.rs
  • rust/lance-encoding/src/encodings/logical/primitive/sparse/writer.rs
  • rust/lance-encoding/src/repdef.rs

@Xuanwo
Xuanwo merged commit 6ca15c8 into main Jul 22, 2026
34 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/sparse-stack-5-auto-sparse branch July 22, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer A-format On-disk format: protos and format spec docs performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants