Skip to content

Compute job time in post-merge-report from the actual GitHub duration #161235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 4 commits into from
Aug 18, 2026
Merged

Compute job time in post-merge-report from the actual GitHub duration #161235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b3b3e9a
Compute job time in post-merge-report from the actual GitHub duration
Kobzol Aug 17, 2026
7994392
Handle possible negative job durations
Kobzol Aug 17, 2026
5fbbb20
Handle job durations that cannot be fetched
Kobzol Aug 17, 2026
119dda9
Modify comments
Kobzol Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 129 additions & 3 deletions src/ci/citool/Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -181,6 +187,7 @@ dependencies = [
"flate2",
"glob-match",
"insta",
"jiff",
"serde",
"serde_json",
"serde_yaml",
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/ci/citool/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
18 changes: 7 additions & 11 deletions src/ci/citool/src/analysis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,17 +237,13 @@ pub fn output_largest_job_duration_changes(
let mut changes: Vec<Entry> = 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::<Duration>();
let duration_after = metrics
.current
.invocations
.iter()
.map(|i| BuildStep::from_invocation(i).duration)
.sum::<Duration>();
// 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 =
job_info_resolver.get_job_duration(job, &metrics.current).unwrap_or(Duration::ZERO);
Comment on lines +243 to +246

@jieyouxu jieyouxu Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: I wonder if we can indicate which of the {github, bootstrap} job duration source is actually being reported, but I think at least pairing the start/end times from the same source is sensible.

@Kobzol Kobzol Aug 18, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we talk about the duration of CI jobs, I think that reportin the GitHub job duration is obvious. We just didn't do the obvious thing previously 😆

@Kobzol Kobzol Aug 18, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some benefits to reporting pure bootstrap time though, as it doesn't take into account Docker builds and other stuff that can be noisy. But the disadvantage is that it doesn't take into account Docker builds and... :)

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
Expand Down
55 changes: 35 additions & 20 deletions src/ci/citool/src/github.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::collections::HashMap;
use std::collections::hash_map::Entry;
use std::time::Duration;

use anyhow::Context;
use build_helper::metrics::{CiMetadata, JsonRoot};
Expand Down Expand Up @@ -60,6 +62,8 @@ struct WorkflowRunJobsResponse {
struct GitHubJob {
name: String,
id: u64,
started_at: Option<jiff::Timestamp>,
completed_at: Option<jiff::Timestamp>,
}

/// Can be used to resolve information about GitHub Actions jobs.
Expand All @@ -78,32 +82,43 @@ 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<String> {
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<u64> {
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);
}
/// Get duration of the given job.
pub fn get_job_duration(&mut self, job_name: &str, metrics: &JsonRoot) -> Option<Duration> {
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 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()?;
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
// 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);
Comment thread
Kobzol marked this conversation as resolved.
Some(duration)
})
})
}

fn get_job_data(&mut self, ci_metadata: &CiMetadata, job_name: &str) -> Option<&GitHubJob> {
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)
}
}
}
}
Loading