Conversation
Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io>
|
🤖 Performance measurements for the export compaction option. We compared buffer compaction enabled versus disabled in Firebolt's Arrow read path, with native decoding disabled so the queries exercise the Arrow exporter. All queries use selection pushdown and retain 99.9% of rows. The table shows the reduction in retired instructions from disabling compaction (medians of three rounds); all query results match.
The array queries scan 17.6M rows and the scalar string queries scan 100M rows. |
Add ArrowExportOptions::get_or_default, drop the option-taking methods from the deprecated ArrowArrayExecutor trait and the extra public varbinview helper, and move the option tests into options/tests.rs. Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
Make ArrowExportVTable::execute_arrow_with_options the required method and deprecate execute_arrow, which now forwards with default options. A plugin that only implemented execute_arrow silently dropped options for its storage export, so e.g. CompactBuffers(false) was ignored for JSON, Variant and WKB columns. Convert all in-tree plugins (JSON, UUID, tensor Vector, Parquet Variant, and the spatial types) and add ParquetVariantArrayExt::to_arrow_with_options for the canonical Variant export path. Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
…arrow Add the options parameter to the plugin's execute_arrow instead of a separate execute_arrow_with_options method plus a deprecated forwarder. Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
…ns exporter ArrowSession::with_options(&options) returns an ArrowExporter that borrows the session and the options for a single export, so the session API keeps one execute_arrow and holds no per-export state. Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
…Ext::to_arrow Replace to_arrow_with_options with an options parameter on to_arrow. Existing callers pass the default options. Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
…t-options Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
| /// assert_eq!(arrow.len(), 2); | ||
| /// # Ok::<(), vortex_error::VortexError>(()) | ||
| /// ``` | ||
| pub fn with_options<'a>(&'a self, options: &'a ArrowExportOptions) -> ArrowExporter<'a> { |
There was a problem hiding this comment.
Q: naming, maybe call it exporter() instead?
Internal export helpers now take the exporter instead of the bare options, so nested exports call exporter.execute_arrow(..) instead of re-fetching the Arrow session from the execution context for every child. Field inference for target = None also uses the exporter's session. Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
Signed-off-by: Lorenz Hübschle <lorenz@firebolt.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns
Merging this PR will improve performance by 12.58%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | compare_int_nullable_neon |
5.6 µs | 4.8 µs | +15.88% |
| ⚡ | WallTime | compare_int_constant_neon |
4.4 µs | 3.8 µs | +15.62% |
| ⚡ | WallTime | compare_u64_neon |
5 µs | 4.3 µs | +15.5% |
| ⚡ | WallTime | compare_int_eq_neon |
4.9 µs | 4.3 µs | +14.32% |
| ⚡ | WallTime | deferred_bool_neon[16384, (ConstantLhs, PartialAccepted)] |
57.2 µs | 50.4 µs | +13.68% |
| ⚡ | WallTime | compare_int_neon |
4.9 µs | 4.3 µs | +12.77% |
| ⚡ | WallTime | deferred_bool_neon[16384, (Columns, PartialAccepted)] |
70.3 µs | 62.8 µs | +11.89% |
| ⚡ | WallTime | compare_float_neon |
8.6 µs | 7.7 µs | +11.68% |
| ⚡ | WallTime | add_constant_shapes_neon[(32768, ConstantPerRow)] |
16.7 µs | 15 µs | +11.49% |
| ⚡ | WallTime | deferred_bool_constant_neon[PerRowConstant] |
6.3 µs | 5.6 µs | +10.91% |
| ⚡ | WallTime | compare_f32_neon |
5.8 µs | 5.2 µs | +10.86% |
| ⚡ | WallTime | infallible_bool_constant_neon[PerRowConstant] |
4.5 µs | 4 µs | +10.73% |
| ⚡ | WallTime | deferred_bool_neon[i64, PerRowPerRow] |
7.3 µs | 6.6 µs | +10.54% |
| ⚡ | WallTime | deferred_bool_constant_neon[ConstantPerRow] |
4.4 µs | 4 µs | +10.46% |
| 🆕 | Simulation | i64_random_chunked[256] |
N/A | 20.6 ms | N/A |
| 🆕 | Simulation | i64_random[256] |
N/A | 5.7 ms | N/A |
| 🆕 | Simulation | nested_list_random[32] |
N/A | 7.4 ms | N/A |
| 🆕 | Simulation | utf8_random[256] |
N/A | 14.4 ms | N/A |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing lorenzhs:lorenz/arrow-export-options (fe235ae) with develop (ca31e64)2
Footnotes
-
503 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. ↩
-
No successful run was found on
develop(c1b204c) during the generation of this report, so ca31e64 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Summary
Arrow export always compacts string and binary view buffers. Callers that consume the exported array right away don't need that scan and copy. This PR adds per-export options to Arrow export, starting with
CompactBuffers, so callers can skip compaction. Compaction stays on by default.Changes
ArrowExportOptions, a container keyed by type, so export plugins can define their own options without changingvortex-arrow. AddCompactBuffers, which defaults totrue.ArrowSession::with_options(&options), which returns anArrowExporterthat borrows the session and the options for one export:session.arrow().with_options(&options).execute_arrow(array, target, ctx). The session stores no options. The options are passed through all nested exports: list, list view, fixed-size list, struct, map, dictionary and run-end.optionsparameter toArrowExportVTable::execute_arrow, so plugins receive the options and pass them to their storage exports. All in-tree plugins are updated: JSON, UUID, tensorVector, Parquet Variant and the spatial types.optionsparameter toParquetVariantArrayExt::to_arrow, which the canonical Variant export path uses. This moves Variant off the deprecatedArrowArrayExecutor, which resolves theTODO(aduffy)there. Other callers pass the default options, so their behavior is unchanged.New tests check that the compaction setting reaches string and binary views inside every nested type, that options reach an external plugin under a list, and that a JSON extension array keeps its buffers with
CompactBuffers(false).API Changes
ArrowExportOptions(with,get,get_or_default),CompactBuffers,ArrowSession::with_optionsandArrowExporter.ArrowExportVTable::execute_arrowtakes a newoptions: &ArrowExportOptionsparameter. Plugins must pass it to their child and storage exports.ParquetVariantArrayExt::to_arrowtakes a newoptions: &ArrowExportOptionsparameter. Pass&ArrowExportOptions::default()to keep the previous behavior.🤖 Generated with Claude Code
https://claude.ai/code/session_01WS45RCSkbTsARypDJ3hkns