perf(cubeorchestrator): keep CubeStore Arrow results in Arrow memory - #11427
Draft
ovr wants to merge 5 commits into
Draft
perf(cubeorchestrator): keep CubeStore Arrow results in Arrow memory#11427ovr wants to merge 5 commits into
ovr wants to merge 5 commits into
Conversation
CubeStore answers with Arrow IPC, but `QueryResult::from_arrow` threw that
away: it walked every cell and materialized a `Vec<DBResponsePrimitive>` per
column (32 B/cell plus a heap `String` per text/decimal cell), then the
transform cloned every cell again into an output dataset that was itself fully
built before serialization began. A 100k x 32 result was materialized twice.
Introduce `QueryResultColumn { Columnar(ColumnarArray) | Arrow(ArrowArray) }`.
Arrow columns stay as Arrow buffers; `ColumnReader` resolves the concrete array
once per column so the per-cell path is a jump table plus an index, never a
`DataType` match and a downcast. Multi-batch streams are concatenated per
column at parse, so a single-batch stream — everything outside CubeStore's
streaming path — is copied not at all.
On top of that, `DirectData` serializes the `compact` and `columnar` response
formats straight from the source columns, so the output dataset is never
materialized either. Cells that need no transform are handed to the serializer
by reference, which is where most of the JSON-side win comes from.
`TransformedData::transform` is unchanged: the SQL API needs the in-memory
`Columnar` form, and it doubles as the test oracle. The vanilla format still
materializes — streaming it needs a duplicate-key guard first, since today's
row `IndexMap` silently dedupes the deprecated-granularity and blending keys.
16 columns x 100k rows, before -> after:
| stage | before | after |
| ------------------------- | -------- | ------- |
| Arrow parse | 18.34 ms | 1.85 ms |
| final JSON, compact | 66.8 ms | 57.1 ms |
| final JSON, columnar | 53.8 ms | 51.3 ms |
| **end-to-end, compact** | 85.1 ms | 59.0 ms |
| **end-to-end, columnar** | 72.1 ms | 53.2 ms |
Peak memory drops by the whole intermediate primitive buffer (~3.2M cells at
that size) for compact and columnar.
Behaviour is held byte-identical by a new oracle test: for every fixture, in
every response format, with both Arrow- and primitive-backed columns, the
streamed JSON must equal the materialized JSON exactly. Two deliberate
differences: plan-time failures on the streaming path surface as a serde error
wrapping the message instead of an `anyhow` error raised before serialization,
and a zero-row column of an unsupported Arrow type still parses (no cell is
ever read), matching the pre-existing behaviour of the per-cell loop.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`QueryResultColumn::to_columnar` and `ArrowArray::to_columnar` had no production callers — only two test sites — and `ArrowArray::data_type`/`::array` had none at all. A materializing helper on the type whose point is *not* to materialize is an invitation to reintroduce the copy this branch removed, so drop all four. The two tests now read columns through `ColumnReader`, which is what the transform paths use, so they exercise the production accessor instead of a test-only one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ovr
force-pushed
the
perf/query-result-arrow-pass
branch
from
July 30, 2026 12:29
e18a190 to
7905950
Compare
github-actions
Bot
added
rust
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11427 +/- ##
==========================================
- Coverage 83.94% 83.94% -0.01%
==========================================
Files 257 257
Lines 80887 80887
==========================================
- Hits 67904 67903 -1
- Misses 12983 12984 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`read_cell!` had to produce a `DBResponsePrimitive`, whose `String` variant owns its bytes — so every cell of an Arrow `Utf8`/`LargeUtf8`/`Utf8View` column cost an allocation even when the caller only wrote the cell out and dropped it. Add `CellRef`: a cell on its way out of a column, which either points at a materialized primitive, borrows text straight from the Arrow buffer, or owns a value decoded on read. The response serializers take that, so text columns now reach the JSON writer with no allocation at all. `Serialize for CellRef` delegates to `Serialize for DBResponsePrimitive` for everything except `Str`, which renders exactly as that impl's `String` arm — one line, so the two cannot drift apart. Borrowing is gated on `is_identity_transform`: a text column annotated as a `time` member is rewritten by `transform_value` and still needs an owned value. `StorageFixture` grew a `Utf8` column typed `time` to cover precisely that, and asserts the reformatted output rather than the raw Arrow text. 16 columns x 100k rows, Arrow source, response JSON: | format | materialized | direct before | direct now | | -------- | ------------ | ------------- | ---------- | | compact | 67.9 ms | 57.1 ms | 48.1 ms | | columnar | 55.2 ms | 51.3 ms | 38.6 ms | The fixture also had to pin down member ordering: `get_members` walks a `HashMap` when appending deprecated-style time members, so a query with two of them yields either order from one call to the next. The fixture now requests one base dimension explicitly, leaving a single appended member — the nondeterminism itself is pre-existing and left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CubeStore answers `SUM` with `Decimal128`, so most measure cells are decimals — and `decimal_to_string` allocated twice for each one: a `String` for the mantissa's digits, then a `format!` for the result. Nothing measured that, because the Arrow bench fixture typed its measures as `Float64`; with `Decimal128` measures the response JSON cost 66% more (compact) and 90% more (columnar) than the numbers this branch had been reporting. Render through `fmt::Write` and a stack digit buffer instead. `DecimalText` holds the mantissa and scale and writes the text on demand, so: - the serializers hand `CellRef::Decimal128`/`Decimal256` to `collect_str`, which streams the digits into the response with no allocation at all; - `decimal_to_string` is that same renderer collected into one `String`, which also speeds up the materializing path the SQL API uses. There is one definition of the format, so the two paths cannot drift. Since this replaces hand-written numeric formatting, `test_decimal_to_string_matches_reference` keeps the previous String-building algorithm as an oracle and checks both agree across `i128` extremes, `i256` extremes and eleven scales, and `test_decimal_cell_serializes_like_owned_string` pins the serialized bytes to what the owned `String` cell produces. 16 columns x 100k rows, Decimal128 measures, response JSON: | path | format | before | after | | ------------ | -------- | -------- | ------- | | direct | compact | 79.9 ms | 45.1 ms | | direct | columnar | 73.4 ms | 40.2 ms | | materialized | compact | 103.5 ms | 78.8 ms | | materialized | columnar | 87.8 ms | 64.2 ms | The bench fixture gained a `MeasureKind` axis so the decimal shape stays measured from here on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rator Each Arrow column in the bench fixture was collected into a `Vec` and handed to `Array::from`, which copies it into the Arrow buffer — so every column existed twice during setup, and the string case held `row_count` live `String` allocations at once (100k of them at the top fixture size). `from_iter_values` fills the buffer from the iterator directly. The emitted IPC payloads are byte-identical, so recorded measurements stay comparable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.