Skip to content

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

Merged
robert3005 merged 5 commits into
developfrom
rk/varbinview-gather-scatter
Aug 7, 2026
Merged

VarBinViewBuilder: support decoding Dict and Sparse arrays directly into the builder#9236
robert3005 merged 5 commits into
developfrom
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 3 benchmarks

⚠️ 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

⚡ 21 improved benchmarks
❌ 3 regressed benchmarks
✅ 1910 untouched benchmarks
⏩ 51 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation fsst_decompress_string 7.2 ms 8.3 ms -12.88%
Simulation decompress_fsst[(10000, 4, 4)] 793.9 µs 899.8 µs -11.76%
Simulation decompress_fsst[(10000, 4, 8)] 815.4 µs 920.9 µs -11.46%
Simulation decompress[u64, (1000, 16)] 134.7 µs 73.9 µs +82.1%
Simulation chunked_varbin_to_varbinview_builder[(10, 100)] 516.5 µs 331.5 µs +55.8%
Simulation chunked_varbin_to_varbinview_builder[(500, 2)] 167.6 µs 116.8 µs +43.53%
Simulation chunked_varbin_into_canonical[(10, 100)] 696.3 µs 489.5 µs +42.25%
Simulation chunked_varbin_opt_to_varbinview_builder[(10, 100)] 790.9 µs 568.7 µs +39.07%
Simulation chunked_varbin_into_canonical[(500, 2)] 178.4 µs 128.6 µs +38.71%
Simulation compact_sliced[(16384, 10)] 2 ms 1.4 ms +37.15%
Simulation chunked_varbin_opt_to_varbinview_builder[(500, 2)] 166.2 µs 121.4 µs +36.83%
Simulation compact_sliced[(4096, 10)] 517.9 µs 385.7 µs +34.29%
Simulation chunked_canonicalize_into[(1000, 50, 8, 4)] 15.4 ms 12.8 ms +20.33%
Simulation chunked_canonicalize_into[(1000, 50, 8, 16)] 16.5 ms 13.9 ms +18.51%
Simulation chunked_into_canonical[(1000, 50, 8, 4)] 17.7 ms 15.1 ms +16.86%
Simulation chunked_into_canonical[(1000, 50, 8, 16)] 18.8 ms 16.2 ms +16.18%
Simulation chunked_into_canonical[(1000, 50, 8, 64)] 20.4 ms 17.7 ms +15.15%
Simulation chunked_canonicalize_into[(1000, 50, 8, 64)] 17.8 ms 15.5 ms +14.73%
Simulation take[small_m/shuffled/primitive/nonnull/chunks=16384/indices=16] 1.3 ms 1.1 ms +13%
Simulation take[small_m/shuffled/primitive/nonnull/chunks=16384/indices=256] 3.7 ms 3.4 ms +11.54%
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

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 (f442d48) with develop (ad5de22)2

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.

  2. No successful run was found on develop (39fde8c) during the generation of this report, so ad5de22 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@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 2 times, most recently from 470b635 to ffeb8fd Compare August 6, 2026 23:22
Comment thread vortex-array/src/builders/varbinview.rs Outdated
Comment thread encodings/sparse/src/canonical.rs
@robert3005
robert3005 force-pushed the rk/varbinview-gather-scatter branch from ffeb8fd to 4eb6087 Compare August 7, 2026 14:52
@robert3005 robert3005 added the changelog/chore A trivial change label Aug 7, 2026
@robert3005
robert3005 requested a review from onursatici August 7, 2026 15:19
Base automatically changed from rk/varbinview-append-buffer to develop August 7, 2026 15:59
robert3005 and others added 5 commits August 7, 2026 16:59
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>
Signed-off-by: Robert Kruszewski <github@robertk.io>
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 08f6e3e to f442d48 Compare August 7, 2026 15:59
@robert3005
robert3005 merged commit 7e06a99 into develop Aug 7, 2026
73 of 74 checks passed
@robert3005
robert3005 deleted the rk/varbinview-gather-scatter branch August 7, 2026 16:25
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