UBarney commented on PR #21090: URL: https://github.com/apache/datafusion/pull/21090#issuecomment-4159654085
> > I added `first_value` benchmarks to `clickbench_extend`. I skipped `last_value` as its implementation logic is identical to `first_value`. @neilconway > > Hmm, I think it is more typical to either add a new criterion-based benchmark or extending an existing one to include a benchmark for `first_` / `last_value` -- for example as done here https://github.com/apache/datafusion/pull/21154/changes#diff-45ef5fab11bdcc96f0716ae2b2dc9685d75a515e660c6d082e1e4c22a83c43aaR299 I believe `clickbench_extend` is more suitable here for two reasons: 1. The dataset is significantly larger, which better demonstrates the performance gains (I'm seeing a ~4x improvement) compared to a micro-benchmark that runs in under 10ms. 2. There is already a precedent for using `clickbench_extend` to test aggregate function performance, for example: https://github.com/apache/datafusion/blob/71459283a93f18b50835543468f506e434ded96d/benchmarks/queries/clickbench/README.md#L64 What do you think? -- 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]
