Skip to content

Commit 6d1482c

Browse files
authored
Implement --perf flag to lintcheck for benchmarking (#14116)
Turns out I was completely overcomplicating myself, there was no need for an external tool such as becnhv2 or even the original becnh, we already had the benchmarking infrastructure right under our noses! This PR implements a new **lintcheck** option called --perf, using it as a flag will mean that lintcheck builds Clippy as a release package and hooks perf to it. The realization that lintcheck is already 90% of what a benchmarking tool needs came to me in a dream ☁️ changelog:none
2 parents 0d3fd98 + 48ae7ec commit 6d1482c

File tree

2 files changed

+52
-10
lines changed

2 files changed

+52
-10
lines changed

lintcheck/src/config.rs

+9
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use clap::{Parser, Subcommand, ValueEnum};
22
use std::num::NonZero;
33
use std::path::PathBuf;
44

5+
#[allow(clippy::struct_excessive_bools)]
56
#[derive(Parser, Clone, Debug)]
67
#[command(args_conflicts_with_subcommands = true)]
78
pub(crate) struct LintcheckConfig {
@@ -11,6 +12,9 @@ pub(crate) struct LintcheckConfig {
1112
short = 'j',
1213
value_name = "N",
1314
default_value_t = 0,
15+
default_value_if("perf", "true", Some("1")), // Limit jobs to 1 when benchmarking
16+
conflicts_with("perf"),
17+
required = false,
1418
hide_default_value = true
1519
)]
1620
pub max_jobs: usize,
@@ -46,6 +50,11 @@ pub(crate) struct LintcheckConfig {
4650
/// Run clippy on the dependencies of crates specified in crates-toml
4751
#[clap(long, conflicts_with("max_jobs"))]
4852
pub recursive: bool,
53+
/// Also produce a `perf.data` file, implies --jobs=1,
54+
/// the `perf.data` file can be found at
55+
/// `target/lintcheck/sources/<package>-<version>/perf.data`
56+
#[clap(long)]
57+
pub perf: bool,
4958
#[command(subcommand)]
5059
pub subcommand: Option<Commands>,
5160
}

lintcheck/src/main.rs

+43-10
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,25 @@ impl Crate {
116116

117117
clippy_args.extend(lint_levels_args.iter().map(String::as_str));
118118

119-
let mut cmd = Command::new("cargo");
119+
let mut cmd;
120+
121+
if config.perf {
122+
cmd = Command::new("perf");
123+
cmd.args(&[
124+
"record",
125+
"-e",
126+
"instructions", // Only count instructions
127+
"-g", // Enable call-graph, useful for flamegraphs and produces richer reports
128+
"--quiet", // Do not tamper with lintcheck's normal output
129+
"-o",
130+
"perf.data",
131+
"--",
132+
"cargo",
133+
]);
134+
} else {
135+
cmd = Command::new("cargo");
136+
}
137+
120138
cmd.arg(if config.fix { "fix" } else { "check" })
121139
.arg("--quiet")
122140
.current_dir(&self.path)
@@ -234,12 +252,22 @@ fn normalize_diag(
234252
}
235253

236254
/// Builds clippy inside the repo to make sure we have a clippy executable we can use.
237-
fn build_clippy() -> String {
238-
let output = Command::new("cargo")
239-
.args(["run", "--bin=clippy-driver", "--", "--version"])
240-
.stderr(Stdio::inherit())
241-
.output()
242-
.unwrap();
255+
fn build_clippy(release_build: bool) -> String {
256+
let mut build_cmd = Command::new("cargo");
257+
build_cmd.args([
258+
"run",
259+
"--bin=clippy-driver",
260+
if release_build { "-r" } else { "" },
261+
"--",
262+
"--version",
263+
]);
264+
265+
if release_build {
266+
build_cmd.env("CARGO_PROFILE_RELEASE_DEBUG", "true");
267+
}
268+
269+
let output = build_cmd.stderr(Stdio::inherit()).output().unwrap();
270+
243271
if !output.status.success() {
244272
eprintln!("Error: Failed to compile Clippy!");
245273
std::process::exit(1);
@@ -270,13 +298,18 @@ fn main() {
270298

271299
#[allow(clippy::too_many_lines)]
272300
fn lintcheck(config: LintcheckConfig) {
273-
let clippy_ver = build_clippy();
274-
let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap();
301+
let clippy_ver = build_clippy(config.perf);
302+
let clippy_driver_path = fs::canonicalize(format!(
303+
"target/{}/clippy-driver{EXE_SUFFIX}",
304+
if config.perf { "release" } else { "debug" }
305+
))
306+
.unwrap();
275307

276308
// assert that clippy is found
277309
assert!(
278310
clippy_driver_path.is_file(),
279-
"target/debug/clippy-driver binary not found! {}",
311+
"target/{}/clippy-driver binary not found! {}",
312+
if config.perf { "release" } else { "debug" },
280313
clippy_driver_path.display()
281314
);
282315

0 commit comments

Comments
 (0)