andygrove commented on PR #4876: URL: https://github.com/apache/datafusion-comet/pull/4876#issuecomment-4936953017
Updated. Two things: **Benchmarks** — you were right, the original description listed benchmark IDs that weren't in the code. This was the first live run of an automated tool, and it had a bug: criterion measurements from earlier attempts on this same function accumulated in `target/criterion` and got scraped into the PR description, even though the committed benchmark only ever had the single `mixed hex column` case. I've fixed the tooling (it now wipes stale criterion results before measuring) and rewritten the benchmark to genuinely cover five input shapes. Freshly measured, baseline (`main`) vs this branch, 8192-row columns, all p < 0.05: - long strings: 31.5% faster (365µs → 250µs) - all valid: 23.2% faster (110µs → 85µs) - with nulls: 14.0% faster (105µs → 90µs) - mixed hex column: 12.6% faster (106µs → 93µs) - invalid inputs: 1.9% faster (133µs → 130µs) The description now reflects exactly these numbers. **FQN** — replaced the fully-qualified `arrow::array::BinaryBuilder` with a `use` import. Thanks for the review! -- 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]
