Skip to content

Correct checking of end bound for triage comparison #1563

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.

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
merged 1 commit into from
Apr 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion site/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ pub mod triage {
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Request {
pub start: Bound,
pub end: Option<Bound>,
pub end: Bound,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down
58 changes: 37 additions & 21 deletions site/src/comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,29 @@ pub async fn handle_triage(
let mut num_comparisons = 0;
let metric = Metric::InstructionsUser;
let benchmark_map = ctxt.get_benchmark_category_map().await;
loop {
let comparison =
match compare_given_commits(before, next.clone(), metric, ctxt, &master_commits)
.await
.map_err(|e| format!("error comparing commits: {}", e))?
{
Some(c) => c,
None => {
log::info!(
"No data found for end bound {:?}. Ending comparison...",
next
);
break;
}
};
// Track whether we are on the known last iteration
let mut last_iteration = false;

let end = loop {
let comparison = match compare_given_commits(
before.clone(),
next.clone(),
metric,
ctxt,
&master_commits,
)
.await
.map_err(|e| format!("error comparing commits: {}", e))?
{
Some(c) => c,
None => {
log::info!(
"No data found for end bound {:?}. Ending comparison...",
next
);
break before;
}
};
num_comparisons += 1;
log::info!(
"Comparing {} to {}",
Expand All @@ -74,17 +82,25 @@ pub async fn handle_triage(
// handle results of comparison
populate_report(&comparison, &benchmark_map, &mut report).await;

// Check that there is a next commit and that the
// after commit is not equal to `end`
// If we already know this is the last iteration, we can stop
if last_iteration {
break before;
}

// Decide whether to keep doing comparisons or not
match comparison.next(&master_commits).map(Bound::Commit) {
Some(n) if Some(&next) != end.as_ref() => {
// There is a next commit, and it is not the end bound.
// We keep doing comparisons...
Some(n) => {
before = next;
// The next iteration is the last if the next bound is equal to the end bound
last_iteration = n != end;
next = n;
}
_ => break,
// There is no next commit so we stop.
None => break before,
}
}
let end = end.unwrap_or(next);
};

// Summarize the entire triage from start commit to end commit
let summary =
Expand Down