kosiew commented on PR #25051: URL: https://github.com/apache/datafusion/pull/25051#issuecomment-5613096613
@jayzhan211 > adapter calls the factory once per new group and update_batch once per group per batch I amended the GroupsAccumulatorAdapter to time the entire grouped dispatch once per input batch, while each DistinctArrayAggAccumulator executes the same update logic without individually recording another timer. > Could you run a benchmark against main for something like ...with ~1M distinct k and 1-2 rows per group, and post the numbers? I added a benchmark and rebased the benchmark to before adding internal submetrics so I can compare benchmark before vs after. [empty commit](https://github.com/apache/datafusion/pull/25051/commits/a997d8313e4c85c5a6ff14ada6876809592df42b) is the mark right before the commits adding internal submetrics. I duplicated this branch to [another PR](https://github.com/apache/datafusion/pull/25096) and ran benchmark there. # 1M distinct -> [did not finish before repo killed it](https://github.com/apache/datafusion/pull/25096#issuecomment-5598236105) # 100k [run 1](https://github.com/apache/datafusion/pull/25096#issuecomment-5605477111) ``` group HEAD test-aggmetrics ----- ---- --------------- array_agg_distinct/Q01 1.00 5.1±0.03ms ? ?/sec 1.04 5.3±0.06ms ? ?/sec ``` [run 2](https://github.com/apache/datafusion/pull/25096#issuecomment-5611653768) ``` group HEAD test-aggmetrics ----- ---- --------------- array_agg_distinct/Q01 1.01 4.8±0.06ms ? ?/sec 1.00 4.8±0.02ms ? ?/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: [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]
