Skip to content

Commit cf0361b

Browse files
committed
show additional context for unrolled perf builds bisections
This is in case the commit has been GC'ed on github: the user would need to map back from the commit to the PR by looking at the comment posted by the bot. This can be automated.
1 parent 73ce27f commit cf0361b

File tree

1 file changed

+114
-17
lines changed

1 file changed

+114
-17
lines changed

src/main.rs

Lines changed: 114 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -570,13 +570,19 @@ impl Config {
570570
let toolchain = &result.searched[result.found];
571571
match self.search_perf_builds(toolchain) {
572572
Ok(result) => {
573+
let bisection = result.bisection;
573574
let url = format!(
574575
"https://github.com/rust-lang-ci/rust/commit/{}",
575-
result.searched[result.found]
576+
bisection.searched[bisection.found]
576577
)
577578
.red()
578579
.bold();
579580
eprintln!("Regression in {url}");
581+
582+
// In case the bisected commit has been garbage-collected by github, we show its
583+
// additional context here.
584+
let context = &result.toolchain_descriptions[bisection.found];
585+
eprintln!("The PR introducing the regression in this rollup is {context}");
580586
}
581587
Err(e) => {
582588
eprintln!("ERROR: {e}");
@@ -1183,7 +1189,7 @@ impl Config {
11831189
})
11841190
}
11851191

1186-
fn search_perf_builds(&self, toolchain: &Toolchain) -> anyhow::Result<BisectionResult> {
1192+
fn search_perf_builds(&self, toolchain: &Toolchain) -> anyhow::Result<PerfBisectionResult> {
11871193
eprintln!("Attempting to search unrolled perf builds");
11881194
let Toolchain {
11891195
spec: ToolchainSpec::Ci { commit, .. },
@@ -1205,13 +1211,19 @@ impl Config {
12051211
.filter(|c| c.user.login == "rust-timer")
12061212
.find(|c| c.body.contains("Perf builds for each rolled up PR"))
12071213
.context("couldn't find perf build comment")?;
1208-
let builds = extract_perf_shas(&perf_comment.body)?;
1209-
let short_sha = builds
1214+
let context = extract_perf_builds(&perf_comment.body)?;
1215+
let short_sha = context
1216+
.builds
12101217
.iter()
12111218
.map(|sha| sha.chars().take(8).collect())
12121219
.collect::<Vec<String>>();
12131220
eprintln!("Found commits {short_sha:?}");
1214-
self.linear_in_commits(&builds)
1221+
1222+
let bisection = self.linear_in_commits(&context.builds)?;
1223+
Ok(PerfBisectionResult {
1224+
bisection,
1225+
toolchain_descriptions: context.descriptions,
1226+
})
12151227
}
12161228

12171229
fn linear_in_commits(&self, commits: &[&str]) -> anyhow::Result<BisectionResult> {
@@ -1257,6 +1269,16 @@ struct BisectionResult {
12571269
dl_spec: DownloadParams,
12581270
}
12591271

1272+
/// The results of a bisection through the unrolled perf builds in a rollup:
1273+
/// - the regular bisection results
1274+
/// - a description of the rolled-up PRs for clearer diagnostics, in case the bisected commit
1275+
/// doesn't exist anymore on github.
1276+
#[derive(Clone)]
1277+
struct PerfBisectionResult {
1278+
bisection: BisectionResult,
1279+
toolchain_descriptions: Vec<String>,
1280+
}
1281+
12601282
fn main() {
12611283
if let Err(err) = run() {
12621284
match err.downcast::<ExitError>() {
@@ -1270,27 +1292,81 @@ fn main() {
12701292
}
12711293
}
12721294

1273-
/// Extracts the commits posted by the rust-timer bot on rollups, for unrolled perf builds.
1295+
/// An in-order mapping from perf build SHA to its description.
1296+
struct PerfBuildsContext<'a> {
1297+
builds: Vec<&'a str>,
1298+
descriptions: Vec<String>,
1299+
}
1300+
1301+
/// Extracts the commits posted by the rust-timer bot on rollups, for unrolled perf builds, with
1302+
/// their associated context: the PR number and title if available.
12741303
///
1275-
/// We're looking for a commit sha, in a comment whose format has changed (and could change in the
1304+
/// We're looking for a commit SHA, in a comment whose format has changed (and could change in the
12761305
/// future), for example:
12771306
/// - v1: https://github.com/rust-lang/rust/pull/113014#issuecomment-1605868471
12781307
/// - v2, the current: https://github.com/rust-lang/rust/pull/113105#issuecomment-1610393473
12791308
///
1280-
/// The sha comes in later columns, so we'll look for a 40-char hex string and give priority to the
1309+
/// The SHA comes in later columns, so we'll look for a 40-char hex string and give priority to the
12811310
/// last we find (to avoid possible conflicts with commits in the PR title column).
1282-
fn extract_perf_shas(body: &str) -> anyhow::Result<Vec<&str>> {
1311+
///
1312+
/// Depending on how recent the perf build commit is, it may have been garbage-collected by github:
1313+
/// perf-builds are force pushed to the `try-perf` branch, and accessing that commit can
1314+
/// 404. Therefore, we try to map back from that commit to the rolled-up PR present in the list of
1315+
/// unrolled builds.
1316+
fn extract_perf_builds(body: &str) -> anyhow::Result<PerfBuildsContext<'_>> {
1317+
let mut builds = Vec::new();
1318+
let mut descriptions = Vec::new();
1319+
12831320
let sha_regex = RegexBuilder::new(r"([0-9a-f]{40})")
12841321
.case_insensitive(true)
12851322
.build()?;
1286-
let builds = body
1323+
for line in body
12871324
.lines()
1288-
// lines of table with PR builds
1325+
// Only look at the lines of the unrolled perf builds table.
12891326
.filter(|l| l.starts_with("|#"))
1290-
// get the last sha we find, to prioritize the 3rd or 2nd columns.
1291-
.filter_map(|l| sha_regex.find_iter(l).last().and_then(|m| Some(m.as_str())))
1292-
.collect();
1293-
Ok(builds)
1327+
{
1328+
// Get the last SHA we find, to prioritize the 3rd or 2nd columns.
1329+
let sha = sha_regex
1330+
.find_iter(line)
1331+
.last()
1332+
.and_then(|m| Some(m.as_str()));
1333+
1334+
// If we did find one, we try to extract the associated description.
1335+
let Some(sha) = sha else { continue };
1336+
1337+
let mut description = String::new();
1338+
1339+
// In v1 and v2, we know that the first column is the PR number.
1340+
//
1341+
// In the unlikely event it's missing because of a parsing discrepancy, we don't want to
1342+
// ignore it, and ask for feedback: we always want to have *some* context per PR, matching
1343+
// the number of SHAs we found.
1344+
let Some(pr) = line.split('|').nth(1) else {
1345+
bail!("Couldn't get rolled-up PR number for SHA {sha}, please open an issue.");
1346+
};
1347+
1348+
description.push_str(pr);
1349+
1350+
// The second column could be a link to the commit (which we don't want in the description),
1351+
// or the PR title (which we want).
1352+
if let Some(title) = line.split('|').nth(2) {
1353+
// For v1, this column would contain the commit, and we won't have the PR title
1354+
// anywhere. So we try to still give some context for that older format: if the column
1355+
// contains the SHA, we don't add that to the description.
1356+
if !title.contains(sha) {
1357+
description.push_str(": ");
1358+
description.push_str(title);
1359+
}
1360+
}
1361+
1362+
builds.push(sha);
1363+
descriptions.push(description);
1364+
}
1365+
1366+
Ok(PerfBuildsContext {
1367+
builds,
1368+
descriptions,
1369+
})
12941370
}
12951371

12961372
#[cfg(test)]
@@ -1381,6 +1457,8 @@ mod tests {
13811457
13821458
In the case of a perf regression, run the following command for each PR you suspect might be the cause: `@rust-timer build $SHA`
13831459
<!-- rust-timer: rollup -->";
1460+
let context =
1461+
extract_perf_builds(body).expect("extracting perf builds context on v1 format failed");
13841462
assert_eq!(
13851463
vec![
13861464
"05b07dad146a6d43ead9bcd1e8bc10cbd017a5f5",
@@ -1389,7 +1467,11 @@ In the case of a perf regression, run the following command for each PR you susp
13891467
"0ed6ba504649ca1cb2672572b4ab41acfb06c86c",
13901468
"18e108ab85b78e6966c5b5bdadfd5b8efeadf080",
13911469
],
1392-
extract_perf_shas(body).expect("extracting perf builds on v1 format failed"),
1470+
context.builds,
1471+
);
1472+
assert_eq!(
1473+
vec!["#113009", "#113008", "#112956", "#112950", "#112937",],
1474+
context.descriptions,
13931475
);
13941476
}
13951477

@@ -1416,6 +1498,8 @@ In the case of a perf regression, run the following command for each PR you susp
14161498
14171499
In the case of a perf regression, run the following command for each PR you suspect might be the cause: `@rust-timer build $SHA`
14181500
<!-- rust-timer: rollup -->";
1501+
let context =
1502+
extract_perf_builds(body).expect("extracting perf builds context on v2 format failed");
14191503
assert_eq!(
14201504
vec![
14211505
"bbec6d6e413aa144c8b9346da27a0f2af299cbeb",
@@ -1427,7 +1511,20 @@ In the case of a perf regression, run the following command for each PR you susp
14271511
"0ce4618dbf5810aabb389edd4950c060b6b4d049",
14281512
"241cd8cd818cdc865cdf02f0c32a40081420b772",
14291513
],
1430-
extract_perf_shas(body).expect("extracting perf builds on v2 format failed"),
1514+
context.builds,
1515+
);
1516+
assert_eq!(
1517+
vec![
1518+
"#112207: Add trustzone and virtualization target features for aarch3…",
1519+
"#112454: Make compiletest aware of targets without dynamic linking",
1520+
"#112628: Allow comparing `Box`es with different allocators",
1521+
"#112692: Provide more context for `rustc +nightly -Zunstable-options…",
1522+
"#112972: Make `UnwindAction::Continue` explicit in MIR dump",
1523+
"#113020: Add tests impl via obj unless denied",
1524+
"#113084: Simplify some conditions",
1525+
"#113103: Normalize types when applying uninhabited predicate.",
1526+
],
1527+
context.descriptions,
14311528
);
14321529
}
14331530
}

0 commit comments

Comments
 (0)