You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "fred1268 (via GitHub)" <gi...@apache.org> on 2026/07/20 09:45:40 UTC
[PR] `array_agg()` performance improvements [datafusion]
fred1268 opened a new pull request, #23716:
URL: https://github.com/apache/datafusion/pull/23716
## Which issue does this PR close?
- Closes #23715.
## Rationale for this change
Queries using `array_agg(DISTINCT col)` were significantly slower than expected.
Profiling revealed that `DistinctArrayAggAccumulator::update_batch` was allocating a
heap-owned `String` on every single input row — even for rows whose value was already
present in the accumulator. For a typical low-cardinality workload (e.g. a column of ~25
database names across thousands of rows), this meant paying the full allocation cost for
every duplicate, which dominated the runtime.
## What changes are included in this PR?
This PR applies the same deduplication strategy already used by `AggregateExec` for
`GROUP BY`: duplicate rows now cost only a hash probe with no heap allocation, and new
distinct values are appended to a single shared buffer rather than allocated individually.
The fix applies to all column types, not just strings, and `retract_batch` support
(required for sliding window frames such as `ROWS BETWEEN N PRECEDING AND CURRENT ROW`)
is fully preserved.
## Are these changes tested?
Four unit tests were added to `DistinctArrayAggAccumulator` — one each for `Utf8`,
`Int64`, `Float64`, and `Date32` — to pin the deduplication contract across the most
common column types and serve as a regression guard for future changes. The existing
sliding window sqllogictest suite (`array_agg_sliding_window.slt`) covers `retract_batch`
correctness end-to-end and passes unchanged.
Two `update_batch` micro-benchmarks were added to measure the before/after on realistic
data: one with low cardinality (~25 distinct database names in 8 192 rows, modelling the
common production case) and one with high cardinality (~7 800 distinct values, modelling
the worst case where almost every row is new). Results on an 8 192-row batch:
| Benchmark | Before | After | Speedup |
|---|---|---|---|
| Low cardinality (~25 distinct DB names) | 648.6 µs | 189.4 µs | **3.42×** |
| High cardinality (~7 800 distinct values) | 1078.3 µs | 269.4 µs | **4.00×** |
## Are there any user-facing changes?
No user-facing changes
No breaking changes to public APIs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "fred1268 (via GitHub)" <gi...@apache.org>.
fred1268 commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5100975217
Thanks @alamb for the thorough review, and sorry if I pushed in two batches which was confusing for you (will try to do better next time). Let me know if there is something else I can do on this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #23716:
URL: https://github.com/apache/datafusion/pull/23716
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3663377381
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -827,9 +827,16 @@ impl GroupsAccumulator for ArrayAggGroupsAccumulator {
struct DistinctState {
/// Converts Arrow arrays to/from the comparable row format.
converter: RowConverter,
- /// Contiguous buffer of one encoded row per distinct value ever inserted.
- /// Dead slots (where `counts[i] == 0`) may exist after `retract_batch`.
- group_values: Rows,
+ /// One owned encoded row per live distinct value, indexed by group index.
+ /// Compacted via swap-remove on eviction so there are never dead slots.
+ group_rows: Vec<OwnedRow>,
Review Comment:
If this turns out to be too expenive (an allocation per OwnedRow) we could also use another Rows here, though managing it will be complicated as Rows doens't have a swap_remove function
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -817,29 +817,40 @@ impl GroupsAccumulator for ArrayAggGroupsAccumulator {
}
}
+/// Resources that are allocated lazily on the first `update_batch` call,
+/// once the concrete runtime Arrow type is known.
+///
+/// Grouping all three fields together makes the "either all present or all
+/// absent" invariant explicit in the type system, replacing the scattered
+/// `.expect()` calls that would otherwise be needed.
#[derive(Debug)]
-pub struct DistinctArrayAggAccumulator {
+struct DistinctState {
Review Comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3672272636
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -1025,10 +1043,12 @@ impl Accumulator for DistinctArrayAggAccumulator {
let arrays = converter.convert_rows(rows)?;
// `convert_rows` always returns the physical (non-dictionary) type.
- // Cast back to the declared logical type when they differ so that
- // e.g. Dictionary columns round-trip correctly through the RowConverter.
+ // Cast back to the declared logical type when they differ AND the
+ // declared type contains a Dictionary somewhere (directly or nested
+ // inside a Struct, List, etc.) — that is the only case where
+ // RowConverter strips the logical type.
Review Comment:
Yes — third site with the same shape: apache/arrow-rs#10413 (`FixedSizeList<Dictionary>` decode panic), the `ListView<Dictionary>` reconstruction in #23523, and now this cast-back. Each hand-maintains a "does the declared type contain a Dictionary anywhere" walk that mirrors arrow-row's internal `corrected_type`.
I filed apache/arrow-rs#10427 proposing `decoded_type(&DataType) -> DataType` plus a round-trip property test — with that, this check collapses to `decoded_type(&t) != t`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5022977080
🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/23716#issuecomment-5022949366)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c5022949366-1176-hk9sf 6.12.85+ #1 SMP Mon May 11 08:17:35 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing perf/array_aggregate (33c5bb1213fd1e335419704f1216eb8a70aea09e) to 1e58928 (merge-base) [diff](https://github.com/apache/datafusion/compare/1e589287c3d6bcf3bc6fa897bb889a6012e1d229..33c5bb1213fd1e335419704f1216eb8a70aea09e) using: array_agg
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5032327320
🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/23716#issuecomment-5032302654)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c5032302654-1202-qdxqv 6.12.85+ #1 SMP Mon May 11 08:17:35 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing perf/array_aggregate (1cd130563b4f8ac948dd9ee08429c6e17a1b7007) to 1fcdef2 (merge-base) [diff](https://github.com/apache/datafusion/compare/1fcdef2fed591e4f5a3144f4d1deb05c57cf343f..1cd130563b4f8ac948dd9ee08429c6e17a1b7007) using: array_agg
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5100859130
run benchmarks array_agg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "fred1268 (via GitHub)" <gi...@apache.org>.
fred1268 commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5101059970
The high cardinality benchmark, although still an improvement, is not as good as in the previous version. That being said, I would expect most people will do `array_agg(DISTINCT col)` when there is a limited number of values in `col` so this should not be too much of a problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5031555928
Let's ship the benchmarks in a preliminary PR, and then come back to this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "fred1268 (via GitHub)" <gi...@apache.org>.
fred1268 commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3657996710
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -890,37 +981,38 @@ impl Accumulator for DistinctArrayAggAccumulator {
}
fn evaluate(&mut self) -> Result<ScalarValue> {
- let mut values: Vec<ScalarValue> = self.values.keys().cloned().collect();
- if values.is_empty() {
+ if self.map.is_empty() {
return Ok(ScalarValue::new_null_list(self.datatype.clone(), true, 1));
}
- if let Some(opts) = self.sort_options {
- let mut delayed_cmp_err = Ok(());
- values.sort_by(|a, b| {
- if a.is_null() {
- return match opts.nulls_first {
- true => Ordering::Less,
- false => Ordering::Greater,
- };
- }
- if b.is_null() {
- return match opts.nulls_first {
- true => Ordering::Greater,
- false => Ordering::Less,
- };
- }
- match opts.descending {
- true => b.try_cmp(a),
- false => a.try_cmp(b),
- }
- .unwrap_or_else(|err| {
- delayed_cmp_err = Err(err);
- Ordering::Equal
- })
- });
- delayed_cmp_err?;
- };
+ let group_values = self
Review Comment:
I implemented something that is my understanding of your comment.
Please let me know if it is not the case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3647671067
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -1546,8 +1682,10 @@ mod tests {
acc2.update_batch(&[string_list_data([vec!["e", "f", "g"]])])?;
acc1 = merge(acc1, acc2)?;
- // without compaction, the size is 16684
- assert_eq!(acc1.size(), 1684);
+ // The GroupValuesRows-based implementation uses a contiguous Rows
+ // buffer + HashTable instead of individual ScalarValue allocations,
+ // so the reported size differs from the previous implementation (was 1684).
Review Comment:
I think the context about the previous impleementation will not be relevant after this PR merges -- it is more a comment for this PR -- just updating without this comment I think would be less confusing
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -876,12 +970,9 @@ impl Accumulator for DistinctArrayAggAccumulator {
assert_eq_or_internal_err!(states.len(), 1, "expects single state");
- // The DISTINCT state schema is `List<value>` — partial accumulators
- // ship the set of values they saw, not multiplicities. Re-ingesting
- // each element here makes the merged counts represent "partitions
- // that emitted this value," which is fine because `evaluate` only
- // reads keys. Refcount semantics for retract are only valid within
- // a single accumulator instance (window execution).
+ // The DISTINCT state is `List<value>`. Partial accumulators ship the
+ // set of live values, not multiplicities. Re-ingesting them here is
Review Comment:
what are multiplicities? I think `DISTINCT state is List<value>` is probably enough for this comment (i see you dodn't add this)
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -890,37 +981,38 @@ impl Accumulator for DistinctArrayAggAccumulator {
}
fn evaluate(&mut self) -> Result<ScalarValue> {
- let mut values: Vec<ScalarValue> = self.values.keys().cloned().collect();
- if values.is_empty() {
+ if self.map.is_empty() {
return Ok(ScalarValue::new_null_list(self.datatype.clone(), true, 1));
}
- if let Some(opts) = self.sort_options {
- let mut delayed_cmp_err = Ok(());
- values.sort_by(|a, b| {
- if a.is_null() {
- return match opts.nulls_first {
- true => Ordering::Less,
- false => Ordering::Greater,
- };
- }
- if b.is_null() {
- return match opts.nulls_first {
- true => Ordering::Greater,
- false => Ordering::Less,
- };
- }
- match opts.descending {
- true => b.try_cmp(a),
- false => a.try_cmp(b),
- }
- .unwrap_or_else(|err| {
- delayed_cmp_err = Err(err);
- Ordering::Equal
- })
- });
- delayed_cmp_err?;
- };
+ let group_values = self
Review Comment:
You can likely avoid these expects / implicit state machines by using an explicit state enum . Maybe something like this:
```rust
enum state {
Init,
Allocated {
map: ...,
group_values: ...:
converter: ...
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5022949366
run benchmarks array_agg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3663357368
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -817,29 +817,40 @@ impl GroupsAccumulator for ArrayAggGroupsAccumulator {
}
}
+/// Resources that are allocated lazily on the first `update_batch` call,
+/// once the concrete runtime Arrow type is known.
+///
Review Comment:
What would be amazing is if we could reuse some of the actual hash aggregate implementations -- but I don't really know how to do that yet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5100975402
🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/23716#issuecomment-5100859130)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group HEAD perf_array_aggregate
----- ---- --------------------
array_agg i64 merge_batch 30% nulls, 0% of nulls point to a zero length array 1.00 501.4±6.42µs ? ?/sec 1.00 500.5±5.68µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 100% of nulls point to a zero length array 1.00 4.6±0.00µs ? ?/sec 1.00 4.6±0.01µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 50% of nulls point to a zero length array 1.00 504.0±6.34µs ? ?/sec 1.00 502.3±5.59µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 90% of nulls point to a zero length array 1.19 592.2±6.54µs ? ?/sec 1.00 499.5±5.91µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 99% of nulls point to a zero length array 1.01 509.4±6.23µs ? ?/sec 1.00 505.2±5.59µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 0% of nulls point to a zero length array 1.00 216.2±2.71µs ? ?/sec 1.00 216.4±2.41µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 100% of nulls point to a zero length array 1.00 4.4±0.00µs ? ?/sec 1.00 4.4±0.00µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 50% of nulls point to a zero length array 1.00 214.1±2.68µs ? ?/sec 1.00 214.2±2.47µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 90% of nulls point to a zero length array 1.00 216.6±2.73µs ? ?/sec 1.00 215.7±2.38µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 99% of nulls point to a zero length array 1.01 217.0±2.74µs ? ?/sec 1.00 214.7±2.68µs ? ?/sec
array_agg i64 merge_batch all nulls, 100% of nulls point to a zero length array 1.04 75.3±3.51ns ? ?/sec 1.00 72.4±2.93ns ? ?/sec
array_agg i64 merge_batch all nulls, 90% of nulls point to a zero length array 1.28 94.0±2.91ns ? ?/sec 1.00 73.2±2.96ns ? ?/sec
array_agg i64 merge_batch no nulls 1.00 82.1±2.50ns ? ?/sec 1.00 82.4±1.42ns ? ?/sec
distinct_array_agg utf8 high cardinality (~7800 distinct, 5% db names) 1.68 1126.6±26.64µs ? ?/sec 1.00 672.3±1.24µs ? ?/sec
distinct_array_agg utf8 low cardinality (~25 distinct) 3.85 586.2±9.97µs ? ?/sec 1.00 152.3±0.58µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**array_agg — base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 350.1s |
| Peak memory | 35.1 MiB |
| Avg memory | 12.2 MiB |
| CPU user | 182.0s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
**array_agg — branch**
| Metric | Value |
|--------|-------|
| Wall time | 345.1s |
| Peak memory | 29.8 MiB |
| Avg memory | 11.3 MiB |
| CPU user | 185.1s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5023137023
Seems like there's no impact in those benchmarks. Do you know if there are any other benchmarks that are using `array_agg`? maybe tpch or tpcds?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5032493643
Nice!
```
distinct_array_agg utf8 high cardinality (~7800 distinct, 5% db names) 4.10 1121.5±2.78µs ? ?/sec 1.00 273.2±0.68µs ? ?/sec
distinct_array_agg utf8 low cardinality (~25 distinct) 3.89 577.0±1.79µs ? ?/sec 1.00 148.3±0.42µs ? ?/sec
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5100857691
Thanks @fred1268 -- I merged up from main to resolve a conflict. The code looks good to me now, though it may be slower as it now does a new allocation for each distinct value (OwnedRow). I'll do the benchmarks one more time
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5100877216
🤖 Benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/23716#issuecomment-5100859130)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c5100859130-1280-9qnh9 6.12.85+ #1 SMP Mon May 11 08:17:35 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing perf/array_aggregate (8389f2ed9a3f5640add32a56759c4441578e1a7d) to bb75d92 (merge-base) [diff](https://github.com/apache/datafusion/compare/bb75d925cb7a6b8e1c5dc22bead85efc49754fe3..8389f2ed9a3f5640add32a56759c4441578e1a7d) using: array_agg
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "fred1268 (via GitHub)" <gi...@apache.org>.
fred1268 commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3657989856
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -1546,8 +1682,10 @@ mod tests {
acc2.update_batch(&[string_list_data([vec!["e", "f", "g"]])])?;
acc1 = merge(acc1, acc2)?;
- // without compaction, the size is 16684
- assert_eq!(acc1.size(), 1684);
+ // The GroupValuesRows-based implementation uses a contiguous Rows
+ // buffer + HashTable instead of individual ScalarValue allocations,
+ // so the reported size differs from the previous implementation (was 1684).
Review Comment:
Sorry about this, I forgot to clean them up
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -876,12 +970,9 @@ impl Accumulator for DistinctArrayAggAccumulator {
assert_eq_or_internal_err!(states.len(), 1, "expects single state");
- // The DISTINCT state schema is `List<value>` — partial accumulators
- // ship the set of values they saw, not multiplicities. Re-ingesting
- // each element here makes the merged counts represent "partitions
- // that emitted this value," which is fine because `evaluate` only
- // reads keys. Refcount semantics for retract are only valid within
- // a single accumulator instance (window execution).
+ // The DISTINCT state is `List<value>`. Partial accumulators ship the
+ // set of live values, not multiplicities. Re-ingesting them here is
Review Comment:
Sorry about this, I forgot to clean them up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5104913112
Performance results definitely look like an improvement to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5021599342
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/23716?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `87.79343%` with `26 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.69%. Comparing base ([`1e58928`](https://app.codecov.io/gh/apache/datafusion/commit/1e589287c3d6bcf3bc6fa897bb889a6012e1d229?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0ae8e11`](https://app.codecov.io/gh/apache/datafusion/commit/0ae8e115ec9abd890ec05c63d39d3aad431af03f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 17 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/23716?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/functions-aggregate/src/array\_agg.rs](https://app.codecov.io/gh/apache/datafusion/pull/23716?src=pr&el=tree&filepath=datafusion%2Ffunctions-aggregate%2Fsrc%2Farray_agg.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9mdW5jdGlvbnMtYWdncmVnYXRlL3NyYy9hcnJheV9hZ2cucnM=) | 87.79% | [6 Missing and 20 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/23716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #23716 +/- ##
==========================================
+ Coverage 80.67% 80.69% +0.02%
==========================================
Files 1086 1088 +2
Lines 366800 367920 +1120
Branches 366800 367920 +1120
==========================================
+ Hits 295912 296896 +984
- Misses 53260 53295 +35
- Partials 17628 17729 +101
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/23716?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5023095886
🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/23716#issuecomment-5022949366)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group HEAD perf_array_aggregate
----- ---- --------------------
array_agg i64 merge_batch 30% nulls, 0% of nulls point to a zero length array 1.02 508.6±10.52µs ? ?/sec 1.00 497.7±6.50µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 100% of nulls point to a zero length array 1.00 4.6±0.00µs ? ?/sec 1.00 4.6±0.00µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 50% of nulls point to a zero length array 1.02 510.3±9.30µs ? ?/sec 1.00 502.0±6.26µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 90% of nulls point to a zero length array 1.01 510.2±10.92µs ? ?/sec 1.00 504.1±5.99µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 99% of nulls point to a zero length array 1.03 515.3±8.71µs ? ?/sec 1.00 501.2±5.86µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 0% of nulls point to a zero length array 1.02 218.6±3.77µs ? ?/sec 1.00 213.5±2.63µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 100% of nulls point to a zero length array 1.00 4.4±0.00µs ? ?/sec 1.00 4.4±0.00µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 50% of nulls point to a zero length array 1.00 215.9±3.92µs ? ?/sec 1.00 216.5±2.39µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 90% of nulls point to a zero length array 1.01 218.1±5.02µs ? ?/sec 1.00 217.0±2.31µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 99% of nulls point to a zero length array 1.02 220.0±4.23µs ? ?/sec 1.00 216.5±2.32µs ? ?/sec
array_agg i64 merge_batch all nulls, 100% of nulls point to a zero length array 1.01 73.6±3.29ns ? ?/sec 1.00 73.1±3.08ns ? ?/sec
array_agg i64 merge_batch all nulls, 90% of nulls point to a zero length array 1.02 74.0±3.03ns ? ?/sec 1.00 72.4±2.89ns ? ?/sec
array_agg i64 merge_batch no nulls 1.00 81.6±1.37ns ? ?/sec 1.01 82.6±1.19ns ? ?/sec
distinct_array_agg utf8 high cardinality (~7800 distinct, 5% db names) 1.00 274.1±0.84µs ? ?/sec
distinct_array_agg utf8 low cardinality (~25 distinct) 1.00 147.5±0.52µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**array_agg — base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 355.1s |
| Peak memory | 27.7 MiB |
| Avg memory | 8.3 MiB |
| CPU user | 156.3s |
| CPU sys | 0.1s |
| Peak spill | 0 B |
**array_agg — branch**
| Metric | Value |
|--------|-------|
| Wall time | 370.1s |
| Peak memory | 30.7 MiB |
| Avg memory | 10.5 MiB |
| CPU user | 180.9s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "fred1268 (via GitHub)" <gi...@apache.org>.
fred1268 commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5070155255
@jayzhan211 @alamb as the original author and reviewer of this code, you may be interested this PR.
(per @gabotechs recommandations, I created a separate PR to add tests and benchmarks before)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: `array_agg()` performance improvements [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3672038558
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -1025,10 +1043,12 @@ impl Accumulator for DistinctArrayAggAccumulator {
let arrays = converter.convert_rows(rows)?;
// `convert_rows` always returns the physical (non-dictionary) type.
- // Cast back to the declared logical type when they differ so that
- // e.g. Dictionary columns round-trip correctly through the RowConverter.
+ // Cast back to the declared logical type when they differ AND the
+ // declared type contains a Dictionary somewhere (directly or nested
+ // inside a Struct, List, etc.) — that is the only case where
+ // RowConverter strips the logical type.
Review Comment:
I think @zhuqi-lucas also observed this too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5032302654
run benchmarks array_agg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] `array_agg()` performance improvements [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#issuecomment-5032406133
🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/23716#issuecomment-5032302654)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group HEAD perf_array_aggregate
----- ---- --------------------
array_agg i64 merge_batch 30% nulls, 0% of nulls point to a zero length array 1.00 502.5±4.80µs ? ?/sec 1.00 501.6±10.91µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 100% of nulls point to a zero length array 1.00 4.6±0.00µs ? ?/sec 1.00 4.6±0.00µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 50% of nulls point to a zero length array 1.01 507.6±5.23µs ? ?/sec 1.00 504.5±11.24µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 90% of nulls point to a zero length array 1.01 509.7±12.25µs ? ?/sec 1.00 507.1±11.09µs ? ?/sec
array_agg i64 merge_batch 30% nulls, 99% of nulls point to a zero length array 1.00 504.8±10.50µs ? ?/sec 1.01 509.2±10.60µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 0% of nulls point to a zero length array 1.00 214.2±2.30µs ? ?/sec 1.00 214.5±4.55µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 100% of nulls point to a zero length array 1.00 4.4±0.00µs ? ?/sec 1.00 4.4±0.01µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 50% of nulls point to a zero length array 1.00 215.4±2.38µs ? ?/sec 1.01 217.2±4.56µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 90% of nulls point to a zero length array 1.00 215.5±2.38µs ? ?/sec 1.01 216.8±4.77µs ? ?/sec
array_agg i64 merge_batch 70% nulls, 99% of nulls point to a zero length array 1.00 215.6±2.63µs ? ?/sec 1.01 217.9±4.56µs ? ?/sec
array_agg i64 merge_batch all nulls, 100% of nulls point to a zero length array 1.00 72.1±2.71ns ? ?/sec 1.04 74.7±3.17ns ? ?/sec
array_agg i64 merge_batch all nulls, 90% of nulls point to a zero length array 1.00 72.2±2.70ns ? ?/sec 1.11 80.2±4.36ns ? ?/sec
array_agg i64 merge_batch no nulls 1.00 81.8±1.32ns ? ?/sec 1.01 82.4±1.25ns ? ?/sec
distinct_array_agg utf8 high cardinality (~7800 distinct, 5% db names) 4.10 1121.5±2.78µs ? ?/sec 1.00 273.2±0.68µs ? ?/sec
distinct_array_agg utf8 low cardinality (~25 distinct) 3.89 577.0±1.79µs ? ?/sec 1.00 148.3±0.42µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**array_agg — base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 260.1s |
| Peak memory | 33.2 MiB |
| Avg memory | 15.7 MiB |
| CPU user | 181.2s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
**array_agg — branch**
| Metric | Value |
|--------|-------|
| Wall time | 255.1s |
| Peak memory | 30.9 MiB |
| Avg memory | 15.5 MiB |
| CPU user | 184.4s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org