Skip to content
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

Add optional backends parameter to benchmarking requests #1992

Merged
merged 5 commits into from
Nov 17, 2024
Merged
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
1 change: 1 addition & 0 deletions collector/src/api.rs
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ pub mod next_artifact {
include: Option<String>,
exclude: Option<String>,
runs: Option<i32>,
backends: Option<String>,
},
}

33 changes: 25 additions & 8 deletions collector/src/bin/collector.rs
Original file line number Diff line number Diff line change
@@ -961,7 +961,26 @@ fn main_result() -> anyhow::Result<i32> {
include,
exclude,
runs,
backends: requested_backends,
} => {
// Parse the requested backends, with LLVM as a fallback if none or no valid
// ones were explicitly specified, which will be the case for the vast
// majority of cases.
let mut backends = vec![];
if let Some(requested_backends) = requested_backends {
let requested_backends = requested_backends.to_lowercase();
if requested_backends.contains("llvm") {
backends.push(CodegenBackend::Llvm);
}
if requested_backends.contains("cranelift") {
backends.push(CodegenBackend::Cranelift);
}
}

if backends.is_empty() {
backends.push(CodegenBackend::Llvm);
}

// FIXME: remove this when/if NextArtifact::Commit's include/exclude
// changed from Option<String> to Vec<String>
// to not to manually parse args
@@ -973,12 +992,10 @@ fn main_result() -> anyhow::Result<i32> {
}
};
let sha = commit.sha.to_string();
let sysroot = Sysroot::install(
sha.clone(),
&target_triple,
vec![CodegenBackend::Llvm],
)
.with_context(|| format!("failed to install sysroot for {:?}", commit))?;
let sysroot = Sysroot::install(sha.clone(), &target_triple, &backends)
.with_context(|| {
format!("failed to install sysroot for {:?}", commit)
})?;

let mut benchmarks = get_compile_benchmarks(
&compile_benchmark_dir,
@@ -1001,7 +1018,7 @@ fn main_result() -> anyhow::Result<i32> {
Profile::Opt,
],
scenarios: Scenario::all(),
backends: vec![CodegenBackend::Llvm],
backends,
iterations: runs.map(|v| v as usize),
is_self_profile: self_profile.self_profile,
bench_rustc: bench_rustc.bench_rustc,
@@ -1174,7 +1191,7 @@ fn main_result() -> anyhow::Result<i32> {
let last_sha = String::from_utf8(last_sha.stdout).expect("utf8");
let last_sha = last_sha.split_whitespace().next().expect(&last_sha);
let commit = get_commit_or_fake_it(last_sha).expect("success");
let mut sysroot = Sysroot::install(commit.sha, &target_triple, codegen_backends.0)?;
let mut sysroot = Sysroot::install(commit.sha, &target_triple, &codegen_backends.0)?;
sysroot.preserve(); // don't delete it

// Print the directory containing the toolchain.
6 changes: 1 addition & 5 deletions collector/src/toolchain.rs
Original file line number Diff line number Diff line change
@@ -20,11 +20,7 @@ pub struct Sysroot {
}

impl Sysroot {
pub fn install(
sha: String,
triple: &str,
backends: Vec<CodegenBackend>,
) -> anyhow::Result<Self> {
pub fn install(sha: String, triple: &str, backends: &[CodegenBackend]) -> anyhow::Result<Self> {
let unpack_into = "cache";

fs::create_dir_all(unpack_into)?;
5 changes: 3 additions & 2 deletions database/schema.md
Original file line number Diff line number Diff line change
@@ -238,11 +238,12 @@ is attached to the entry in this table, it can be benchmarked.
* exclude: which benchmarks should be excluded (corresponds to the `--exclude` benchmark parameter)
* runs: how many iterations should be used by default for the benchmark run
* commit_date: when was the commit created
* backends: the codegen backends to use for the benchmarks (corresponds to the `--backends` benchmark parameter)

```
sqlite> select * from pull_request_build limit 1;
bors_sha pr parent_sha complete requested include exclude runs commit_date
---------- -- ---------- -------- --------- ------- ------- ---- -----------
bors_sha pr parent_sha complete requested include exclude runs commit_date backends
---------- -- ---------- -------- --------- ------- ------- ---- ----------- --------
1w0p83... 42 fq24xq... true <timestamp> 3 <timestamp>
```

5 changes: 3 additions & 2 deletions database/src/bin/postgres-to-sqlite.rs
Original file line number Diff line number Diff line change
@@ -275,13 +275,13 @@ impl Table for PullRequestBuild {
}

fn postgres_select_statement(&self, _since_weeks_ago: Option<u32>) -> String {
"select bors_sha, pr, parent_sha, complete, requested, include, exclude, runs from "
"select bors_sha, pr, parent_sha, complete, requested, include, exclude, runs, backends from "
.to_string()
+ self.name()
}

fn sqlite_insert_statement(&self) -> &'static str {
"insert into pull_request_build (bors_sha, pr, parent_sha, complete, requested, include, exclude, runs) VALUES (?, ?, ?, ?, ?, ?, ?, ?)"
"insert into pull_request_build (bors_sha, pr, parent_sha, complete, requested, include, exclude, runs, backends) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"
}

fn sqlite_execute_insert(&self, statement: &mut rusqlite::Statement, row: tokio_postgres::Row) {
@@ -296,6 +296,7 @@ impl Table for PullRequestBuild {
row.get::<_, Option<&str>>(5),
row.get::<_, Option<&str>>(6),
row.get::<_, Option<i32>>(7),
row.get::<_, Option<&str>>(8),
])
.unwrap();
}
6 changes: 4 additions & 2 deletions database/src/bin/sqlite-to-postgres.rs
Original file line number Diff line number Diff line change
@@ -369,6 +369,7 @@ struct PullRequestBuildRow<'a> {
exclude: Nullable<&'a str>,
runs: Nullable<i32>,
commit_date: Nullable<DateTime<Utc>>,
backends: Nullable<&'a str>,
}

impl Table for PullRequestBuild {
@@ -377,11 +378,11 @@ impl Table for PullRequestBuild {
}

fn sqlite_attributes() -> &'static str {
"bors_sha, pr, parent_sha, complete, requested, include, exclude, runs, commit_date"
"bors_sha, pr, parent_sha, complete, requested, include, exclude, runs, commit_date, backends"
}

fn postgres_attributes() -> &'static str {
"bors_sha, pr, parent_sha, complete, requested, include, exclude, runs, commit_date"
"bors_sha, pr, parent_sha, complete, requested, include, exclude, runs, commit_date, backends"
}

fn postgres_generated_id_attribute() -> Option<&'static str> {
@@ -407,6 +408,7 @@ impl Table for PullRequestBuild {
commit_date: Nullable(
commit_date.map(|seconds| Utc.timestamp_opt(seconds, 0).unwrap()),
),
backends: row.get_ref(9).unwrap().try_into().unwrap(),
})
.unwrap();
}
1 change: 1 addition & 0 deletions database/src/lib.rs
Original file line number Diff line number Diff line change
@@ -28,6 +28,7 @@ pub struct QueuedCommit {
pub exclude: Option<String>,
pub runs: Option<i32>,
pub commit_date: Option<Date>,
pub backends: Option<String>,
}

#[derive(Debug, Hash, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
1 change: 1 addition & 0 deletions database/src/pool.rs
Original file line number Diff line number Diff line change
@@ -121,6 +121,7 @@ pub trait Connection: Send + Sync {
include: Option<&str>,
exclude: Option<&str>,
runs: Option<i32>,
backends: Option<&str>,
);
/// Returns true if this PR was queued waiting for a commit
async fn pr_attach_commit(
14 changes: 9 additions & 5 deletions database/src/pool/postgres.rs
Original file line number Diff line number Diff line change
@@ -278,6 +278,7 @@ static MIGRATIONS: &[&str] = &[
alter table pstat_series drop constraint pstat_series_crate_profile_cache_statistic_key;
alter table pstat_series add constraint test_case UNIQUE(crate, profile, scenario, backend, metric);
"#,
r#"alter table pull_request_build add column backends text;"#,
];

#[async_trait::async_trait]
@@ -736,14 +737,15 @@ where
include: Option<&str>,
exclude: Option<&str>,
runs: Option<i32>,
backends: Option<&str>,
) {
if let Err(e) = self.conn()
.execute(
"insert into pull_request_build (pr, complete, requested, include, exclude, runs) VALUES ($1, false, CURRENT_TIMESTAMP, $2, $3, $4)",
&[&(pr as i32), &include, &exclude, &runs],
"insert into pull_request_build (pr, complete, requested, include, exclude, runs, backends) VALUES ($1, false, CURRENT_TIMESTAMP, $2, $3, $4, $5)",
&[&(pr as i32), &include, &exclude, &runs, &backends],
)
.await {
log::error!("failed to queue_pr({}, {:?}, {:?}, {:?}): {:?}", pr, include, exclude, runs, e);
log::error!("failed to queue_pr({}, {:?}, {:?}, {:?}, {:?}): {:?}", pr, include, exclude, runs, backends, e);
}
}
async fn pr_attach_commit(
@@ -767,7 +769,7 @@ where
let rows = self
.conn()
.query(
"select pr, bors_sha, parent_sha, include, exclude, runs, commit_date from pull_request_build
"select pr, bors_sha, parent_sha, include, exclude, runs, commit_date, backends from pull_request_build
where complete is false and bors_sha is not null
order by requested asc",
&[],
@@ -783,6 +785,7 @@ where
exclude: row.get(4),
runs: row.get(5),
commit_date: row.get::<_, Option<_>>(6).map(Date),
backends: row.get(7),
})
.collect()
}
@@ -792,7 +795,7 @@ where
.query_opt(
"update pull_request_build SET complete = true
where bors_sha = $1
returning pr, bors_sha, parent_sha, include, exclude, runs, commit_date",
returning pr, bors_sha, parent_sha, include, exclude, runs, commit_date, backends",
&[&sha],
)
.await
@@ -805,6 +808,7 @@ where
exclude: row.get(4),
runs: row.get(5),
commit_date: row.get::<_, Option<_>>(6).map(Date),
backends: row.get(7),
})
}
async fn collection_id(&self, version: &str) -> CollectionId {
16 changes: 10 additions & 6 deletions database/src/pool/sqlite.rs
Original file line number Diff line number Diff line change
@@ -385,6 +385,7 @@ static MIGRATIONS: &[Migration] = &[
alter table pstat_series_new rename to pstat_series;
"#,
),
Migration::new("alter table pull_request_build add column backends text"),
];

#[async_trait::async_trait]
@@ -909,13 +910,14 @@ impl Connection for SqliteConnection {
include: Option<&str>,
exclude: Option<&str>,
runs: Option<i32>,
backends: Option<&str>,
) {
self.raw_ref()
.prepare_cached(
"insert into pull_request_build (pr, complete, requested, include, exclude, runs) VALUES (?, 0, strftime('%s','now'), ?, ?, ?)",
"insert into pull_request_build (pr, complete, requested, include, exclude, runs, backends) VALUES (?, 0, strftime('%s','now'), ?, ?, ?, ?)",
)
.unwrap()
.execute(params![pr, include, exclude, &runs])
.execute(params![pr, include, exclude, &runs, backends])
.unwrap();
}
async fn pr_attach_commit(
@@ -939,7 +941,7 @@ impl Connection for SqliteConnection {
async fn queued_commits(&self) -> Vec<QueuedCommit> {
self.raw_ref()
.prepare_cached(
"select pr, bors_sha, parent_sha, include, exclude, runs, commit_date from pull_request_build
"select pr, bors_sha, parent_sha, include, exclude, runs, commit_date, backends from pull_request_build
where complete is false and bors_sha is not null
order by requested asc",
)
@@ -954,7 +956,8 @@ impl Connection for SqliteConnection {
include: row.get(3).unwrap(),
exclude: row.get(4).unwrap(),
runs: row.get(5).unwrap(),
commit_date: row.get::<_, Option<i64>>(6).unwrap().map(|timestamp| Date(DateTime::from_timestamp(timestamp, 0).unwrap()))
commit_date: row.get::<_, Option<i64>>(6).unwrap().map(|timestamp| Date(DateTime::from_timestamp(timestamp, 0).unwrap())),
backends: row.get(7).unwrap(),
})
})
.collect::<Result<Vec<_>, _>>()
@@ -974,7 +977,7 @@ impl Connection for SqliteConnection {
assert_eq!(count, 1, "sha is unique column");
self.raw_ref()
.query_row(
"select pr, sha, parent_sha, include, exclude, runs, commit_date from pull_request_build
"select pr, sha, parent_sha, include, exclude, runs, commit_date, backends from pull_request_build
where sha = ?",
params![sha],
|row| {
@@ -985,7 +988,8 @@ impl Connection for SqliteConnection {
include: row.get(3).unwrap(),
exclude: row.get(4).unwrap(),
runs: row.get(5).unwrap(),
commit_date: row.get::<_, Option<i64>>(6).unwrap().map(|timestamp| Date(DateTime::from_timestamp(timestamp, 0).unwrap()))
commit_date: row.get::<_, Option<i64>>(6).unwrap().map(|timestamp| Date(DateTime::from_timestamp(timestamp, 0).unwrap())),
backends: row.get(7).unwrap(),
})
},
)
1 change: 1 addition & 0 deletions site/frontend/src/pages/status/data.ts
Original file line number Diff line number Diff line change
@@ -39,6 +39,7 @@ export type MissingReason =
include: string | null;
exclude: string | null;
runs: number | null;
backends: string | null;
};
}
| {
10 changes: 10 additions & 0 deletions site/src/load.rs
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ pub enum MissingReason {
include: Option<String>,
exclude: Option<String>,
runs: Option<i32>,
backends: Option<String>,
},
InProgress(Option<Box<MissingReason>>),
}
@@ -382,6 +383,7 @@ fn calculate_missing_from(
exclude,
runs,
commit_date,
backends,
} in queued_pr_commits
.into_iter()
// filter out any queued PR master commits (leaving only try commits)
@@ -407,6 +409,7 @@ fn calculate_missing_from(
include,
exclude,
runs,
backends,
},
));
}
@@ -579,6 +582,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
QueuedCommit {
sha: "b".into(),
@@ -588,6 +592,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
QueuedCommit {
sha: "a".into(),
@@ -597,6 +602,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
];
let in_progress_artifacts = vec![];
@@ -640,6 +646,7 @@ mod tests {
include: None,
exclude: None,
runs: None,
backends: None,
},
),
];
@@ -689,6 +696,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
// A try run
QueuedCommit {
@@ -699,6 +707,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
];
let in_progress_artifacts = vec![];
@@ -746,6 +755,7 @@ mod tests {
include: None,
exclude: None,
runs: None,
backends: None,
},
),
];
Loading
Loading