apache / apache/datafusion-comet

Reduce allocations in approx_percentile QuantileSummaries merge and flush paths

Open
#4,874 2 comments 0 reactions 1 assignee Claimed by @peterxcli View on GitHub
area:aggregation area:expressions enhancement performance
Dominant language
Scala
Stars
1.3k
Forks
375
Avg merge
2d 4h
Merged PRs (30d)
198

Description

This is a follow-up to a review comment on #4801 (https://github.com/apache/datafusion-comet/pull/4801#discussion_r3552521171).

## What is the problem the feature request solves?

The native `QuantileSummaries` port behind `approx_percentile` / `percentile_approx` allocates more than necessary on its hot paths:

- `QuantileSummaries::merge` takes `&self` / `&other` and allocates a fresh summary, so `merge_batch` reallocates on every incoming digest.
- `with_head_buffer_inserted` rebuilds the whole `sampled` vector on every flush.

These are not correctness issues, and results are bit-identical to Spark, but they show up as avoidable allocation churn when many partial digests are merged or when a single accumulator ingests many batches.

## Describe the potential solution

- A double-buffer for the flush so `with_head_buffer_inserted` reuses a scratch `Vec` instead of allocating a new `sampled` each time.
- A consuming `merge(self, other: &Self) -> Self` (or an in-place `merge_into(&mut self, other: &Self)`) so `merge_batch` does not reallocate per digest.

Both changes must preserve the existing bit-for-bit results; the load-bearing invariants are documented at the top of `native/spark-expr/src/agg_funcs/quantile_summaries.rs`.

## Additional context

Worth doing only if a profile shows this path is hot. Noted during review of #4801 as an explicit out-of-scope follow-up.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.