Skip to content

Use term 'profile' instead of 'build' as that's what we're using elsewhere #1055

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 2 commits into from
Oct 5, 2021
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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
"--exclude webrender-wrench,style-servo,cargo,rustc",
"--include webrender-wrench,style-servo,cargo --exclude rustc",
]
BUILD_KINDS: [
PROFILE_KINDS: [
"Check,Doc,Debug",
"Opt",
]
Expand Down Expand Up @@ -98,7 +98,7 @@ jobs:
env:
JEMALLOC_OVERRIDE: /usr/lib/x86_64-linux-gnu/libjemalloc.so
BENCH_INCLUDE_EXCLUDE_OPTS: ${{ matrix.BENCH_INCLUDE_EXCLUDE_OPTS }}
BUILD_KINDS: ${{ matrix.BUILD_KINDS }}
PROFILE_KINDS: ${{ matrix.PROFILE_KINDS }}
SHELL: "/bin/bash"

test_profiling:
Expand Down
2 changes: 1 addition & 1 deletion ci/check-benchmarks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ RUST_BACKTRACE=1 \
RUST_LOG=raw_cargo_messages=trace,collector=debug,rust_sysroot=debug \
cargo run -p collector --bin collector -- \
bench_local $bindir/rustc Test \
--builds $BUILD_KINDS \
--builds $PROFILE_KINDS \
--cargo $bindir/cargo \
--runs All \
--rustdoc $bindir/rustdoc \
Expand Down
50 changes: 25 additions & 25 deletions collector/src/execute.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Execute benchmarks.

use crate::{BuildKind, Compiler, ScenarioKind};
use crate::{Compiler, ProfileKind, ScenarioKind};
use anyhow::{anyhow, bail, Context};
use collector::command_output;
use collector::etw_parser;
Expand Down 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: BuildKind) -> Option<&'static str> {
fn subcommand(&self, build_kind: ProfileKind) -> Option<&'static str> {
match self {
Profiler::PerfStat
| Profiler::PerfStatSelfProfile
Expand All @@ -247,15 +247,15 @@ impl Profiler {
| Profiler::DepGraph
| Profiler::MonoItems
| Profiler::Eprintln => {
if build_kind == BuildKind::Doc {
if build_kind == ProfileKind::Doc {
Some("rustdoc")
} else {
Some("rustc")
}
}
Profiler::LlvmLines => match build_kind {
BuildKind::Debug | BuildKind::Opt => Some("llvm-lines"),
BuildKind::Check | BuildKind::Doc => None,
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: BuildKind,
build_kind: ProfileKind,
incremental: bool,
processor_etc: Option<(
&'a mut dyn Processor,
Expand Down Expand Up @@ -393,20 +393,20 @@ impl<'a> CargoProcess<'a> {
}
} else {
match self.build_kind {
BuildKind::Doc => "rustdoc",
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 {
BuildKind::Check => {
ProfileKind::Check => {
cmd.arg("--profile").arg("check");
}
BuildKind::Debug => {}
BuildKind::Doc => {}
BuildKind::Opt => {
ProfileKind::Debug => {}
ProfileKind::Doc => {}
ProfileKind::Opt => {
cmd.arg("--release");
}
}
Expand Down Expand Up @@ -534,7 +534,7 @@ pub enum Retry {
pub struct ProcessOutputData<'a> {
name: BenchmarkName,
cwd: &'a Path,
build_kind: BuildKind,
build_kind: ProfileKind,
scenario_kind: ScenarioKind,
scenario_kind_str: &'a str,
patch: Option<&'a Patch>,
Expand All @@ -544,7 +544,7 @@ pub struct ProcessOutputData<'a> {
/// processing.
pub trait Processor {
/// The `Profiler` being used.
fn profiler(&self, _: BuildKind) -> Profiler;
fn profiler(&self, _: ProfileKind) -> Profiler;

/// Process the output produced by the particular `Profiler` being used.
fn process_output(
Expand All @@ -563,7 +563,7 @@ pub trait Processor {
///
/// Return "true" if planning on doing something different for second
/// iteration.
fn finished_first_collection(&mut self, _: BuildKind) -> bool {
fn finished_first_collection(&mut self, _: ProfileKind) -> bool {
false
}

Expand Down Expand Up @@ -626,7 +626,7 @@ impl<'a> MeasureProcessor<'a> {
fn insert_stats(
&mut self,
scenario: database::Scenario,
build_kind: BuildKind,
build_kind: ProfileKind,
stats: (Stats, Option<SelfProfile>, Option<SelfProfileFiles>),
) {
let version = String::from_utf8(
Expand All @@ -643,10 +643,10 @@ impl<'a> MeasureProcessor<'a> {

let collection = self.rt.block_on(self.conn.collection_id(&version));
let profile = match build_kind {
BuildKind::Check => database::Profile::Check,
BuildKind::Debug => database::Profile::Debug,
BuildKind::Doc => database::Profile::Doc,
BuildKind::Opt => database::Profile::Opt,
ProfileKind::Check => database::Profile::Check,
ProfileKind::Debug => database::Profile::Debug,
ProfileKind::Doc => database::Profile::Doc,
ProfileKind::Opt => database::Profile::Opt,
};

if let Some(files) = stats.2 {
Expand Down Expand Up @@ -806,7 +806,7 @@ impl Upload {
}

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

fn finished_first_collection(&mut self, build: BuildKind) -> bool {
fn finished_first_collection(&mut self, build: ProfileKind) -> bool {
let original = self.profiler(build);
self.is_first_collection = false;
// We need to run again if we're going to use a different profiler
Expand Down Expand Up @@ -919,7 +919,7 @@ impl<'a> ProfileProcessor<'a> {
}

impl<'a> Processor for ProfileProcessor<'a> {
fn profiler(&self, _: BuildKind) -> Profiler {
fn profiler(&self, _: ProfileKind) -> Profiler {
self.profiler
}

Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl Benchmark {
&'a self,
compiler: Compiler<'a>,
cwd: &'a Path,
build_kind: BuildKind,
build_kind: ProfileKind,
) -> CargoProcess<'a> {
let mut cargo_args = self
.config
Expand Down Expand Up @@ -1339,7 +1339,7 @@ impl Benchmark {
pub fn measure(
&self,
processor: &mut dyn Processor,
build_kinds: &[BuildKind],
build_kinds: &[ProfileKind],
scenario_kinds: &[ScenarioKind],
compiler: Compiler<'_>,
iterations: Option<usize>,
Expand Down Expand Up @@ -1429,7 +1429,7 @@ impl Benchmark {
}

// Rustdoc does not support incremental compilation
if build_kind != BuildKind::Doc {
if build_kind != ProfileKind::Doc {
// An incremental build from scratch (slowest incremental case).
// This is required for any subsequent incremental builds.
if scenario_kinds.contains(&ScenarioKind::IncrFull)
Expand Down
52 changes: 26 additions & 26 deletions collector/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,26 @@ impl<'a> Compiler<'a> {
}

#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub enum BuildKind {
pub enum ProfileKind {
Check,
Debug,
Doc,
Opt,
}

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

fn default() -> Vec<Self> {
// Don't run rustdoc by default.
vec![BuildKind::Check, BuildKind::Debug, BuildKind::Opt]
vec![ProfileKind::Check, ProfileKind::Debug, ProfileKind::Opt]
}
}

Expand Down Expand Up @@ -102,12 +102,12 @@ impl ScenarioKind {
}
}

// How the --builds arg maps to BuildKinds.
const STRINGS_AND_BUILD_KINDS: &[(&str, BuildKind)] = &[
("Check", BuildKind::Check),
("Debug", BuildKind::Debug),
("Doc", BuildKind::Doc),
("Opt", BuildKind::Opt),
// How the --builds arg maps to ProfileKinds.
const STRINGS_AND_PROFILE_KINDS: &[(&str, ProfileKind)] = &[
("Check", ProfileKind::Check),
("Debug", ProfileKind::Debug),
("Doc", ProfileKind::Doc),
("Opt", ProfileKind::Opt),
];

// How the --runs arg maps to ScenarioKinds.
Expand All @@ -118,11 +118,11 @@ const STRINGS_AND_SCENARIO_KINDS: &[(&str, ScenarioKind)] = &[
("IncrPatched", ScenarioKind::IncrPatched),
];

fn build_kinds_from_arg(arg: &Option<&str>) -> anyhow::Result<Vec<BuildKind>> {
fn build_kinds_from_arg(arg: &Option<&str>) -> anyhow::Result<Vec<ProfileKind>> {
if let Some(arg) = arg {
kinds_from_arg("build", STRINGS_AND_BUILD_KINDS, arg)
kinds_from_arg("build", STRINGS_AND_PROFILE_KINDS, arg)
} else {
Ok(BuildKind::default())
Ok(ProfileKind::default())
}
}

Expand Down Expand Up @@ -211,7 +211,7 @@ fn bench(
rt: &mut Runtime,
pool: database::Pool,
artifact_id: &ArtifactId,
build_kinds: &[BuildKind],
build_kinds: &[ProfileKind],
scenario_kinds: &[ScenarioKind],
compiler: Compiler<'_>,
benchmarks: &[Benchmark],
Expand Down Expand Up @@ -398,7 +398,7 @@ fn get_benchmarks(
/// - `cargo`: if one is given, check if it is acceptable. Otherwise, look
/// for the nightly Cargo via `rustup`.
fn get_local_toolchain(
build_kinds: &[BuildKind],
build_kinds: &[ProfileKind],
rustc: &str,
rustdoc: Option<&str>,
cargo: Option<&str>,
Expand Down Expand Up @@ -456,7 +456,7 @@ fn get_local_toolchain(
Some(PathBuf::from(rustdoc).canonicalize().with_context(|| {
format!("failed to canonicalize rustdoc executable '{}'", rustdoc)
})?)
} else if build_kinds.contains(&BuildKind::Doc) {
} else if build_kinds.contains(&ProfileKind::Doc) {
// We need a `rustdoc`. Look for one next to `rustc`.
if let Ok(rustdoc) = rustc.with_file_name("rustdoc").canonicalize() {
debug!("found rustdoc: {:?}", &rustdoc);
Expand Down Expand Up @@ -500,7 +500,7 @@ fn generate_cachegrind_diffs(
id2: &str,
out_dir: &Path,
benchmarks: &[Benchmark],
build_kinds: &[BuildKind],
build_kinds: &[ProfileKind],
scenario_kinds: &[ScenarioKind],
errors: &mut BenchmarkErrors,
) {
Expand All @@ -510,7 +510,7 @@ fn generate_cachegrind_diffs(
if let ScenarioKind::IncrPatched = scenario_kind {
continue;
}
if build_kind == BuildKind::Doc && scenario_kind.is_incr() {
if build_kind == ProfileKind::Doc && scenario_kind.is_incr() {
continue;
}
let filename = |prefix, id| {
Expand Down Expand Up @@ -614,7 +614,7 @@ fn profile(
profiler: Profiler,
out_dir: &Path,
benchmarks: &[Benchmark],
build_kinds: &[BuildKind],
build_kinds: &[ProfileKind],
scenario_kinds: &[ScenarioKind],
errors: &mut BenchmarkErrors,
) {
Expand Down Expand Up @@ -875,7 +875,7 @@ fn main_result() -> anyhow::Result<i32> {
&mut rt,
pool,
&ArtifactId::Commit(commit),
&BuildKind::all(),
&ProfileKind::all(),
&ScenarioKind::all(),
Compiler::from_sysroot(&sysroot),
&benchmarks,
Expand Down Expand Up @@ -912,10 +912,10 @@ fn main_result() -> anyhow::Result<i32> {
ScenarioKind::all_non_incr()
};
let build_kinds = if collector::version_supports_doc(toolchain) {
BuildKind::all()
ProfileKind::all()
} else {
let mut all = BuildKind::all();
let doc = all.iter().position(|bk| *bk == BuildKind::Doc).unwrap();
let mut all = ProfileKind::all();
let doc = all.iter().position(|bk| *bk == ProfileKind::Doc).unwrap();
all.remove(doc);
all
};
Expand Down