Skip to content

Add optional backends parameter to benchmarking requests #1992

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

Expand Down
33 changes: 25 additions & 8 deletions collector/src/bin/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 1 addition & 5 deletions collector/src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
5 changes: 3 additions & 2 deletions database/schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
```

Expand Down
5 changes: 3 additions & 2 deletions database/src/bin/postgres-to-sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
}
Expand Down
6 changes: 4 additions & 2 deletions database/src/bin/sqlite-to-postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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> {
Expand All @@ -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();
}
Expand Down
1 change: 1 addition & 0 deletions database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions database/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
14 changes: 9 additions & 5 deletions database/src/pool/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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(
Expand All @@ -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",
&[],
Expand All @@ -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()
}
Expand All @@ -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
Expand All @@ -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 {
Expand Down
16 changes: 10 additions & 6 deletions database/src/pool/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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(
Expand All @@ -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",
)
Expand All @@ -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<_>, _>>()
Expand All @@ -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| {
Expand All @@ -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(),
})
},
)
Expand Down
1 change: 1 addition & 0 deletions site/frontend/src/pages/status/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export type MissingReason =
include: string | null;
exclude: string | null;
runs: number | null;
backends: string | null;
};
}
| {
Expand Down
10 changes: 10 additions & 0 deletions site/src/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub enum MissingReason {
include: Option<String>,
exclude: Option<String>,
runs: Option<i32>,
backends: Option<String>,
},
InProgress(Option<Box<MissingReason>>),
}
Expand Down Expand Up @@ -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)
Expand All @@ -407,6 +409,7 @@ fn calculate_missing_from(
include,
exclude,
runs,
backends,
},
));
}
Expand Down Expand Up @@ -579,6 +582,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
QueuedCommit {
sha: "b".into(),
Expand All @@ -588,6 +592,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
QueuedCommit {
sha: "a".into(),
Expand All @@ -597,6 +602,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
];
let in_progress_artifacts = vec![];
Expand Down Expand Up @@ -640,6 +646,7 @@ mod tests {
include: None,
exclude: None,
runs: None,
backends: None,
},
),
];
Expand Down Expand Up @@ -689,6 +696,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
// A try run
QueuedCommit {
Expand All @@ -699,6 +707,7 @@ mod tests {
exclude: None,
runs: None,
commit_date: None,
backends: None,
},
];
let in_progress_artifacts = vec![];
Expand Down Expand Up @@ -746,6 +755,7 @@ mod tests {
include: None,
exclude: None,
runs: None,
backends: None,
},
),
];
Expand Down
Loading
Loading