From 062eef4703706b754ed43312169f9cf07ac89fe9 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 21 Apr 2026 11:01:39 -0400 Subject: [PATCH 1/3] docs: Update contributing guidelines with benchmark results --- .github/pull_request_template.md | 2 ++ CONTRIBUTING.md | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index c2d07f49ab88..0e10040fe5a9 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -27,6 +27,8 @@ We typically require tests for all PRs in order to: 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? + +If this PR claims performance improvement, please include evidence, such as benchmark results. --> # Are there any user-facing changes? diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a375917e3a3b..4e8b69fc7652 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -198,9 +198,11 @@ Search for `allow(clippy::` in the codebase to identify lints that are ignored/a - If you have several lints on a function or module, you may disable the lint on the function or module. - If a lint is pervasive across multiple modules, you may disable it at the crate level. -## Running Benchmarks +## Performance Improvements -Running benchmarks are a good way to test the performance of a change. As benchmarks usually take a long time to run, we recommend running targeted tests instead of the full suite. +Pull requests to improve performance, especially those that add non trivial complexity or `unsafe` should be accompanied by evidence of improvement, such as benchmarks. + +As benchmarks usually take a long time to run, we recommend running targeted tests instead of the full suite. ```bash # run all benchmarks @@ -225,6 +227,11 @@ git checkout feature cargo bench --bench parse_time -- --baseline main ``` +If your PR proposes a performance improvement, please include a summary of the benchmark results (e.g. from `cargo-criterion` or `critcmp`) in the PR description. +If you need to add new benchmarks to cover your change, please make a separate PR (for example [#9729]) first so we can run the benchmarks with automated runner. + + +[#9729]: https://github.com/apache/arrow-rs/pull/9729 ## Git Pre-Commit Hook We can use [git pre-commit hook](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) to automate various kinds of git pre-commit checking/formatting. From 69a4e7b783a67b63d3676a66d3f192d205d7a85d Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 21 Apr 2026 11:07:13 -0400 Subject: [PATCH 2/3] tweaks --- .github/pull_request_template.md | 2 +- CONTRIBUTING.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 0e10040fe5a9..7daafd5b38df 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -28,7 +28,7 @@ We typically require tests for all PRs in order to: If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? -If this PR claims performance improvement, please include evidence, such as benchmark results. +If this PR claims a performance improvement, please include evidence such as benchmark results. --> # Are there any user-facing changes? diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4e8b69fc7652..e8d64e293c61 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -200,7 +200,7 @@ Search for `allow(clippy::` in the codebase to identify lints that are ignored/a ## Performance Improvements -Pull requests to improve performance, especially those that add non trivial complexity or `unsafe` should be accompanied by evidence of improvement, such as benchmarks. +Pull requests that improve performance, especially those that add non-trivial complexity or use `unsafe`, should include evidence of the improvement, such as benchmarks. As benchmarks usually take a long time to run, we recommend running targeted tests instead of the full suite. @@ -227,8 +227,8 @@ git checkout feature cargo bench --bench parse_time -- --baseline main ``` -If your PR proposes a performance improvement, please include a summary of the benchmark results (e.g. from `cargo-criterion` or `critcmp`) in the PR description. -If you need to add new benchmarks to cover your change, please make a separate PR (for example [#9729]) first so we can run the benchmarks with automated runner. +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. [#9729]: https://github.com/apache/arrow-rs/pull/9729 From 5e2a35fc5dccb53048f48a800e87036c5004845c Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 21 Apr 2026 11:13:07 -0400 Subject: [PATCH 3/3] rettier --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8d64e293c61..83ee468c32e5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -230,8 +230,8 @@ 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. - [#9729]: https://github.com/apache/arrow-rs/pull/9729 + ## Git Pre-Commit Hook We can use [git pre-commit hook](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) to automate various kinds of git pre-commit checking/formatting.