VarBinViewBuilder: support appending buffers directly into the builder storage - #9235
VarBinViewBuilder: support appending buffers directly into the builder storage#9235robert3005 wants to merge 2 commits into
Conversation
Merging this PR will regress 5 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decompress[u32, (10000, 1024)] |
78.5 µs | 95.2 µs | -17.55% |
| ❌ | Simulation | fsst_decompress_string |
7.2 ms | 8.3 ms | -12.77% |
| ❌ | Simulation | decompress_fsst[(10000, 4, 4)] |
794.2 µs | 901 µs | -11.85% |
| ❌ | Simulation | decompress_fsst[(10000, 4, 8)] |
815.5 µs | 922.1 µs | -11.56% |
| ❌ | Simulation | canonicalize_compare[(10000, 4, 4)] |
955.9 µs | 1,064.1 µs | -10.17% |
| ⚡ | Simulation | chunked_varbin_to_varbinview_builder[(10, 100)] |
509.3 µs | 329.1 µs | +54.76% |
| ⚡ | Simulation | chunked_varbin_opt_to_varbinview_builder[(10, 100)] |
794.1 µs | 554.8 µs | +43.14% |
| ⚡ | Simulation | chunked_varbin_to_varbinview_builder[(500, 2)] |
164.5 µs | 119.1 µs | +38.13% |
| ⚡ | Simulation | chunked_varbin_into_canonical[(10, 100)] |
670.9 µs | 488 µs | +37.47% |
| ⚡ | Simulation | chunked_varbin_into_canonical[(500, 2)] |
175.4 µs | 130.9 µs | +33.94% |
| ⚡ | Simulation | chunked_varbin_opt_to_varbinview_builder[(500, 2)] |
162.4 µs | 122.5 µs | +32.57% |
| ⚡ | Simulation | chunked_into_canonical[(1000, 50, 8, 4)] |
17.9 ms | 15.7 ms | +14.22% |
| ⚡ | Simulation | chunked_canonicalize_into[(1000, 50, 8, 4)] |
15.1 ms | 13.2 ms | +13.66% |
| ⚡ | Simulation | chunked_canonicalize_into[(1000, 50, 8, 16)] |
16.2 ms | 14.3 ms | +12.98% |
| ⚡ | Simulation | chunked_into_canonical[(1000, 50, 8, 16)] |
19 ms | 16.8 ms | +12.83% |
| ⚡ | Simulation | decompress[u64, (1000, 16)] |
73 µs | 65.2 µs | +11.87% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/varbinview-append-buffer (a0bec35) with develop (a1057db)
Footnotes
-
8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
541d03c to
bc773a1
Compare
Every encoding that decodes a byte heap + lengths for a VarBinViewBuilder (FSST, OnPair, VarBin) repeated the same ritual: predict the index its buffer would land at (completed_block_count() + in_progress()), run build_views under a ptype match, then push_buffer_and_adjusted_views. The index prediction leaked the builder's flush behavior into four crates, and the VarBin path paid two avoidable copies: into_mut() on a shared heap slice memcpy'd every value byte just to satisfy build_views' ByteBufferMut parameter (only the rare >2GiB rolling path mutates), and offsets_to_lengths materialized a lengths buffer that build_views immediately re-accumulated back into offsets. - build_views now takes a frozen ByteBuffer and returns zero-copy slices of it (the rolling path slices instead of split_off), with an offsets-driven twin (build_views_from_offsets) and a shared extend_views core that writes views straight into an existing views buffer. - VarBinViewBuilder grows append_buffer_with_lengths / append_buffer_with_offsets, which flush, number and adopt the heap internally and build views directly into the builder's views storage (no intermediate views allocation). - The bulk appends measure utilization from the lengths alone when the builder compacts (only non-inlinable values reference the heap) and rewrite under-utilized heaps to just the referencing values, so VarBin's canonicalize-then-append compaction fallback is gone and compacts_buffers() is private. - Zstd, whose lengths are interleaved in its frames and so cannot use the lengths-driven appends, moves to append_views_built_at: the builder hands the start index into a callback and adopts the buffers and views it returns, so buffer numbering never escapes the builder. push_buffer_and_adjusted_views and the in_progress() accessor are gone with it. - varbin_decode_views is absorbed: the builder path uses append_buffer_with_offsets and varbin_to_canonical uses build_views_from_offsets, both zero-copy over the offsets range. Checks: cargo nextest -p vortex-array -p vortex-fsst -p vortex-onpair -p vortex-zstd; cargo check -p vortex-cuda; cargo +nightly fmt --all; cargo clippy --all-targets on the touched crates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uet46qdrbQcF61HXRG7EKc Signed-off-by: Robert Kruszewski <github@robertk.io>
bc773a1 to
a0bec35
Compare
throughout string encodings we have a repeated pattern that want to directly
write values to the varbinview builder without additional copy. This pr
generalizes this functionality
Stack created with GitHub Stacks CLI • Give Feedback 💬