Skip to content

Commit 1c8f187

Browse files
JonathanBrouwerJonathanBrouwer
authored
Merge pull request #2545 from JonathanBrouwer/triage-run-update
Update the triage comment with results when a benchmark run completes
2 parents 4a6df5e + 4430c9d commit 1c8f187

4 files changed

Lines changed: 252 additions & 16 deletions

File tree

‎site/src/github/client.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,33 @@ impl GraphQLClient {
189189
Ok(comments)
190190
}
191191

192+
pub async fn update_comment_content(
193+
&self,
194+
comment_id: &str,
195+
new_body: &str,
196+
) -> anyhow::Result<()> {
197+
#[derive(serde::Deserialize)]
198+
struct UpdateCommentData {}
199+
200+
const UPDATE_COMMENT: &str = "mutation($id: ID!, $body: String!) {
201+
updateIssueComment(input: {id: $id, body: $body}) {
202+
issueComment {
203+
id
204+
}
205+
}
206+
}";
207+
208+
self.send::<UpdateCommentData, _>(
209+
UPDATE_COMMENT,
210+
serde_json::json!({
211+
"id": comment_id,
212+
"body": new_body,
213+
}),
214+
)
215+
.await?;
216+
Ok(())
217+
}
218+
192219
pub async fn hide_comment(&self, comment_id: &str, reason: &str) -> anyhow::Result<()> {
193220
#[derive(serde::Deserialize)]
194221
struct MinimizeData {}

‎site/src/github/comparison_summary.rs‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::load::SiteCtxt;
66

77
use database::{metric::Metric, QueuedCommit};
88

9-
use crate::github::triage::{is_triage_run, TriageBuild};
9+
use crate::github::triage::{is_triage_run, update_triage_body, TriageBuild};
1010
use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL};
1111
use humansize::BINARY;
1212
use std::fmt::Write;
@@ -38,7 +38,30 @@ pub async fn post_comparison_comment(
3838

3939
let source = if is_master_commit {
4040
PerfRunSource::MasterCommit
41-
} else if let Some(triage_run) = is_triage_run(&commit, &mut client, &mut graph_client).await? {
41+
} else if let Some(mut triage_run) =
42+
is_triage_run(&commit, &mut client, &mut graph_client).await?
43+
{
44+
// Update the triage comment
45+
let triage_summary = metrics_result(ctxt, &commit)
46+
.await
47+
.unwrap_or_else(|error| error);
48+
match update_triage_body(&mut triage_run.triage_comment.body, pr, triage_summary) {
49+
Ok(()) => {
50+
// While in theory this is racy (the comment could have been edited since querying it),
51+
// in practice this should never happen
52+
if let Err(err) = graph_client
53+
.update_comment_content(
54+
&triage_run.triage_comment.id,
55+
&triage_run.triage_comment.body,
56+
)
57+
.await
58+
{
59+
log::error!("Failed to update triage comment: {err:?}")
60+
}
61+
}
62+
Err(err) => log::error!("Failed to update triage body: {err:?}"),
63+
}
64+
4265
PerfRunSource::TriageBuild(triage_run)
4366
} else {
4467
PerfRunSource::TryBuild
@@ -47,7 +70,6 @@ pub async fn post_comparison_comment(
4770
let body = summarize_run(ctxt, commit, source)
4871
.await
4972
.unwrap_or_else(|error| error);
50-
5173
client.post_comment(pr, body).await;
5274

5375
Ok(())
@@ -180,47 +202,53 @@ async fn summarize_run(
180202
let bootstrap = summarize_bootstrap(&inst_comparison);
181203
let artifact_size = summarize_artifact_size(&inst_comparison);
182204

205+
write!(&mut message, "{}", metrics_result(ctxt, &commit).await?).unwrap();
206+
write!(&mut message, "\n{bootstrap}").unwrap();
207+
write!(&mut message, "\n{artifact_size}").unwrap();
208+
209+
Ok(message)
210+
}
211+
212+
pub async fn metrics_result(ctxt: &SiteCtxt, commit: &QueuedCommit) -> Result<String, String> {
213+
let benchmark_map = ctxt.get_benchmark_category_map().await;
214+
let mut metrics_result = String::new();
183215
let metrics = vec![
184216
(
185217
"Instruction count",
186218
Metric::InstructionsUser,
187219
DefaultMetricVisibility::Shown,
188-
inst_comparison,
220+
calculate_metric_comparison(ctxt, commit, Metric::InstructionsUser).await?,
189221
),
190222
(
191223
"Max RSS (memory usage)",
192224
Metric::MaxRSS,
193225
DefaultMetricVisibility::Hidden,
194-
calculate_metric_comparison(ctxt, &commit, Metric::MaxRSS).await?,
226+
calculate_metric_comparison(ctxt, commit, Metric::MaxRSS).await?,
195227
),
196228
(
197229
"Cycles",
198230
Metric::CyclesUser,
199231
DefaultMetricVisibility::Hidden,
200-
calculate_metric_comparison(ctxt, &commit, Metric::CyclesUser).await?,
232+
calculate_metric_comparison(ctxt, commit, Metric::CyclesUser).await?,
201233
),
202234
(
203235
"Binary size",
204236
Metric::LinkedArtifactSize,
205237
DefaultMetricVisibility::Hidden,
206-
calculate_metric_comparison(ctxt, &commit, Metric::LinkedArtifactSize).await?,
238+
calculate_metric_comparison(ctxt, commit, Metric::LinkedArtifactSize).await?,
207239
),
208240
];
209241

210242
for (title, metric, visibility, comparison) in metrics {
211-
message.push_str(&format!(
243+
metrics_result.push_str(&format!(
212244
"\n### [{title}]({})\n",
213-
make_comparison_url(&commit, metric)
245+
make_comparison_url(commit, metric)
214246
));
215247

216248
let (primary, secondary) = comparison.summarize_compile_by_category(&benchmark_map);
217-
write_metric_summary(primary, secondary, visibility, &mut message);
249+
write_metric_summary(primary, secondary, visibility, &mut metrics_result);
218250
}
219-
220-
write!(&mut message, "\n{bootstrap}").unwrap();
221-
write!(&mut message, "\n{artifact_size}").unwrap();
222-
223-
Ok(message)
251+
Ok(metrics_result)
224252
}
225253

226254
fn summarize_artifact_size(comparison: &ArtifactComparison) -> String {

‎site/src/github/triage.rs‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::github::client::{Client, GraphQLClient, ResponseComment};
22
use crate::request_handlers::parse_unrolled_build_message;
3+
use anyhow::bail;
34
use database::QueuedCommit;
45

56
pub struct TriageBuild {
@@ -9,6 +10,13 @@ pub struct TriageBuild {
910

1011
pub const TRIAGE_MARKER: &str = "<!-- rust-timer: triage -->";
1112

13+
pub fn triage_body_start_marker(pr: u32) -> String {
14+
format!("<!-- rust-timer:triage-body-start-pr-{pr} -->\n")
15+
}
16+
pub fn triage_body_end_marker(pr: u32) -> String {
17+
format!("<!-- rust-timer:triage-body-end-pr-{pr} -->\n")
18+
}
19+
1220
/// Returns `Some` if this commit is part of a triage run.
1321
pub async fn is_triage_run(
1422
commit: &QueuedCommit,
@@ -38,3 +46,164 @@ pub async fn is_triage_run(
3846
triage_comment,
3947
}))
4048
}
49+
50+
pub fn update_triage_body(
51+
body: &mut String,
52+
pr: u32,
53+
triage_summary: String,
54+
) -> anyhow::Result<()> {
55+
let Some(body_start) = body.find(&triage_body_start_marker(pr)) else {
56+
bail!("Failed to triage body start")
57+
};
58+
let body_start = body_start + triage_body_start_marker(pr).len();
59+
let Some(body_end) = body.find(&triage_body_end_marker(pr)) else {
60+
bail!("Failed to triage body end")
61+
};
62+
63+
body.replace_range(body_start..body_end, &triage_summary);
64+
Ok(())
65+
}
66+
67+
#[cfg(test)]
68+
mod tests {
69+
use crate::github::triage::update_triage_body;
70+
71+
const BEFORE_BODY: &str = "### #157428 364c9dea926885d60daef4bf1a15fa03efa35a84 allocator: refactor for stabilisation
72+
<!-- rust-timer:triage-body-start-pr-157428 -->
73+
Queued 364c9dea926885d60daef4bf1a15fa03efa35a84 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=364c9dea926885d60daef4bf1a15fa03efa35a84).
74+
There are currently 0 preceding artifacts in the [queue](https://perf.rust-lang.org/status.html).
75+
It will probably take at least ~1.0 hours until the benchmark run finishes.
76+
<!-- rust-timer:triage-body-end-pr-157428 -->
77+
78+
### #160077 a96bde1411c3b0fe68417c04e85624f33b645516 Don't compute FnAbi for LLVM intrinsics
79+
<!-- rust-timer:triage-body-start-pr-160077 -->
80+
Queued a96bde1411c3b0fe68417c04e85624f33b645516 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=a96bde1411c3b0fe68417c04e85624f33b645516).
81+
There is currently 1 preceding artifact in the [queue](https://perf.rust-lang.org/status.html).
82+
It will probably take at least ~2.0 hours until the benchmark run finishes.
83+
<!-- rust-timer:triage-body-end-pr-160077 -->
84+
85+
### #160288 77cb04ee49e4f29833262112d5abf705c96faa98 rustdoc: use anonymous constant for primitives/keywords/attribute docs
86+
<!-- rust-timer:triage-body-start-pr-160288 -->
87+
Queued 77cb04ee49e4f29833262112d5abf705c96faa98 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=77cb04ee49e4f29833262112d5abf705c96faa98).
88+
There are currently 2 preceding artifacts in the [queue](https://perf.rust-lang.org/status.html).
89+
It will probably take at least ~3.0 hours until the benchmark run finishes.
90+
<!-- rust-timer:triage-body-end-pr-160288 -->
91+
92+
<!-- rust-timer: triage -->";
93+
94+
#[test]
95+
fn test_update_first() {
96+
let mut body = BEFORE_BODY.to_string();
97+
update_triage_body(
98+
&mut body,
99+
157428,
100+
"NEW BODY1\nNEW BODY2\nNEW BODY3\n".to_string(),
101+
)
102+
.unwrap();
103+
104+
const EXPECTED_RESULT: &str = "### #157428 364c9dea926885d60daef4bf1a15fa03efa35a84 allocator: refactor for stabilisation
105+
<!-- rust-timer:triage-body-start-pr-157428 -->
106+
NEW BODY1
107+
NEW BODY2
108+
NEW BODY3
109+
<!-- rust-timer:triage-body-end-pr-157428 -->
110+
111+
### #160077 a96bde1411c3b0fe68417c04e85624f33b645516 Don't compute FnAbi for LLVM intrinsics
112+
<!-- rust-timer:triage-body-start-pr-160077 -->
113+
Queued a96bde1411c3b0fe68417c04e85624f33b645516 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=a96bde1411c3b0fe68417c04e85624f33b645516).
114+
There is currently 1 preceding artifact in the [queue](https://perf.rust-lang.org/status.html).
115+
It will probably take at least ~2.0 hours until the benchmark run finishes.
116+
<!-- rust-timer:triage-body-end-pr-160077 -->
117+
118+
### #160288 77cb04ee49e4f29833262112d5abf705c96faa98 rustdoc: use anonymous constant for primitives/keywords/attribute docs
119+
<!-- rust-timer:triage-body-start-pr-160288 -->
120+
Queued 77cb04ee49e4f29833262112d5abf705c96faa98 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=77cb04ee49e4f29833262112d5abf705c96faa98).
121+
There are currently 2 preceding artifacts in the [queue](https://perf.rust-lang.org/status.html).
122+
It will probably take at least ~3.0 hours until the benchmark run finishes.
123+
<!-- rust-timer:triage-body-end-pr-160288 -->
124+
125+
<!-- rust-timer: triage -->";
126+
assert_eq!(body, EXPECTED_RESULT);
127+
}
128+
129+
#[test]
130+
fn test_update_second() {
131+
let mut body = BEFORE_BODY.to_string();
132+
update_triage_body(
133+
&mut body,
134+
160077,
135+
"NEW BODY1\nNEW BODY2\nNEW BODY3\n".to_string(),
136+
)
137+
.unwrap();
138+
139+
const EXPECTED_RESULT: &str = "### #157428 364c9dea926885d60daef4bf1a15fa03efa35a84 allocator: refactor for stabilisation
140+
<!-- rust-timer:triage-body-start-pr-157428 -->
141+
Queued 364c9dea926885d60daef4bf1a15fa03efa35a84 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=364c9dea926885d60daef4bf1a15fa03efa35a84).
142+
There are currently 0 preceding artifacts in the [queue](https://perf.rust-lang.org/status.html).
143+
It will probably take at least ~1.0 hours until the benchmark run finishes.
144+
<!-- rust-timer:triage-body-end-pr-157428 -->
145+
146+
### #160077 a96bde1411c3b0fe68417c04e85624f33b645516 Don't compute FnAbi for LLVM intrinsics
147+
<!-- rust-timer:triage-body-start-pr-160077 -->
148+
NEW BODY1
149+
NEW BODY2
150+
NEW BODY3
151+
<!-- rust-timer:triage-body-end-pr-160077 -->
152+
153+
### #160288 77cb04ee49e4f29833262112d5abf705c96faa98 rustdoc: use anonymous constant for primitives/keywords/attribute docs
154+
<!-- rust-timer:triage-body-start-pr-160288 -->
155+
Queued 77cb04ee49e4f29833262112d5abf705c96faa98 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=77cb04ee49e4f29833262112d5abf705c96faa98).
156+
There are currently 2 preceding artifacts in the [queue](https://perf.rust-lang.org/status.html).
157+
It will probably take at least ~3.0 hours until the benchmark run finishes.
158+
<!-- rust-timer:triage-body-end-pr-160288 -->
159+
160+
<!-- rust-timer: triage -->";
161+
assert_eq!(body, EXPECTED_RESULT);
162+
}
163+
164+
#[test]
165+
fn test_update_last() {
166+
let mut body = BEFORE_BODY.to_string();
167+
update_triage_body(
168+
&mut body,
169+
160288,
170+
"NEW BODY1\nNEW BODY2\nNEW BODY3\n".to_string(),
171+
)
172+
.unwrap();
173+
174+
const EXPECTED_RESULT: &str = "### #157428 364c9dea926885d60daef4bf1a15fa03efa35a84 allocator: refactor for stabilisation
175+
<!-- rust-timer:triage-body-start-pr-157428 -->
176+
Queued 364c9dea926885d60daef4bf1a15fa03efa35a84 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=364c9dea926885d60daef4bf1a15fa03efa35a84).
177+
There are currently 0 preceding artifacts in the [queue](https://perf.rust-lang.org/status.html).
178+
It will probably take at least ~1.0 hours until the benchmark run finishes.
179+
<!-- rust-timer:triage-body-end-pr-157428 -->
180+
181+
### #160077 a96bde1411c3b0fe68417c04e85624f33b645516 Don't compute FnAbi for LLVM intrinsics
182+
<!-- rust-timer:triage-body-start-pr-160077 -->
183+
Queued a96bde1411c3b0fe68417c04e85624f33b645516 with parent 93c9086fdd5b80d286480a19ac047746ecc5fa1f, future [comparison URL](https://perf.rust-lang.org/compare.html?start=93c9086fdd5b80d286480a19ac047746ecc5fa1f&end=a96bde1411c3b0fe68417c04e85624f33b645516).
184+
There is currently 1 preceding artifact in the [queue](https://perf.rust-lang.org/status.html).
185+
It will probably take at least ~2.0 hours until the benchmark run finishes.
186+
<!-- rust-timer:triage-body-end-pr-160077 -->
187+
188+
### #160288 77cb04ee49e4f29833262112d5abf705c96faa98 rustdoc: use anonymous constant for primitives/keywords/attribute docs
189+
<!-- rust-timer:triage-body-start-pr-160288 -->
190+
NEW BODY1
191+
NEW BODY2
192+
NEW BODY3
193+
<!-- rust-timer:triage-body-end-pr-160288 -->
194+
195+
<!-- rust-timer: triage -->";
196+
assert_eq!(body, EXPECTED_RESULT);
197+
}
198+
199+
#[test]
200+
fn test_update_not_in_there() {
201+
let mut body = BEFORE_BODY.to_string();
202+
assert!(update_triage_body(
203+
&mut body,
204+
1234,
205+
"NEW BODY1\nNEW BODY2\nNEW BODY3\n".to_string()
206+
)
207+
.is_err());
208+
}
209+
}

‎site/src/request_handlers/github.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::load::SiteCtxt;
66
use std::fmt::Write;
77

88
use crate::github::client::Client;
9-
use crate::github::triage::TRIAGE_MARKER;
9+
use crate::github::triage::{triage_body_end_marker, triage_body_start_marker, TRIAGE_MARKER};
1010
use database::{
1111
parse_backends, parse_profiles, parse_targets, BenchmarkRequest, BenchmarkRequestInsertResult,
1212
CodegenBackend, Profile, Target,
@@ -263,6 +263,12 @@ async fn handle_rust_timer(
263263
.unwrap();
264264

265265
// Enqueue the sha build and write result
266+
write!(
267+
&mut result,
268+
"{}",
269+
triage_body_start_marker(unrolled_build_message.member_pr_number)
270+
)
271+
.unwrap();
266272
let (Ok(msg) | Err(msg)) = enqueue_sha_build(
267273
&ctxt,
268274
main_client,
@@ -274,6 +280,12 @@ async fn handle_rust_timer(
274280
)
275281
.await;
276282
writeln!(&mut result, "{msg}\n").unwrap();
283+
write!(
284+
&mut result,
285+
"{}",
286+
triage_body_end_marker(unrolled_build_message.member_pr_number)
287+
)
288+
.unwrap();
277289
}
278290
// Add a marker to the comment which should help to find it again later
279291
writeln!(&mut result, "{}", TRIAGE_MARKER).unwrap();

0 commit comments

Comments
 (0)