This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 5255de4bac docs: Update contributing guidelines with benchmark results
(#9782)
5255de4bac is described below
commit 5255de4bacd2131c3e3fde5cbeba008fc835d6b3
Author: Andrew Lamb <[email protected]>
AuthorDate: Wed Apr 29 16:44:21 2026 -0400
docs: Update contributing guidelines with benchmark results (#9782)
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax.
-->
- Closes #NNN.
# Rationale for this change
As we get more PRs with performance improvements, I think it is
important to make sure our expectations are documented -- namely that
any PR for improving performance includes benchmark results
# What changes are included in this PR?
1. Update contributing guidelines
2. Add a reminder in the PR template
# Are these changes tested?
N/a
# Are there any user-facing changes?
New docs
---
.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 c2d07f49ab..7daafd5b38 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 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 a375917e3a..83ee468c32 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 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.
```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, 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.
