Skip to content

Commit df45b9a

Browse files
KobzolKobzol
committed
Generalize statistic description querying
1 parent 2c2a889 commit df45b9a

File tree

7 files changed

+83
-58
lines changed

7 files changed

+83
-58
lines changed

‎database/src/bin/import-sqlite.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ async fn main() {
4545
let sqlite_aid = sqlite_conn.artifact_id(&aid).await;
4646
let postgres_aid = postgres_conn.artifact_id(&aid).await;
4747

48-
for (&(benchmark, profile, scenario, metric), id) in sqlite_idx.all_statistic_descriptions()
48+
for (&(benchmark, profile, scenario, metric), id) in
49+
sqlite_idx.compile_statistic_descriptions()
4950
{
5051
if benchmarks.insert(benchmark) {
5152
postgres_conn

‎database/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ pub struct Index {
408408
artifacts: Indexed<Box<str>>,
409409
/// Id lookup of the errors for a crate
410410
errors: Indexed<Benchmark>,
411-
/// Id lookup of stat description ids
411+
/// Id lookup of compile stat description ids
412412
/// For legacy reasons called `pstat_series` in the database, and so the name is kept here.
413413
pstat_series: Indexed<(Benchmark, Profile, Scenario, Metric)>,
414414
}
@@ -613,7 +613,7 @@ impl Index {
613613
// millions of queries and labels and iterating all of them is eventually
614614
// going to be impractical. But for now it performs quite well, so we'll go
615615
// for it as keeping indices around would be annoying.
616-
pub fn all_statistic_descriptions(
616+
pub fn compile_statistic_descriptions(
617617
&self,
618618
) -> impl Iterator<
619619
Item = (

‎site/src/comparison.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::api;
66
use crate::db::{ArtifactId, Benchmark, Lookup, Profile, Scenario};
77
use crate::github;
88
use crate::load::SiteCtxt;
9-
use crate::selector::{self, CompileBenchmarkQuery, CompileTestCase, TestCase};
9+
use crate::selector::{self, BenchmarkQuery, CompileBenchmarkQuery, TestCase};
1010

1111
use collector::benchmark::category::Category;
1212
use collector::Bound;
@@ -802,15 +802,19 @@ async fn compare_given_commits(
802802

803803
// `responses` contains series iterators. The first element in the iterator is the data
804804
// for `a` and the second is the data for `b`
805-
let mut responses = ctxt.compile_statistic_series(query.clone(), aids).await?;
805+
let mut responses = ctxt.statistic_series(query.clone(), aids).await?;
806806

807807
let conn = ctxt.conn().await;
808808
let statistics_for_a = statistics_from_series(&mut responses);
809809
let statistics_for_b = statistics_from_series(&mut responses);
810810

811-
let mut historical_data =
812-
HistoricalDataMap::<CompileTestCase>::calculate(ctxt, a.clone(), master_commits, query)
813-
.await?;
811+
let mut historical_data = HistoricalDataMap::<CompileBenchmarkQuery>::calculate(
812+
ctxt,
813+
a.clone(),
814+
master_commits,
815+
query,
816+
)
817+
.await?;
814818
let comparisons = statistics_for_a
815819
.into_iter()
816820
.filter_map(|(test_case, a)| {
@@ -1048,19 +1052,19 @@ impl ArtifactComparison {
10481052
}
10491053

10501054
/// The historical data for a certain benchmark
1051-
pub struct HistoricalDataMap<TestCase> {
1055+
pub struct HistoricalDataMap<Query: BenchmarkQuery> {
10521056
/// Historical data on a per test case basis
1053-
pub data: HashMap<TestCase, HistoricalData>,
1057+
pub data: HashMap<Query::TestCase, HistoricalData>,
10541058
}
10551059

1056-
impl HistoricalDataMap<CompileTestCase> {
1060+
impl<Query: BenchmarkQuery> HistoricalDataMap<Query> {
10571061
const NUM_PREVIOUS_COMMITS: usize = 30;
10581062

10591063
async fn calculate(
10601064
ctxt: &SiteCtxt,
10611065
from: ArtifactId,
10621066
master_commits: &[collector::MasterCommit],
1063-
query: CompileBenchmarkQuery,
1067+
query: Query,
10641068
) -> Result<Self, BoxedError> {
10651069
let mut historical_data = HashMap::new();
10661070

@@ -1078,7 +1082,7 @@ impl HistoricalDataMap<CompileTestCase> {
10781082
}
10791083

10801084
let mut previous_commit_series = ctxt
1081-
.compile_statistic_series(query, previous_commits.clone())
1085+
.statistic_series(query, previous_commits.clone())
10821086
.await?;
10831087

10841088
for _ in previous_commits.iter() {

‎site/src/request_handlers/dashboard.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ pub async fn handle_dashboard(ctxt: Arc<SiteCtxt>) -> ServerResult<dashboard::Re
9898
let mut cases = dashboard::Cases::default();
9999
for scenario in summary_scenarios.iter() {
100100
let responses = ctxt
101-
.compile_statistic_series(
101+
.statistic_series(
102102
query
103103
.clone()
104104
.profile(selector::Selector::One(profile))

‎site/src/request_handlers/graph.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ async fn create_graph(
5757
) -> ServerResult<graph::Response> {
5858
let artifact_ids = artifact_ids_for_range(&ctxt, request.start, request.end);
5959
let mut series_iterator = ctxt
60-
.compile_statistic_series(
60+
.statistic_series(
6161
CompileBenchmarkQuery::default()
6262
.benchmark(Selector::One(request.benchmark))
6363
.profile(Selector::One(request.profile.parse()?))
@@ -100,7 +100,7 @@ async fn create_graphs(
100100
create_selector(&request.scenario).try_map(|v| v.parse::<Scenario>())?;
101101

102102
let interpolated_responses: Vec<_> = ctxt
103-
.compile_statistic_series(
103+
.statistic_series(
104104
CompileBenchmarkQuery::default()
105105
.benchmark(benchmark_selector)
106106
.profile(profile_selector)

‎site/src/request_handlers/self_profile.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -602,9 +602,7 @@ pub async fn handle_self_profile(
602602
}
603603
let commits = Arc::new(commits);
604604

605-
let mut cpu_responses = ctxt
606-
.compile_statistic_series(query, commits.clone())
607-
.await?;
605+
let mut cpu_responses = ctxt.statistic_series(query, commits.clone()).await?;
608606
assert_eq!(cpu_responses.len(), 1, "all selectors are exact");
609607
let mut cpu_response = cpu_responses.remove(0).series;
610608

‎site/src/selector.rs

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use collector::Bound;
2929
use database::{Benchmark, Commit, Index, Lookup};
3030

3131
use crate::comparison::Metric;
32-
use std::fmt;
32+
use std::fmt::Debug;
3333
use std::hash::Hash;
3434
use std::ops::RangeInclusive;
3535
use std::sync::Arc;
@@ -146,21 +146,10 @@ impl<T> Selector<T> {
146146
Selector::All => true,
147147
}
148148
}
149-
150-
pub fn assert_one(&self) -> &T
151-
where
152-
T: fmt::Debug,
153-
{
154-
if let Selector::One(one) = self {
155-
one
156-
} else {
157-
panic!("{:?} != One", self)
158-
}
159-
}
160149
}
161150

162151
/// Represents the parameters of a single benchmark execution that collects a set of statistics.
163-
pub trait TestCase: std::fmt::Debug + Clone + Hash + Eq + PartialEq {}
152+
pub trait TestCase: Debug + Clone + Hash + PartialEq + Eq + PartialOrd + Ord {}
164153

165154
#[derive(Debug)]
166155
pub struct SeriesResponse<Case, T> {
@@ -189,6 +178,17 @@ impl<TestCase, T> SeriesResponse<TestCase, T> {
189178
}
190179
}
191180

181+
pub trait BenchmarkQuery: Debug {
182+
type TestCase: TestCase;
183+
184+
/// Returns all known statistic descriptions (test cases + metrics) and their corresponding
185+
/// row IDs.
186+
fn get_statistic_descriptions(
187+
&self,
188+
index: &Index,
189+
) -> Vec<(Self::TestCase, database::Metric, u32)>;
190+
}
191+
192192
#[derive(Clone, Hash, Eq, PartialEq, Debug)]
193193
pub struct CompileBenchmarkQuery {
194194
benchmark: Selector<String>,
@@ -239,7 +239,37 @@ impl Default for CompileBenchmarkQuery {
239239
}
240240
}
241241

242-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
242+
impl BenchmarkQuery for CompileBenchmarkQuery {
243+
type TestCase = CompileTestCase;
244+
245+
fn get_statistic_descriptions(
246+
&self,
247+
index: &Index,
248+
) -> Vec<(Self::TestCase, database::Metric, u32)> {
249+
index
250+
.compile_statistic_descriptions()
251+
.filter(|(&(b, p, s, m), _)| {
252+
self.benchmark.matches(b)
253+
&& self.profile.matches(p)
254+
&& self.scenario.matches(s)
255+
&& self.metric.matches(m)
256+
})
257+
.map(|(&(benchmark, profile, scenario, metric), sid)| {
258+
(
259+
CompileTestCase {
260+
benchmark,
261+
profile,
262+
scenario,
263+
},
264+
metric,
265+
sid,
266+
)
267+
})
268+
.collect()
269+
}
270+
}
271+
272+
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
243273
pub struct CompileTestCase {
244274
pub benchmark: Benchmark,
245275
pub profile: Profile,
@@ -249,11 +279,11 @@ pub struct CompileTestCase {
249279
impl TestCase for CompileTestCase {}
250280

251281
impl SiteCtxt {
252-
pub async fn compile_statistic_series(
282+
pub async fn statistic_series<Q: BenchmarkQuery>(
253283
&self,
254-
query: CompileBenchmarkQuery,
284+
query: Q,
255285
artifact_ids: Arc<Vec<ArtifactId>>,
256-
) -> Result<Vec<SeriesResponse<CompileTestCase, StatisticSeries>>, String> {
286+
) -> Result<Vec<SeriesResponse<Q::TestCase, StatisticSeries>>, String> {
257287
StatisticSeries::execute_compile_query(artifact_ids, self, query).await
258288
}
259289
}
@@ -264,27 +294,23 @@ pub struct StatisticSeries {
264294
}
265295

266296
impl StatisticSeries {
267-
async fn execute_compile_query(
297+
async fn execute_compile_query<Q: BenchmarkQuery>(
268298
artifact_ids: Arc<Vec<ArtifactId>>,
269299
ctxt: &SiteCtxt,
270-
query: CompileBenchmarkQuery,
271-
) -> Result<Vec<SeriesResponse<CompileTestCase, Self>>, String> {
300+
query: Q,
301+
) -> Result<Vec<SeriesResponse<Q::TestCase, Self>>, String> {
272302
let dumped = format!("{:?}", query);
273303

274304
let index = ctxt.index.load();
275-
let mut statistic_descriptions = index
276-
.all_statistic_descriptions()
277-
.filter(|(&(b, p, s, m), _)| {
278-
query.benchmark.matches(b)
279-
&& query.profile.matches(p)
280-
&& query.scenario.matches(s)
281-
&& query.metric.matches(m)
282-
})
283-
.collect::<Vec<_>>();
305+
let mut statistic_descriptions = query.get_statistic_descriptions(&index);
284306

285307
statistic_descriptions.sort_unstable();
308+
let descriptions_count = statistic_descriptions.len();
286309

287-
let sids: Vec<_> = statistic_descriptions.iter().map(|(_, sid)| *sid).collect();
310+
let sids: Vec<_> = statistic_descriptions
311+
.iter()
312+
.map(|(_, _, sid)| *sid)
313+
.collect();
288314
let aids = artifact_ids
289315
.iter()
290316
.map(|aid| aid.lookup(&index))
@@ -299,13 +325,13 @@ impl StatisticSeries {
299325
.get_pstats(&sids, &aids)
300326
.await
301327
.into_iter()
302-
.zip(&statistic_descriptions)
328+
.zip(statistic_descriptions)
303329
.filter(|(points, _)| points.iter().any(|value| value.is_some()))
304-
.map(|(points, (&(benchmark, profile, scenario, metric), _))| {
330+
.map(|(points, (test_case, metric, _))| {
305331
SeriesResponse {
306332
series: StatisticSeries {
307333
artifact_ids: ArtifactIdIter::new(artifact_ids.clone()),
308-
points: if metric == *"cpu-clock" || metric == *"task-clock" {
334+
points: if *metric == *"cpu-clock" || *metric == *"task-clock" {
309335
// Convert to seconds -- perf reports these measurements in
310336
// milliseconds
311337
points
@@ -317,18 +343,14 @@ impl StatisticSeries {
317343
points.into_iter()
318344
},
319345
},
320-
test_case: CompileTestCase {
321-
benchmark,
322-
profile,
323-
scenario,
324-
},
346+
test_case,
325347
}
326348
})
327349
.collect::<Vec<_>>();
328350
log::trace!(
329351
"{:?}: run {} from {}",
330352
start.elapsed(),
331-
statistic_descriptions.len(),
353+
descriptions_count,
332354
dumped
333355
);
334356
Ok(res)

0 commit comments

Comments
 (0)