Merge pull request #2264 from Jamesbarford/fix/job-queue-job_kind

Fix; Runtime & Rustc benchmarks to run once

Author: Kobzol

collector/src/benchmark_set/mod.rs

@@ -29,11 +29,6 @@ impl BenchmarkSetId {
 pub enum BenchmarkSetMember {
     /// Benchmark a specific compile-time benchmark
     CompileBenchmark(BenchmarkName),
-    /// Benchmark *all* the runtime benchmarks.
-    /// For simplicity, we currently always benchmark all of them on a single collector.
-    RuntimeBenchmarks,
-    /// Benchmark the rustc bootstrap
-    Rustc,
 }
 
 /// Return the number of benchmark sets for the given target.
@@ -102,8 +97,6 @@ pub fn expand_benchmark_set(id: BenchmarkSetId) -> Vec<BenchmarkSetMember> {
                 compile(UNUSED_WARNINGS),
                 compile(WF_PROJECTION_STRESS_65510),
                 compile(WG_GRAMMAR),
-                BenchmarkSetMember::Rustc,
-                BenchmarkSetMember::RuntimeBenchmarks,
             ]
         }
         (Target::X86_64UnknownLinuxGnu, 1..) => {
@@ -169,8 +162,6 @@ mod tests {
 
         // Check that the union of all sets contains all the required benchmarks
         let all_members = sets.iter().flatten().collect::<HashSet<_>>();
-        assert!(all_members.contains(&BenchmarkSetMember::Rustc));
-        assert!(all_members.contains(&BenchmarkSetMember::RuntimeBenchmarks));
 
         const BENCHMARK_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/compile-benchmarks");
         let all_compile_benchmarks =
@@ -187,13 +178,12 @@ mod tests {
             );
         }
         for benchmark in &all_members {
-            if let BenchmarkSetMember::CompileBenchmark(name) = benchmark {
-                assert!(
-                    all_compile_benchmarks.contains(name),
-                    "Compile-time benchmark {name} does not exist on disk or is a stable benchmark"
-                );
-            }
+            let BenchmarkSetMember::CompileBenchmark(name) = benchmark;
+            assert!(
+                all_compile_benchmarks.contains(name),
+                "Compile-time benchmark {name} does not exist on disk or is a stable benchmark"
+            );
         }
-        assert_eq!(all_members.len(), all_compile_benchmarks.len() + 2);
+        assert_eq!(all_members.len(), all_compile_benchmarks.len());
     }
 }

collector/src/bin/collector.rs

@@ -1699,13 +1699,22 @@ async fn create_benchmark_configs(
     let mut bench_rustc = false;
     let mut bench_runtime = false;
     let mut bench_compile_benchmarks = HashSet::new();
-    for member in benchmark_set_members {
-        match member {
-            BenchmarkSetMember::CompileBenchmark(benchmark) => {
-                bench_compile_benchmarks.insert(benchmark);
+
+    match job.kind() {
+        database::BenchmarkJobKind::Runtime => {
+            bench_runtime = true;
+        }
+        database::BenchmarkJobKind::Compiletime => {
+            for member in benchmark_set_members {
+                match member {
+                    BenchmarkSetMember::CompileBenchmark(benchmark) => {
+                        bench_compile_benchmarks.insert(benchmark);
+                    }
+                }
             }
-            BenchmarkSetMember::RuntimeBenchmarks => bench_runtime = true,
-            BenchmarkSetMember::Rustc => bench_rustc = true,
+        }
+        database::BenchmarkJobKind::Rustc => {
+            bench_rustc = true;
         }
     }
 

database/src/lib.rs

@@ -1132,6 +1132,7 @@ pub struct BenchmarkJob {
     created_at: DateTime<Utc>,
     status: BenchmarkJobStatus,
     deque_counter: u32,
+    kind: BenchmarkJobKind,
 }
 
 impl BenchmarkJob {
@@ -1179,6 +1180,10 @@ impl BenchmarkJob {
     pub fn created_at(&self) -> DateTime<Utc> {
         self.created_at
     }
+
+    pub fn kind(&self) -> BenchmarkJobKind {
+        self.kind
+    }
 }
 
 /// Describes the final state of a job
@@ -1256,3 +1261,32 @@ pub struct BenchmarkRequestWithErrors {
     /// Benchmark (name) -> error
     pub errors: HashMap<String, String>,
 }
+
+#[derive(Debug, PartialEq, Clone, Copy, serde::Deserialize, serde::Serialize)]
+pub enum BenchmarkJobKind {
+    Runtime,
+    Compiletime,
+    Rustc,
+}
+
+impl fmt::Display for BenchmarkJobKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            BenchmarkJobKind::Runtime => write!(f, "runtime"),
+            BenchmarkJobKind::Compiletime => write!(f, "compiletime"),
+            BenchmarkJobKind::Rustc => write!(f, "rustc"),
+        }
+    }
+}
+
+impl FromStr for BenchmarkJobKind {
+    type Err = String;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s.to_ascii_lowercase().as_str() {
+            "runtime" => BenchmarkJobKind::Runtime,
+            "compiletime" => BenchmarkJobKind::Compiletime,
+            "rustc" => BenchmarkJobKind::Rustc,
+            _ => return Err(format!("{s} is not a codegen backend")),
+        })
+    }
+}

database/src/pool.rs

@@ -1,9 +1,9 @@
 use crate::selector::CompileTestCase;
 use crate::{
     ArtifactCollection, ArtifactId, ArtifactIdNumber, BenchmarkJob, BenchmarkJobConclusion,
-    BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestWithErrors,
-    BenchmarkSet, CodegenBackend, CollectorConfig, CompileBenchmark, PendingBenchmarkRequests,
-    Target,
+    BenchmarkJobKind, BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus,
+    BenchmarkRequestWithErrors, BenchmarkSet, CodegenBackend, CollectorConfig, CompileBenchmark,
+    PendingBenchmarkRequests, Target,
 };
 use crate::{CollectionId, Index, Profile, QueuedCommit, Scenario, Step};
 use chrono::{DateTime, Utc};
@@ -233,6 +233,7 @@ pub trait Connection: Send + Sync {
         backend: CodegenBackend,
         profile: Profile,
         benchmark_set: u32,
+        kind: BenchmarkJobKind,
     ) -> anyhow::Result<Option<u32>>;
 
     /// Add a benchmark job which is explicitly using a `parent_sha` we split
@@ -245,6 +246,7 @@ pub trait Connection: Send + Sync {
         backend: CodegenBackend,
         profile: Profile,
         benchmark_set: u32,
+        kind: BenchmarkJobKind,
     ) -> (bool, anyhow::Result<u32>);
 
     /// Returns a set of compile-time benchmark test cases that were already computed for the
@@ -706,6 +708,7 @@ mod tests {
                     CodegenBackend::Llvm,
                     Profile::Opt,
                     0u32,
+                    BenchmarkJobKind::Runtime,
                 )
                 .await;
             assert!(result.is_ok());
@@ -860,6 +863,7 @@ mod tests {
                 CodegenBackend::Llvm,
                 Profile::Opt,
                 1u32,
+                BenchmarkJobKind::Runtime,
             )
             .await
             .unwrap();
@@ -956,6 +960,7 @@ mod tests {
                 CodegenBackend::Llvm,
                 Profile::Opt,
                 benchmark_set.0,
+                BenchmarkJobKind::Runtime,
             )
             .await
             .unwrap();
@@ -1213,6 +1218,7 @@ mod tests {
                     CodegenBackend::Llvm,
                     Profile::Debug,
                     0,
+                    BenchmarkJobKind::Runtime,
                 )
                 .await;
 

database/src/pool/postgres.rs

@@ -2,13 +2,13 @@ use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction}
 use crate::selector::CompileTestCase;
 use crate::{
     ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, BenchmarkJob,
-    BenchmarkJobConclusion, BenchmarkJobStatus, BenchmarkRequest, BenchmarkRequestIndex,
-    BenchmarkRequestStatus, BenchmarkRequestType, BenchmarkRequestWithErrors, BenchmarkSet,
-    CodegenBackend, CollectionId, CollectorConfig, Commit, CommitType, CompileBenchmark, Date,
-    Index, PendingBenchmarkRequests, Profile, QueuedCommit, Scenario, Target,
-    BENCHMARK_JOB_STATUS_FAILURE_STR, BENCHMARK_JOB_STATUS_IN_PROGRESS_STR,
-    BENCHMARK_JOB_STATUS_QUEUED_STR, BENCHMARK_JOB_STATUS_SUCCESS_STR,
-    BENCHMARK_REQUEST_MASTER_STR, BENCHMARK_REQUEST_RELEASE_STR,
+    BenchmarkJobConclusion, BenchmarkJobKind, BenchmarkJobStatus, BenchmarkRequest,
+    BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestType,
+    BenchmarkRequestWithErrors, BenchmarkSet, CodegenBackend, CollectionId, CollectorConfig,
+    Commit, CommitType, CompileBenchmark, Date, Index, PendingBenchmarkRequests, Profile,
+    QueuedCommit, Scenario, Target, BENCHMARK_JOB_STATUS_FAILURE_STR,
+    BENCHMARK_JOB_STATUS_IN_PROGRESS_STR, BENCHMARK_JOB_STATUS_QUEUED_STR,
+    BENCHMARK_JOB_STATUS_SUCCESS_STR, BENCHMARK_REQUEST_MASTER_STR, BENCHMARK_REQUEST_RELEASE_STR,
     BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR, BENCHMARK_REQUEST_STATUS_COMPLETED_STR,
     BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR, BENCHMARK_REQUEST_STATUS_WAITING_FOR_ARTIFACTS_STR,
     BENCHMARK_REQUEST_TRY_STR,
@@ -417,6 +417,9 @@ static MIGRATIONS: &[&str] = &[
     r#"
     CREATE INDEX benchmark_request_completed_idx ON benchmark_request(completed_at);
     "#,
+    r#"
+    ALTER TABLE job_queue ADD COLUMN kind TEXT NOT NULL;
+    "#,
 ];
 
 #[async_trait::async_trait]
@@ -1753,6 +1756,7 @@ where
         backend: CodegenBackend,
         profile: Profile,
         benchmark_set: u32,
+        kind: BenchmarkJobKind,
     ) -> (bool, anyhow::Result<u32>) {
         let row_result = self
             .conn()
@@ -1764,9 +1768,10 @@ where
                 backend,
                 profile,
                 benchmark_set,
-                status
+                status,
+                kind
             )
-            VALUES ($1, $2, $3, $4, $5, $6)
+            VALUES ($1, $2, $3, $4, $5, $6, $7)
             ON CONFLICT DO NOTHING
             RETURNING job_queue.id
                 "#,
@@ -1777,6 +1782,7 @@ where
                     &profile,
                     &(benchmark_set as i32),
                     &BENCHMARK_JOB_STATUS_QUEUED_STR,
+                    &kind,
                 ],
             )
             .await;
@@ -1811,6 +1817,7 @@ where
         backend: CodegenBackend,
         profile: Profile,
         benchmark_set: u32,
+        kind: BenchmarkJobKind,
     ) -> anyhow::Result<Option<u32>> {
         // This will return zero rows if the job already exists
         let rows = self
@@ -1823,9 +1830,10 @@ where
                 backend,
                 profile,
                 benchmark_set,
-                status
+                status,
+                kind
             )
-            VALUES ($1, $2, $3, $4, $5, $6)
+            VALUES ($1, $2, $3, $4, $5, $6, $7)
             ON CONFLICT DO NOTHING
             RETURNING job_queue.id
                 "#,
@@ -1836,6 +1844,7 @@ where
                     &profile,
                     &(benchmark_set as i32),
                     &BENCHMARK_JOB_STATUS_QUEUED_STR,
+                    &kind,
                 ],
             )
             .await
@@ -2021,6 +2030,7 @@ where
                     updated.created_at,
                     updated.started_at,
                     updated.retry,
+                    updated.kind,
                     br.commit_type,
                     br.commit_date
                 FROM updated
@@ -2054,9 +2064,11 @@ where
                         collector_name: collector_name.into(),
                     },
                     deque_counter: row.get::<_, i32>(6) as u32,
+                    kind: BenchmarkJobKind::from_str(row.get::<_, &str>(7))
+                        .map_err(|e| anyhow::anyhow!(e))?,
                 };
-                let commit_type = row.get::<_, &str>(7);
-                let commit_date = row.get::<_, Option<DateTime<Utc>>>(8);
+                let commit_type = row.get::<_, &str>(8);
+                let commit_date = row.get::<_, Option<DateTime<Utc>>>(9);
 
                 let commit_date = Date(commit_date.ok_or_else(|| {
                     anyhow::anyhow!("Dequeuing job for a benchmark request without commit date")
@@ -2262,6 +2274,8 @@ where
                 created_at: row.get::<_, DateTime<Utc>>(6),
                 status,
                 deque_counter: row.get::<_, i32>(10) as u32,
+                kind: BenchmarkJobKind::from_str(row.get::<_, &str>(12))
+                    .map_err(|e| anyhow::anyhow!(e))?,
             };
             request_to_jobs
                 .entry(job.request_tag.clone())
@@ -2429,6 +2443,7 @@ impl_to_postgresql_via_to_string!(BenchmarkRequestType);
 impl_to_postgresql_via_to_string!(Target);
 impl_to_postgresql_via_to_string!(CodegenBackend);
 impl_to_postgresql_via_to_string!(Profile);
+impl_to_postgresql_via_to_string!(BenchmarkJobKind);
 
 #[cfg(test)]
 mod tests {

database/src/pool/sqlite.rs

@@ -2,9 +2,9 @@ use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction}
 use crate::selector::CompileTestCase;
 use crate::{
     ArtifactCollection, ArtifactId, Benchmark, BenchmarkJob, BenchmarkJobConclusion,
-    BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestWithErrors,
-    BenchmarkSet, CodegenBackend, CollectionId, CollectorConfig, Commit, CommitType,
-    CompileBenchmark, Date, PendingBenchmarkRequests, Profile, Target,
+    BenchmarkJobKind, BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus,
+    BenchmarkRequestWithErrors, BenchmarkSet, CodegenBackend, CollectionId, CollectorConfig,
+    Commit, CommitType, CompileBenchmark, Date, PendingBenchmarkRequests, Profile, Target,
 };
 use crate::{ArtifactIdNumber, Index, QueuedCommit};
 use chrono::{DateTime, TimeZone, Utc};
@@ -1335,6 +1335,7 @@ impl Connection for SqliteConnection {
         _backend: CodegenBackend,
         _profile: Profile,
         _benchmark_set: u32,
+        _kind: BenchmarkJobKind,
     ) -> anyhow::Result<Option<u32>> {
         no_queue_implementation_abort!()
     }
@@ -1346,6 +1347,7 @@ impl Connection for SqliteConnection {
         _backend: CodegenBackend,
         _profile: Profile,
         _benchmark_set: u32,
+        _kind: BenchmarkJobKind,
     ) -> (bool, anyhow::Result<u32>) {
         no_queue_implementation_abort!()
     }

database/src/tests/builder.rs

@@ -1,6 +1,7 @@
 use crate::{
-    BenchmarkJob, BenchmarkJobConclusion, BenchmarkRequest, BenchmarkRequestStatus, BenchmarkSet,
-    CodegenBackend, CollectorConfig, Connection, Profile, Target,
+    BenchmarkJob, BenchmarkJobConclusion, BenchmarkJobKind, BenchmarkRequest,
+    BenchmarkRequestStatus, BenchmarkSet, CodegenBackend, CollectorConfig, Connection, Profile,
+    Target,
 };
 use chrono::Utc;
 use hashbrown::{HashMap, HashSet};
@@ -46,6 +47,7 @@ impl RequestBuilder {
                     job.backend,
                     job.profile,
                     job.benchmark_set,
+                    job.kind,
                 )
                 .await
                 .unwrap();
@@ -109,6 +111,7 @@ pub struct JobBuilder {
     profile: Profile,
     benchmark_set: u32,
     conclusion: BenchmarkJobConclusion,
+    kind: BenchmarkJobKind,
 }
 
 impl JobBuilder {
@@ -126,6 +129,7 @@ impl Default for JobBuilder {
             profile: Profile::Check,
             benchmark_set: 0,
             conclusion: BenchmarkJobConclusion::Success,
+            kind: BenchmarkJobKind::Compiletime,
         }
     }
 }