Skip to content

Move the rest of 'build_kind' to 'profile_kind' #1059

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.

Sign up for GitHub

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 1 commit into from
Oct 6, 2021
Merged

Move the rest of 'build_kind' to 'profile_kind' #1059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
89 changes: 43 additions & 46 deletions collector/src/execute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl Profiler {

// What cargo subcommand do we need to run for this profiler? If not
// `rustc`, must be a subcommand that itself invokes `rustc`.
fn subcommand(&self, build_kind: ProfileKind) -> Option<&'static str> {
fn subcommand(&self, profile_kind: ProfileKind) -> Option<&'static str> {
match self {
Profiler::PerfStat
| Profiler::PerfStatSelfProfile
Expand All @@ -247,13 +247,13 @@ impl Profiler {
| Profiler::DepGraph
| Profiler::MonoItems
| Profiler::Eprintln => {
if build_kind == ProfileKind::Doc {
if profile_kind == ProfileKind::Doc {
Some("rustdoc")
} else {
Some("rustc")
}
}
Profiler::LlvmLines => match build_kind {
Profiler::LlvmLines => match profile_kind {
ProfileKind::Debug | ProfileKind::Opt => Some("llvm-lines"),
ProfileKind::Check | ProfileKind::Doc => None,
},
Expand Down Expand Up @@ -286,7 +286,7 @@ impl Profiler {
struct CargoProcess<'a> {
compiler: Compiler<'a>,
cwd: &'a Path,
build_kind: ProfileKind,
profile_kind: ProfileKind,
incremental: bool,
processor_etc: Option<(
&'a mut dyn Processor,
Expand Down Expand Up @@ -361,9 +361,9 @@ impl<'a> CargoProcess<'a> {
// really.
fn run_rustc(&mut self, needs_final: bool) -> anyhow::Result<()> {
log::info!(
"run_rustc with incremental={}, build_kind={:?}, scenario_kind={:?}, patch={:?}",
"run_rustc with incremental={}, profile_kind={:?}, scenario_kind={:?}, patch={:?}",
self.incremental,
self.build_kind,
self.profile_kind,
self.processor_etc.as_ref().map(|v| v.1),
self.processor_etc.as_ref().and_then(|v| v.3)
);
Expand All @@ -374,33 +374,33 @@ impl<'a> CargoProcess<'a> {
// machinery works).
let subcommand =
if let Some((ref mut processor, scenario_kind, ..)) = self.processor_etc {
let profiler = processor.profiler(self.build_kind);
let profiler = processor.profiler(self.profile_kind);
if !profiler.is_scenario_kind_allowed(scenario_kind) {
return Err(anyhow::anyhow!(
"this profiler doesn't support {:?} scenarios",
scenario_kind
));
}

match profiler.subcommand(self.build_kind) {
match profiler.subcommand(self.profile_kind) {
None => {
return Err(anyhow::anyhow!(
"this profiler doesn't support {:?} builds",
self.build_kind
"this profiler doesn't support the {:?} profile",
self.profile_kind
))
}
Some(sub) => sub,
}
} else {
match self.build_kind {
match self.profile_kind {
ProfileKind::Doc => "rustdoc",
_ => "rustc",
}
};

let mut cmd = self.base_command(self.cwd, subcommand);
cmd.arg("-p").arg(self.get_pkgid(self.cwd)?);
match self.build_kind {
match self.profile_kind {
ProfileKind::Check => {
cmd.arg("--profile").arg("check");
}
Expand Down Expand Up @@ -428,7 +428,7 @@ impl<'a> CargoProcess<'a> {
.as_mut()
.map(|v| &mut v.0)
.expect("needs_final needs a processor");
let profiler = processor.profiler(self.build_kind).name();
let profiler = processor.profiler(self.profile_kind).name();
// If we're using a processor, we expect that only the crate
// we're interested in benchmarking will be built, not any
// dependencies.
Expand Down Expand Up @@ -484,7 +484,7 @@ impl<'a> CargoProcess<'a> {
let data = ProcessOutputData {
name: self.processor_name.clone(),
cwd: self.cwd,
build_kind: self.build_kind,
profile_kind: self.profile_kind,
scenario_kind,
scenario_kind_str,
patch,
Expand Down Expand Up @@ -534,7 +534,7 @@ pub enum Retry {
pub struct ProcessOutputData<'a> {
name: BenchmarkName,
cwd: &'a Path,
build_kind: ProfileKind,
profile_kind: ProfileKind,
scenario_kind: ScenarioKind,
scenario_kind_str: &'a str,
patch: Option<&'a Patch>,
Expand Down Expand Up @@ -626,7 +626,7 @@ impl<'a> MeasureProcessor<'a> {
fn insert_stats(
&mut self,
scenario: database::Scenario,
build_kind: ProfileKind,
profile_kind: ProfileKind,
stats: (Stats, Option<SelfProfile>, Option<SelfProfileFiles>),
) {
let version = String::from_utf8(
Expand All @@ -642,7 +642,7 @@ impl<'a> MeasureProcessor<'a> {
.unwrap();

let collection = self.rt.block_on(self.conn.collection_id(&version));
let profile = match build_kind {
let profile = match profile_kind {
ProfileKind::Check => database::Profile::Check,
ProfileKind::Debug => database::Profile::Debug,
ProfileKind::Doc => database::Profile::Doc,
Expand Down Expand Up @@ -806,7 +806,7 @@ impl Upload {
}

impl<'a> Processor for MeasureProcessor<'a> {
fn profiler(&self, _build: ProfileKind) -> Profiler {
fn profiler(&self, _profile: ProfileKind) -> Profiler {
if self.is_first_collection && self.is_self_profile {
if cfg!(unix) {
Profiler::PerfStatSelfProfile
Expand All @@ -826,11 +826,11 @@ impl<'a> Processor for MeasureProcessor<'a> {
self.is_first_collection = true;
}

fn finished_first_collection(&mut self, build: ProfileKind) -> bool {
let original = self.profiler(build);
fn finished_first_collection(&mut self, profile: ProfileKind) -> bool {
let original = self.profiler(profile);
self.is_first_collection = false;
// We need to run again if we're going to use a different profiler
self.profiler(build) != original
self.profiler(profile) != original
}

fn process_output(
Expand All @@ -842,27 +842,27 @@ impl<'a> Processor for MeasureProcessor<'a> {
Ok(res) => {
match data.scenario_kind {
ScenarioKind::Full => {
self.insert_stats(database::Scenario::Empty, data.build_kind, res);
self.insert_stats(database::Scenario::Empty, data.profile_kind, res);
}
ScenarioKind::IncrFull => {
self.insert_stats(
database::Scenario::IncrementalEmpty,
data.build_kind,
data.profile_kind,
res,
);
}
ScenarioKind::IncrUnchanged => {
self.insert_stats(
database::Scenario::IncrementalFresh,
data.build_kind,
data.profile_kind,
res,
);
}
ScenarioKind::IncrPatched => {
let patch = data.patch.unwrap();
self.insert_stats(
database::Scenario::IncrementalPatch(patch.name),
data.build_kind,
data.profile_kind,
res,
);
}
Expand Down Expand Up @@ -934,7 +934,7 @@ impl<'a> Processor for ProfileProcessor<'a> {
let out_file = |prefix: &str| -> String {
format!(
"{}-{}-{}-{:?}-{}",
prefix, self.id, data.name, data.build_kind, data.scenario_kind_str
prefix, self.id, data.name, data.profile_kind, data.scenario_kind_str
)
};

Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl Benchmark {
&'a self,
compiler: Compiler<'a>,
cwd: &'a Path,
build_kind: ProfileKind,
profile_kind: ProfileKind,
) -> CargoProcess<'a> {
let mut cargo_args = self
.config
Expand All @@ -1313,7 +1313,7 @@ impl Benchmark {
compiler,
processor_name: self.name.clone(),
cwd,
build_kind,
profile_kind,
incremental: false,
processor_etc: None,
manifest_path: self
Expand All @@ -1339,7 +1339,7 @@ impl Benchmark {
pub fn measure(
&self,
processor: &mut dyn Processor,
build_kinds: &[ProfileKind],
profile_kinds: &[ProfileKind],
scenario_kinds: &[ScenarioKind],
compiler: Compiler<'_>,
iterations: Option<usize>,
Expand All @@ -1350,13 +1350,13 @@ impl Benchmark {
return processor.measure_rustc(compiler).context("measure rustc");
}

if self.config.disabled || build_kinds.is_empty() {
if self.config.disabled || profile_kinds.is_empty() {
eprintln!("Skipping {}: disabled", self.name);
bail!("disabled benchmark");
}

eprintln!("Preparing {}", self.name);
let build_kind_dirs = build_kinds
let profile_kind_dirs = profile_kinds
.iter()
.map(|kind| Ok((*kind, self.make_temp_dir(&self.path)?)))
.collect::<anyhow::Result<Vec<_>>>()?;
Expand Down Expand Up @@ -1385,10 +1385,10 @@ impl Benchmark {
// target-directory global lock during compilation.
crossbeam_utils::thread::scope::<_, anyhow::Result<()>>(|s| {
let server = jobserver::Client::new(num_cpus::get()).context("jobserver::new")?;
for (build_kind, prep_dir) in &build_kind_dirs {
for (profile_kind, prep_dir) in &profile_kind_dirs {
let server = server.clone();
s.spawn::<_, anyhow::Result<()>>(move |_| {
self.mk_cargo_process(compiler, prep_dir.path(), *build_kind)
self.mk_cargo_process(compiler, prep_dir.path(), *profile_kind)
.jobserver(server)
.run_rustc(false)?;
Ok(())
Expand All @@ -1398,18 +1398,18 @@ impl Benchmark {
})
.unwrap()?;

for (build_kind, prep_dir) in build_kind_dirs {
for (profile_kind, prep_dir) in profile_kind_dirs {
eprintln!(
"Running {}: {:?} + {:?}",
self.name, build_kind, scenario_kinds
self.name, profile_kind, scenario_kinds
);

// We want at least two runs for all benchmarks (since we run
// self-profile separately).
processor.start_first_collection();
for i in 0..cmp::max(iterations, 2) {
if i == 1 {
let different = processor.finished_first_collection(build_kind);
let different = processor.finished_first_collection(profile_kind);
if iterations == 1 && !different {
// Don't run twice if this processor doesn't need it and
// we've only been asked to run once.
Expand All @@ -1423,28 +1423,25 @@ impl Benchmark {

// A full non-incremental build.
if scenario_kinds.contains(&ScenarioKind::Full) {
self.mk_cargo_process(compiler, cwd, build_kind)
self.mk_cargo_process(compiler, cwd, profile_kind)
.processor(processor, ScenarioKind::Full, "Full", None)
.run_rustc(true)?;
}

// Rustdoc does not support incremental compilation
if build_kind != ProfileKind::Doc {
// An incremental build from scratch (slowest incremental case).
if profile_kind != ProfileKind::Doc {
// An incremental from scratch (slowest incremental case).
// This is required for any subsequent incremental builds.
if scenario_kinds.contains(&ScenarioKind::IncrFull)
|| scenario_kinds.contains(&ScenarioKind::IncrUnchanged)
|| scenario_kinds.contains(&ScenarioKind::IncrPatched)
{
self.mk_cargo_process(compiler, cwd, build_kind)
if scenario_kinds.iter().any(|s| s.is_incr()) {
self.mk_cargo_process(compiler, cwd, profile_kind)
.incremental(true)
.processor(processor, ScenarioKind::IncrFull, "IncrFull", None)
.run_rustc(true)?;
}

// An incremental build with no changes (fastest incremental case).
if scenario_kinds.contains(&ScenarioKind::IncrUnchanged) {
self.mk_cargo_process(compiler, cwd, build_kind)
self.mk_cargo_process(compiler, cwd, profile_kind)
.incremental(true)
.processor(
processor,
Expand All @@ -1463,7 +1460,7 @@ impl Benchmark {
// An incremental build with some changes (realistic
// incremental case).
let scenario_kind_str = format!("IncrPatched{}", i);
self.mk_cargo_process(compiler, cwd, build_kind)
self.mk_cargo_process(compiler, cwd, profile_kind)
.incremental(true)
.processor(
processor,
Expand Down
7 changes: 3 additions & 4 deletions collector/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ pub fn robocopy(
}

fn run_command_with_output(cmd: &mut Command) -> anyhow::Result<process::Output> {
log::trace!("running: {:?}", cmd);
let mut child = cmd.stdout(Stdio::piped()).stderr(Stdio::piped()).spawn()?;

let mut stdout = Vec::new();
Expand Down Expand Up @@ -218,8 +217,8 @@ fn run_command_with_output(cmd: &mut Command) -> anyhow::Result<process::Output>

Ok(process::Output {
status,
stdout: stdout,
stderr: stderr,
stdout,
stderr,
})
}

Expand All @@ -228,7 +227,7 @@ pub fn command_output(cmd: &mut Command) -> anyhow::Result<process::Output> {

if !output.status.success() {
return Err(anyhow::anyhow!(
"expected success, got {}\n\nstderr={}\n\n stdout={}",
"expected success, got {}\n\nstderr={}\n\n stdout={}\n",
output.status,
String::from_utf8_lossy(&output.stderr),
String::from_utf8_lossy(&output.stdout)
Expand Down
Loading