From 9c2f4def0db701672aa6048e2fbc96b98de16d99 Mon Sep 17 00:00:00 2001 From: James Barford-Evans Date: Fri, 3 Oct 2025 14:49:40 +0100 Subject: [PATCH 1/2] Fix; Runtime & Rustc benchmarks to run once --- collector/src/benchmark_set/mod.rs | 22 +++++----------- collector/src/bin/collector.rs | 21 ++++++++++----- database/src/lib.rs | 34 +++++++++++++++++++++++++ database/src/pool.rs | 12 ++++++--- database/src/pool/postgres.rs | 41 ++++++++++++++++++++---------- database/src/pool/sqlite.rs | 8 +++--- database/src/tests/builder.rs | 8 ++++-- site/src/job_queue/mod.rs | 35 ++++++++++++++++++++++--- 8 files changed, 134 insertions(+), 47 deletions(-) diff --git a/collector/src/benchmark_set/mod.rs b/collector/src/benchmark_set/mod.rs index 337bb1f4a..baf521acc 100644 --- a/collector/src/benchmark_set/mod.rs +++ b/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 { 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::>(); - 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()); } } diff --git a/collector/src/bin/collector.rs b/collector/src/bin/collector.rs index c8e838a23..cd8555c4d 100644 --- a/collector/src/bin/collector.rs +++ b/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; } } diff --git a/database/src/lib.rs b/database/src/lib.rs index c1e04a0c8..996dfc4dd 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -1132,6 +1132,7 @@ pub struct BenchmarkJob { created_at: DateTime, status: BenchmarkJobStatus, deque_counter: u32, + kind: BenchmarkJobKind, } impl BenchmarkJob { @@ -1179,6 +1180,10 @@ impl BenchmarkJob { pub fn created_at(&self) -> DateTime { 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, } + +#[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 { + 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")), + }) + } +} diff --git a/database/src/pool.rs b/database/src/pool.rs index d3a78dca7..e950757cb 100644 --- a/database/src/pool.rs +++ b/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>; /// 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); /// 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; diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index dd8ae9549..fa35bd14b 100644 --- a/database/src/pool/postgres.rs +++ b/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] @@ -1746,6 +1749,7 @@ where backend: CodegenBackend, profile: Profile, benchmark_set: u32, + kind: BenchmarkJobKind, ) -> (bool, anyhow::Result) { let row_result = self .conn() @@ -1757,9 +1761,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 "#, @@ -1770,6 +1775,7 @@ where &profile, &(benchmark_set as i32), &BENCHMARK_JOB_STATUS_QUEUED_STR, + &kind, ], ) .await; @@ -1804,6 +1810,7 @@ where backend: CodegenBackend, profile: Profile, benchmark_set: u32, + kind: BenchmarkJobKind, ) -> anyhow::Result> { // This will return zero rows if the job already exists let rows = self @@ -1816,9 +1823,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 "#, @@ -1829,6 +1837,7 @@ where &profile, &(benchmark_set as i32), &BENCHMARK_JOB_STATUS_QUEUED_STR, + &kind, ], ) .await @@ -2014,6 +2023,7 @@ where updated.created_at, updated.started_at, updated.retry, + updated.kind, br.commit_type, br.commit_date FROM updated @@ -2047,9 +2057,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>>(8); + let commit_type = row.get::<_, &str>(8); + let commit_date = row.get::<_, Option>>(9); let commit_date = Date(commit_date.ok_or_else(|| { anyhow::anyhow!("Dequeuing job for a benchmark request without commit date") @@ -2255,6 +2267,8 @@ where created_at: row.get::<_, DateTime>(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()) @@ -2422,6 +2436,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 { diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index ff6f259c8..eae0432e1 100644 --- a/database/src/pool/sqlite.rs +++ b/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> { no_queue_implementation_abort!() } @@ -1346,6 +1347,7 @@ impl Connection for SqliteConnection { _backend: CodegenBackend, _profile: Profile, _benchmark_set: u32, + _kind: BenchmarkJobKind, ) -> (bool, anyhow::Result) { no_queue_implementation_abort!() } diff --git a/database/src/tests/builder.rs b/database/src/tests/builder.rs index 0ea1f47d3..11020a551 100644 --- a/database/src/tests/builder.rs +++ b/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, } } } diff --git a/site/src/job_queue/mod.rs b/site/src/job_queue/mod.rs index 29e12e631..753301487 100644 --- a/site/src/job_queue/mod.rs +++ b/site/src/job_queue/mod.rs @@ -6,8 +6,8 @@ use crate::load::{partition_in_place, SiteCtxt}; use chrono::Utc; use collector::benchmark_set::benchmark_set_count; use database::{ - BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestType, Date, - PendingBenchmarkRequests, QueuedCommit, Target, + BenchmarkJobKind, BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, + BenchmarkRequestType, Date, PendingBenchmarkRequests, Profile, QueuedCommit, Target, }; use parking_lot::RwLock; use std::{str::FromStr, sync::Arc}; @@ -234,6 +234,30 @@ pub async fn enqueue_benchmark_request( // Target x benchmark_set x backend x profile -> BenchmarkJob for target in Target::all() { + // enqueue Runtime job and Rustc job, for each backend + for &backend in backends.iter() { + tx.conn() + .enqueue_benchmark_job( + request_tag, + target, + backend, + Profile::Opt, + 0u32, + BenchmarkJobKind::Runtime, + ) + .await?; + tx.conn() + .enqueue_benchmark_job( + request_tag, + target, + backend, + Profile::Opt, + 0u32, + BenchmarkJobKind::Rustc, + ) + .await?; + } + for benchmark_set in 0..benchmark_set_count(target.into()) { for &backend in backends.iter() { for &profile in profiles.iter() { @@ -244,6 +268,7 @@ pub async fn enqueue_benchmark_request( backend, profile, benchmark_set as u32, + BenchmarkJobKind::Compiletime, ) .await?; // If there is a parent, we create a job for it too. The @@ -264,6 +289,7 @@ pub async fn enqueue_benchmark_request( // backend, // profile, // benchmark_set as u32, + // BenchmarkJobKind::Compiletime, // ) // .await; // @@ -438,8 +464,8 @@ mod tests { use chrono::Utc; use database::tests::run_postgres_test; use database::{ - BenchmarkJobConclusion, BenchmarkRequest, BenchmarkRequestStatus, BenchmarkSet, - CodegenBackend, Profile, Target, + BenchmarkJobConclusion, BenchmarkJobKind, BenchmarkRequest, BenchmarkRequestStatus, + BenchmarkSet, CodegenBackend, Profile, Target, }; fn create_master(sha: &str, parent: &str, pr: u32) -> BenchmarkRequest { @@ -477,6 +503,7 @@ mod tests { CodegenBackend::Llvm, Profile::Opt, benchmark_set, + BenchmarkJobKind::Compiletime, ) .await .unwrap(); From e335e3657f3c881f93c31d065fd1e13cc9046869 Mon Sep 17 00:00:00 2001 From: James Barford-Evans Date: Mon, 6 Oct 2025 08:31:35 +0100 Subject: [PATCH 2/2] PR Feedback; have more specific enqueues of `Runtime` and `Rustc` benchmarks --- site/src/job_queue/mod.rs | 53 +++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/site/src/job_queue/mod.rs b/site/src/job_queue/mod.rs index 753301487..45622bf12 100644 --- a/site/src/job_queue/mod.rs +++ b/site/src/job_queue/mod.rs @@ -7,7 +7,8 @@ use chrono::Utc; use collector::benchmark_set::benchmark_set_count; use database::{ BenchmarkJobKind, BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, - BenchmarkRequestType, Date, PendingBenchmarkRequests, Profile, QueuedCommit, Target, + BenchmarkRequestType, CodegenBackend, Date, PendingBenchmarkRequests, Profile, QueuedCommit, + Target, }; use parking_lot::RwLock; use std::{str::FromStr, sync::Arc}; @@ -232,31 +233,35 @@ pub async fn enqueue_benchmark_request( // Prevent the error from spamming the logs // let mut has_emitted_parent_sha_error = false; + // Enqueue Rustc job for only for x86_64 & llvm. This benchmark is how long + // it takes to build the rust compiler. It takes a while to run and is + // assumed that if the compilation of other rust project improve then this + // too would improve. + tx.conn() + .enqueue_benchmark_job( + request_tag, + Target::X86_64UnknownLinuxGnu, + CodegenBackend::Llvm, + Profile::Opt, + 0u32, + BenchmarkJobKind::Rustc, + ) + .await?; + // Target x benchmark_set x backend x profile -> BenchmarkJob for target in Target::all() { - // enqueue Runtime job and Rustc job, for each backend - for &backend in backends.iter() { - tx.conn() - .enqueue_benchmark_job( - request_tag, - target, - backend, - Profile::Opt, - 0u32, - BenchmarkJobKind::Runtime, - ) - .await?; - tx.conn() - .enqueue_benchmark_job( - request_tag, - target, - backend, - Profile::Opt, - 0u32, - BenchmarkJobKind::Rustc, - ) - .await?; - } + // Enqueue Runtime job for all targets using LLVM as the backend for + // runtime benchmarks + tx.conn() + .enqueue_benchmark_job( + request_tag, + target, + CodegenBackend::Llvm, + Profile::Opt, + 0u32, + BenchmarkJobKind::Runtime, + ) + .await?; for benchmark_set in 0..benchmark_set_count(target.into()) { for &backend in backends.iter() {