Skip to content

Commit 6905a48

Browse files
committed
Add docs on how to add/remove/update a benchmark.
This is an improved version of the docs in https://hackmd.io/d9uE7qgtTWKDLivy0uoVQw.
1 parent e269f50 commit 6905a48

File tree

2 files changed

+82
-1
lines changed

2 files changed

+82
-1
lines changed

collector/benchmarks/README.md

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,84 @@ Rust code being written today.
147147
- **syn**: See above. This is an older version (0.11.11) of the crate.
148148
- **tokio-webpush-simple**: A simple web server built with a very old version
149149
of tokio. Uses futures a lot, but doesn't use `async`/`await`.
150+
151+
# How to update/add/remove benchmarks
152+
153+
## Add a new benchmark
154+
155+
- Decide on which category it belongs to.
156+
- If it's a third-party crate:
157+
- If you are keen: talk with a maintainer of the crate to see if there is
158+
anything we should be aware of when using this crate as a compile-time
159+
benchmark.
160+
- Look at [crates.io](https://crates.io) to find the latest (non-prerelease) version.
161+
- Download it with `collector download -c $CATEGORY crate $NAME $VERSION`.
162+
The `$CATEGORY` is probably `primary`.
163+
- It makes it easier for reviewers if you split things into two commits.
164+
- In the first commit, just add the new code.
165+
- Do this by doing `git add` on the new directory.
166+
- In the second commit, do everything else.
167+
- Add `[workspace]` to the very bottom of the benchmark's `Cargo.toml`, if
168+
doesn't already have a `[workspace]` section. This means commands like
169+
`cargo build` will work within the benchmark directory.
170+
- Add any necessary stuff to the `perf-config.json` file.
171+
- If the benchmark is a sub-crate within a top-level crate, you'll need a
172+
`"cargo_toml"` entry.
173+
- If you get a "non-wrapped rustc" error when running it, you'll need a
174+
`"touch_file"` entry.
175+
- Consider adding one or more `N-*.patch` files.
176+
- If it's a primary benchmark, you should definitely do this.
177+
- Creating the diff against what you've committed so far might be useful.
178+
Use `git diff` from the repository root, or `git diff --relative` within
179+
the benchmark directory. Copy the output into the `N-*.patch` file.
180+
- Do a test run with an `IncrPatched` scenario to make sure the patch
181+
applies correctly.
182+
- `git grep` for occurrences of the old benchmark name (e.g. in
183+
`.github/workflows/ci.yml` or `ci/check-*.sh`) and see if anything similar
184+
needs doing for the new benchmark... usually not.
185+
- Add the new entry to `collector/benchmarks/README.md`. Don't remove the
186+
entry for the old benchmark yet.
187+
- Compare benchmarking time for the old and new versions of the benchmark.
188+
(It's useful to know if the new one is a lot slower than the old one.)
189+
- First, consider the entire compilation time with something like this, by
190+
doing this within the benchmark directory is good:
191+
```
192+
CARGO_INCREMENTAL=0 cargo check ; \rm -rf target
193+
CARGO_INCREMENTAL=0 cargo build ; \rm -rf target
194+
CARGO_INCREMENTAL=0 cargo build --release ; \rm -rf target
195+
```
196+
Put this info in the commit message.
197+
- Second, compare the final crate time with these commands:
198+
```
199+
target/release/collector bench_local +nightly --id Test \
200+
--profiles=Check,Debug,Opt --scenarios=Full --include=$OLD,$NEW,helloworld
201+
target/release/site results.db
202+
```
203+
Then switch to wall-times, compare `Test` against itself, and toggle the
204+
"Show non-relevant results"/"Display raw data" check boxes to make sure it
205+
hasn't changed drastically.
206+
- E.g. `futures` was changed so it's just a facade for a bunch of
207+
sub-crates, and the final crate time became very similar to `helloworld`,
208+
which wasn't interesting.
209+
- File a PR.
210+
211+
## Remove a benchmark
212+
213+
- It makes it easier for reviewers if you split things into two commits.
214+
- In the first commit just remove the old code.
215+
- Do this with `git rm -r` on the directory.
216+
- In the second commit do everything else.
217+
- Remove the entry from `collector/benchmarks/README.md`.
218+
- `git grep` for occurrences of the old benchmark name (e.g. in
219+
`.github/workflows/ci.yml` or `ci/check-*.sh`) and see if anything needs
220+
changing... usually not.
221+
- File a PR.
222+
223+
## Update a benchmark
224+
225+
- Do this in two steps. First add the new version of the benchmark. Once the PR
226+
is merged, make sure it's running correctly. Second, remove the old version
227+
of the benchmark. Doing it in two steps ensures we have continuity of
228+
profiling coverage in the case where something goes wrong.
229+
- When adding the new version, for `perf-config.json` and the `N-*.patch`
230+
files, use the corresponding files for the old version as a starting point.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
{
22
"touch_file": "src/lib.rs",
33
"category": "stable"
4-
}
4+
}

0 commit comments

Comments
 (0)