Skip to content

Commit d7894a2

Browse files
committed
Make creation of benchmark sets more robust
Avoid creating two separate functions that could diverge.
1 parent d729dd7 commit d7894a2

File tree

4 files changed

+45
-42
lines changed

4 files changed

+45
-42
lines changed

‎collector/src/benchmark_set/compile_benchmarks.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! This file contains an exhaustive list of all compile-time benchmarks
22
//! located in the `collector/compile-benchmarks` directory that are benchmarked in production.
33
//! If new benchmarks are added/removed, they have to also be added/removed here, and in
4-
//! the [super::expand_benchmark_set] function.
4+
//! the [super::get_benchmark_sets_for_target] function.
55
66
// Stable benchmarks
77
pub(super)constCARGO:&str = "cargo";

‎collector/src/benchmark_set/mod.rs‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,28 @@ pub enum BenchmarkSetMember{
3131
CompileBenchmark(BenchmarkName),
3232
}
3333

34-
/// Return the number of benchmark sets for the given target.
35-
pubfnbenchmark_set_count(target:Target) -> usize{
36-
match target {
37-
Target::X86_64UnknownLinuxGnu => 1,
34+
#[derive(Debug)]
35+
pubstructBenchmarkSet{
36+
members:Vec<BenchmarkSetMember>,
37+
}
38+
39+
implBenchmarkSet{
40+
pubfnmembers(&self) -> &[BenchmarkSetMember]{
41+
&self.members
3842
}
3943
}
4044

41-
/// Expand all the benchmarks that should be performed by a single collector.
42-
pubfnexpand_benchmark_set(id:BenchmarkSetId) -> Vec<BenchmarkSetMember>{
45+
/// Return all benchmark sets for the given target.
46+
pubfnget_benchmark_sets_for_target(target:Target) -> Vec<BenchmarkSet>{
4347
use compile_benchmarks::*;
4448

45-
match(id.target, id.index){
46-
(Target::X86_64UnknownLinuxGnu,0) => {
47-
vec![
49+
fncompile(name:&str) -> BenchmarkSetMember{
50+
BenchmarkSetMember::CompileBenchmark(BenchmarkName::from(name))
51+
}
52+
53+
match target {
54+
Target::X86_64UnknownLinuxGnu => {
55+
let all = vec![
4856
compile(AWAIT_CALL_TREE),
4957
compile(BITMAPS_3_2_1),
5058
compile(BITMAPS_3_2_1_NEW_SOLVER),
@@ -106,24 +114,21 @@ pub fn expand_benchmark_set(id: BenchmarkSetId) -> Vec<BenchmarkSetMember>{
106114
compile(UNUSED_WARNINGS),
107115
compile(WF_PROJECTION_STRESS_65510),
108116
compile(WG_GRAMMAR),
109-
]
110-
}
111-
(Target::X86_64UnknownLinuxGnu,1..) => {
112-
panic!("Unknown benchmark set id{id:?}");
117+
];
118+
vec![BenchmarkSet{ members: all }]
113119
}
114120
}
115121
}
116122

117-
/// Helper function for creating compile-time benchmark member sets.
118-
fncompile(name:&str) -> BenchmarkSetMember{
119-
BenchmarkSetMember::CompileBenchmark(BenchmarkName::from(name))
123+
/// Expand all the benchmarks that should be performed by a single collector.
124+
pubfnget_benchmark_set(id:BenchmarkSetId) -> BenchmarkSet{
125+
letmut sets = get_benchmark_sets_for_target(id.target);
126+
sets.remove(id.indexasusize)
120127
}
121128

122129
#[cfg(test)]
123130
mod tests {
124-
usecrate::benchmark_set::{
125-
benchmark_set_count, expand_benchmark_set,BenchmarkSetId,BenchmarkSetMember,
126-
};
131+
usecrate::benchmark_set::{get_benchmark_sets_for_target,BenchmarkSetMember};
127132
usecrate::compile::benchmark::target::Target;
128133
usecrate::compile::benchmark::{
129134
get_compile_benchmarks,BenchmarkName,CompileBenchmarkFilter,
@@ -135,21 +140,13 @@ mod tests{
135140
/// complete, i.e. they don't miss any benchmarks.
136141
#[test]
137142
fncheck_benchmark_set_x64(){
138-
let target = Target::X86_64UnknownLinuxGnu;
139-
let sets = (0..benchmark_set_count(target))
140-
.map(|index| {
141-
expand_benchmark_set(BenchmarkSetId{
142-
target,
143-
index: index asu32,
144-
})
145-
})
146-
.collect::<Vec<Vec<BenchmarkSetMember>>>();
143+
let sets = get_benchmark_sets_for_target(Target::X86_64UnknownLinuxGnu);
147144

148145
// Assert set is unique
149146
for set in&sets {
150-
let hashset = set.iter().collect::<HashSet<_>>();
147+
let hashset = set.members().iter().collect::<HashSet<_>>();
151148
assert_eq!(
152-
set.len(),
149+
set.members().len(),
153150
hashset.len(),
154151
"Benchmark set{set:?} contains duplicates"
155152
);
@@ -160,8 +157,8 @@ mod tests{
160157
for j in i + 1..sets.len(){
161158
let set_a = &sets[i];
162159
let set_b = &sets[j];
163-
let hashset_a = set_a.iter().collect::<HashSet<_>>();
164-
let hashset_b = set_b.iter().collect::<HashSet<_>>();
160+
let hashset_a = set_a.members().iter().collect::<HashSet<_>>();
161+
let hashset_b = set_b.members().iter().collect::<HashSet<_>>();
165162
assert!(
166163
hashset_a.is_disjoint(&hashset_b),
167164
"Benchmark sets{set_a:?} and{set_b:?} overlap"
@@ -170,7 +167,10 @@ mod tests{
170167
}
171168

172169
// Check that the union of all sets contains all the required benchmarks
173-
let all_members = sets.iter().flatten().collect::<HashSet<_>>();
170+
let all_members = sets
171+
.iter()
172+
.flat_map(|s| s.members())
173+
.collect::<HashSet<_>>();
174174

175175
constBENCHMARK_DIR:&str = concat!(env!("CARGO_MANIFEST_DIR"),"/compile-benchmarks");
176176
let all_compile_benchmarks =
@@ -189,7 +189,7 @@ mod tests{
189189
letBenchmarkSetMember::CompileBenchmark(name) = benchmark;
190190
assert!(
191191
all_compile_benchmarks.contains(name),
192-
"Compile-time benchmark{name} does not exist on disk or is a stable benchmark"
192+
"Compile-time benchmark{name} does not exist on disk"
193193
);
194194
}
195195
assert_eq!(all_members.len(), all_compile_benchmarks.len());

‎collector/src/bin/collector.rs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use collector::api::next_artifact::NextArtifact;
3434
use collector::artifact_stats::{
3535
compile_and_get_stats,ArtifactStats,ArtifactWithStats,CargoProfile,
3636
};
37-
use collector::benchmark_set::{expand_benchmark_set,BenchmarkSetId,BenchmarkSetMember};
37+
use collector::benchmark_set::{get_benchmark_set,BenchmarkSetId,BenchmarkSetMember};
3838
use collector::codegen::{codegen_diff,CodegenType};
3939
use collector::compile::benchmark::category::Category;
4040
use collector::compile::benchmark::codegen_backend::CodegenBackend;
@@ -1777,9 +1777,12 @@ async fn create_benchmark_configs(
17771777
Option<RuntimeBenchmarkConfig>,
17781778
)>{
17791779
// Expand the benchmark set and figure out which benchmarks should be executed
1780-
let benchmark_set = BenchmarkSetId::new(job.target().into(), job.benchmark_set().get_id());
1781-
let benchmark_set_members = expand_benchmark_set(benchmark_set);
1782-
log::debug!("Expanded benchmark set members:{benchmark_set_members:?}");
1780+
let benchmark_set_id = BenchmarkSetId::new(job.target().into(), job.benchmark_set().get_id());
1781+
let benchmark_set = get_benchmark_set(benchmark_set_id);
1782+
log::debug!(
1783+
"Expanded benchmark set members:{:?}",
1784+
benchmark_set.members()
1785+
);
17831786

17841787
letmut bench_rustc = false;
17851788
letmut bench_runtime = false;
@@ -1795,7 +1798,7 @@ async fn create_benchmark_configs(
17951798
bench_runtime = true;
17961799
}
17971800
database::BenchmarkJobKind::Compiletime => {
1798-
for member inbenchmark_set_members{
1801+
for member inbenchmark_set.members(){
17991802
match member {
18001803
BenchmarkSetMember::CompileBenchmark(benchmark) => {
18011804
bench_compile_benchmarks.insert(benchmark);

‎site/src/job_queue/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::job_queue::utils::{parse_release_string, ExtractIf};
55
usecrate::load::{partition_in_place,SiteCtxt};
66
use anyhow::Context;
77
use chrono::Utc;
8-
use collector::benchmark_set::benchmark_set_count;
8+
use collector::benchmark_set::get_benchmark_sets_for_target;
99
use database::pool::{JobEnqueueResult,Transaction};
1010
use database::{
1111
BenchmarkJobKind,BenchmarkRequest,BenchmarkRequestIndex,BenchmarkRequestInsertResult,
@@ -304,7 +304,7 @@ pub async fn enqueue_benchmark_request(
304304

305305
// Target x benchmark_set x backend x profile -> BenchmarkJob
306306
for target inTarget::all(){
307-
for benchmark_set in0..benchmark_set_count(target.into()){
307+
for benchmark_set in0..get_benchmark_sets_for_target(target.into()).len(){
308308
for&backend in backends.iter(){
309309
for&profile in profiles.iter(){
310310
enqueue_job(

0 commit comments

Comments
(0)