Skip to content

Commit cabcfba

Browse files
JamesbarfordJamesbarford
committed
pr feedback - reorder target, add documentation, add target to the sql conversion scripts
1 parent 7cb728f commit cabcfba

File tree

12 files changed

+55
-40
lines changed

12 files changed

+55
-40
lines changed

‎collector/src/compile/benchmark/target.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
1+
/// Target representing an Rust target tripple, 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
16
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, serde::Deserialize)]
27
pub enum Target {
8+
/// `x86_64-unknown-linux-gnu`
39
X86_64UnknownLinuxGnu
410
}
511

‎collector/src/compile/execute/bencher.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@ impl<'a> BenchProcessor<'a> {
113113
profile,
114114
scenario,
115115
backend,
116+
target,
116117
stat,
117118
value,
118-
target,
119119
));
120120
}
121121

‎collector/src/compile/execute/mod.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use std::process::{self, Command};
2323
use std::str;
2424
use std::sync::LazyLock;
2525

26-
2726
pub mod bencher;
2827
mod etw_parser;
2928
pub mod profiler;

‎database/schema.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Here is the diagram for compile-time benchmarks:
3535
│ scenario │ │ cid │ │
3636
│ backend │ │ value ├───┘
3737
│ metric │ └──────────┘
38-
│ compiler_tar..│
38+
│ target │
3939
└───────────────┘
4040
```
4141

‎database/src/bin/import-sqlite.rs‎

Lines changed: 2 additions & 2 deletions
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, target), 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,9 +74,9 @@ async fn main() {
7474
profile,
7575
scenario,
7676
backend,
77+
target,
7778
metric.as_str(),
7879
stat,
79-
target,
8080
)
8181
.await;
8282
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,11 +246,11 @@ 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() + self.name()
250250
}
251251

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

256256
fn sqlite_execute_insert(&self, statement: &mut rusqlite::Statement, row: tokio_postgres::Row) {
@@ -262,6 +262,7 @@ impl Table for PstatSeries {
262262
row.get::<_, &str>(3),
263263
row.get::<_, &str>(4),
264264
row.get::<_, &str>(5),
265+
row.get::<_, &str>(6),
265266
])
266267
.unwrap();
267268
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ struct PstatSeriesRow<'a> {
322322
profile: &'a str,
323323
scenario: &'a str,
324324
backend: &'a str,
325+
target: &'a str,
325326
metric: &'a str,
326327
}
327328

@@ -331,11 +332,11 @@ impl Table for PstatSeries {
331332
}
332333

333334
fn sqlite_attributes() -> &'static str {
334-
"id, crate, profile, scenario, backend, metric"
335+
"id, crate, profile, scenario, backend, target, metric"
335336
}
336337

337338
fn postgres_attributes() -> &'static str {
338-
"id, crate, profile, scenario, backend, metric"
339+
"id, crate, profile, scenario, backend, target, metric"
339340
}
340341

341342
fn postgres_generated_id_attribute() -> Option<&'static str> {
@@ -350,7 +351,8 @@ impl Table for PstatSeries {
350351
profile: row.get_ref(2).unwrap().as_str().unwrap(),
351352
scenario: row.get_ref(3).unwrap().as_str().unwrap(),
352353
backend: row.get_ref(4).unwrap().as_str().unwrap(),
353-
metric: row.get_ref(5).unwrap().as_str().unwrap(),
354+
target: row.get_ref(5).unwrap().as_str().unwrap(),
355+
metric: row.get_ref(6).unwrap().as_str().unwrap(),
354356
})
355357
.unwrap();
356358
}

‎database/src/lib.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,17 @@ impl PartialOrd for Scenario {
339339
Some(self.cmp(other))
340340
}
341341
}
342-
/// The codegen backend used for compilation.
342+
343+
/// Target representing an Rust target tripple, for a full list of targets and
344+
/// their support see;
345+
/// https://doc.rust-lang.org/nightly/rustc/platform-support.html
346+
///
347+
/// Presently we only support x86_64
343348
#[derive(
344349
Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, serde::Serialize, serde::Deserialize,
345350
)]
346351
pub enum Target {
352+
/// `x86_64-unknown-linux-gnu`
347353
X86_64UnknownLinuxGnu
348354
}
349355

@@ -503,7 +509,7 @@ pub struct Index {
503509
artifacts: Indexed<Box<str>>,
504510
/// Id lookup of compile stat description ids
505511
/// For legacy reasons called `pstat_series` in the database, and so the name is kept here.
506-
pstat_series: Indexed<(Benchmark, Profile, Scenario, CodegenBackend, Metric, Target)>,
512+
pstat_series: Indexed<(Benchmark, Profile, Scenario, CodegenBackend, Target, Metric)>,
507513
/// Id lookup of runtime stat description ids
508514
runtime_pstat_series: Indexed<(Benchmark, Metric)>,
509515
}
@@ -617,7 +623,7 @@ mod index_serde {
617623
}
618624
}
619625

620-
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
626+
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
621627
pub enum DbLabel {
622628
StatisticDescription {
623629
benchmark: Benchmark,
@@ -647,7 +653,7 @@ impl Lookup for DbLabel {
647653
target,
648654
} => index
649655
.pstat_series
650-
.get(&(*benchmark, *profile, *scenario, *backend, *metric, *target)),
656+
.get(&(*benchmark, *profile, *scenario, *backend, *target, *metric)),
651657
}
652658
}
653659
}
@@ -723,7 +729,7 @@ impl Index {
723729
&self,
724730
) -> impl Iterator<
725731
Item = (
726-
&(Benchmark, Profile, Scenario, CodegenBackend, Metric, Target),
732+
&(Benchmark, Profile, Scenario, CodegenBackend, Target, Metric),
727733
StatisticalDescriptionId,
728734
),
729735
> + '_ {

‎database/src/pool.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ pub trait Connection: Send + Sync {
4545
profile: Profile,
4646
scenario: Scenario,
4747
backend: CodegenBackend,
48+
target: Target,
4849
metric: &str,
4950
value: f64,
50-
target: Target,
5151
);
5252
async fn record_runtime_statistic(
5353
&self,

‎database/src/pool/postgres.rs‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ static MIGRATIONS: &[&str] = &[
278278
alter table pstat_series drop constraint pstat_series_crate_profile_cache_statistic_key;
279279
alter table pstat_series add constraint test_case UNIQUE(crate, profile, scenario, backend, metric);
280280
"#,
281+
r#"alter table pull_request_build add column backends text;"#,
281282
// A pstat series shows 1 target
282283
r#"
283284
alter table pstat_series add target text not null default 'x86_64-unknown-linux-gnu';
@@ -471,8 +472,8 @@ impl PostgresConnection {
471472
.await
472473
.unwrap(),
473474
get_error: conn.prepare("select benchmark, error from error where aid = $1").await.unwrap(),
474-
insert_pstat_series: conn.prepare("insert into pstat_series (crate, profile, scenario, backend, metric, target) VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(),
475-
select_pstat_series: conn.prepare("select id from pstat_series where crate = $1 and profile = $2 and scenario = $3 and backend = $4 and metric = $5 and target = $6").await.unwrap(),
475+
insert_pstat_series: conn.prepare("insert into pstat_series (crate, profile, scenario, backend, target, metric) VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(),
476+
select_pstat_series: conn.prepare("select id from pstat_series where crate = $1 and profile = $2 and scenario = $3 and backend = $4 and target = $5 and metric = $6").await.unwrap(),
476477
collection_id: conn.prepare("insert into collection (perf_commit) VALUES ($1) returning id").await.unwrap(),
477478
record_duration: conn.prepare("
478479
insert into artifact_collection_duration (
@@ -622,7 +623,7 @@ where
622623
pstat_series: self
623624
.conn()
624625
.query(
625-
"select id, crate, profile, scenario, backend, metric, target, from pstat_series;",
626+
"select id, crate, profile, scenario, backend, target, metric from pstat_series;",
626627
&[],
627628
)
628629
.await
@@ -636,8 +637,8 @@ where
636637
Profile::from_str(row.get::<_, String>(2).as_str()).unwrap(),
637638
row.get::<_, String>(3).as_str().parse().unwrap(),
638639
CodegenBackend::from_str(row.get::<_, String>(4).as_str()).unwrap(),
639-
row.get::<_, String>(5).as_str().into(),
640640
Target::from_str(row.get::<_, String>(6).as_str()).unwrap(),
641+
row.get::<_, String>(5).as_str().into(),
641642
),
642643
)
643644
})
@@ -835,9 +836,9 @@ where
835836
profile: Profile,
836837
scenario: Scenario,
837838
backend: CodegenBackend,
839+
target: Target,
838840
metric: &str,
839841
stat: f64,
840-
target: Target,
841842
) {
842843
let profile = profile.to_string();
843844
let scenario = scenario.to_string();
@@ -847,7 +848,7 @@ where
847848
.conn()
848849
.query_opt(
849850
&self.statements().select_pstat_series,
850-
&[&benchmark, &profile, &scenario, &backend, &metric, &target],
851+
&[&benchmark, &profile, &scenario, &backend, &target, &metric],
851852
)
852853
.await
853854
.unwrap();
@@ -857,14 +858,14 @@ where
857858
self.conn()
858859
.query_opt(
859860
&self.statements().insert_pstat_series,
860-
&[&benchmark, &profile, &scenario, &backend, &metric, &target],
861+
&[&benchmark, &profile, &scenario, &backend, &target, &metric],
861862
)
862863
.await
863864
.unwrap();
864865
self.conn()
865866
.query_one(
866867
&self.statements().select_pstat_series,
867-
&[&benchmark, &profile, &scenario, &backend, &metric, &target],
868+
&[&benchmark, &profile, &scenario, &backend, &target, &metric],
868869
)
869870
.await
870871
.unwrap()

0 commit comments

Comments
 (0)