Skip to content

Commit

Permalink
Merge pull request #1724 from blyxyas/clippy-timer
Browse files Browse the repository at this point in the history
Integrate Clippy into the project
  • Loading branch information
Kobzol authored Sep 26, 2023
2 parents 1d1400b + 2f6ae0f commit 2a9fade
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 30 deletions.
11 changes: 9 additions & 2 deletions collector/benchlib/src/benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,15 @@ impl<'a> BenchmarkGroup<'a> {

fn profile_benchmark(self, args: ProfileArgs) -> anyhow::Result<()> {
let Some(benchmark) = self.benchmarks.get(args.benchmark.as_str()) else {
return Err(anyhow::anyhow!("Benchmark `{}` not found. Available benchmarks: {}", args.benchmark,
self.benchmarks.keys().map(|s| s.to_string()).collect::<Vec<_>>().join(", ")));
return Err(anyhow::anyhow!(
"Benchmark `{}` not found. Available benchmarks: {}",
args.benchmark,
self.benchmarks
.keys()
.map(|s| s.to_string())
.collect::<Vec<_>>()
.join(", ")
));
};
(benchmark.profile_fn)();

Expand Down
39 changes: 22 additions & 17 deletions collector/src/bin/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use collector::runtime::{
use collector::runtime::{profile_runtime, RuntimeCompilationOpts};
use collector::toolchain::{
create_toolchain_from_published_version, get_local_toolchain, Sysroot, Toolchain,
ToolchainConfig,
};
use collector::utils::cachegrind::cachegrind_diff;
use collector::utils::{is_installed, wait_for_future};
Expand Down Expand Up @@ -375,6 +376,10 @@ struct CompileTimeOptions {
/// The path to the local rustdoc to measure
#[arg(long)]
rustdoc: Option<PathBuf>,

/// The path to the local clippy to measure
#[arg(long)]
clippy: Option<PathBuf>,
}

#[derive(Debug, clap::Args)]
Expand Down Expand Up @@ -660,9 +665,7 @@ fn main_result() -> anyhow::Result<i32> {
let toolchain = get_local_toolchain(
&[Profile::Opt],
rustc,
None,
None,
None,
ToolchainConfig::default(),
id,
target_triple.clone(),
)?;
Expand Down Expand Up @@ -718,9 +721,7 @@ fn main_result() -> anyhow::Result<i32> {
let toolchain = get_local_toolchain(
&[Profile::Opt],
rustc,
None,
None,
None,
ToolchainConfig::default(),
id,
target_triple.clone(),
)?;
Expand Down Expand Up @@ -756,9 +757,11 @@ fn main_result() -> anyhow::Result<i32> {
let toolchain = get_local_toolchain(
&profiles,
&local.rustc,
opts.rustdoc.as_deref(),
local.cargo.as_deref(),
local.id.as_deref(),
*ToolchainConfig::default()
.rustdoc(opts.rustdoc.as_deref())
.clippy(opts.clippy.as_deref())
.cargo(local.cargo.as_deref())
.id(local.id.as_deref()),
"",
target_triple,
)?;
Expand Down Expand Up @@ -948,9 +951,11 @@ fn main_result() -> anyhow::Result<i32> {
let toolchain = get_local_toolchain(
profiles,
rustc,
opts.rustdoc.as_deref(),
local.cargo.as_deref(),
local.id.as_deref(),
*ToolchainConfig::default()
.rustdoc(opts.rustdoc.as_deref())
.clippy(opts.clippy.as_deref())
.cargo(local.cargo.as_deref())
.id(local.id.as_deref()),
suffix,
target_triple.clone(),
)?;
Expand Down Expand Up @@ -1062,9 +1067,9 @@ fn get_local_toolchain_for_runtime_benchmarks(
get_local_toolchain(
&[Profile::Opt],
&local.rustc,
None,
local.cargo.as_deref(),
local.id.as_deref(),
*ToolchainConfig::default()
.cargo(local.cargo.as_deref())
.id(local.id.as_deref()),
"",
target_triple.to_string(),
)
Expand Down Expand Up @@ -1193,9 +1198,9 @@ fn bench_published_artifact(
let artifact_id = ArtifactId::Tag(toolchain.id.clone());

let profiles = if collector::version_supports_doc(&toolchain.id) {
Profile::all()
vec![Profile::Check, Profile::Debug, Profile::Doc, Profile::Opt]
} else {
Profile::all_non_doc()
vec![Profile::Check, Profile::Debug, Profile::Opt]
};
let scenarios = if collector::version_supports_incremental(&toolchain.id) {
Scenario::all()
Expand Down
3 changes: 3 additions & 0 deletions collector/src/bin/rustc-fake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ fn main() {
let mut args = args_os.collect::<Vec<_>>();
let rustc = env::var_os("RUSTC_REAL").unwrap();
let actually_rustdoc = name.ends_with("rustdoc-fake");
let actually_clippy = name.ends_with("clippy-fake");
let tool = if actually_rustdoc {
env::var_os("RUSTDOC_REAL").unwrap()
} else if actually_clippy {
env::var_os("CLIPPY_REAL").unwrap()
} else {
rustc
};
Expand Down
2 changes: 2 additions & 0 deletions collector/src/compile/benchmark/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ pub enum Profile {
Debug,
Doc,
Opt,
Clippy,
}

impl Profile {
pub fn all() -> Vec<Self> {
vec![Profile::Check, Profile::Debug, Profile::Doc, Profile::Opt]
}

// This also leaves Clippy out
pub fn all_non_doc() -> Vec<Self> {
vec![Profile::Check, Profile::Debug, Profile::Opt]
}
Expand Down
1 change: 1 addition & 0 deletions collector/src/compile/execute/bencher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ impl<'a> BenchProcessor<'a> {
Profile::Debug => database::Profile::Debug,
Profile::Doc => database::Profile::Doc,
Profile::Opt => database::Profile::Opt,
Profile::Clippy => database::Profile::Clippy,
};

if let Some(files) = stats.2 {
Expand Down
22 changes: 21 additions & 1 deletion collector/src/compile/execute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl PerfTool {
}
ProfileTool(LlvmLines) => match profile {
Profile::Debug | Profile::Opt => Some("llvm-lines"),
Profile::Check | Profile::Doc => None,
Profile::Check | Profile::Doc | Profile::Clippy => None,
},
}
}
Expand Down Expand Up @@ -168,6 +168,10 @@ impl<'a> CargoProcess<'a> {

if let Some(r) = &self.toolchain.components.rustdoc {
cmd.env("RUSTDOC", &*FAKE_RUSTDOC).env("RUSTDOC_REAL", r);
};

if let Some(c) = &self.toolchain.components.clippy {
cmd.env("CLIPPY", &*FAKE_CLIPPY).env("CLIPPY_REAL", c);
}
cmd
}
Expand Down Expand Up @@ -236,6 +240,7 @@ impl<'a> CargoProcess<'a> {
}
Profile::Debug => {}
Profile::Doc => {}
Profile::Clippy => {}
Profile::Opt => {
cmd.arg("--release");
}
Expand Down Expand Up @@ -354,6 +359,21 @@ lazy_static::lazy_static! {
}
fake_rustdoc
};
static ref FAKE_CLIPPY: PathBuf = {
let mut fake_clippy = env::current_exe().unwrap();
fake_clippy.pop();
fake_clippy.push("clippy-fake");
// link from rustc-fake to rustdoc-fake
if !fake_clippy.exists() {
#[cfg(unix)]
use std::os::unix::fs::symlink;
#[cfg(windows)]
use std::os::windows::fs::symlink_file as symlink;

symlink(&*FAKE_RUSTC, &fake_clippy).expect("failed to make symbolic link");
}
fake_clippy
};
}

/// Used to indicate if we need to retry a run.
Expand Down
82 changes: 72 additions & 10 deletions collector/src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ impl SysrootDownload {
let components = ToolchainComponents::from_binaries_and_libdir(
sysroot_bin("rustc")?,
Some(sysroot_bin("rustdoc")?),
sysroot_bin("cargo-clippy").ok(),
sysroot_bin("cargo")?,
&self.directory.join(&self.rust_sha).join("lib"),
)?;
Expand Down Expand Up @@ -241,6 +242,7 @@ impl Toolchain {
pub struct ToolchainComponents {
pub rustc: PathBuf,
pub rustdoc: Option<PathBuf>,
pub clippy: Option<PathBuf>,
pub cargo: PathBuf,
pub lib_rustc: Option<PathBuf>,
pub lib_std: Option<PathBuf>,
Expand All @@ -252,12 +254,14 @@ impl ToolchainComponents {
fn from_binaries_and_libdir(
rustc: PathBuf,
rustdoc: Option<PathBuf>,
clippy: Option<PathBuf>,
cargo: PathBuf,
libdir: &Path,
) -> anyhow::Result<Self> {
let mut component = ToolchainComponents {
rustc,
rustdoc,
clippy,
cargo,
..Default::default()
};
Expand Down Expand Up @@ -288,6 +292,36 @@ impl ToolchainComponents {
}
}

#[derive(Clone, Copy, Default)]
pub struct ToolchainConfig<'a> {
rustdoc: Option<&'a Path>,
clippy: Option<&'a Path>,
cargo: Option<&'a Path>,
id: Option<&'a str>,
}

impl<'a> ToolchainConfig<'a> {
pub fn rustdoc(&mut self, rustdoc: Option<&'a Path>) -> &mut Self {
self.rustdoc = rustdoc;
self
}

pub fn clippy(&mut self, clippy: Option<&'a Path>) -> &mut Self {
self.clippy = clippy;
self
}

pub fn cargo(&mut self, cargo: Option<&'a Path>) -> &mut Self {
self.cargo = cargo;
self
}

pub fn id(&mut self, id: Option<&'a str>) -> &mut Self {
self.id = id;
self
}
}

/// Get a toolchain from the input.
/// - `rustc`: check if the given one is acceptable.
/// - `rustdoc`: if one is given, check if it is acceptable. Otherwise, if
Expand All @@ -297,9 +331,7 @@ impl ToolchainComponents {
pub fn get_local_toolchain(
profiles: &[Profile],
rustc: &str,
rustdoc: Option<&Path>,
cargo: Option<&Path>,
id: Option<&str>,
toolchain_config: ToolchainConfig<'_>,
id_suffix: &str,
target_triple: String,
) -> anyhow::Result<Toolchain> {
Expand Down Expand Up @@ -354,7 +386,7 @@ pub fn get_local_toolchain(
debug!("found rustc: {:?}", &rustc);

// When the id comes from a +toolchain, the suffix is *not* added.
let id = if let Some(id) = id {
let id = if let Some(id) = toolchain_config.id {
let mut id = id.to_owned();
id.push_str(id_suffix);
id
Expand All @@ -369,7 +401,7 @@ pub fn get_local_toolchain(

// When specifying rustc via a path, the suffix is always added to the
// id.
let mut id = if let Some(id) = id {
let mut id = if let Some(id) = toolchain_config.id {
id.to_owned()
} else {
"Id".to_string()
Expand All @@ -380,7 +412,7 @@ pub fn get_local_toolchain(
};

let rustdoc =
if let Some(rustdoc) = &rustdoc {
if let Some(rustdoc) = &toolchain_config.rustdoc {
Some(rustdoc.canonicalize().with_context(|| {
format!("failed to canonicalize rustdoc executable {:?}", rustdoc)
})?)
Expand All @@ -400,7 +432,28 @@ pub fn get_local_toolchain(
None
};

let cargo = if let Some(cargo) = &cargo {
let clippy = if let Some(clippy) = &toolchain_config.clippy {
Some(
clippy.canonicalize().with_context(|| {
format!("failed to canonicalize clippy executable {:?}", clippy)
})?,
)
} else if profiles.contains(&Profile::Clippy) {
// We need a `clippy`. Look for one next to `rustc`.
if let Ok(clippy) = rustc.with_file_name("cargo-clippy").canonicalize() {
debug!("found clippy: {:?}", &clippy);
Some(clippy)
} else {
anyhow::bail!(
"'Clippy' build specified but '--cargo-clippy' not specified and no 'cargo-clippy' found \
next to 'rustc'"
);
}
} else {
// No `clippy` provided, but none needed.
None
};
let cargo = if let Some(cargo) = &toolchain_config.cargo {
cargo
.canonicalize()
.with_context(|| format!("failed to canonicalize cargo executable {:?}", cargo))?
Expand Down Expand Up @@ -428,7 +481,9 @@ pub fn get_local_toolchain(
let lib_dir = get_lib_dir_from_rustc(&rustc).context("Cannot find libdir for rustc")?;

Ok(Toolchain {
components: ToolchainComponents::from_binaries_and_libdir(rustc, rustdoc, cargo, &lib_dir)?,
components: ToolchainComponents::from_binaries_and_libdir(
rustc, rustdoc, clippy, cargo, &lib_dir,
)?,
id,
triple: target_triple,
})
Expand Down Expand Up @@ -465,16 +520,23 @@ pub fn create_toolchain_from_published_version(
};
let rustc = which("rustc")?;
let rustdoc = which("rustdoc")?;
let clippy = which("clippy")?;
let cargo = which("cargo")?;

debug!("Found rustc: {}", rustc.display());
debug!("Found rustdoc: {}", rustdoc.display());
debug!("Found clippy: {}", clippy.display());
debug!("Found cargo: {}", cargo.display());

let lib_dir = get_lib_dir_from_rustc(&rustc)?;

let components =
ToolchainComponents::from_binaries_and_libdir(rustc, Some(rustdoc), cargo, &lib_dir)?;
let components = ToolchainComponents::from_binaries_and_libdir(
rustc,
Some(rustdoc),
Some(clippy),
cargo,
&lib_dir,
)?;

Ok(Toolchain {
components,
Expand Down
4 changes: 4 additions & 0 deletions database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ pub enum Profile {
Doc,
/// An optimized "release" build
Opt,
/// A Clippy run
Clippy,
}

impl Profile {
Expand All @@ -233,6 +235,7 @@ impl Profile {
Profile::Opt => "opt",
Profile::Debug => "debug",
Profile::Doc => "doc",
Profile::Clippy => "clippy",
}
}
}
Expand All @@ -245,6 +248,7 @@ impl std::str::FromStr for Profile {
"debug" => Profile::Debug,
"doc" => Profile::Doc,
"opt" => Profile::Opt,
"clippy" => Profile::Clippy,
_ => return Err(format!("{} is not a profile", s)),
})
}
Expand Down
Loading

0 comments on commit 2a9fade

Please sign in to comment.