Skip to content

VarBinViewBuilder: support appending buffers directly into the builder storage - #9235

Open
robert3005 wants to merge 2 commits into
developfrom
rk/varbinview-append-buffer
Open

VarBinViewBuilder: support appending buffers directly into the builder storage#9235
robert3005 wants to merge 2 commits into
developfrom
rk/varbinview-append-buffer

Conversation

@robert3005

Copy link
Copy Markdown
Contributor

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 CLIGive Feedback 💬

@robert3005
robert3005 requested a review from onursatici August 6, 2026 14:35
@robert3005 robert3005 added the changelog/chore A trivial change label Aug 6, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 6, 2026

Copy link
Copy Markdown

Merging this PR will regress 5 benchmarks

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 11 improved benchmarks
❌ 5 regressed benchmarks
✅ 1952 untouched benchmarks
⏩ 8 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

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)

Open in CodSpeed

Footnotes

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

@robert3005
robert3005 force-pushed the rk/varbinview-append-buffer branch from 541d03c to bc773a1 Compare August 6, 2026 15:36
Comment thread vortex-array/src/builders/varbinview.rs Outdated
robert3005 and others added 2 commits August 6, 2026 22:47
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>
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 force-pushed the rk/varbinview-append-buffer branch from bc773a1 to a0bec35 Compare August 6, 2026 22:48
@robert3005
robert3005 requested a review from onursatici August 6, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants