Compute job time in post-merge-report from the actual GitHub duration - #161235
Conversation
|
Rofl. I wrote the function in a way that it only compiles under Polonius. |
a870429 to
4314166
Compare
This comment has been minimized.
This comment has been minimized.
4314166 to
5fbbb20
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😆
There was a problem hiding this comment.
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... :)
|
@bors r=jieyouxu rollup |
…uwer Rollup of 11 pull requests Successful merges: - #161024 (resolving cyclic glob vis-max) - #161231 (passes: `rustc_scalable_vector` fields are not dead) - #161238 (remove scalar size mismatch interpreter error, make it an ICE instead) - #160345 (Resolver: add `checked` methods for `Cm(Ref)Cell`) - #161129 (Avoid ICE when recovering parenthesized type parameters) - #161235 (Compute job time in post-merge-report from the actual GitHub duration) - #161239 (Fix `#[repr(transparent)]` wrapper types not working with `Complex<T>`) - #161244 (Add regression test for indexing an unsized static without a body) - #161257 (Ignore target feature test when LLVM fails to compile minicore) - #161258 (perf: return early from in_external_macro for root contexts) - #161278 (Add regression test for normalization failure on erased closure in async block)
Rollup merge of #161235 - Kobzol:citool-actual-job-time, r=jieyouxu Compute job time in post-merge-report from the actual GitHub duration Up until now, the post-merge report analysis showed job durations computed from the sum of executed bootstrap steps. But this is quite confusing, because it does not correspond to the actual times we see on GitHub. I got quite worried recently about some durations, until I realized that I'm looking at something different than I thought I was. r? jieyouxu
…uwer Rollup of 11 pull requests Successful merges: - rust-lang/rust#161024 (resolving cyclic glob vis-max) - rust-lang/rust#161231 (passes: `rustc_scalable_vector` fields are not dead) - rust-lang/rust#161238 (remove scalar size mismatch interpreter error, make it an ICE instead) - rust-lang/rust#160345 (Resolver: add `checked` methods for `Cm(Ref)Cell`) - rust-lang/rust#161129 (Avoid ICE when recovering parenthesized type parameters) - rust-lang/rust#161235 (Compute job time in post-merge-report from the actual GitHub duration) - rust-lang/rust#161239 (Fix `#[repr(transparent)]` wrapper types not working with `Complex<T>`) - rust-lang/rust#161244 (Add regression test for indexing an unsized static without a body) - rust-lang/rust#161257 (Ignore target feature test when LLVM fails to compile minicore) - rust-lang/rust#161258 (perf: return early from in_external_macro for root contexts) - rust-lang/rust#161278 (Add regression test for normalization failure on erased closure in async block)
Up until now, the post-merge report analysis showed job durations computed from the sum of executed bootstrap steps. But this is quite confusing, because it does not correspond to the actual times we see on GitHub. I got quite worried recently about some durations, until I realized that I'm looking at something different than I thought I was.
r? jieyouxu