From 6463590f0ce9ae9113bba171d8f9ea0d64c7e47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 15 Feb 2025 14:25:43 +0100 Subject: [PATCH 1/4] Postprocess test suite metrics into GitHub summary --- .github/workflows/ci.yml | 17 +++++ src/ci/citool/Cargo.lock | 9 +++ src/ci/citool/Cargo.toml | 2 + src/ci/citool/src/main.rs | 15 ++++ src/ci/citool/src/metrics.rs | 138 +++++++++++++++++++++++++++++++++++ 5 files changed, 181 insertions(+) create mode 100644 src/ci/citool/src/metrics.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0cfbb4e77c465..a91598586b9fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -182,6 +182,13 @@ jobs: - name: show the current environment run: src/ci/scripts/dump-environment.sh + # Pre-build citool before the following step uninstalls rustup + # Build is into the build directory, to avoid modifying sources + - name: build citool + run: | + cd src/ci/citool + CARGO_TARGET_DIR=../../../build/citool cargo build + - name: run the build # Redirect stderr to stdout to avoid reordering the two streams in the GHA logs. run: src/ci/scripts/run-build-from-ci.sh 2>&1 @@ -218,6 +225,16 @@ jobs: # erroring about invalid credentials instead. if: github.event_name == 'push' || env.DEPLOY == '1' || env.DEPLOY_ALT == '1' + - name: postprocess metrics into the summary + run: | + if [ -f build/metrics.json ]; then + ./build/citool/debug/citool postprocess-metrics build/metrics.json ${GITHUB_STEP_SUMMARY} + elif [ -f obj/build/metrics.json ]; then + ./build/citool/debug/citool postprocess-metrics obj/build/metrics.json ${GITHUB_STEP_SUMMARY} + else + echo "No metrics.json found" + fi + - name: upload job metrics to DataDog if: needs.calculate_matrix.outputs.run_type != 'pr' env: diff --git a/src/ci/citool/Cargo.lock b/src/ci/citool/Cargo.lock index 39b6b44da643b..e5be2d42472b7 100644 --- a/src/ci/citool/Cargo.lock +++ b/src/ci/citool/Cargo.lock @@ -58,11 +58,20 @@ version = "1.0.95" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04" +[[package]] +name = "build_helper" +version = "0.1.0" +dependencies = [ + "serde", + "serde_derive", +] + [[package]] name = "citool" version = "0.1.0" dependencies = [ "anyhow", + "build_helper", "clap", "insta", "serde", diff --git a/src/ci/citool/Cargo.toml b/src/ci/citool/Cargo.toml index e77c67c71477a..11172832cb818 100644 --- a/src/ci/citool/Cargo.toml +++ b/src/ci/citool/Cargo.toml @@ -10,6 +10,8 @@ serde = { version = "1", features = ["derive"] } serde_yaml = "0.9" serde_json = "1" +build_helper = { path = "../../build_helper" } + [dev-dependencies] insta = "1" diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs index ad9cc8b82a6f9..cef92e998da1a 100644 --- a/src/ci/citool/src/main.rs +++ b/src/ci/citool/src/main.rs @@ -1,3 +1,5 @@ +mod metrics; + use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::Command; @@ -6,6 +8,8 @@ use anyhow::Context; use clap::Parser; use serde_yaml::Value; +use crate::metrics::postprocess_metrics; + const CI_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/.."); const DOCKER_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/../docker"); const JOBS_YML_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/../github-actions/jobs.yml"); @@ -338,6 +342,14 @@ enum Args { #[clap(long = "type", default_value = "auto")] job_type: JobType, }, + /// Postprocess the metrics.json file generated by bootstrap. + PostprocessMetrics { + /// Path to the metrics.json file + metrics_path: PathBuf, + /// Path to a file where the postprocessed metrics summary will be stored. + /// Usually, this will be GITHUB_STEP_SUMMARY on CI. + summary_path: PathBuf, + }, } #[derive(clap::ValueEnum, Clone)] @@ -369,6 +381,9 @@ fn main() -> anyhow::Result<()> { Args::RunJobLocally { job_type, name } => { run_workflow_locally(load_db(default_jobs_file)?, job_type, name)? } + Args::PostprocessMetrics { metrics_path, summary_path } => { + postprocess_metrics(&metrics_path, &summary_path)?; + } } Ok(()) diff --git a/src/ci/citool/src/metrics.rs b/src/ci/citool/src/metrics.rs new file mode 100644 index 0000000000000..9a1c7c4d910ad --- /dev/null +++ b/src/ci/citool/src/metrics.rs @@ -0,0 +1,138 @@ +use std::collections::BTreeMap; +use std::fs::File; +use std::io::Write; +use std::path::Path; + +use anyhow::Context; +use build_helper::metrics::{JsonNode, JsonRoot, TestOutcome, TestSuite, TestSuiteMetadata}; + +pub fn postprocess_metrics(metrics_path: &Path, summary_path: &Path) -> anyhow::Result<()> { + let metrics = load_metrics(metrics_path)?; + + let mut file = File::options() + .append(true) + .create(true) + .open(summary_path) + .with_context(|| format!("Cannot open summary file at {summary_path:?}"))?; + + record_test_suites(&metrics, &mut file)?; + + Ok(()) +} + +fn record_test_suites(metrics: &JsonRoot, file: &mut File) -> anyhow::Result<()> { + let suites = get_test_suites(&metrics); + + if !suites.is_empty() { + let aggregated = aggregate_test_suites(&suites); + let table = render_table(aggregated); + writeln!(file, "\n# Test results\n")?; + writeln!(file, "{table}")?; + } else { + eprintln!("No test suites found in metrics"); + } + + Ok(()) +} + +fn render_table(suites: BTreeMap) -> String { + use std::fmt::Write; + + let mut table = "| Test suite | Passed ✅ | Ignored 🚫 | Failed ❌ |\n".to_string(); + writeln!(table, "|:------|------:|------:|------:|").unwrap(); + + fn write_row( + buffer: &mut String, + name: &str, + record: &TestSuiteRecord, + surround: &str, + ) -> std::fmt::Result { + let TestSuiteRecord { passed, ignored, failed } = record; + let total = (record.passed + record.ignored + record.failed) as f64; + let passed_pct = ((*passed as f64) / total) * 100.0; + let ignored_pct = ((*ignored as f64) / total) * 100.0; + let failed_pct = ((*failed as f64) / total) * 100.0; + + write!(buffer, "| {surround}{name}{surround} |")?; + write!(buffer, " {surround}{passed} ({passed_pct:.0}%){surround} |")?; + write!(buffer, " {surround}{ignored} ({ignored_pct:.0}%){surround} |")?; + writeln!(buffer, " {surround}{failed} ({failed_pct:.0}%){surround} |")?; + + Ok(()) + } + + let mut total = TestSuiteRecord::default(); + for (name, record) in suites { + write_row(&mut table, &name, &record, "").unwrap(); + total.passed += record.passed; + total.ignored += record.ignored; + total.failed += record.failed; + } + write_row(&mut table, "Total", &total, "**").unwrap(); + table +} + +#[derive(Default)] +struct TestSuiteRecord { + passed: u64, + ignored: u64, + failed: u64, +} + +fn aggregate_test_suites(suites: &[&TestSuite]) -> BTreeMap { + let mut records: BTreeMap = BTreeMap::new(); + for suite in suites { + let name = match &suite.metadata { + TestSuiteMetadata::CargoPackage { crates, stage, .. } => { + format!("{} (stage {stage})", crates.join(", ")) + } + TestSuiteMetadata::Compiletest { suite, stage, .. } => { + format!("{suite} (stage {stage})") + } + }; + let record = records.entry(name).or_default(); + for test in &suite.tests { + match test.outcome { + TestOutcome::Passed => { + record.passed += 1; + } + TestOutcome::Failed => { + record.failed += 1; + } + TestOutcome::Ignored { .. } => { + record.ignored += 1; + } + } + } + } + records +} + +fn get_test_suites(metrics: &JsonRoot) -> Vec<&TestSuite> { + fn visit_test_suites<'a>(nodes: &'a [JsonNode], suites: &mut Vec<&'a TestSuite>) { + for node in nodes { + match node { + JsonNode::RustbuildStep { children, .. } => { + visit_test_suites(&children, suites); + } + JsonNode::TestSuite(suite) => { + suites.push(&suite); + } + } + } + } + + let mut suites = vec![]; + for invocation in &metrics.invocations { + visit_test_suites(&invocation.children, &mut suites); + } + suites +} + +fn load_metrics(path: &Path) -> anyhow::Result { + let metrics = std::fs::read_to_string(path) + .with_context(|| format!("Cannot read JSON metrics from {path:?}"))?; + let metrics: JsonRoot = serde_json::from_str(&metrics) + .with_context(|| format!("Cannot deserialize JSON metrics from {path:?}"))?; + Ok(metrics) +} From 2e5ab4e6a07a9ebdb6e593ca19112148250e6906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 19 Feb 2025 16:06:47 +0100 Subject: [PATCH 2/4] Store bootstrap command-line into metrics --- src/bootstrap/src/utils/metrics.rs | 8 ++++++++ src/build_helper/src/metrics.rs | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/bootstrap/src/utils/metrics.rs b/src/bootstrap/src/utils/metrics.rs index b51fd490535a3..57766fd63fb11 100644 --- a/src/bootstrap/src/utils/metrics.rs +++ b/src/bootstrap/src/utils/metrics.rs @@ -200,6 +200,14 @@ impl BuildMetrics { } }; invocations.push(JsonInvocation { + // The command-line invocation with which bootstrap was invoked. + // Skip the first argument, as it is a potentially long absolute + // path that is not interesting. + cmdline: std::env::args_os() + .skip(1) + .map(|arg| arg.to_string_lossy().to_string()) + .collect::>() + .join(" "), start_time: state .invocation_start .duration_since(SystemTime::UNIX_EPOCH) diff --git a/src/build_helper/src/metrics.rs b/src/build_helper/src/metrics.rs index 538c33e9b1591..c9be63ed534b0 100644 --- a/src/build_helper/src/metrics.rs +++ b/src/build_helper/src/metrics.rs @@ -12,6 +12,8 @@ pub struct JsonRoot { #[derive(Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub struct JsonInvocation { + // Remembers the command-line invocation with which bootstrap was invoked. + pub cmdline: String, // Unix timestamp in seconds // // This is necessary to easily correlate this invocation with logs or other data. From ead58ea6f8bf860459fe5351a83717a8e591b0af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 4 Mar 2025 12:29:51 +0100 Subject: [PATCH 3/4] Move `BuildStep` and metric logging into `build_helper` --- src/build_helper/src/metrics.rs | 80 +++++++++++++++++++++++++++++++ src/tools/opt-dist/src/metrics.rs | 73 ++-------------------------- 2 files changed, 83 insertions(+), 70 deletions(-) diff --git a/src/build_helper/src/metrics.rs b/src/build_helper/src/metrics.rs index c9be63ed534b0..4b585c1518246 100644 --- a/src/build_helper/src/metrics.rs +++ b/src/build_helper/src/metrics.rs @@ -1,3 +1,5 @@ +use std::time::Duration; + use serde_derive::{Deserialize, Serialize}; #[derive(Serialize, Deserialize)] @@ -100,3 +102,81 @@ fn null_as_f64_nan<'de, D: serde::Deserializer<'de>>(d: D) -> Result::deserialize(d).map(|f| f.unwrap_or(f64::NAN)) } + +/// Represents a single bootstrap step, with the accumulated duration of all its children. +#[derive(Clone, Debug)] +pub struct BuildStep { + pub r#type: String, + pub children: Vec, + pub duration: Duration, +} + +impl BuildStep { + /// Create a `BuildStep` representing a single invocation of bootstrap. + /// The most important thing is that the build step aggregates the + /// durations of all children, so that it can be easily accessed. + pub fn from_invocation(invocation: &JsonInvocation) -> Self { + fn parse(node: &JsonNode) -> Option { + match node { + JsonNode::RustbuildStep { + type_: kind, + children, + duration_excluding_children_sec, + .. + } => { + let children: Vec<_> = children.into_iter().filter_map(parse).collect(); + let children_duration = children.iter().map(|c| c.duration).sum::(); + Some(BuildStep { + r#type: kind.to_string(), + children, + duration: children_duration + + Duration::from_secs_f64(*duration_excluding_children_sec), + }) + } + JsonNode::TestSuite(_) => None, + } + } + + let duration = Duration::from_secs_f64(invocation.duration_including_children_sec); + let children: Vec<_> = invocation.children.iter().filter_map(parse).collect(); + Self { r#type: "total".to_string(), children, duration } + } + + pub fn find_all_by_type(&self, r#type: &str) -> Vec<&Self> { + let mut result = Vec::new(); + self.find_by_type(r#type, &mut result); + result + } + + fn find_by_type<'a>(&'a self, r#type: &str, result: &mut Vec<&'a Self>) { + if self.r#type == r#type { + result.push(self); + } + for child in &self.children { + child.find_by_type(r#type, result); + } + } +} + +/// Writes build steps into a nice indented table. +pub fn format_build_steps(root: &BuildStep) -> String { + use std::fmt::Write; + + let mut substeps: Vec<(u32, &BuildStep)> = Vec::new(); + + fn visit<'a>(step: &'a BuildStep, level: u32, substeps: &mut Vec<(u32, &'a BuildStep)>) { + substeps.push((level, step)); + for child in &step.children { + visit(child, level + 1, substeps); + } + } + + visit(root, 0, &mut substeps); + + let mut output = String::new(); + for (level, step) in substeps { + let label = format!("{}{}", ".".repeat(level as usize), step.r#type); + writeln!(output, "{label:<65}{:>8.2}s", step.duration.as_secs_f64()).unwrap(); + } + output +} diff --git a/src/tools/opt-dist/src/metrics.rs b/src/tools/opt-dist/src/metrics.rs index 89c4cb12d4c3f..0a745566eb514 100644 --- a/src/tools/opt-dist/src/metrics.rs +++ b/src/tools/opt-dist/src/metrics.rs @@ -1,33 +1,10 @@ use std::time::Duration; -use build_helper::metrics::{JsonNode, JsonRoot}; +use build_helper::metrics::{BuildStep, JsonRoot, format_build_steps}; use camino::Utf8Path; use crate::timer::TimerSection; -#[derive(Clone, Debug)] -pub struct BuildStep { - r#type: String, - children: Vec, - duration: Duration, -} - -impl BuildStep { - pub fn find_all_by_type(&self, r#type: &str) -> Vec<&BuildStep> { - let mut result = Vec::new(); - self.find_by_type(r#type, &mut result); - result - } - fn find_by_type<'a>(&'a self, r#type: &str, result: &mut Vec<&'a BuildStep>) { - if self.r#type == r#type { - result.push(self); - } - for child in &self.children { - child.find_by_type(r#type, result); - } - } -} - /// Loads the metrics of the most recent bootstrap execution from a metrics.json file. pub fn load_metrics(path: &Utf8Path) -> anyhow::Result { let content = std::fs::read(path.as_std_path())?; @@ -37,30 +14,7 @@ pub fn load_metrics(path: &Utf8Path) -> anyhow::Result { .pop() .ok_or_else(|| anyhow::anyhow!("No bootstrap invocation found in metrics file"))?; - fn parse(node: JsonNode) -> Option { - match node { - JsonNode::RustbuildStep { - type_: kind, - children, - duration_excluding_children_sec, - .. - } => { - let children: Vec<_> = children.into_iter().filter_map(parse).collect(); - let children_duration = children.iter().map(|c| c.duration).sum::(); - Some(BuildStep { - r#type: kind.to_string(), - children, - duration: children_duration - + Duration::from_secs_f64(duration_excluding_children_sec), - }) - } - JsonNode::TestSuite(_) => None, - } - } - - let duration = Duration::from_secs_f64(invocation.duration_including_children_sec); - let children: Vec<_> = invocation.children.into_iter().filter_map(parse).collect(); - Ok(BuildStep { r#type: "root".to_string(), children, duration }) + Ok(BuildStep::from_invocation(&invocation)) } /// Logs the individual metrics in a table and add Rustc and LLVM durations to the passed @@ -82,27 +36,6 @@ pub fn record_metrics(metrics: &BuildStep, timer: &mut TimerSection) { timer.add_duration("Rustc", rustc_duration); } - log_metrics(metrics); -} - -fn log_metrics(metrics: &BuildStep) { - use std::fmt::Write; - - let mut substeps: Vec<(u32, &BuildStep)> = Vec::new(); - - fn visit<'a>(step: &'a BuildStep, level: u32, substeps: &mut Vec<(u32, &'a BuildStep)>) { - substeps.push((level, step)); - for child in &step.children { - visit(child, level + 1, substeps); - } - } - - visit(metrics, 0, &mut substeps); - - let mut output = String::new(); - for (level, step) in substeps { - let label = format!("{}{}", ".".repeat(level as usize), step.r#type); - writeln!(output, "{label:<65}{:>8.2}s", step.duration.as_secs_f64()).unwrap(); - } + let output = format_build_steps(metrics); log::info!("Build step durations\n{output}"); } From 7b53ac7ee67fbd62cadd883c860a6df263b6a481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 2 Mar 2025 18:23:11 +0100 Subject: [PATCH 4/4] Record bootstrap step durations into GitHub summary in citool --- src/build_helper/src/metrics.rs | 10 ++++++++-- src/ci/citool/src/metrics.rs | 30 ++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/build_helper/src/metrics.rs b/src/build_helper/src/metrics.rs index 4b585c1518246..eb306550fc4a6 100644 --- a/src/build_helper/src/metrics.rs +++ b/src/build_helper/src/metrics.rs @@ -175,8 +175,14 @@ pub fn format_build_steps(root: &BuildStep) -> String { let mut output = String::new(); for (level, step) in substeps { - let label = format!("{}{}", ".".repeat(level as usize), step.r#type); - writeln!(output, "{label:<65}{:>8.2}s", step.duration.as_secs_f64()).unwrap(); + let label = format!( + "{}{}", + ".".repeat(level as usize), + // Bootstrap steps can be generic and thus contain angle brackets (<...>). + // However, Markdown interprets these as HTML, so we need to escap ethem. + step.r#type.replace('<', "<").replace('>', ">") + ); + writeln!(output, "{label:.<65}{:>8.2}s", step.duration.as_secs_f64()).unwrap(); } output } diff --git a/src/ci/citool/src/metrics.rs b/src/ci/citool/src/metrics.rs index 9a1c7c4d910ad..2386413ed6b1d 100644 --- a/src/ci/citool/src/metrics.rs +++ b/src/ci/citool/src/metrics.rs @@ -4,7 +4,9 @@ use std::io::Write; use std::path::Path; use anyhow::Context; -use build_helper::metrics::{JsonNode, JsonRoot, TestOutcome, TestSuite, TestSuiteMetadata}; +use build_helper::metrics::{ + BuildStep, JsonNode, JsonRoot, TestOutcome, TestSuite, TestSuiteMetadata, format_build_steps, +}; pub fn postprocess_metrics(metrics_path: &Path, summary_path: &Path) -> anyhow::Result<()> { let metrics = load_metrics(metrics_path)?; @@ -15,7 +17,31 @@ pub fn postprocess_metrics(metrics_path: &Path, summary_path: &Path) -> anyhow:: .open(summary_path) .with_context(|| format!("Cannot open summary file at {summary_path:?}"))?; - record_test_suites(&metrics, &mut file)?; + if !metrics.invocations.is_empty() { + writeln!(file, "# Bootstrap steps")?; + record_bootstrap_step_durations(&metrics, &mut file)?; + record_test_suites(&metrics, &mut file)?; + } + + Ok(()) +} + +fn record_bootstrap_step_durations(metrics: &JsonRoot, file: &mut File) -> anyhow::Result<()> { + for invocation in &metrics.invocations { + let step = BuildStep::from_invocation(invocation); + let table = format_build_steps(&step); + eprintln!("Step `{}`\n{table}\n", invocation.cmdline); + writeln!( + file, + r"
+{} +
{table}
+
+", + invocation.cmdline + )?; + } + eprintln!("Recorded {} bootstrap invocation(s)", metrics.invocations.len()); Ok(()) }