Skip to content

Commit 4cb8a8d

Browse files
authored
Merge pull request #2059 from Jamesbarford/feat/add-target-column
Add `target` column, defaulting to `x86_64-unknown-linux-gnu`
2 parents f9123e1 + 0b40d8f commit 4cb8a8d

File tree

14 files changed

+211
-59
lines changed

14 files changed

+211
-59
lines changed

collector/src/bin/collector.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use collector::compile::benchmark::category::Category;
3737
use collector::compile::benchmark::codegen_backend::CodegenBackend;
3838
use collector::compile::benchmark::profile::Profile;
3939
use collector::compile::benchmark::scenario::Scenario;
40+
use collector::compile::benchmark::target::Target;
4041
use collector::compile::benchmark::{
4142
compile_benchmark_dir, get_compile_benchmarks, ArtifactType, Benchmark, BenchmarkName,
4243
};
@@ -99,6 +100,7 @@ struct CompileBenchmarkConfig {
99100
iterations: Option<usize>,
100101
is_self_profile: bool,
101102
bench_rustc: bool,
103+
targets: Vec<Target>,
102104
}
103105

104106
struct RuntimeBenchmarkConfig {
@@ -200,6 +202,7 @@ fn profile_compile(
200202
scenarios: &[Scenario],
201203
backends: &[CodegenBackend],
202204
errors: &mut BenchmarkErrors,
205+
targets: &[Target],
203206
) {
204207
eprintln!("Profiling {} with {:?}", toolchain.id, profiler);
205208
if let Profiler::SelfProfile = profiler {
@@ -220,6 +223,7 @@ fn profile_compile(
220223
backends,
221224
toolchain,
222225
Some(1),
226+
targets,
223227
));
224228
eprintln!("Finished benchmark {benchmark_id}");
225229

@@ -910,6 +914,7 @@ fn main_result() -> anyhow::Result<i32> {
910914
iterations: Some(iterations),
911915
is_self_profile: self_profile.self_profile,
912916
bench_rustc: bench_rustc.bench_rustc,
917+
targets: vec![Target::default()],
913918
};
914919

915920
run_benchmarks(&mut rt, conn, shared, Some(config), None)?;
@@ -1024,6 +1029,7 @@ fn main_result() -> anyhow::Result<i32> {
10241029
iterations: runs.map(|v| v as usize),
10251030
is_self_profile: self_profile.self_profile,
10261031
bench_rustc: bench_rustc.bench_rustc,
1032+
targets: vec![Target::default()],
10271033
};
10281034
let runtime_suite = rt.block_on(load_runtime_benchmarks(
10291035
conn.as_mut(),
@@ -1136,6 +1142,7 @@ fn main_result() -> anyhow::Result<i32> {
11361142
scenarios,
11371143
backends,
11381144
&mut errors,
1145+
&[Target::default()],
11391146
);
11401147
Ok(id)
11411148
};
@@ -1734,6 +1741,7 @@ fn bench_published_artifact(
17341741
iterations: Some(3),
17351742
is_self_profile: false,
17361743
bench_rustc: false,
1744+
targets: vec![Target::default()],
17371745
}),
17381746
Some(RuntimeBenchmarkConfig::new(
17391747
runtime_suite,
@@ -1834,6 +1842,7 @@ fn bench_compile(
18341842
&config.backends,
18351843
&shared.toolchain,
18361844
config.iterations,
1845+
&config.targets,
18371846
)))
18381847
.with_context(|| anyhow::anyhow!("Cannot compile {}", benchmark.name))
18391848
},

collector/src/compile/benchmark/mod.rs

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::compile::benchmark::codegen_backend::CodegenBackend;
33
use crate::compile::benchmark::patch::Patch;
44
use crate::compile::benchmark::profile::Profile;
55
use crate::compile::benchmark::scenario::Scenario;
6+
use crate::compile::benchmark::target::Target;
67
use crate::compile::execute::{CargoProcess, Processor};
78
use crate::toolchain::Toolchain;
89
use crate::utils::wait_for_future;
@@ -20,6 +21,7 @@ pub mod codegen_backend;
2021
pub(crate) mod patch;
2122
pub mod profile;
2223
pub mod scenario;
24+
pub mod target;
2325

2426
fn default_runs() -> usize {
2527
3
@@ -180,6 +182,7 @@ impl Benchmark {
180182
cwd: &'a Path,
181183
profile: Profile,
182184
backend: CodegenBackend,
185+
target: Target,
183186
) -> CargoProcess<'a> {
184187
let mut cargo_args = self
185188
.config
@@ -220,10 +223,12 @@ impl Benchmark {
220223
.collect(),
221224
touch_file: self.config.touch_file.clone(),
222225
jobserver: None,
226+
target,
223227
}
224228
}
225229

226230
/// Run a specific benchmark under a processor + profiler combination.
231+
#[allow(clippy::too_many_arguments)]
227232
pub async fn measure(
228233
&self,
229234
processor: &mut dyn Processor,
@@ -232,6 +237,7 @@ impl Benchmark {
232237
backends: &[CodegenBackend],
233238
toolchain: &Toolchain,
234239
iterations: Option<usize>,
240+
targets: &[Target],
235241
) -> anyhow::Result<()> {
236242
if self.config.disabled {
237243
eprintln!("Skipping {}: disabled", self.name);
@@ -263,10 +269,15 @@ impl Benchmark {
263269
}
264270

265271
eprintln!("Preparing {}", self.name);
266-
let mut target_dirs: Vec<((CodegenBackend, Profile), TempDir)> = vec![];
272+
let mut target_dirs: Vec<((CodegenBackend, Profile, Target), TempDir)> = vec![];
267273
for backend in backends {
268274
for profile in &profiles {
269-
target_dirs.push(((*backend, *profile), self.make_temp_dir(&self.path)?));
275+
for target in targets {
276+
target_dirs.push((
277+
(*backend, *profile, *target),
278+
self.make_temp_dir(&self.path)?,
279+
));
280+
}
270281
}
271282
}
272283

@@ -304,15 +315,21 @@ impl Benchmark {
304315
)
305316
.context("jobserver::new")?;
306317
let mut threads = Vec::with_capacity(target_dirs.len());
307-
for ((backend, profile), prep_dir) in &target_dirs {
318+
for ((backend, profile, target), prep_dir) in &target_dirs {
308319
let server = server.clone();
309320
let thread = s.spawn::<_, anyhow::Result<()>>(move || {
310321
wait_for_future(async move {
311322
let server = server.clone();
312-
self.mk_cargo_process(toolchain, prep_dir.path(), *profile, *backend)
313-
.jobserver(server)
314-
.run_rustc(false)
315-
.await?;
323+
self.mk_cargo_process(
324+
toolchain,
325+
prep_dir.path(),
326+
*profile,
327+
*backend,
328+
*target,
329+
)
330+
.jobserver(server)
331+
.run_rustc(false)
332+
.await?;
316333
Ok::<(), anyhow::Error>(())
317334
})?;
318335
Ok(())
@@ -343,12 +360,13 @@ impl Benchmark {
343360
let mut timing_dirs: Vec<ManuallyDrop<TempDir>> = vec![];
344361

345362
let benchmark_start = std::time::Instant::now();
346-
for ((backend, profile), prep_dir) in &target_dirs {
363+
for ((backend, profile, target), prep_dir) in &target_dirs {
347364
let backend = *backend;
348365
let profile = *profile;
366+
let target = *target;
349367
eprintln!(
350-
"Running {}: {:?} + {:?} + {:?}",
351-
self.name, profile, scenarios, backend
368+
"Running {}: {:?} + {:?} + {:?} + {:?}",
369+
self.name, profile, scenarios, backend, target,
352370
);
353371

354372
// We want at least two runs for all benchmarks (since we run
@@ -370,7 +388,7 @@ impl Benchmark {
370388

371389
// A full non-incremental build.
372390
if scenarios.contains(&Scenario::Full) {
373-
self.mk_cargo_process(toolchain, cwd, profile, backend)
391+
self.mk_cargo_process(toolchain, cwd, profile, backend, target)
374392
.processor(processor, Scenario::Full, "Full", None)
375393
.run_rustc(true)
376394
.await?;
@@ -381,7 +399,7 @@ impl Benchmark {
381399
// An incremental from scratch (slowest incremental case).
382400
// This is required for any subsequent incremental builds.
383401
if scenarios.iter().any(|s| s.is_incr()) {
384-
self.mk_cargo_process(toolchain, cwd, profile, backend)
402+
self.mk_cargo_process(toolchain, cwd, profile, backend, target)
385403
.incremental(true)
386404
.processor(processor, Scenario::IncrFull, "IncrFull", None)
387405
.run_rustc(true)
@@ -390,7 +408,7 @@ impl Benchmark {
390408

391409
// An incremental build with no changes (fastest incremental case).
392410
if scenarios.contains(&Scenario::IncrUnchanged) {
393-
self.mk_cargo_process(toolchain, cwd, profile, backend)
411+
self.mk_cargo_process(toolchain, cwd, profile, backend, target)
394412
.incremental(true)
395413
.processor(processor, Scenario::IncrUnchanged, "IncrUnchanged", None)
396414
.run_rustc(true)
@@ -405,7 +423,7 @@ impl Benchmark {
405423
// An incremental build with some changes (realistic
406424
// incremental case).
407425
let scenario_str = format!("IncrPatched{}", i);
408-
self.mk_cargo_process(toolchain, cwd, profile, backend)
426+
self.mk_cargo_process(toolchain, cwd, profile, backend, target)
409427
.incremental(true)
410428
.processor(
411429
processor,
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// Target representing an Rust target triple, for a full list of targets and
2+
/// their support see;
3+
/// https://doc.rust-lang.org/nightly/rustc/platform-support.html
4+
///
5+
/// Presently we only support x86_64
6+
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, serde::Deserialize)]
7+
pub enum Target {
8+
/// `x86_64-unknown-linux-gnu`
9+
X86_64UnknownLinuxGnu,
10+
}
11+
12+
impl Default for Target {
13+
fn default() -> Self {
14+
Self::X86_64UnknownLinuxGnu
15+
}
16+
}

collector/src/compile/execute/bencher.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::compile::benchmark::codegen_backend::CodegenBackend;
22
use crate::compile::benchmark::profile::Profile;
33
use crate::compile::benchmark::scenario::Scenario;
4+
use crate::compile::benchmark::target::Target;
45
use crate::compile::benchmark::BenchmarkName;
56
use crate::compile::execute;
67
use crate::compile::execute::{
@@ -91,13 +92,18 @@ impl<'a> BenchProcessor<'a> {
9192
scenario: database::Scenario,
9293
profile: database::Profile,
9394
backend: CodegenBackend,
95+
target: Target,
9496
stats: Stats,
9597
) {
9698
let backend = match backend {
9799
CodegenBackend::Llvm => database::CodegenBackend::Llvm,
98100
CodegenBackend::Cranelift => database::CodegenBackend::Cranelift,
99101
};
100102

103+
let target = match target {
104+
Target::X86_64UnknownLinuxGnu => database::Target::X86_64UnknownLinuxGnu,
105+
};
106+
101107
let mut buf = FuturesUnordered::new();
102108
for (stat, value) in stats.iter() {
103109
buf.push(self.conn.record_statistic(
@@ -107,6 +113,7 @@ impl<'a> BenchProcessor<'a> {
107113
profile,
108114
scenario,
109115
backend,
116+
target,
110117
stat,
111118
value,
112119
));
@@ -199,8 +206,15 @@ impl Processor for BenchProcessor<'_> {
199206
res.0.stats.retain(|key, _| key.starts_with("size:"));
200207
}
201208

202-
self.insert_stats(collection, scenario, profile, data.backend, res.0)
203-
.await;
209+
self.insert_stats(
210+
collection,
211+
scenario,
212+
profile,
213+
data.backend,
214+
data.target,
215+
res.0,
216+
)
217+
.await;
204218

205219
Ok(Retry::No)
206220
}

collector/src/compile/execute/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::compile::benchmark::codegen_backend::CodegenBackend;
44
use crate::compile::benchmark::patch::Patch;
55
use crate::compile::benchmark::profile::Profile;
66
use crate::compile::benchmark::scenario::Scenario;
7+
use crate::compile::benchmark::target::Target;
78
use crate::compile::benchmark::BenchmarkName;
89
use crate::toolchain::Toolchain;
910
use crate::utils::fs::EnsureImmutableFile;
@@ -129,6 +130,7 @@ pub struct CargoProcess<'a> {
129130
pub rustc_args: Vec<String>,
130131
pub touch_file: Option<String>,
131132
pub jobserver: Option<jobserver::Client>,
133+
pub target: Target,
132134
}
133135
/// Returns an optional list of Performance CPU cores, if the system has P and E cores.
134136
/// This list *should* be in a format suitable for the `taskset` command.
@@ -273,12 +275,13 @@ impl<'a> CargoProcess<'a> {
273275
// really.
274276
pub async fn run_rustc(&mut self, needs_final: bool) -> anyhow::Result<()> {
275277
log::info!(
276-
"run_rustc with incremental={}, profile={:?}, scenario={:?}, patch={:?}, backend={:?}, phase={}",
278+
"run_rustc with incremental={}, profile={:?}, scenario={:?}, patch={:?}, backend={:?}, target={:?}, phase={}",
277279
self.incremental,
278280
self.profile,
279281
self.processor_etc.as_ref().map(|v| v.1),
280282
self.processor_etc.as_ref().and_then(|v| v.3),
281283
self.backend,
284+
self.target,
282285
if needs_final { "benchmark" } else { "dependencies" }
283286
);
284287

@@ -420,6 +423,7 @@ impl<'a> CargoProcess<'a> {
420423
scenario_str,
421424
patch,
422425
backend: self.backend,
426+
target: self.target,
423427
};
424428
match processor.process_output(&data, output).await {
425429
Ok(Retry::No) => return Ok(()),
@@ -484,6 +488,7 @@ pub struct ProcessOutputData<'a> {
484488
scenario_str: &'a str,
485489
patch: Option<&'a Patch>,
486490
backend: CodegenBackend,
491+
target: Target,
487492
}
488493

489494
/// Trait used by `Benchmark::measure()` to provide different kinds of

database/schema.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Here is the diagram for compile-time benchmarks:
3535
│ scenario │ │ cid │ │
3636
│ backend │ │ value ├───┘
3737
│ metric │ └──────────┘
38+
│ target │
3839
└───────────────┘
3940
```
4041

@@ -151,9 +152,9 @@ many times in the `pstat` table.
151152

152153
```
153154
sqlite> select * from pstat_series limit 1;
154-
id crate profile scenario backend metric
155-
---------- ---------- ---------- ---------- ------- ------------
156-
1 helloworld check full llvm task-clock:u
155+
id crate profile scenario backend target metric
156+
---------- ---------- ---------- ---------- ------- ------------ ------------
157+
1 helloworld check full llvm x86_64-linux-unknown-gnu task-clock:u
157158
```
158159

159160
### pstat

database/src/bin/import-sqlite.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ async fn main() {
4545
let sqlite_aid = sqlite_conn.artifact_id(&aid).await;
4646
let postgres_aid = postgres_conn.artifact_id(&aid).await;
4747

48-
for (&(benchmark, profile, scenario, backend, metric), id) in
48+
for (&(benchmark, profile, scenario, backend, target, metric), id) in
4949
sqlite_idx.compile_statistic_descriptions()
5050
{
5151
if benchmarks.insert(benchmark) {
@@ -74,6 +74,7 @@ async fn main() {
7474
profile,
7575
scenario,
7676
backend,
77+
target,
7778
metric.as_str(),
7879
stat,
7980
)

database/src/bin/postgres-to-sqlite.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,11 +246,12 @@ impl Table for PstatSeries {
246246
}
247247

248248
fn postgres_select_statement(&self, _since_weeks_ago: Option<u32>) -> String {
249-
"select id, crate, profile, scenario, backend, metric from ".to_string() + self.name()
249+
"select id, crate, profile, scenario, backend, target, metric from ".to_string()
250+
+ self.name()
250251
}
251252

252253
fn sqlite_insert_statement(&self) -> &'static str {
253-
"insert into pstat_series (id, crate, profile, scenario, backend, metric) VALUES (?, ?, ?, ?, ?, ?)"
254+
"insert into pstat_series (id, crate, profile, scenario, backend, target, metric) VALUES (?, ?, ?, ?, ?, ?, ?)"
254255
}
255256

256257
fn sqlite_execute_insert(&self, statement: &mut rusqlite::Statement, row: tokio_postgres::Row) {
@@ -262,6 +263,7 @@ impl Table for PstatSeries {
262263
row.get::<_, &str>(3),
263264
row.get::<_, &str>(4),
264265
row.get::<_, &str>(5),
266+
row.get::<_, &str>(6),
265267
])
266268
.unwrap();
267269
}

0 commit comments

Comments
 (0)