diff options
| -rw-r--r-- | src/bootstrap/metrics.rs | 119 | ||||
| -rw-r--r-- | src/bootstrap/test.rs | 49 |
2 files changed, 144 insertions, 24 deletions
diff --git a/src/bootstrap/metrics.rs b/src/bootstrap/metrics.rs index e19d56ccd6a..5990f33b9bc 100644 --- a/src/bootstrap/metrics.rs +++ b/src/bootstrap/metrics.rs @@ -14,6 +14,25 @@ use std::io::BufWriter; use std::time::{Duration, Instant, SystemTime}; use sysinfo::{CpuExt, System, SystemExt}; +// Update this number whenever a breaking change is made to the build metrics. +// +// The output format is versioned for two reasons: +// +// - The metadata is intended to be consumed by external tooling, and exposing a format version +// helps the tools determine whether they're compatible with a metrics file. +// +// - If a developer enables build metrics in their local checkout, making a breaking change to the +// metrics format would result in a hard-to-diagnose error message when an existing metrics file +// is not compatible with the new changes. With a format version number, bootstrap can discard +// incompatible metrics files instead of appending metrics to them. +// +// Version changelog: +// +// - v0: initial version +// - v1: replaced JsonNode::Test with JsonNode::TestSuite +// +const CURRENT_FORMAT_VERSION: usize = 1; + pub(crate) struct BuildMetrics { state: RefCell<MetricsState>, } @@ -57,7 +76,7 @@ impl BuildMetrics { duration_excluding_children_sec: Duration::ZERO, children: Vec::new(), - tests: Vec::new(), + test_suites: Vec::new(), }); } @@ -84,6 +103,17 @@ impl BuildMetrics { } } + pub(crate) fn begin_test_suite(&self, metadata: TestSuiteMetadata, builder: &Builder<'_>) { + // Do not record dry runs, as they'd be duplicates of the actual steps. + if builder.config.dry_run() { + return; + } + + let mut state = self.state.borrow_mut(); + let step = state.running_steps.last_mut().unwrap(); + step.test_suites.push(TestSuite { metadata, tests: Vec::new() }); + } + pub(crate) fn record_test(&self, name: &str, outcome: TestOutcome, builder: &Builder<'_>) { // Do not record dry runs, as they'd be duplicates of the actual steps. if builder.config.dry_run() { @@ -91,12 +121,13 @@ impl BuildMetrics { } let mut state = self.state.borrow_mut(); - state - .running_steps - .last_mut() - .unwrap() - .tests - .push(Test { name: name.to_string(), outcome }); + let step = state.running_steps.last_mut().unwrap(); + + if let Some(test_suite) = step.test_suites.last_mut() { + test_suite.tests.push(Test { name: name.to_string(), outcome }); + } else { + panic!("metrics.record_test() called without calling metrics.begin_test_suite() first"); + } } fn collect_stats(&self, state: &mut MetricsState) { @@ -131,7 +162,20 @@ impl BuildMetrics { // Some of our CI builds consist of multiple independent CI invocations. Ensure all the // previous invocations are still present in the resulting file. let mut invocations = match std::fs::read(&dest) { - Ok(contents) => t!(serde_json::from_slice::<JsonRoot>(&contents)).invocations, + Ok(contents) => { + // We first parse just the format_version field to have the check succeed even if + // the rest of the contents are not valid anymore. + let version: OnlyFormatVersion = t!(serde_json::from_slice(&contents)); + if version.format_version == CURRENT_FORMAT_VERSION { + t!(serde_json::from_slice::<JsonRoot>(&contents)).invocations + } else { + println!( + "warning: overriding existing build/metrics.json, as it's not \ + compatible with build metrics format version {CURRENT_FORMAT_VERSION}." + ); + Vec::new() + } + } Err(err) => { if err.kind() != std::io::ErrorKind::NotFound { panic!("failed to open existing metrics file at {}: {err}", dest.display()); @@ -149,7 +193,7 @@ impl BuildMetrics { children: steps.into_iter().map(|step| self.prepare_json_step(step)).collect(), }); - let json = JsonRoot { system_stats, invocations }; + let json = JsonRoot { format_version: CURRENT_FORMAT_VERSION, system_stats, invocations }; t!(std::fs::create_dir_all(dest.parent().unwrap())); let mut file = BufWriter::new(t!(File::create(&dest))); @@ -159,11 +203,7 @@ impl BuildMetrics { fn prepare_json_step(&self, step: StepMetrics) -> JsonNode { let mut children = Vec::new(); children.extend(step.children.into_iter().map(|child| self.prepare_json_step(child))); - children.extend( - step.tests - .into_iter() - .map(|test| JsonNode::Test { name: test.name, outcome: test.outcome }), - ); + children.extend(step.test_suites.into_iter().map(JsonNode::TestSuite)); JsonNode::RustbuildStep { type_: step.type_, @@ -198,17 +238,14 @@ struct StepMetrics { duration_excluding_children_sec: Duration, children: Vec<StepMetrics>, - tests: Vec<Test>, -} - -struct Test { - name: String, - outcome: TestOutcome, + test_suites: Vec<TestSuite>, } #[derive(Serialize, Deserialize)] #[serde(rename_all = "snake_case")] struct JsonRoot { + #[serde(default)] // For version 0 the field was not present. + format_version: usize, system_stats: JsonInvocationSystemStats, invocations: Vec<JsonInvocation>, } @@ -237,14 +274,42 @@ enum JsonNode { children: Vec<JsonNode>, }, - Test { - name: String, - #[serde(flatten)] - outcome: TestOutcome, + TestSuite(TestSuite), +} + +#[derive(Serialize, Deserialize)] +struct TestSuite { + metadata: TestSuiteMetadata, + tests: Vec<Test>, +} + +#[derive(Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub(crate) enum TestSuiteMetadata { + CargoPackage { + crates: Vec<String>, + target: String, + host: String, + stage: u32, + }, + Compiletest { + suite: String, + mode: String, + compare_mode: Option<String>, + target: String, + host: String, + stage: u32, }, } #[derive(Serialize, Deserialize)] +pub(crate) struct Test { + name: String, + #[serde(flatten)] + outcome: TestOutcome, +} + +#[derive(Serialize, Deserialize)] #[serde(tag = "outcome", rename_all = "snake_case")] pub(crate) enum TestOutcome { Passed, @@ -266,3 +331,9 @@ struct JsonInvocationSystemStats { struct JsonStepSystemStats { cpu_utilization_percent: f64, } + +#[derive(Deserialize)] +struct OnlyFormatVersion { + #[serde(default)] // For version 0 the field was not present. + format_version: usize, +} diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index b829e19784a..44cd84be705 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -317,6 +317,17 @@ impl Step for Cargo { cargo.env("CARGO_TEST_DISABLE_NIGHTLY", "1"); cargo.env("PATH", &path_for_cargo(builder, compiler)); + #[cfg(feature = "build-metrics")] + builder.metrics.begin_test_suite( + crate::metrics::TestSuiteMetadata::CargoPackage { + crates: vec!["cargo".into()], + target: self.host.triple.to_string(), + host: self.host.triple.to_string(), + stage: self.stage, + }, + builder, + ); + let _time = util::timeit(&builder); add_flags_and_try_run_tests(builder, &mut cargo); } @@ -1699,6 +1710,19 @@ note: if you're sure you want to do this, please open an issue as to why. In the builder.ci_env.force_coloring_in_ci(&mut cmd); + #[cfg(feature = "build-metrics")] + builder.metrics.begin_test_suite( + crate::metrics::TestSuiteMetadata::Compiletest { + suite: suite.into(), + mode: mode.into(), + compare_mode: None, + target: self.target.triple.to_string(), + host: self.compiler.host.triple.to_string(), + stage: self.compiler.stage, + }, + builder, + ); + builder.info(&format!( "Check compiletest suite={} mode={} ({} -> {})", suite, mode, &compiler.host, target @@ -1708,6 +1732,20 @@ note: if you're sure you want to do this, please open an issue as to why. In the if let Some(compare_mode) = compare_mode { cmd.arg("--compare-mode").arg(compare_mode); + + #[cfg(feature = "build-metrics")] + builder.metrics.begin_test_suite( + crate::metrics::TestSuiteMetadata::Compiletest { + suite: suite.into(), + mode: mode.into(), + compare_mode: Some(compare_mode.into()), + target: self.target.triple.to_string(), + host: self.compiler.host.triple.to_string(), + stage: self.compiler.stage, + }, + builder, + ); + builder.info(&format!( "Check compiletest suite={} mode={} compare_mode={} ({} -> {})", suite, mode, compare_mode, &compiler.host, target @@ -2034,6 +2072,17 @@ fn run_cargo_test( let mut cargo = prepare_cargo_test(cargo, libtest_args, crates, primary_crate, compiler, target, builder); let _time = util::timeit(&builder); + + #[cfg(feature = "build-metrics")] + builder.metrics.begin_test_suite( + crate::metrics::TestSuiteMetadata::CargoPackage { + crates: crates.iter().map(|c| c.to_string()).collect(), + target: target.triple.to_string(), + host: compiler.host.triple.to_string(), + stage: compiler.stage, + }, + builder, + ); add_flags_and_try_run_tests(builder, &mut cargo) } |