diff --git a/collector/src/bin/collector.rs b/collector/src/bin/collector.rs index 7f7c5ab59..c9ccd2425 100644 --- a/collector/src/bin/collector.rs +++ b/collector/src/bin/collector.rs @@ -121,14 +121,21 @@ struct RuntimeBenchmarkConfig { runtime_suite: BenchmarkSuite, filter: RuntimeBenchmarkFilter, iterations: u32, + target: Target, } impl RuntimeBenchmarkConfig { - fn new(suite: BenchmarkSuite, filter: RuntimeBenchmarkFilter, iterations: u32) -> Self { + fn new( + suite: BenchmarkSuite, + filter: RuntimeBenchmarkFilter, + iterations: u32, + target: Target, + ) -> Self { Self { runtime_suite: suite.filter(&filter), filter, iterations, + target, } } } @@ -860,8 +867,9 @@ fn main_result() -> anyhow::Result { purge, } => { log_db(&db); - let toolchain = - get_local_toolchain_for_runtime_benchmarks(&local, &require_host_target_tuple())?; + + let host_tuple = require_host_target_tuple(); + let toolchain = get_local_toolchain_for_runtime_benchmarks(&local, &host_tuple)?; let pool = Pool::open(&db.db); let isolation_mode = if no_isolate { @@ -899,6 +907,7 @@ fn main_result() -> anyhow::Result { runtime_suite, RuntimeBenchmarkFilter::new(local.exclude, local.include), iterations, + Target::from_str(&host_tuple).expect("Found unexpected host target"), ); rt.block_on(run_benchmarks(conn.as_mut(), shared, None, Some(config)))?; Ok(0) @@ -1189,6 +1198,7 @@ fn main_result() -> anyhow::Result { runtime_suite, filter: RuntimeBenchmarkFilter::keep_all(), iterations: DEFAULT_RUNTIME_ITERATIONS, + target: Target::default(), }; let shared = SharedBenchmarkConfig { artifact_id, @@ -1806,6 +1816,7 @@ async fn create_benchmark_configs( runtime_suite, filter: RuntimeBenchmarkFilter::keep_all(), iterations: DEFAULT_RUNTIME_ITERATIONS, + target: job.target().into(), }) } else { None @@ -2232,6 +2243,7 @@ async fn run_benchmarks( &collector, runtime.filter, runtime.iterations, + runtime.target, ) .await .context("Runtime benchmarks failed") @@ -2306,6 +2318,7 @@ async fn bench_published_artifact( runtime_suite, RuntimeBenchmarkFilter::keep_all(), DEFAULT_RUNTIME_ITERATIONS, + Target::default(), )), ) .await diff --git a/collector/src/runtime/mod.rs b/collector/src/runtime/mod.rs index 94b4a7335..b906f00a3 100644 --- a/collector/src/runtime/mod.rs +++ b/collector/src/runtime/mod.rs @@ -20,6 +20,7 @@ use crate::{command_output, CollectorCtx}; mod benchmark; mod profile; +use crate::compile::benchmark::target::Target; pub use benchmark::RuntimeCompilationOpts; pub use profile::{profile_runtime, RuntimeProfiler}; @@ -35,6 +36,7 @@ pub async fn bench_runtime( collector: &CollectorCtx, filter: RuntimeBenchmarkFilter, iterations: u32, + target: Target, ) -> anyhow::Result<()> { let filtered = suite.filtered_benchmark_count(&filter); println!("Executing {filtered} benchmarks\n"); @@ -74,6 +76,7 @@ pub async fn bench_runtime( tx.conn(), collector.artifact_row_id, &rustc_perf_version, + target, result, ) .await; @@ -126,6 +129,7 @@ async fn record_stats( conn: &dyn Connection, artifact_id: ArtifactIdNumber, rustc_perf_version: &str, + target: Target, result: BenchmarkResult, ) { async fn record<'a>( @@ -133,6 +137,7 @@ async fn record_stats( artifact_id: ArtifactIdNumber, collection_id: CollectionId, result: &'a BenchmarkResult, + target: Target, value: Option, metric: &'a str, ) { @@ -142,6 +147,7 @@ async fn record_stats( artifact_id, &result.name, metric, + target.into(), value as f64, ) .await; @@ -156,6 +162,7 @@ async fn record_stats( artifact_id, collection_id, &result, + target, stat.instructions, "instructions:u", ) @@ -165,6 +172,7 @@ async fn record_stats( artifact_id, collection_id, &result, + target, stat.cycles, "cycles:u", ) @@ -174,6 +182,7 @@ async fn record_stats( artifact_id, collection_id, &result, + target, stat.branch_misses, "branch-misses", ) @@ -183,6 +192,7 @@ async fn record_stats( artifact_id, collection_id, &result, + target, stat.cache_misses, "cache-misses", ) @@ -192,6 +202,7 @@ async fn record_stats( artifact_id, collection_id, &result, + target, Some(stat.wall_time.as_nanos() as u64), "wall-time", ) diff --git a/database/src/bin/import-sqlite.rs b/database/src/bin/import-sqlite.rs index 8b47e4992..c30512315 100644 --- a/database/src/bin/import-sqlite.rs +++ b/database/src/bin/import-sqlite.rs @@ -82,7 +82,7 @@ async fn main() { } } - for (&(benchmark, metric), id) in sqlite_idx.runtime_statistic_descriptions() { + for (&(benchmark, target, metric), id) in sqlite_idx.runtime_statistic_descriptions() { let stat = sqlite_conn .get_runtime_pstats(&[id], &[Some(sqlite_aid)]) .await @@ -92,7 +92,14 @@ async fn main() { .unwrap(); if let Some(stat) = stat { postgres_conn - .record_runtime_statistic(cid, postgres_aid, &benchmark, metric.as_str(), stat) + .record_runtime_statistic( + cid, + postgres_aid, + &benchmark, + metric.as_str(), + target, + stat, + ) .await; } } diff --git a/database/src/bin/postgres-to-sqlite.rs b/database/src/bin/postgres-to-sqlite.rs index 20615fd34..5a57c3316 100644 --- a/database/src/bin/postgres-to-sqlite.rs +++ b/database/src/bin/postgres-to-sqlite.rs @@ -397,11 +397,11 @@ impl Table for RuntimePstatSeries { } fn postgres_select_statement(&self, _since_weeks_ago: Option) -> String { - "select id, benchmark, metric from ".to_string() + self.name() + "SELECT id, benchmark, target, metric FROM ".to_string() + self.name() } fn sqlite_insert_statement(&self) -> &'static str { - "insert into runtime_pstat_series (id, benchmark, metric) VALUES (?, ?, ?)" + "INSERT INTO runtime_pstat_series (id, benchmark, target, metric) VALUES (?, ?, ?, ?)" } fn sqlite_execute_insert(&self, statement: &mut rusqlite::Statement, row: tokio_postgres::Row) { @@ -410,6 +410,7 @@ impl Table for RuntimePstatSeries { row.get::<_, i32>(0), row.get::<_, &str>(1), row.get::<_, &str>(2), + row.get::<_, &str>(3), ]) .unwrap(); } diff --git a/database/src/bin/sqlite-to-postgres.rs b/database/src/bin/sqlite-to-postgres.rs index a290dea95..35eefd62b 100644 --- a/database/src/bin/sqlite-to-postgres.rs +++ b/database/src/bin/sqlite-to-postgres.rs @@ -543,6 +543,7 @@ struct RuntimePstatSeries; struct RuntimePstatSeriesRow<'a> { id: i32, benchmark: &'a str, + target: &'a str, metric: &'a str, } @@ -552,11 +553,11 @@ impl Table for RuntimePstatSeries { } fn sqlite_attributes() -> &'static str { - "id, benchmark, metric" + "id, benchmark, target, metric" } fn postgres_attributes() -> &'static str { - "id, benchmark, metric" + "id, benchmark, target, metric" } fn postgres_generated_id_attribute() -> Option<&'static str> { @@ -568,7 +569,8 @@ impl Table for RuntimePstatSeries { .serialize(RuntimePstatSeriesRow { id: row.get(0).unwrap(), benchmark: row.get_ref(1).unwrap().as_str().unwrap(), - metric: row.get_ref(2).unwrap().as_str().unwrap(), + target: row.get_ref(2).unwrap().as_str().unwrap(), + metric: row.get_ref(3).unwrap().as_str().unwrap(), }) .unwrap(); } diff --git a/database/src/lib.rs b/database/src/lib.rs index 996dfc4dd..bac85fd55 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -522,7 +522,7 @@ pub struct Index { /// For legacy reasons called `pstat_series` in the database, and so the name is kept here. pstat_series: Indexed<(Benchmark, Profile, Scenario, CodegenBackend, Target, Metric)>, /// Id lookup of runtime stat description ids - runtime_pstat_series: Indexed<(Benchmark, Metric)>, + runtime_pstat_series: Indexed<(Benchmark, Target, Metric)>, } /// An index lookup @@ -725,7 +725,7 @@ impl Index { self.runtime_pstat_series .map .keys() - .map(|(_, metric)| metric) + .map(|(_, _, metric)| metric) .collect::>() .into_iter() .map(|s| s.to_string()) @@ -752,7 +752,7 @@ impl Index { pub fn runtime_statistic_descriptions( &self, - ) -> impl Iterator + '_ { + ) -> impl Iterator + '_ { self.runtime_pstat_series .map .iter() diff --git a/database/src/pool.rs b/database/src/pool.rs index e79fe6d75..5269be392 100644 --- a/database/src/pool.rs +++ b/database/src/pool.rs @@ -64,6 +64,7 @@ pub trait Connection: Send + Sync { artifact: ArtifactIdNumber, benchmark: &str, metric: &str, + target: Target, value: f64, ); /// Records a self-profile artifact in S3. diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index c52e676a2..f96161245 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -432,6 +432,12 @@ static MIGRATIONS: &[&str] = &[ benchmark_set ); "#, + // Add target to runtime_pstat_series + r#" + ALTER TABLE runtime_pstat_series ADD target TEXT NOT NULL DEFAULT 'x86_64-unknown-linux-gnu'; + ALTER TABLE runtime_pstat_series DROP CONSTRAINT runtime_pstat_series_benchmark_metric_key; + ALTER TABLE runtime_pstat_series ADD CONSTRAINT runtime_test_case UNIQUE(benchmark, target, metric); + "#, ]; #[async_trait::async_trait] @@ -660,8 +666,8 @@ impl PostgresConnection { select name, category from benchmark ").await.unwrap(), - insert_runtime_pstat_series: conn.prepare("insert into runtime_pstat_series (benchmark, metric) VALUES ($1, $2) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(), - select_runtime_pstat_series: conn.prepare("select id from runtime_pstat_series where benchmark = $1 and metric = $2").await.unwrap(), + insert_runtime_pstat_series: conn.prepare("INSERT INTO runtime_pstat_series (benchmark, target, metric) VALUES ($1, $2, $3) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(), + select_runtime_pstat_series: conn.prepare("SELECT id FROM runtime_pstat_series WHERE benchmark = $1 AND target = $2 AND metric = $3").await.unwrap(), insert_runtime_pstat: conn .prepare("insert into runtime_pstat (series, aid, cid, value) VALUES ($1, $2, $3, $4)") .await @@ -883,7 +889,7 @@ where runtime_pstat_series: self .conn() .query( - "select id, benchmark, metric from runtime_pstat_series;", + "SELECT id, benchmark, target, metric FROM runtime_pstat_series;", &[], ) .await @@ -893,8 +899,9 @@ where ( row.get::<_, i32>(0) as u32, ( - row.get::<_, String>(1).as_str().into(), - row.get::<_, String>(2).as_str().into(), + row.get::<_, &str>(1).into(), + Target::from_str(row.get::<_, &str>(2)).unwrap(), + row.get::<_, &str>(3).into(), ), ) }) @@ -1123,13 +1130,15 @@ where artifact: ArtifactIdNumber, benchmark: &str, metric: &str, + target: Target, value: f64, ) { + let target = target.to_string(); let sid = self .conn() .query_opt( &self.statements().select_runtime_pstat_series, - &[&benchmark, &metric], + &[&benchmark, &target, &metric], ) .await .unwrap(); @@ -1139,14 +1148,14 @@ where self.conn() .query_opt( &self.statements().insert_runtime_pstat_series, - &[&benchmark, &metric], + &[&benchmark, &target, &metric], ) .await .unwrap(); self.conn() .query_one( &self.statements().select_runtime_pstat_series, - &[&benchmark, &metric], + &[&benchmark, &target, &metric], ) .await .unwrap() diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index 6434ddd54..b192ddd38 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -431,6 +431,21 @@ static MIGRATIONS: &[Migration] = &[ CREATE INDEX error_artifact_idx ON error(aid); "#, ), + // Add runtime target as a unique constraint, defaulting to 'x86_64-unknown-linux-gnu' + Migration::without_foreign_key_constraints( + r#" + CREATE TABLE runtime_pstat_series_with_target( + id integer primary key not null, + benchmark text not null, + target text not null default 'x86_64-unknown-linux-gnu', + metric text not null, + UNIQUE(benchmark, target, metric) + ); + INSERT INTO runtime_pstat_series_with_target SELECT id, benchmark, 'x86_64-unknown-linux-gnu', metric FROM runtime_pstat_series; + DROP TABLE runtime_pstat_series; + ALTER TABLE runtime_pstat_series_with_target RENAME TO runtime_pstat_series; + "#, + ), ]; #[async_trait::async_trait] @@ -579,14 +594,15 @@ impl Connection for SqliteConnection { .collect(); let runtime_pstat_series = self .raw() - .prepare("select id, benchmark, metric from runtime_pstat_series;") + .prepare("SELECT id, benchmark, target, metric FROM runtime_pstat_series;") .unwrap() .query_map(params![], |row| { Ok(( row.get::<_, i32>(0)? as u32, ( row.get::<_, String>(1)?.as_str().into(), - row.get::<_, String>(2)?.as_str().into(), + Target::from_str(row.get::<_, String>(2)?.as_str()).unwrap(), + row.get::<_, String>(3)?.as_str().into(), ), )) }) @@ -750,25 +766,27 @@ impl Connection for SqliteConnection { artifact: ArtifactIdNumber, benchmark: &str, metric: &str, + target: Target, value: f64, ) { + let target = target.to_string(); self.raw_ref() .execute( - "insert or ignore into runtime_pstat_series (benchmark, metric) VALUES (?, ?)", - params![&benchmark, &metric,], + "INSERT OR IGNORE INTO runtime_pstat_series (benchmark, target, metric) VALUES (?, ?, ?)", + params![&benchmark, &target, &metric], ) .unwrap(); let sid: i32 = self .raw_ref() .query_row( - "select id from runtime_pstat_series where benchmark = ? and metric = ?", - params![&benchmark, &metric,], + "SELECT id FROM runtime_pstat_series WHERE benchmark = ? AND target = ? AND metric = ?", + params![&benchmark, &target, &metric], |r| r.get(0), ) .unwrap(); self.raw_ref() .execute( - "insert into runtime_pstat (series, aid, cid, value) VALUES (?, ?, ?, ?)", + "INSERT INTO runtime_pstat (series, aid, cid, value) VALUES (?, ?, ?, ?)", params![&sid, &artifact.0, &collection.0, &value], ) .unwrap(); diff --git a/database/src/selector.rs b/database/src/selector.rs index e4f49cfea..1f9d4e475 100644 --- a/database/src/selector.rs +++ b/database/src/selector.rs @@ -334,6 +334,7 @@ impl TestCase for CompileTestCase {} #[derive(Clone, Hash, Eq, PartialEq, Debug)] pub struct RuntimeBenchmarkQuery { benchmark: Selector, + target: Selector, metric: Selector, } @@ -351,6 +352,7 @@ impl RuntimeBenchmarkQuery { pub fn all_for_metric(metric: Metric) -> Self { Self { benchmark: Selector::All, + target: Selector::All, metric: Selector::One(metric.as_str().into()), } } @@ -360,6 +362,7 @@ impl Default for RuntimeBenchmarkQuery { fn default() -> Self { Self { benchmark: Selector::All, + target: Selector::All, metric: Selector::All, } } @@ -376,8 +379,10 @@ impl BenchmarkQuery for RuntimeBenchmarkQuery { ) -> Result>, String> { let mut statistic_descriptions: Vec<_> = index .runtime_statistic_descriptions() - .filter(|(&(b, m), _)| self.benchmark.matches(b) && self.metric.matches(m)) - .map(|(&(benchmark, _), sid)| (RuntimeTestCase { benchmark }, sid)) + .filter(|(&(b, t, m), _)| { + self.benchmark.matches(b) && self.target.matches(t) && self.metric.matches(m) + }) + .map(|(&(benchmark, target, _), sid)| (RuntimeTestCase { benchmark, target }, sid)) .collect(); statistic_descriptions.sort_unstable(); @@ -409,6 +414,7 @@ impl BenchmarkQuery for RuntimeBenchmarkQuery { #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct RuntimeTestCase { pub benchmark: Benchmark, + pub target: Target, } impl TestCase for RuntimeTestCase {} diff --git a/site/frontend/src/pages/compare/compile/common.ts b/site/frontend/src/pages/compare/compile/common.ts index f11211e48..1226f08d4 100644 --- a/site/frontend/src/pages/compare/compile/common.ts +++ b/site/frontend/src/pages/compare/compile/common.ts @@ -1,6 +1,11 @@ -import {BenchmarkFilter, CompareResponse, StatComparison} from "../types"; +import { + BenchmarkFilter, + CompareResponse, + StatComparison, + TargetSet, +} from "../types"; import {calculateComparison, TestCaseComparison} from "../data"; -import {benchmarkNameMatchesFilter} from "../shared"; +import {benchmarkNameMatchesFilter, targetMatchesFilter} from "../shared"; export type CompileBenchmarkFilter = { profile: { @@ -19,9 +24,7 @@ export type CompileBenchmarkFilter = { llvm: boolean; cranelift: boolean; }; - target: { - x86_64_unknown_linux_gnu: boolean; - }; + target: TargetSet; category: { primary: boolean; secondary: boolean; @@ -160,15 +163,6 @@ export function computeCompileComparisonsWithNonRelevant( } } - function targetFilter(target: Target): boolean { - if (target === "x86_64-unknown-linux-gnu") { - return filter.target.x86_64_unknown_linux_gnu; - } else { - // Unknown, but by default we should show things - return true; - } - } - function artifactFilter(metadata: CompileBenchmarkMetadata | null): boolean { if (metadata?.binary === null) return true; @@ -201,7 +195,7 @@ export function computeCompileComparisonsWithNonRelevant( profileFilter(comparison.testCase.profile) && scenarioFilter(comparison.testCase.scenario) && backendFilter(comparison.testCase.backend) && - targetFilter(comparison.testCase.target) && + targetMatchesFilter(comparison.testCase.target, filter.target) && categoryFilter(comparison.testCase.category) && artifactFilter(benchmarkMap[comparison.testCase.benchmark] ?? null) && changeFilter(comparison) && diff --git a/site/frontend/src/pages/compare/compile/compile-page.vue b/site/frontend/src/pages/compare/compile/compile-page.vue index b63a234ee..e81e60d29 100644 --- a/site/frontend/src/pages/compare/compile/compile-page.vue +++ b/site/frontend/src/pages/compare/compile/compile-page.vue @@ -18,7 +18,7 @@ import { } from "./common"; import {BenchmarkInfo} from "../../../api"; import {importantCompileMetrics} from "../metrics"; -import {getBoolOrDefault} from "../shared"; +import {getBoolOrDefault, loadTargetSetFromUrl} from "../shared"; const props = defineProps<{ data: CompareResponse; @@ -78,13 +78,7 @@ function loadFilterFromUrl( defaultFilter.backend.cranelift ), }, - target: { - x86_64_unknown_linux_gnu: getBoolOrDefault( - urlParams, - "target-x86_64-unknown-linux-gnu", - defaultFilter.target.x86_64_unknown_linux_gnu - ), - }, + target: loadTargetSetFromUrl(urlParams, defaultFilter.target), category: { primary: getBoolOrDefault( urlParams, diff --git a/site/frontend/src/pages/compare/runtime/common.ts b/site/frontend/src/pages/compare/runtime/common.ts index 966cfaa12..e13dd93b5 100644 --- a/site/frontend/src/pages/compare/runtime/common.ts +++ b/site/frontend/src/pages/compare/runtime/common.ts @@ -1,21 +1,29 @@ -import {BenchmarkFilter, StatComparison} from "../types"; +import {BenchmarkFilter, StatComparison, TargetSet} from "../types"; import {calculateComparison, TestCaseComparison} from "../data"; -import {benchmarkNameMatchesFilter} from "../shared"; +import {benchmarkNameMatchesFilter, targetMatchesFilter} from "../shared"; +import {Target} from "../compile/common"; export interface RuntimeTestCase { benchmark: string; + target: Target; } -export type RuntimeBenchmarkFilter = BenchmarkFilter; +export type RuntimeBenchmarkFilter = { + target: TargetSet; +} & BenchmarkFilter; export const defaultRuntimeFilter: RuntimeBenchmarkFilter = { name: null, nonRelevant: false, showRawData: false, + target: { + x86_64_unknown_linux_gnu: true, + }, }; export interface RuntimeBenchmarkComparison { benchmark: string; + target: Target; comparison: StatComparison; } @@ -26,9 +34,9 @@ export function computeRuntimeComparisonsWithNonRelevant( function shouldShowTestCase( comparison: TestCaseComparison ): boolean { - return benchmarkNameMatchesFilter( - comparison.testCase.benchmark, - filter.name + return ( + benchmarkNameMatchesFilter(comparison.testCase.benchmark, filter.name) && + targetMatchesFilter(comparison.testCase.target, filter.target) ); } @@ -37,6 +45,7 @@ export function computeRuntimeComparisonsWithNonRelevant( (c: RuntimeBenchmarkComparison): TestCaseComparison => { let testCase = { benchmark: c.benchmark, + target: c.target, }; return calculateComparison(c.comparison, testCase); } diff --git a/site/frontend/src/pages/compare/runtime/comparisons-table.vue b/site/frontend/src/pages/compare/runtime/comparisons-table.vue index bdc60bce2..a1cb7f8fc 100644 --- a/site/frontend/src/pages/compare/runtime/comparisons-table.vue +++ b/site/frontend/src/pages/compare/runtime/comparisons-table.vue @@ -1,7 +1,7 @@