Skip to content

Commit c033a4c

Browse files
authored
Document and improve (a lot) lintcheck --perf (#14194)
In #14116 we added a benchmarking option for Lintcheck, this commit adds a new chapter to the book AND improves that option into a more usable state. It's recommended to review one commit at a time. - **Document how to benchmark with lintcheck --perf** - **Several improvements on lintcheck perf (desc.)** - Now lintcheck perf deletes target directory after benchmarking, benchmarking with a cache isn't very useful or telling of any precise outcome. - Support for benchmarking several times without having to do a cargo clean. - Compress perf.data changelog: none
2 parents 6166f60 + 17351bd commit c033a4c

File tree

3 files changed

+95
-2
lines changed

3 files changed

+95
-2
lines changed

book/src/SUMMARY.md

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
- [Updating the Changelog](development/infrastructure/changelog_update.md)
3131
- [Release a New Version](development/infrastructure/release.md)
3232
- [The Clippy Book](development/infrastructure/book.md)
33+
- [Benchmarking Clippy](development/infrastructure/benchmarking.md)
3334
- [Proposals](development/proposals/README.md)
3435
- [Roadmap 2021](development/proposals/roadmap-2021.md)
3536
- [Syntax Tree Patterns](development/proposals/syntax-tree-patterns.md)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Benchmarking Clippy
2+
3+
Benchmarking Clippy is similar to using our Lintcheck tool, in fact, it even
4+
uses the same tool! Just by adding a `--perf` flag it will transform Lintcheck
5+
into a very simple but powerful benchmarking tool!
6+
7+
It requires having the [`perf` tool][perf] installed, as `perf` is what's actually
8+
profiling Clippy under the hood.
9+
10+
The lintcheck `--perf` tool generates a series of `perf.data` in the
11+
`target/lintcheck/sources/<package>-<version>` directories. Each `perf.data`
12+
corresponds to the package which is contained.
13+
14+
Lintcheck uses the `-g` flag, meaning that you can get stack traces for richer
15+
analysis, including with tools such as [flamegraph][flamegraph-perf]
16+
(or [`flamegraph-rs`][flamegraph-rs]).
17+
18+
Currently, we only measure instruction count, as it's the most reproducible metric
19+
and [rustc-perf][rustc-perf] also considers it the main number to focus on.
20+
21+
## Benchmarking a PR
22+
23+
Having a benchmarking tool directly implemented into lintcheck gives us the
24+
ability to benchmark any given PR just by making a before and after
25+
26+
Here's the way you can get into any PR, benchmark it, and then benchmark
27+
`master`.
28+
29+
The first `perf.data` will not have any numbers appended, but any subsequent
30+
benchmark will be written to `perf.data.number` with a number growing for 0.
31+
All benchmarks are compressed so that you can
32+
33+
```bash
34+
git fetch upstream pull/<PR_NUMBER>/head:<BRANCH_NAME>
35+
git switch BRANCHNAME
36+
37+
# Bench
38+
cargo lintcheck --perf
39+
40+
# Get last common commit, checkout that
41+
LAST_COMMIT=$(git log BRANCHNAME..master --oneline | tail -1 | cut -c 1-11)
42+
git switch -c temporary $LAST_COMMIT
43+
44+
# We're now on master
45+
46+
# Bench
47+
cargo lintcheck --perf
48+
perf diff ./target/lintcheck/sources/CRATE/perf.data ./target/lintcheck/sources/CRATE/perf.data.0
49+
```
50+
51+
52+
[perf]: https://perfwiki.github.io/main/
53+
[flamegraph-perf]: https://github.com/brendangregg/FlameGraph
54+
[flamegraph-rs]: https://github.com/flamegraph-rs/flamegraph
55+
[rustc-perf]: https://github.com/rust-lang/rustc-perf

lintcheck/src/main.rs

+39-2
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,17 @@ impl Crate {
120120

121121
if config.perf {
122122
cmd = Command::new("perf");
123+
let perf_data_filename = get_perf_data_filename(&self.path);
123124
cmd.args(&[
124125
"record",
125126
"-e",
126127
"instructions", // Only count instructions
127128
"-g", // Enable call-graph, useful for flamegraphs and produces richer reports
128129
"--quiet", // Do not tamper with lintcheck's normal output
130+
"--compression-level=22",
131+
"--freq=3000", // Slow down program to capture all events
129132
"-o",
130-
"perf.data",
133+
&perf_data_filename,
131134
"--",
132135
"cargo",
133136
]);
@@ -165,7 +168,7 @@ impl Crate {
165168
return Vec::new();
166169
}
167170

168-
if !config.fix {
171+
if !config.fix && !config.perf {
169172
cmd.arg("--message-format=json");
170173
}
171174

@@ -203,6 +206,11 @@ impl Crate {
203206
return Vec::new();
204207
}
205208

209+
// We don't want to keep target directories if benchmarking
210+
if config.perf {
211+
let _ = fs::remove_dir_all(&shared_target_dir);
212+
}
213+
206214
// get all clippy warnings and ICEs
207215
let mut entries: Vec<ClippyCheckOutput> = Message::parse_stream(stdout.as_bytes())
208216
.filter_map(|msg| match msg {
@@ -441,6 +449,35 @@ fn lintcheck(config: LintcheckConfig) {
441449
fs::write(&config.lintcheck_results_path, text).unwrap();
442450
}
443451

452+
/// Traverse a directory looking for `perf.data.<number>` files, and adds one
453+
/// to the most recent of those files, returning the new most recent `perf.data`
454+
/// file name.
455+
fn get_perf_data_filename(source_path: &Path) -> String {
456+
if source_path.join("perf.data").exists() {
457+
let mut max_number = 0;
458+
fs::read_dir(source_path)
459+
.unwrap()
460+
.filter_map(Result::ok)
461+
.filter(|path| {
462+
path.file_name()
463+
.as_os_str()
464+
.to_string_lossy() // We don't care about data loss, as we're checking for equality
465+
.starts_with("perf.data")
466+
})
467+
.for_each(|path| {
468+
let file_name = path.file_name();
469+
let file_name = file_name.as_os_str().to_str().unwrap().split('.').next_back().unwrap();
470+
if let Ok(parsed_file_name) = file_name.parse::<usize>() {
471+
if parsed_file_name >= max_number {
472+
max_number = parsed_file_name + 1;
473+
}
474+
}
475+
});
476+
return format!("perf.data.{max_number}");
477+
}
478+
String::from("perf.data")
479+
}
480+
444481
/// Returns the path to the Clippy project directory
445482
#[must_use]
446483
fn clippy_project_root() -> &'static Path {

0 commit comments

Comments
 (0)