Repository navigation
perf: Reduce record batch memory accounting overhead - #24319
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24319 +/- ##
========================================
Coverage 81.23% 81.24%
========================================
Files 1112 1112
Lines 390635 390912 +277
Branches 390635 390912 +277
========================================
+ Hits 317350 317602 +252
- Misses 54650 54668 +18
- Partials 18635 18642 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ryux1
force-pushed
the
perf/batch-memory-accounting
branch
from
August 13, 2026 18:14
e584d8b to
890be61
Compare
Contributor
Author
|
Thanks @comphead — added in 890be61:
The updated A/B tables are in the PR description. The row-count cases stay essentially flat as expected for buffer traversal; measured speedups are 7.49–7.97x for four primitive columns, 9.39x for lists, and 8.86x for structs.
|
Contributor
|
Thanks @ryux1 -- I'll check it out shortly |
Contributor
|
run benchmark window_query_sql |
1 similar comment
Contributor
|
run benchmark window_query_sql |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/batch-memory-accounting (890be61) to 8c099dc (merge-base) diff Run configurationrun benchmark window_query_sqlResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/batch-memory-accounting (890be61) to 8c099dc (merge-base) diff Run configurationrun benchmark window_query_sqlResults will be posted here when complete File an issue against this benchmark runner |
comphead
reviewed
Aug 13, 2026
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/batch-memory-accounting (890be61) to 8c099dc (merge-base) diff Run configurationrun benchmark window_query_sqlCPU Details (lscpu)Details
Resource Usagewindow_query_sql — base (merge-base)
window_query_sql — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/batch-memory-accounting (890be61) to 8c099dc (merge-base) diff Run configurationrun benchmark window_query_sqlCPU Details (lscpu)Details
Resource Usagewindow_query_sql — base (merge-base)
window_query_sql — branch
File an issue against this benchmark runner |
Contributor
|
Looks like a small but real improvement to me -- thank you @ryux1 |
Contributor
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/batch-memory-accounting (db959c3) to 8c099dc (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/batch-memory-accounting (db959c3) to 8c099dc (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/batch-memory-accounting (db959c3) to 8c099dc (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
alamb
reviewed
Aug 14, 2026
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/batch-memory-accounting (db959c3) to 8c099dc (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/batch-memory-accounting (db959c3) to 8c099dc (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/batch-memory-accounting (db959c3) to 8c099dc (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
comphead
reviewed
Aug 14, 2026
comphead
left a comment
comphead
left a comment
Contributor
There was a problem hiding this comment.
We may want to consider adding a bench that reuses a RecordBatchMemoryCounter across N sliced batches (the shared-buffer case), which is the hash-join build-side workload at datafusion/physical-plan/src/joins/hash_join/exec.rs:2134. That's where the inline-16 fast path matters most.
Extend the microbenchmark across row counts and List/Struct layouts. Exercise all legal run-end index types in the ArrayData parity test.
Clarify that the 16-entry threshold avoids allocations for small buffer sets while bounding inline storage and linear lookup. The threshold is a performance heuristic rather than a semantic limit.
Move buffer traversal helpers onto RecordBatchMemoryCounter and replace panic-based array dispatch with safe generic accounting fallbacks. Add concrete byte/list view parity coverage and benchmark a counter reused across zero-copy slices.
ryux1
force-pushed
the
perf/batch-memory-accounting
branch
from
August 15, 2026 13:50
db959c3 to
b6414cd
Compare
Contributor
Author
|
Addressed the latest review feedback in b6414cd and rebased the branch onto current Changes since the last review:
Fresh sequential A/B results against current
Validation:
All six review threads are addressed and resolved. @alamb @comphead, ready for another look when convenient. |
Contributor
Author
|
CI status update: the labeler completed successfully on |
Contributor
|
run clickbench_partitioned |
alamb
approved these changes
Aug 16, 2026
| } | ||
| // Arrow only permits Int16, Int32, and Int64 run-end indexes. A | ||
| // custom Array implementation may still expose malformed data; | ||
| // retain correct accounting for it without panicking. |
jayzhan211
approved these changes
Aug 16, 2026
alamb
enabled auto-merge
August 17, 2026 19:04
Contributor
|
Thanks again @jayzhan211 and @ryux1 and @comphead |
lilic
added a commit
to influxdata/arrow-datafusion
that referenced
this pull request
Sep 24, 2026
datafusion/common/src/utils/memory.rs as of apache/datafusion main 9951f0b (2026-09-24): apache#24319 introduced the counter and apache#24364, apache#24837, apache#25335 refined it. Counts each buffer once across a sequence of batches; replaces the fork-local helpers.
discord9
pushed a commit
to GreptimeTeam/datafusion
that referenced
this pull request
Nov 6, 2026
## Which issue does this PR close? - Closes apache#24310. ## Rationale for this change Record batch memory accounting runs on hot execution paths. The current implementation materializes ArrayData for every array and allocates a hash set even for small batches, adding measurable overhead to queries that frequently update memory reservations. ## What changes are included in this PR? - Traverse Arrow arrays directly and recursively count their backing buffers without materializing ArrayData. - Track the first 16 buffer identities inline, then promote to a hash set for wider batches or counters spanning many batches. - Preserve shared-buffer deduplication and full buffer-capacity accounting semantics. - Add parity coverage against the previous ArrayData traversal for primitive, binary/view, list/view, fixed-size, struct, union, dictionary, map, and all legal run-end index layouts. - Add a focused Criterion benchmark across column counts, row counts, and primitive/list/struct layouts. Criterion point estimates from `cargo bench -p datafusion-common --bench record_batch_memory`, measured sequentially on upstream `main` and this PR on the same machine: ### Column count (8,192 rows, Int64) | Columns | main | this PR | Speedup | |---:|---:|---:|---:| | 1 | 81.999 ns | 10.645 ns | 7.70x | | 4 | 308.11 ns | 38.669 ns | 7.97x | | 16 | 1.2543 us | 173.55 ns | 7.23x | | 64 | 4.7457 us | 1.3914 us | 3.41x | ### Row count (4 Int64 columns) | Rows | main | this PR | Speedup | |---:|---:|---:|---:| | 1 | 299.20 ns | 39.939 ns | 7.49x | | 128 | 302.99 ns | 38.914 ns | 7.79x | | 8,192 | 307.64 ns | 39.740 ns | 7.74x | | 65,536 | 310.61 ns | 38.994 ns | 7.97x | ### Array layout (4 columns, 8,192 rows) | Layout | main | this PR | Speedup | |---|---:|---:|---:| | Primitive Int64 | 311.01 ns | 40.140 ns | 7.75x | | List of Int64 | 778.29 ns | 82.865 ns | 9.39x | | Struct of two Int64 fields | 1.0072 us | 113.70 ns | 8.86x | Each list row contains two Int64 values. Each struct column contains two Int64 child fields. ## Are these changes tested? Yes. - `cargo fmt --all -- --check` - `cargo clippy --all-targets --all-features -- -D warnings` - `cargo test -p datafusion-common utils::memory --lib` (10 passed) - `RUST_BACKTRACE=1 cargo test --profile ci --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption` - Focused Criterion comparison shown above ## Are there any user-facing changes? No API or behavior changes. This reduces CPU and allocation overhead in record batch memory accounting. AI assistance: OpenAI Codex assisted with implementation and test execution. I reviewed the change and its behavior end to end. Backport provenance: - Upstream PR: apache#24319 - Upstream commit: c7be84a - The upstream diff is unchanged, including its own tests and benchmark. - No fork adaptation is included in this commit. Local Arrow 59 validity-buffer regressions are deliberately deferred to the final additional-test section. - The body above is the original upstream description; its benchmark estimates and verification statements describe upstream, not this fork's measurements. Signed-off-by: discord9 <[email protected]>
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
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.
Which issue does this PR close?
Rationale for this change
Record batch memory accounting runs on hot execution paths. The current implementation materializes ArrayData for every array and allocates a hash set even for small batches, adding measurable overhead to queries that frequently update memory reservations.
What changes are included in this PR?
Criterion point estimates from
cargo bench -p datafusion-common --bench record_batch_memory, measured sequentially on upstreammainand this PR on the same machine:Column count (8,192 rows, Int64)
Row count (4 Int64 columns)
Array layout (4 columns, 8,192 rows)
Each list row contains two Int64 values. Each struct column contains two Int64 child fields.
Are these changes tested?
Yes.
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo test -p datafusion-common utils::memory --lib(10 passed)RUST_BACKTRACE=1 cargo test --profile ci --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryptionAre there any user-facing changes?
No API or behavior changes. This reduces CPU and allocation overhead in record batch memory accounting.
AI assistance: OpenAI Codex assisted with implementation and test execution. I reviewed the change and its behavior end to end.