From b3b3e9ae3e0030a4fb9c8774f3b01bb2333696ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 17 Aug 2026 14:03:37 +0200 Subject: [PATCH 1/4] Compute job time in post-merge-report from the actual GitHub duration --- src/ci/citool/Cargo.toml | 1 + src/ci/citool/src/analysis.rs | 29 ++++++++++++++++++----------- src/ci/citool/src/github.rs | 32 +++++++++++++++++++++++++------- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/ci/citool/Cargo.toml b/src/ci/citool/Cargo.toml index 4e954f77c081e..83d57b3294bbf 100644 --- a/src/ci/citool/Cargo.toml +++ b/src/ci/citool/Cargo.toml @@ -11,6 +11,7 @@ csv = "1" diff = "0.1" flate2 = { version = "1.1.9", default-features = false, features = ["zlib-rs"] } # For feature flag only glob-match = "0.2" +jiff = { version = "0.2", features = ["serde"] } serde = { version = "1", features = ["derive"] } serde_yaml = "0.9" serde_json = "1" diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs index b8048f341ea2e..23833744394ff 100644 --- a/src/ci/citool/src/analysis.rs +++ b/src/ci/citool/src/analysis.rs @@ -237,17 +237,24 @@ pub fn output_largest_job_duration_changes( let mut changes: Vec = vec![]; for (job, metrics) in job_metrics { if let Some(parent) = &metrics.parent { - let duration_before = parent - .invocations - .iter() - .map(|i| BuildStep::from_invocation(i).duration) - .sum::(); - let duration_after = metrics - .current - .invocations - .iter() - .map(|i| BuildStep::from_invocation(i).duration) - .sum::(); + // Try to get duration from GitHub, if it fails, compute it from bootstrap metrics. + let duration_before = + job_info_resolver.get_job_duration(job, parent).unwrap_or_else(|| { + parent + .invocations + .iter() + .map(|i| BuildStep::from_invocation(i).duration) + .sum::() + }); + let duration_after = + job_info_resolver.get_job_duration(job, &metrics.current).unwrap_or_else(|| { + metrics + .current + .invocations + .iter() + .map(|i| BuildStep::from_invocation(i).duration) + .sum::() + }); let pct_change = duration_after.as_secs_f64() / duration_before.as_secs_f64(); let pct_change = pct_change * 100.0; // Normalize around 100, to get + for regression and - for improvements diff --git a/src/ci/citool/src/github.rs b/src/ci/citool/src/github.rs index 35e4c3f9599d6..6279d96f37e4c 100644 --- a/src/ci/citool/src/github.rs +++ b/src/ci/citool/src/github.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::time::Duration; use anyhow::Context; use build_helper::metrics::{CiMetadata, JsonRoot}; @@ -60,6 +61,8 @@ struct WorkflowRunJobsResponse { struct GitHubJob { name: String, id: u64, + started_at: Option, + completed_at: Option, } /// Can be used to resolve information about GitHub Actions jobs. @@ -78,22 +81,34 @@ impl JobInfoResolver { /// Get a link to a job summary for the given job name and bootstrap execution. pub fn get_job_summary_link(&mut self, job_name: &str, metrics: &JsonRoot) -> Option { metrics.ci_metadata.as_ref().and_then(|metadata| { - self.get_job_id(metadata, job_name).map(|job_id| { + self.get_job_data(metadata, job_name).map(|job| { format!( - "https://github.com/{}/actions/runs/{}#summary-{job_id}", - metadata.repository, metadata.workflow_run_id + "https://github.com/{}/actions/runs/{}#summary-{}", + metadata.repository, metadata.workflow_run_id, job.id ) }) }) } - fn get_job_id(&mut self, ci_metadata: &CiMetadata, job_name: &str) -> Option { + /// Get duration of the given job. + pub fn get_job_duration(&mut self, job_name: &str, metrics: &JsonRoot) -> Option { + metrics.ci_metadata.as_ref().and_then(|metadata| { + self.get_job_data(metadata, job_name).and_then(|job| { + let start = job.started_at?; + let end = job.completed_at?; + let duration = Duration::try_from(end - start).ok()?; + Some(duration) + }) + }) + } + + fn get_job_data(&mut self, ci_metadata: &CiMetadata, job_name: &str) -> Option<&GitHubJob> { if let Some(job) = self .workflow_job_cache .get(&ci_metadata.workflow_run_id) .and_then(|jobs| jobs.iter().find(|j| j.name == job_name)) { - return Some(job.id); + return Some(job); } let jobs = self @@ -101,9 +116,12 @@ impl JobInfoResolver { .get_workflow_run_jobs(&ci_metadata.repository, ci_metadata.workflow_run_id) .inspect_err(|e| eprintln!("Cannot download workflow jobs: {e:?}")) .ok()?; - let job_id = jobs.iter().find(|j| j.name == job_name).map(|j| j.id); // Save the cache even if the job name was not found, it could be useful for further lookups self.workflow_job_cache.insert(ci_metadata.workflow_run_id, jobs); - job_id + self.workflow_job_cache + .get(&ci_metadata.workflow_run_id) + .unwrap() + .iter() + .find(|j| j.name == job_name) } } From 7994392d6eb417ebf86a1aec81e9662e4bca20b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 17 Aug 2026 14:41:12 +0200 Subject: [PATCH 2/4] Handle possible negative job durations --- src/ci/citool/src/github.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/citool/src/github.rs b/src/ci/citool/src/github.rs index 6279d96f37e4c..e61066bd6d933 100644 --- a/src/ci/citool/src/github.rs +++ b/src/ci/citool/src/github.rs @@ -96,7 +96,7 @@ impl JobInfoResolver { self.get_job_data(metadata, job_name).and_then(|job| { let start = job.started_at?; let end = job.completed_at?; - let duration = Duration::try_from(end - start).ok()?; + let duration = Duration::try_from(end - start).unwrap_or(Duration::ZERO); Some(duration) }) }) From 5fbbb209695409c2ceb8cf03c374a3cdc7493365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 17 Aug 2026 14:42:11 +0200 Subject: [PATCH 3/4] Handle job durations that cannot be fetched --- src/ci/citool/Cargo.lock | 132 +++++++++++++++++++++++++++++++++- src/ci/citool/src/analysis.rs | 17 +---- src/ci/citool/src/github.rs | 32 ++++----- 3 files changed, 144 insertions(+), 37 deletions(-) diff --git a/src/ci/citool/Cargo.lock b/src/ci/citool/Cargo.lock index e455a45415c7d..4e0f51ee855e9 100644 --- a/src/ci/citool/Cargo.lock +++ b/src/ci/citool/Cargo.lock @@ -132,6 +132,12 @@ dependencies = [ "serde", ] +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "bitflags" version = "2.13.1" @@ -181,6 +187,7 @@ dependencies = [ "flate2", "glob-match", "insta", + "jiff", "serde", "serde_json", "serde_yaml", @@ -303,6 +310,37 @@ dependencies = [ "memchr", ] +[[package]] +name = "defmt" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2953bfe4f93bbd20cc71198842756f77d161884c99ebbabc41d80231ded88d1" +dependencies = [ + "bitflags 1.3.2", + "defmt-macros", +] + +[[package]] +name = "defmt-macros" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bad9c72e7ca2137e0dc3813245a0d282fd6daad32fd800af018306a9169b5fe8" +dependencies = [ + "defmt-parser", + "proc-macro2", + "quote", + "syn 2.0.119", +] + +[[package]] +name = "defmt-parser" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10d60334b3b2e7c9d91ef8150abfb6fa4c1c39ebbcf4a81c2e346aad939fee3e" +dependencies = [ + "thiserror", +] + [[package]] name = "deranged" version = "0.5.8" @@ -589,6 +627,59 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "jiff" +version = "0.2.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "668b7183bd07af9a4885f5c35b0cc5c83c4607a913c16b7e17291832910d2dcc" +dependencies = [ + "defmt", + "jiff-core", + "jiff-static", + "jiff-tzdb-platform", + "log", + "portable-atomic", + "portable-atomic-util", + "serde_core", + "windows-link", +] + +[[package]] +name = "jiff-core" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7feca88439efe53da3754500c1851dedf3cb36c524dd5cf8225cc0794de95d09" +dependencies = [ + "defmt", +] + +[[package]] +name = "jiff-static" +version = "0.2.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a69dcb3a21cfb32ce1cd056169337ca284af0766dd766e7878819b251a49204" +dependencies = [ + "jiff-core", + "proc-macro2", + "quote", + "syn 2.0.119", +] + +[[package]] +name = "jiff-tzdb" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "142bd39932ad231f10513df9ab62661fead8719872150b7ad02a2df79f4e141e" + +[[package]] +name = "jiff-tzdb-platform" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "875a5a69ac2bab1a891711cf5eccbec1ce0341ea805560dcd90b7a2e925132e8" +dependencies = [ + "jiff-tzdb", +] + [[package]] name = "libc" version = "0.2.189" @@ -659,6 +750,21 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" +[[package]] +name = "portable-atomic" +version = "1.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05c8b63e8d9609db387f0324918f81d68fe27748f084ef092fb35954d0539a85" + +[[package]] +name = "portable-atomic-util" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a106d1259c23fac8e543272398ae0e3c0b8d33c88ed73d0cc71b0f1d902618" +dependencies = [ + "portable-atomic", +] + [[package]] name = "potential_utf" version = "0.1.6" @@ -724,7 +830,7 @@ version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" dependencies = [ - "bitflags", + "bitflags 2.13.1", "errno", "libc", "linux-raw-sys", @@ -916,6 +1022,26 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "thiserror" +version = "2.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", +] + [[package]] name = "time" version = "0.3.55" @@ -1234,9 +1360,9 @@ dependencies = [ [[package]] name = "zerovec-derive" -version = "0.11.4" +version = "0.11.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47402523226a02bfe5230160dc3ccc089aa6f6f19e7fcbb4e6f824bbb1b4aa62" +checksum = "9f212a141d820099d57ffafb9569be9617a6f27d3dc881fbee8fb56642f917a9" dependencies = [ "proc-macro2", "quote", diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs index 23833744394ff..3941f6af15c0c 100644 --- a/src/ci/citool/src/analysis.rs +++ b/src/ci/citool/src/analysis.rs @@ -239,22 +239,9 @@ pub fn output_largest_job_duration_changes( if let Some(parent) = &metrics.parent { // Try to get duration from GitHub, if it fails, compute it from bootstrap metrics. let duration_before = - job_info_resolver.get_job_duration(job, parent).unwrap_or_else(|| { - parent - .invocations - .iter() - .map(|i| BuildStep::from_invocation(i).duration) - .sum::() - }); + job_info_resolver.get_job_duration(job, parent).unwrap_or(Duration::ZERO); let duration_after = - job_info_resolver.get_job_duration(job, &metrics.current).unwrap_or_else(|| { - metrics - .current - .invocations - .iter() - .map(|i| BuildStep::from_invocation(i).duration) - .sum::() - }); + job_info_resolver.get_job_duration(job, &metrics.current).unwrap_or(Duration::ZERO); let pct_change = duration_after.as_secs_f64() / duration_before.as_secs_f64(); let pct_change = pct_change * 100.0; // Normalize around 100, to get + for regression and - for improvements diff --git a/src/ci/citool/src/github.rs b/src/ci/citool/src/github.rs index e61066bd6d933..a3e1e1771e703 100644 --- a/src/ci/citool/src/github.rs +++ b/src/ci/citool/src/github.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::collections::hash_map::Entry; use std::time::Duration; use anyhow::Context; @@ -103,25 +104,18 @@ impl JobInfoResolver { } fn get_job_data(&mut self, ci_metadata: &CiMetadata, job_name: &str) -> Option<&GitHubJob> { - if let Some(job) = self - .workflow_job_cache - .get(&ci_metadata.workflow_run_id) - .and_then(|jobs| jobs.iter().find(|j| j.name == job_name)) - { - return Some(job); + match self.workflow_job_cache.entry(ci_metadata.workflow_run_id) { + Entry::Occupied(jobs) => jobs.into_mut().iter().find(|j| j.name == job_name), + Entry::Vacant(entry) => { + let jobs = self + .client + .get_workflow_run_jobs(&ci_metadata.repository, ci_metadata.workflow_run_id) + .inspect_err(|e| eprintln!("Cannot download workflow jobs: {e:?}")) + .ok()?; + // Save the cache even if the job name was not found, it could be useful for further lookups + let jobs = entry.insert(jobs); + jobs.iter().find(|j| j.name == job_name) + } } - - let jobs = self - .client - .get_workflow_run_jobs(&ci_metadata.repository, ci_metadata.workflow_run_id) - .inspect_err(|e| eprintln!("Cannot download workflow jobs: {e:?}")) - .ok()?; - // Save the cache even if the job name was not found, it could be useful for further lookups - self.workflow_job_cache.insert(ci_metadata.workflow_run_id, jobs); - self.workflow_job_cache - .get(&ci_metadata.workflow_run_id) - .unwrap() - .iter() - .find(|j| j.name == job_name) } } From 119dda983b136316307a2057611c8bfd70ad900d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 18 Aug 2026 13:04:08 +0200 Subject: [PATCH 4/4] Modify comments --- src/ci/citool/src/analysis.rs | 4 +++- src/ci/citool/src/github.rs | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs index 3941f6af15c0c..2d04726df399c 100644 --- a/src/ci/citool/src/analysis.rs +++ b/src/ci/citool/src/analysis.rs @@ -237,7 +237,9 @@ pub fn output_largest_job_duration_changes( let mut changes: Vec = vec![]; for (job, metrics) in job_metrics { if let Some(parent) = &metrics.parent { - // Try to get duration from GitHub, if it fails, compute it from bootstrap metrics. + // Try to get duration from GitHub. + // If it fails, treat the duration as zero - it should be obvious in the post-merge + // report that something failed in that case. let duration_before = job_info_resolver.get_job_duration(job, parent).unwrap_or(Duration::ZERO); let duration_after = diff --git a/src/ci/citool/src/github.rs b/src/ci/citool/src/github.rs index a3e1e1771e703..01ee5114842e5 100644 --- a/src/ci/citool/src/github.rs +++ b/src/ci/citool/src/github.rs @@ -97,6 +97,9 @@ impl JobInfoResolver { self.get_job_data(metadata, job_name).and_then(|job| { let start = job.started_at?; let end = job.completed_at?; + + // If `end` is for whatever reason earlier than `start`, then we assume that the + // duration is zero, because we do not want negative durations. let duration = Duration::try_from(end - start).unwrap_or(Duration::ZERO); Some(duration) })