mbutrovich commented on code in PR #12107: URL: https://github.com/apache/arrow-rs/pull/12107#discussion_r4502295502
########## CONTRIBUTING.md: ########## @@ -307,6 +307,8 @@ identifier that can be changed without affecting test semantics. Pull requests that improve performance, especially those that add non-trivial complexity or use `unsafe`, should include evidence of the improvement, such as benchmarks. +A benchmark that improves is not enough on its own. State which part of the change produces the gain, for example by removing it and rerunning. Cover both well-predicted input (constant lengths, all valid) and poorly predicted input (random), because branchless code wins on the second and can lose on the first. Name the inputs that get slower and say whether real workloads are likely to hit them. Review Comment: So what I’ve been doing on any perf PR is doing the benchmark as the first commit. Then you can just ref the commit hash of the benchmark and we don’t need two PRs for every performance change. -- 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]
