Skip to content

Document and improve (a lot) lintcheck --perf #14194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
- [Updating the Changelog](development/infrastructure/changelog_update.md)
- [Release a New Version](development/infrastructure/release.md)
- [The Clippy Book](development/infrastructure/book.md)
- [Benchmarking Clippy](development/infrastructure/benchmarking.md)
- [Proposals](development/proposals/README.md)
- [Roadmap 2021](development/proposals/roadmap-2021.md)
- [Syntax Tree Patterns](development/proposals/syntax-tree-patterns.md)
Expand Down
55 changes: 55 additions & 0 deletions book/src/development/infrastructure/benchmarking.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Benchmarking Clippy

Benchmarking Clippy is similar to using our Lintcheck tool, in fact, it even
uses the same tool! Just by adding a `--perf` flag it will transform Lintcheck
into a very simple but powerful benchmarking tool!

It requires having the [`perf` tool][perf] installed, as `perf` is what's actually
profiling Clippy under the hood.

The lintcheck `--perf` tool generates a series of `perf.data` in the
`target/lintcheck/sources/<package>-<version>` directories. Each `perf.data`
corresponds to the package which is contained.

Lintcheck uses the `-g` flag, meaning that you can get stack traces for richer
analysis, including with tools such as [flamegraph][flamegraph-perf]
(or [`flamegraph-rs`][flamegraph-rs]).

Currently, we only measure instruction count, as it's the most reproducible metric
and [rustc-perf][rustc-perf] also considers it the main number to focus on.

## Benchmarking a PR

Having a benchmarking tool directly implemented into lintcheck gives us the
ability to benchmark any given PR just by making a before and after

Here's the way you can get into any PR, benchmark it, and then benchmark
`master`.

The first `perf.data` will not have any numbers appended, but any subsequent
benchmark will be written to `perf.data.number` with a number growing for 0.
All benchmarks are compressed so that you can
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the end of this sentence is missing! :)


```bash
git fetch upstream pull/<PR_NUMBER>/head:<BRANCH_NAME>
git switch BRANCHNAME

# Bench
cargo lintcheck --perf

# Get last common commit, checkout that
LAST_COMMIT=$(git log BRANCHNAME..master --oneline | tail -1 | cut -c 1-11)
git switch -c temporary $LAST_COMMIT

# We're now on master

# Bench
cargo lintcheck --perf
perf diff ./target/lintcheck/sources/CRATE/perf.data ./target/lintcheck/sources/CRATE/perf.data.0
```


[perf]: https://perfwiki.github.io/main/
[flamegraph-perf]: https://github.com/brendangregg/FlameGraph
[flamegraph-rs]: https://github.com/flamegraph-rs/flamegraph
[rustc-perf]: https://github.com/rust-lang/rustc-perf
41 changes: 39 additions & 2 deletions lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,17 @@ impl Crate {

if config.perf {
cmd = Command::new("perf");
let perf_data_filename = get_perf_data_filename(&self.path);
cmd.args(&[
"record",
"-e",
"instructions", // Only count instructions
"-g", // Enable call-graph, useful for flamegraphs and produces richer reports
"--quiet", // Do not tamper with lintcheck's normal output
"--compression-level=22",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not slow down the capture? zstd 22 is pretty slow

"--freq=3000", // Slow down program to capture all events
"-o",
"perf.data",
&perf_data_filename,
"--",
"cargo",
]);
Expand Down Expand Up @@ -165,7 +168,7 @@ impl Crate {
return Vec::new();
}

if !config.fix {
if !config.fix && !config.perf {
cmd.arg("--message-format=json");
}

Expand Down Expand Up @@ -203,6 +206,11 @@ impl Crate {
return Vec::new();
}

// We don't want to keep target directories if benchmarking
if config.perf {
let _ = fs::remove_dir_all(&shared_target_dir);
}

// get all clippy warnings and ICEs
let mut entries: Vec<ClippyCheckOutput> = Message::parse_stream(stdout.as_bytes())
.filter_map(|msg| match msg {
Expand Down Expand Up @@ -441,6 +449,35 @@ fn lintcheck(config: LintcheckConfig) {
fs::write(&config.lintcheck_results_path, text).unwrap();
}

/// Traverse a directory looking for `perf.data.<number>` files, and adds one
/// to the most recent of those files, returning the new most recent `perf.data`
/// file name.
fn get_perf_data_filename(source_path: &Path) -> String {
if source_path.join("perf.data").exists() {
let mut max_number = 0;
fs::read_dir(source_path)
.unwrap()
.filter_map(Result::ok)
.filter(|path| {
path.file_name()
.as_os_str()
.to_string_lossy() // We don't care about data loss, as we're checking for equality
.starts_with("perf.data")
})
.for_each(|path| {
let file_name = path.file_name();
let file_name = file_name.as_os_str().to_str().unwrap().split('.').next_back().unwrap();
if let Ok(parsed_file_name) = file_name.parse::<usize>() {
if parsed_file_name >= max_number {
max_number = parsed_file_name + 1;
}
}
});
return format!("perf.data.{max_number}");
}
String::from("perf.data")
}

/// Returns the path to the Clippy project directory
#[must_use]
fn clippy_project_root() -> &'static Path {
Expand Down