Rich-T-kid commented on code in PR #12107: URL: https://github.com/apache/arrow-rs/pull/12107#discussion_r4502550295
########## CONTRIBUTING.md: ########## @@ -333,7 +333,16 @@ cargo bench --bench parse_time -- --baseline main ``` If your PR proposes a performance improvement, include a summary of the benchmark results (for example, from `cargo-criterion` or `critcmp`) in the PR description. -If you need to add new benchmarks to cover your change, make a separate PR first (for example, [#9729]) so we can run the benchmarks on an automated runner. + +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. + +If you need to add new benchmarks to cover your change, add them in a separate PR first (for example, [#9729]) or as the first commit of your PR. Either way, the automated benchmark runner can compare the change against a revision that already has the benchmarks. For a first commit, pass its SHA as the baseline: Review Comment: can we add a line about pointing out which benchmark(s) the change targets. not sure if it fits better here or in the `AGENTS.md`. minor but makes it easier to know which benchmarks to run without needing to dig through the PR -- 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]
