Skip to content

VarBin: build views straight into a VarBinViewBuilder - #9127

Merged
robert3005 merged 3 commits into
developfrom
rk/varbin-to-varbinview-builder
Aug 5, 2026
Merged

VarBin: build views straight into a VarBinViewBuilder#9127
robert3005 merged 3 commits into
developfrom
rk/varbin-to-varbinview-builder

Conversation

@robert3005

@robert3005 robert3005 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Instead of requiring canonical for varbin to export to varbinview builder we export directly to varbinview builder

@codspeed-hq

codspeed-hq Bot commented Jul 31, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 9.77%

⚡ 1 improved benchmark
❌ 7 regressed benchmarks
✅ 1887 untouched benchmarks
🆕 6 new benchmarks
⏩ 43 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)] 62.2 µs 78 µs -20.25%
Simulation chunked_canonicalize_into[(1000, 50, 8, 4)] 14.9 ms 17 ms -11.97%
Simulation chunked_canonicalize_into[(1000, 50, 8, 16)] 16 ms 18.1 ms -11.43%
Simulation chunked_dict_primitive_canonical_into[f32, (1000, 10, 100)] 1.4 ms 1.6 ms -11.33%
Simulation decompress[u64, (1000, 16)] 65.2 µs 73.5 µs -11.28%
Simulation chunked_canonicalize_into[(1000, 50, 8, 64)] 17.3 ms 19.4 ms -10.62%
Simulation chunked_dict_primitive_canonical_into[u32, (1000, 100, 100)] 1.5 ms 1.6 ms -10.05%
Simulation compress_fsst[(1000, 64, 8)] 1.2 ms 1 ms +11.71%
🆕 Simulation chunked_varbin_into_canonical[(10, 100)] N/A 680.4 µs N/A
🆕 Simulation chunked_varbin_into_canonical[(500, 2)] N/A 169.8 µs N/A
🆕 Simulation chunked_varbin_opt_to_varbinview_builder[(10, 100)] N/A 900.7 µs N/A
🆕 Simulation chunked_varbin_opt_to_varbinview_builder[(500, 2)] N/A 157.8 µs N/A
🆕 Simulation chunked_varbin_to_varbinview_builder[(10, 100)] N/A 512.7 µs N/A
🆕 Simulation chunked_varbin_to_varbinview_builder[(500, 2)] N/A 158.5 µs N/A

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing rk/varbin-to-varbinview-builder (957a3ed) with develop (8316455)

Open in CodSpeed

Footnotes

  1. 43 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/varbin-to-varbinview-builder branch from 30b5bcc to d4aaedf Compare August 1, 2026 10:30
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch 2 times, most recently from 3cfce8e to 5f89fb3 Compare August 4, 2026 23:14
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch from 5f89fb3 to 4a4beb0 Compare August 4, 2026 23:15
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch 2 times, most recently from 0e6e08d to ca493a9 Compare August 5, 2026 13:56

@onursatici onursatici left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should follow up with refactoring the varbinview builder so it allows us to write views directly. This change helps us avoid re-writing the views but we still have a temporary views buffer that we copy from

@robert3005

Copy link
Copy Markdown
Contributor Author

yes, there's a cleanup pass after all the builder changes. I noticed there's patterns that could be simplified/improved but they're spread out all over

@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch from ca493a9 to 7cab066 Compare August 5, 2026 15:02
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch from 7cab066 to fa6b46c Compare August 5, 2026 16:12
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch 2 times, most recently from e1b2d4e to e629da9 Compare August 5, 2026 16:58
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch from e629da9 to 16fb788 Compare August 5, 2026 17:00
@robert3005 robert3005 added the changelog/chore A trivial change label Aug 5, 2026
Base automatically changed from rk/onpair-decode-into-builder to develop August 5, 2026 19:04
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch from 16fb788 to aeb9331 Compare August 5, 2026 19:04
Appending a `VarBin` array to a `VarBinViewBuilder` canonicalized it first, which
built the views, wrapped them in a `VarBinViewArray` the builder immediately
unwrapped, and then had `append_varbinview_array` rewrite every view to rebase
its buffer index onto the builder's.

Number the buffer up front instead: `varbin_decode_views` takes the index the
pushed buffer will land at, so the views come out already correct and the append
is one view per row plus a buffer push. `varbin_to_canonical` now shares that
helper, which also shrinks what `match_each_integer_ptype!` duplicates.

A builder configured to compact still goes the canonical route — it chooses per
buffer whether to keep, slice or rewrite it by measuring the finished views, and
`push_buffer_and_adjusted_views` would bypass that.

chunk_array_builder, fastest of 100 samples, 3 runs each:

| benchmark (rows x chunks)             | before  | after   | speedup |
| ------------------------------------- | ------- | ------- | ------- |
| varbin_to_varbinview_builder 10x1000  | 313 µs  | 204 µs  | 1.53x   |
| varbin_to_varbinview_builder 100x100  | 87.3 µs | 69.5 µs | 1.26x   |
| varbin_to_varbinview_builder 1000x10  | 58.6 µs | 54.8 µs | 1.07x   |
| varbin_opt_to_varbinview_bldr 10x1000 | 378 µs  | 266 µs  | 1.42x   |
| varbin_into_canonical 10x1000         | 357 µs  | 253 µs  | 1.41x   |

The last row is the control: it never touches the builder, so its gain is from
the shared-helper extraction alone. Relative to it, the append itself drops from
0.88x to 0.81x of a canonicalization.

Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 force-pushed the rk/varbin-to-varbinview-builder branch from aeb9331 to 9ba66f9 Compare August 5, 2026 19:04
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 merged commit 5ef3d70 into develop Aug 5, 2026
73 of 74 checks passed
@robert3005
robert3005 deleted the rk/varbin-to-varbinview-builder branch August 5, 2026 22:11
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