Skip to content

Commit 7c04613

Browse files
committed
Do not update collector progress when the job queue is used
Otherwise the old system would pick up benchmark requests and that breaks it.
1 parent 6c1460f commit 7c04613

File tree

3 files changed

+41
-23
lines changed

3 files changed

+41
-23
lines changed

collector/src/bin/collector.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ impl RuntimeBenchmarkConfig {
136136
struct SharedBenchmarkConfig {
137137
artifact_id: ArtifactId,
138138
toolchain: Toolchain,
139-
record_duration: bool,
140139
job_id: Option<u32>,
141140
}
142141

@@ -854,7 +853,6 @@ fn main_result() -> anyhow::Result<i32> {
854853
let shared = SharedBenchmarkConfig {
855854
artifact_id,
856855
toolchain,
857-
record_duration: true,
858856
job_id: None,
859857
};
860858
let config = RuntimeBenchmarkConfig::new(
@@ -998,7 +996,6 @@ fn main_result() -> anyhow::Result<i32> {
998996
let shared = SharedBenchmarkConfig {
999997
toolchain,
1000998
artifact_id,
1001-
record_duration: true,
1002999
job_id: None,
10031000
};
10041001
let config = CompileBenchmarkConfig {
@@ -1152,7 +1149,6 @@ fn main_result() -> anyhow::Result<i32> {
11521149
let shared = SharedBenchmarkConfig {
11531150
artifact_id,
11541151
toolchain,
1155-
record_duration: true,
11561152
job_id: None,
11571153
};
11581154

@@ -1672,7 +1668,6 @@ async fn run_benchmark_job(
16721668
let shared = SharedBenchmarkConfig {
16731669
artifact_id,
16741670
toolchain,
1675-
record_duration: false,
16761671
job_id: Some(job.id()),
16771672
};
16781673

@@ -2131,7 +2126,7 @@ async fn init_collection(
21312126
runtime: Option<&RuntimeBenchmarkConfig>,
21322127
) -> CollectorCtx {
21332128
assert!(runtime.is_some() || compile.is_some());
2134-
let mut builder = CollectorStepBuilder::default();
2129+
let mut builder = CollectorStepBuilder::new(shared.job_id);
21352130
if let Some(compile) = compile {
21362131
builder = builder.record_compile_benchmarks(&compile.benchmarks, compile.bench_rustc);
21372132
}
@@ -2174,15 +2169,14 @@ async fn run_benchmarks(
21742169
&collector,
21752170
runtime.filter,
21762171
runtime.iterations,
2177-
shared.job_id,
21782172
)
21792173
.await
21802174
.context("Runtime benchmarks failed")
21812175
} else {
21822176
Ok(())
21832177
};
21842178

2185-
if shared.record_duration {
2179+
if shared.job_id.is_none() {
21862180
let end = start.elapsed();
21872181
connection
21882182
.record_duration(collector.artifact_row_id, end)
@@ -2230,7 +2224,6 @@ async fn bench_published_artifact(
22302224
let shared = SharedBenchmarkConfig {
22312225
artifact_id,
22322226
toolchain,
2233-
record_duration: true,
22342227
job_id: None,
22352228
};
22362229
run_benchmarks(

collector/src/lib.rs

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,17 @@ pub async fn master_commits() -> anyhow::Result<Vec<MasterCommit>> {
302302
#[derive(Default)]
303303
pub struct CollectorStepBuilder {
304304
steps: Vec<String>,
305+
job_id: Option<u32>,
305306
}
306307

307308
impl CollectorStepBuilder {
309+
pub fn new(job_id: Option<u32>) -> Self {
310+
Self {
311+
steps: vec![],
312+
job_id,
313+
}
314+
}
315+
308316
pub fn record_compile_benchmarks(
309317
mut self,
310318
benchmarks: &[Benchmark],
@@ -338,9 +346,11 @@ impl CollectorStepBuilder {
338346
let artifact_row_id = {
339347
let mut tx = conn.transaction().await;
340348
let artifact_row_id = tx.conn().artifact_id(artifact_id).await;
341-
tx.conn()
342-
.collector_start(artifact_row_id, &self.steps)
343-
.await;
349+
if self.job_id.is_none() {
350+
tx.conn()
351+
.collector_start(artifact_row_id, &self.steps)
352+
.await;
353+
}
344354
tx.commit().await.unwrap();
345355
artifact_row_id
346356
};
@@ -353,6 +363,7 @@ impl CollectorStepBuilder {
353363
CollectorCtx {
354364
artifact_row_id,
355365
measured_compile_test_cases,
366+
job_id: self.job_id,
356367
}
357368
}
358369
}
@@ -362,17 +373,26 @@ pub struct CollectorCtx {
362373
pub artifact_row_id: ArtifactIdNumber,
363374
/// Which tests cases were already computed **before** this collection began?
364375
pub measured_compile_test_cases: HashSet<CompileTestCase>,
376+
pub job_id: Option<u32>,
365377
}
366378

367379
impl CollectorCtx {
380+
pub fn is_from_job_queue(&self) -> bool {
381+
self.job_id.is_some()
382+
}
383+
368384
pub async fn start_compile_step(&self, conn: &dyn Connection, benchmark_name: &BenchmarkName) {
369-
conn.collector_start_step(self.artifact_row_id, &benchmark_name.0)
370-
.await;
385+
if !self.is_from_job_queue() {
386+
conn.collector_start_step(self.artifact_row_id, &benchmark_name.0)
387+
.await;
388+
}
371389
}
372390

373391
pub async fn end_compile_step(&self, conn: &dyn Connection, benchmark_name: &BenchmarkName) {
374-
conn.collector_end_step(self.artifact_row_id, &benchmark_name.0)
375-
.await
392+
if !self.is_from_job_queue() {
393+
conn.collector_end_step(self.artifact_row_id, &benchmark_name.0)
394+
.await;
395+
}
376396
}
377397

378398
/// Starts a new runtime benchmark collector step.
@@ -384,14 +404,20 @@ impl CollectorCtx {
384404
group: &BenchmarkGroup,
385405
) -> Option<String> {
386406
let step_name = runtime_group_step_name(&group.name);
387-
conn.collector_start_step(self.artifact_row_id, &step_name)
388-
.await
389-
.then_some(step_name)
407+
if self.is_from_job_queue() {
408+
Some(step_name)
409+
} else {
410+
conn.collector_start_step(self.artifact_row_id, &step_name)
411+
.await
412+
.then_some(step_name)
413+
}
390414
}
391415

392416
pub async fn end_runtime_step(&self, conn: &dyn Connection, group: &BenchmarkGroup) {
393-
conn.collector_end_step(self.artifact_row_id, &runtime_group_step_name(&group.name))
394-
.await
417+
if !self.is_from_job_queue() {
418+
conn.collector_end_step(self.artifact_row_id, &runtime_group_step_name(&group.name))
419+
.await;
420+
}
395421
}
396422
}
397423

collector/src/runtime/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ pub async fn bench_runtime(
3535
collector: &CollectorCtx,
3636
filter: RuntimeBenchmarkFilter,
3737
iterations: u32,
38-
job_id: Option<u32>,
3938
) -> anyhow::Result<()> {
4039
let filtered = suite.filtered_benchmark_count(&filter);
4140
println!("Executing {filtered} benchmarks\n");
@@ -94,7 +93,7 @@ pub async fn bench_runtime(
9493
collector.artifact_row_id,
9594
&step_name,
9695
&format!("{error:?}"),
97-
job_id,
96+
collector.job_id,
9897
)
9998
.await;
10099
};

0 commit comments

Comments
 (0)