perf(encoding): avoid inline bitpacking input copy#7748
Conversation
Replace the unconditional input copy in `InlineBitpacking::unchunk` with a typed view borrowed over the inline bitpacking chunk words. Output buffers continue to be zero-initialized before decoding. Also: - Validate chunk headers up front and reject malformed inline bitpacking chunks with clear errors. - Expand `unchunk` roundtrip and corruption tests to cover header sizing, alignment, payload length mismatches, invalid bit widths, and excessive value counts. - Initialize bitpacking output with Default. Local benchmark (WSL2, i7-10700, `cargo bench -p lance-encoding --bench decoder decode_inline_bitpacking_unchunk --features bitpacking -- --noplot`): | Case | Path | Time | Throughput | |---------------|-------------|--------:|-------------:| | u32_bw12_1024 | legacy_copy | 285 ns | 5.0 GiB/s | | u32_bw12_1024 | typed_view | 247 ns | 5.8 GiB/s | | u64_bw23_1024 | legacy_copy | 434 ns | 6.3 GiB/s | | u64_bw23_1024 | typed_view | 340 ns | 8.1 GiB/s | typed_view is ~13% faster for u32_bw12_1024 and ~22% faster for u64_bw23_1024.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughInline bitpacking unchunking now uses typed header/payload views with explicit validation and structured corruption errors. Tests cover valid and invalid inputs, while Criterion benchmarks compare legacy copying with typed-view decoding. ChangesInline bitpacking decoding
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Criterion
participant ChunkGenerator
participant LegacyDecoder
participant InlineBitpacking
Criterion->>ChunkGenerator: generate inline bitpacked chunk
Criterion->>LegacyDecoder: decode with legacy_copy
Criterion->>InlineBitpacking: decode with typed_view
LegacyDecoder-->>Criterion: return decoded payload
InlineBitpacking-->>Criterion: return decoded payload
Criterion->>Criterion: compare decoded buffers and record timings
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Closed: changes will be force-pushed to the original branch #7696 as a single commit. |
Supersedes #7696. This PR contains the same changes as #7696, but squashed into a single commit with a consolidated commit message.
Summary
InlineBitpacking::unchunkby borrowing a typed view over the inline bitpacking chunk words.unchunkroundtrip coverage and a Criterion benchmark comparing the old copy path with the new typed-view path.Benchmark
Local machine: WSL2 on Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 12 logical CPUs.
Command:
Results:
u32_bw12_1024legacy_copyu32_bw12_1024typed_viewu64_bw23_1024legacy_copyu64_bw23_1024typed_viewPath-to-path deltas:
u32_bw12_1024:typed_viewwas 13.4% faster thanlegacy_copyu64_bw23_1024:typed_viewwas 21.6% faster thanlegacy_copyThe benchmark uses aligned chunks built with
LanceBuffer::reinterpret_vec, so it measures the alignedborrow_to_typed_viewfast path. Misaligned buffers can still fall back to a copy.Test Plan
cargo fmt --allcargo test -p lance-encoding --features bitpackingcargo clippy -p lance-encoding --all-features --tests --benches -- -D warningscargo clippy --all --tests --benches -- -D warningscargo bench -p lance-encoding --bench decoder decode_inline_bitpacking_unchunk --features bitpacking -- --noplotSummary by CodeRabbit
Bug Fixes
Performance
Tests