andygrove opened a new issue, #4874: URL: https://github.com/apache/datafusion-comet/issues/4874
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. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
