Skip to content

VarBinViewBuilder: support decoding Dict and Sparse arrays directly into the builder - #9236

Open
robert3005 wants to merge 2 commits into
rk/varbinview-append-bufferfrom
rk/varbinview-gather-scatter
Open

VarBinViewBuilder: support decoding Dict and Sparse arrays directly into the builder#9236
robert3005 wants to merge 2 commits into
rk/varbinview-append-bufferfrom
rk/varbinview-gather-scatter

Conversation

@robert3005

Copy link
Copy Markdown
Contributor

We can avoid copies if we support scatter and gather operations on builder which
is essentially what dict and sparse array append_to_builder should be


Stack created with GitHub Stacks CLIGive Feedback 💬

@codspeed-hq

codspeed-hq Bot commented Aug 6, 2026

Copy link
Copy Markdown

Merging this PR will regress 2 benchmarks

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 1919 untouched benchmarks
⏩ 51 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation decode_varbin[(1000, 2)] 61.9 µs 79 µs -21.62%
Simulation decompress[u64, (1000, 16)] 65.2 µs 73 µs -10.59%
Simulation compact_sliced[(16384, 10)] 1.9 ms 1.4 ms +35.99%
Simulation compact_sliced[(4096, 10)] 514 µs 385.8 µs +33.23%
Simulation decompress[u32, (10000, 1024)] 95.2 µs 78.1 µs +21.92%
Simulation compact[(16384, 10)] 4.9 ms 4.4 ms +10.35%

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-gather-scatter (ffeb8fd) with rk/varbinview-append-buffer (a0bec35)

Open in CodSpeed

Footnotes

  1. 51 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-gather-scatter branch from fa8eda8 to 4b3b54f Compare August 6, 2026 15:36
Comment thread vortex-array/src/builders/varbinview.rs Outdated
@robert3005
robert3005 force-pushed the rk/varbinview-gather-scatter branch from 4b3b54f to 470b635 Compare August 6, 2026 22:48
robert3005 and others added 2 commits August 6, 2026 23:58
Dict was the one string encoding with no direct VarBinViewBuilder path:
it took the dictionary to full logical length (allocating an
intermediate views buffer proportional to the row count) and then
append_varbinview_array walked all those views a second time to rebase
their buffer indices. Sparse had no append_to_builder override at all
and paid the same double pass over its scattered views.

- VarBinViewBuilder grows push_buffers (flush-aware, dedup-aware buffer
  adoption returning the index each buffer landed at) and two bulk view
  appends built on it: append_views_gathered (one rebased view write per
  row through an index lookup, null rows skip the lookup) and
  append_views_scattered (one bulk fill-view write plus one write per
  patch).
- Dict gathers views through its codes straight into the builder: the
  dictionary's buffers are adopted once — deduplicated across chunks
  sharing the dictionary — with no byte copy and no intermediate array.
- Sparse overrides append_to_builder for strings: the view builder gets
  the scatter directly, and VarBinBuilder gets an in-order walk of fill
  runs (append_n_values) and patches, preserving last-wins semantics for
  duplicate patch indices. Non-string dtypes keep the canonicalize
  fallback.
- execute_varbin_inner no longer pushes a data buffer for a fill value
  short enough to inline — the view never referenced it, so it was pure
  dead weight in every canonical decode of a short-filled sparse array.

Checks: cargo nextest -p vortex-array -p vortex-sparse; 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-gather-scatter branch from 470b635 to ffeb8fd Compare August 6, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants