Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 database/schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ Columns:
* `completed`: Completed request.
* **backends** (`text NOT NULL`): Comma-separated list of codegen backends to benchmark. If empty, the default set of codegen backends will be benchmarked.
* **profiles** (`text NOT NULL`): Comma-separated list of profiles to benchmark. If empty, the default set of profiles will be benchmarked.
* **targets** (`text NOT NULL`): Comma-separated list of targets to benchmark. If empty, the default `x86_64-unknown-linux-gnu` will be benchmarked.

### collector_config

Expand Down
50 changes: 42 additions & 8 deletions database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,14 @@ impl Target {
pub fn all() -> Vec<Self> {
vec![Self::X86_64UnknownLinuxGnu]
}

pub fn default_targets() -> Vec<Self> {
vec![Self::X86_64UnknownLinuxGnu]
}

pub fn is_optional(self) -> bool {
self != Target::X86_64UnknownLinuxGnu
}
}

impl FromStr for Target {
Expand Down Expand Up @@ -913,6 +921,7 @@ pub struct BenchmarkRequest {
status: BenchmarkRequestStatus,
backends: String,
profiles: String,
targets: String,
}

impl BenchmarkRequest {
Expand All @@ -927,11 +936,17 @@ impl BenchmarkRequest {
status: BenchmarkRequestStatus::ArtifactsReady,
backends: String::new(),
profiles: String::new(),
targets: String::new(),
}
}

/// Create a try request that is in the `WaitingForArtifacts` status.
pub fn create_try_without_artifacts(pr: u32, backends: &str, profiles: &str) -> Self {
pub fn create_try_without_artifacts(
pr: u32,
backends: &str,
profiles: &str,
targets: &str,
) -> Self {
Self {
commit_type: BenchmarkRequestType::Try {
pr,
Expand All @@ -943,6 +958,7 @@ impl BenchmarkRequest {
status: BenchmarkRequestStatus::WaitingForArtifacts,
backends: backends.to_string(),
profiles: profiles.to_string(),
targets: targets.to_string(),
}
}

Expand All @@ -959,6 +975,7 @@ impl BenchmarkRequest {
status: BenchmarkRequestStatus::ArtifactsReady,
backends: String::new(),
profiles: String::new(),
targets: String::new(),
}
}

Expand Down Expand Up @@ -1037,6 +1054,15 @@ impl BenchmarkRequest {
parse_profiles(&self.profiles).map_err(|e| anyhow::anyhow!("{e}"))
}

/// Get the targets for the request
pub fn targets(&self) -> anyhow::Result<Vec<Target>> {
if self.targets.trim().is_empty() {
return Ok(Target::default_targets());
}

parse_targets(&self.targets).map_err(|e| anyhow::anyhow!("{e}"))
}

pub fn is_completed(&self) -> bool {
matches!(self.status, BenchmarkRequestStatus::Completed { .. })
}
Expand All @@ -1056,18 +1082,26 @@ pub enum BenchmarkRequestInsertResult {
NothingInserted,
}

pub fn parse_backends(backends: &str) -> Result<Vec<CodegenBackend>, String> {
backends
fn parse_comma_separated<T>(raw_string: &str, name: &str) -> Result<Vec<T>, String>
where
T: FromStr,
{
raw_string
.split(',')
.map(|s| CodegenBackend::from_str(s).map_err(|_| format!("Invalid backend: {s}")))
.map(|s| T::from_str(s).map_err(|_| format!("Invalid {name}: {s}")))
.collect()
}

pub fn parse_backends(backends: &str) -> Result<Vec<CodegenBackend>, String> {
parse_comma_separated(backends, "backend")
}

pub fn parse_profiles(profiles: &str) -> Result<Vec<Profile>, String> {
profiles
.split(',')
.map(|s| Profile::from_str(s).map_err(|_| format!("Invalid profile: {s}")))
.collect()
parse_comma_separated(profiles, "profile")
}

pub fn parse_targets(targets: &str) -> Result<Vec<Target>, String> {
parse_comma_separated(targets, "target")
}

/// Cached information about benchmark requests in the DB
Expand Down
4 changes: 2 additions & 2 deletions database/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ mod tests {
// But this should fail, as we can't have two queued requests at once
let result = db
.insert_benchmark_request(&BenchmarkRequest::create_try_without_artifacts(
42, "", "",
42, "", "", "",
))
.await
.unwrap();
Expand Down Expand Up @@ -675,7 +675,7 @@ mod tests {
run_postgres_test(|ctx| async {
let db = ctx.db();

let req = BenchmarkRequest::create_try_without_artifacts(42, "", "");
let req = BenchmarkRequest::create_try_without_artifacts(42, "", "", "");

db.insert_benchmark_request(&req).await.unwrap();
assert!(db
Expand Down
11 changes: 8 additions & 3 deletions database/src/pool/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ static MIGRATIONS: &[&str] = &[
ALTER TABLE runtime_pstat_series ADD CONSTRAINT runtime_test_case UNIQUE(benchmark, target, metric);
"#,
r#"ALTER TABLE job_queue ADD COLUMN is_optional BOOLEAN NOT NULL DEFAULT FALSE"#,
r#"ALTER TABLE benchmark_request ADD COLUMN targets TEXT NOT NULL DEFAULT 'x86_64-unknown-linux-gnu'"#,
];

#[async_trait::async_trait]
Expand Down Expand Up @@ -801,7 +802,7 @@ impl PostgresConnection {

// `tag` should be kept as the first column
const BENCHMARK_REQUEST_COLUMNS: &str =
"tag, parent_sha, pr, commit_type, status, created_at, completed_at, backends, profiles, commit_date, duration_ms";
"tag, parent_sha, pr, commit_type, status, created_at, completed_at, backends, profiles, commit_date, duration_ms, targets";

#[async_trait::async_trait]
impl<P> Connection for P
Expand Down Expand Up @@ -2193,8 +2194,8 @@ where

for row in rows {
let tag = row.get::<_, &str>(0);
let error_benchmark = row.get::<_, Option<String>>(11);
let error_content = row.get::<_, Option<String>>(12);
let error_benchmark = row.get::<_, Option<String>>(12);
let error_content = row.get::<_, Option<String>>(13);

// We already saw this request, just add errors
if let Some(errors) = errors.get_mut(tag) {
Expand Down Expand Up @@ -2350,6 +2351,7 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRe
let profiles = row.get::<_, String>(8 + row_offset);
let commit_date = row.get::<_, Option<DateTime<Utc>>>(9 + row_offset);
let duration_ms = row.get::<_, Option<i32>>(10 + row_offset);
let targets = row.get::<_, String>(11 + row_offset);

let pr = pr.map(|v| v as u32);

Expand All @@ -2371,6 +2373,7 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRe
status,
backends,
profiles,
targets,
},
BENCHMARK_REQUEST_MASTER_STR => BenchmarkRequest {
commit_type: BenchmarkRequestType::Master {
Expand All @@ -2383,6 +2386,7 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRe
status,
backends,
profiles,
targets,
},
BENCHMARK_REQUEST_RELEASE_STR => BenchmarkRequest {
commit_type: BenchmarkRequestType::Release {
Expand All @@ -2393,6 +2397,7 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRe
status,
backends,
profiles,
targets,
},
_ => panic!("Invalid `commit_type` for `BenchmarkRequest` {commit_type}",),
}
Expand Down
2 changes: 1 addition & 1 deletion database/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl TestContext {

/// Create a new try benchmark request without artifacts and add it to the DB.
pub async fn insert_try_request(&self, pr: u32) -> BenchmarkRequest {
let req = BenchmarkRequest::create_try_without_artifacts(pr, "", "");
let req = BenchmarkRequest::create_try_without_artifacts(pr, "", "", "");
self.db().insert_benchmark_request(&req).await.unwrap();
req
}
Expand Down
7 changes: 4 additions & 3 deletions site/src/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ pub async fn enqueue_benchmark_request(

let backends = request.backends()?;
let profiles = request.profiles()?;
let targets = request.targets()?;

#[derive(PartialEq, Debug)]
enum EnqueueMode {
Expand Down Expand Up @@ -327,12 +328,12 @@ pub async fn enqueue_benchmark_request(
};

// Target x benchmark_set x backend x profile -> BenchmarkJob
for target in Target::all() {
for &target in targets.iter() {
// We only have X86_64 at the moment and when we add other targets do
// not want to block the Benchmark request from completing to wait on
// other targets. Essentially, for the time being, other targets will
// run in the background
let is_optional = target != Target::X86_64UnknownLinuxGnu;
let is_optional = target.is_optional();

for benchmark_set in 0..get_benchmark_sets_for_target(target.into()).len() {
for &backend in backends.iter() {
Expand Down Expand Up @@ -629,7 +630,7 @@ mod tests {
}

fn create_try(pr: u32) -> BenchmarkRequest {
BenchmarkRequest::create_try_without_artifacts(pr, "", "")
BenchmarkRequest::create_try_without_artifacts(pr, "", "", "")
}

fn create_release(tag: &str) -> BenchmarkRequest {
Expand Down
Loading
Loading