Merge pull request #1651 from Kobzol/runtime-error-handling

Store runtime errors into the database

Author: Kobzol

collector/src/bin/collector.rs

@@ -14,6 +14,7 @@ use collector::{runtime, utils, CollectorCtx, CollectorStepBuilder};
 use database::{ArtifactId, ArtifactIdNumber, Commit, CommitType, Connection, Pool};
 use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator};
 use std::cmp::Ordering;
+use std::collections::HashMap;
 use std::ffi::OsStr;
 use std::fs;
 use std::fs::File;
@@ -30,8 +31,8 @@ use tokio::runtime::Runtime;
 use collector::compile::execute::bencher::BenchProcessor;
 use collector::compile::execute::profiler::{ProfileProcessor, Profiler};
 use collector::runtime::{
-    bench_runtime, runtime_benchmark_dir, BenchmarkFilter, BenchmarkSuite, CargoIsolationMode,
-    DEFAULT_RUNTIME_ITERATIONS,
+    bench_runtime, runtime_benchmark_dir, BenchmarkFilter, BenchmarkSuite,
+    BenchmarkSuiteCompilation, CargoIsolationMode, DEFAULT_RUNTIME_ITERATIONS,
 };
 use collector::toolchain::{
     create_toolchain_from_published_version, get_local_toolchain, Sysroot, Toolchain,
@@ -655,12 +656,16 @@ fn main_result() -> anyhow::Result<i32> {
             } else {
                 CargoIsolationMode::Isolated
             };
-            let runtime_suite = runtime::prepare_runtime_benchmark_suite(
-                &toolchain,
+
+            let mut conn = rt.block_on(pool.connection());
+            let artifact_id = ArtifactId::Tag(toolchain.id.clone());
+            let runtime_suite = rt.block_on(load_runtime_benchmarks(
+                conn.as_mut(),
                 &runtime_benchmark_dir,
                 isolation_mode,
-            )?;
-            let artifact_id = ArtifactId::Tag(toolchain.id.clone());
+                &toolchain,
+                &artifact_id,
+            ))?;
 
             let shared = SharedBenchmarkConfig {
                 artifact_id,
@@ -671,7 +676,6 @@ fn main_result() -> anyhow::Result<i32> {
                 filter: BenchmarkFilter::new(local.exclude, local.include),
                 iterations,
             };
-            let conn = rt.block_on(pool.connection());
             run_benchmarks(&mut rt, conn, shared, None, Some(config))?;
             Ok(0)
         }
@@ -966,6 +970,35 @@ Make sure to modify `{dir}/perf-config.json` if the category/artifact don't matc
     }
 }
 
+async fn load_runtime_benchmarks(
+    conn: &mut dyn Connection,
+    benchmark_dir: &Path,
+    isolation_mode: CargoIsolationMode,
+    toolchain: &Toolchain,
+    artifact_id: &ArtifactId,
+) -> anyhow::Result<BenchmarkSuite> {
+    let BenchmarkSuiteCompilation {
+        suite,
+        failed_to_compile,
+    } = runtime::prepare_runtime_benchmark_suite(toolchain, benchmark_dir, isolation_mode)?;
+
+    record_runtime_compilation_errors(conn, artifact_id, failed_to_compile).await;
+    Ok(suite)
+}
+
+async fn record_runtime_compilation_errors(
+    connection: &mut dyn Connection,
+    artifact_id: &ArtifactId,
+    errors: HashMap<String, String>,
+) {
+    let artifact_row_number = connection.artifact_id(artifact_id).await;
+    for (krate, error) in errors {
+        connection
+            .record_error(artifact_row_number, &krate, &error)
+            .await;
+    }
+}
+
 fn log_db(db_option: &DbOption) {
     println!("Using database `{}`", db_option.db);
 }
@@ -1045,7 +1078,7 @@ fn run_benchmarks(
 
 /// Perform benchmarks on a published artifact.
 fn bench_published_artifact(
-    connection: Box<dyn Connection>,
+    mut connection: Box<dyn Connection>,
     rt: &mut Runtime,
     toolchain: Toolchain,
     dirs: &BenchmarkDirs,
@@ -1067,11 +1100,13 @@ fn bench_published_artifact(
     let mut compile_benchmarks = get_compile_benchmarks(dirs.compile, None, None, None)?;
     compile_benchmarks.retain(|b| b.category().is_stable());
 
-    let runtime_suite = runtime::prepare_runtime_benchmark_suite(
-        &toolchain,
+    let runtime_suite = rt.block_on(load_runtime_benchmarks(
+        connection.as_mut(),
         dirs.runtime,
         CargoIsolationMode::Isolated,
-    )?;
+        &toolchain,
+        &artifact_id,
+    ))?;
 
     let shared = SharedBenchmarkConfig {
         artifact_id,

collector/src/lib.rs

@@ -299,8 +299,12 @@ impl CollectorStepBuilder {
     }
 
     pub fn record_runtime_benchmarks(mut self, suite: &BenchmarkSuite) -> Self {
-        self.steps
-            .extend(suite.groups.iter().map(runtime_group_step_name));
+        self.steps.extend(
+            suite
+                .groups
+                .iter()
+                .map(|group| runtime_group_step_name(&group.name)),
+        );
         self
     }
 
@@ -344,17 +348,26 @@ impl CollectorCtx {
             .await
     }
 
-    pub async fn start_runtime_step(&self, conn: &dyn Connection, group: &BenchmarkGroup) -> bool {
-        conn.collector_start_step(self.artifact_row_id, &runtime_group_step_name(group))
+    /// Starts a new runtime benchmark collector step.
+    /// If this step was already computed, returns None.
+    /// Otherwise returns Some(<name of step>).
+    pub async fn start_runtime_step(
+        &self,
+        conn: &dyn Connection,
+        group: &BenchmarkGroup,
+    ) -> Option<String> {
+        let step_name = runtime_group_step_name(&group.name);
+        conn.collector_start_step(self.artifact_row_id, &step_name)
             .await
+            .then_some(step_name)
     }
 
     pub async fn end_runtime_step(&self, conn: &dyn Connection, group: &BenchmarkGroup) {
-        conn.collector_end_step(self.artifact_row_id, &runtime_group_step_name(group))
+        conn.collector_end_step(self.artifact_row_id, &runtime_group_step_name(&group.name))
             .await
     }
 }
 
-fn runtime_group_step_name(group: &BenchmarkGroup) -> String {
-    format!("runtime:{}", group.name)
+pub fn runtime_group_step_name(benchmark_name: &str) -> String {
+    format!("runtime:{}", benchmark_name)
 }

collector/src/runtime/benchmark.rs

@@ -1,3 +1,4 @@
+use crate::runtime_group_step_name;
 use crate::toolchain::Toolchain;
 use anyhow::Context;
 use benchlib::benchmark::passes_filter;
@@ -90,6 +91,12 @@ pub enum CargoIsolationMode {
     Isolated,
 }
 
+pub struct BenchmarkSuiteCompilation {
+    pub suite: BenchmarkSuite,
+    // Maps benchmark group name to compilation error
+    pub failed_to_compile: HashMap<String, String>,
+}
+
 /// Find all runtime benchmark crates in `benchmark_dir` and compile them.
 /// We assume that each binary defines a benchmark suite using `benchlib`.
 /// We then execute each benchmark suite with the `list-benchmarks` command to find out its
@@ -98,7 +105,7 @@ pub fn prepare_runtime_benchmark_suite(
     toolchain: &Toolchain,
     benchmark_dir: &Path,
     isolation_mode: CargoIsolationMode,
-) -> anyhow::Result<BenchmarkSuite> {
+) -> anyhow::Result<BenchmarkSuiteCompilation> {
     let benchmark_crates = get_runtime_benchmark_groups(benchmark_dir)?;
 
     let temp_dir: Option<TempDir> = match isolation_mode {
@@ -120,6 +127,7 @@ pub fn prepare_runtime_benchmark_suite(
     println!("Compiling {group_count} runtime benchmark groups");
 
     let mut groups = Vec::new();
+    let mut failed_to_compile = HashMap::new();
     for (index, benchmark_crate) in benchmark_crates.into_iter().enumerate() {
         println!(
             "Compiling {:<22} ({}/{group_count})",
@@ -129,25 +137,41 @@ pub fn prepare_runtime_benchmark_suite(
 
         let target_dir = temp_dir.as_ref().map(|d| d.path());
 
-        let cargo_process = start_cargo_build(toolchain, &benchmark_crate.path, target_dir)
+        let result = start_cargo_build(toolchain, &benchmark_crate.path, target_dir)
             .with_context(|| {
                 anyhow::anyhow!("Cannot start compilation of {}", benchmark_crate.name)
-            })?;
-        let group =
-            parse_benchmark_group(cargo_process, &benchmark_crate.name).with_context(|| {
-                anyhow::anyhow!("Cannot compile runtime benchmark {}", benchmark_crate.name)
-            })?;
-        groups.push(group);
+            })
+            .and_then(|process| {
+                parse_benchmark_group(process, &benchmark_crate.name).with_context(|| {
+                    anyhow::anyhow!("Cannot compile runtime benchmark {}", benchmark_crate.name)
+                })
+            });
+        match result {
+            Ok(group) => groups.push(group),
+            Err(error) => {
+                log::error!(
+                    "Cannot compile runtime benchmark group `{}`",
+                    benchmark_crate.name
+                );
+                failed_to_compile.insert(
+                    runtime_group_step_name(&benchmark_crate.name),
+                    format!("{error:?}"),
+                );
+            }
+        }
     }
 
     groups.sort_unstable_by(|a, b| a.binary.cmp(&b.binary));
     log::debug!("Found binaries: {:?}", groups);
 
     check_duplicates(&groups)?;
 
-    Ok(BenchmarkSuite {
-        groups,
-        _tmp_artifacts_dir: temp_dir,
+    Ok(BenchmarkSuiteCompilation {
+        suite: BenchmarkSuite {
+            groups,
+            _tmp_artifacts_dir: temp_dir,
+        },
+        failed_to_compile,
     })
 }
 
@@ -181,6 +205,7 @@ fn parse_benchmark_group(
     let mut group: Option<BenchmarkGroup> = None;
 
     let stream = BufReader::new(cargo_process.stdout.take().unwrap());
+    let mut messages = String::new();
     for message in Message::parse_stream(stream) {
         let message = message?;
         match message {
@@ -210,25 +235,28 @@ fn parse_benchmark_group(
                     }
                 }
             }
-            Message::TextLine(line) => println!("{}", line),
+            Message::TextLine(line) => {
+                println!("{line}")
+            }
             Message::CompilerMessage(msg) => {
-                print!("{}", msg.message.rendered.unwrap_or(msg.message.message))
+                let message = msg.message.rendered.unwrap_or(msg.message.message);
+                messages.push_str(&message);
+                print!("{message}");
             }
             _ => {}
         }
     }
 
-    let group = group.ok_or_else(|| {
-        anyhow::anyhow!("Runtime benchmark group `{group_name}` has not produced any binary")
-    })?;
-
     let output = cargo_process.wait()?;
     if !output.success() {
         Err(anyhow::anyhow!(
-            "Failed to compile runtime benchmark, exit code {}",
-            output.code().unwrap_or(1)
+            "Failed to compile runtime benchmark, exit code {}\n{messages}",
+            output.code().unwrap_or(1),
         ))
     } else {
+        let group = group.ok_or_else(|| {
+            anyhow::anyhow!("Runtime benchmark group `{group_name}` has not produced any binary")
+        })?;
         Ok(group)
     }
 }
@@ -246,7 +274,7 @@ fn start_cargo_build(
         .arg("build")
         .arg("--release")
         .arg("--message-format")
-        .arg("json-diagnostic-rendered-ansi")
+        .arg("json-diagnostic-short")
         .current_dir(benchmark_dir)
         .stdin(Stdio::null())
         .stdout(Stdio::piped())