Skip to content

Commit 3547fd1

Browse files
committed
Update perf. triage documentation
There were some quite old information about the compare page and rollup merge handling.
1 parent 59a4f6c commit 3547fd1

File tree

1 file changed

+20
-22
lines changed

1 file changed

+20
-22
lines changed

triage/README.md

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ usage.
88
- Mark-Simulacrum
99
- rylev
1010
- pnkfelix
11+
- kobzol
1112

1213
Monday evening to Tuesday afternoon in North America is a good time to do it
1314
because This Week in Rust (see below) is usually published on Wednesday, US time,
@@ -43,6 +44,7 @@ are actually not regressions and have only been labeled as such due to noise.
4344
Look for significant changes (regressions or improvements) in the following:
4445
- instruction count
4546
- max rss
47+
- binary size
4648

4749
When working with graphs:
4850
- Click and drag a region of a graph to zoom in on it. This is useful when data
@@ -54,12 +56,11 @@ To view the code changes:
5456
open the page of commits in the merge.
5557

5658
Understanding the comparison page:
57-
- Each benchmark is listed with the min, max and the avg percentage change
59+
- Each benchmark is listed with the percentage change
5860
(red indicates regressions, green indicates improvements) across the various
5961
benchmarks run (e.g., full, incremental-full, incremental-unchanged, etc.).
60-
- Clicking on a specific benchmark will show the results for each of the various
61-
benchmarks. Clicking on the percentages will open a more specific detail view
62-
of timing for queries run during compilation.
62+
- Clicking on a specific benchmark run will show a detailed view of the results, including
63+
history chart and links to self-profile query timings.
6364

6465
### Interpreting results
6566

@@ -73,37 +74,34 @@ For help understanding how to interpret results, consult the [comparison analysi
7374
### Ping PR Author/Reviewer
7475

7576
Single PR in Merge:
76-
- Add a comment to the PR pointing to the "compare" page (unless someone else
77-
has already done that).
7877
- In the case of a regression, ask the author for a response. If it's a big
7978
regression, consider requesting the author revert their changes. It may
80-
be worth looking through the comments to see if any perf CI runs were done,
81-
and whether the regression was expected.
79+
be worth looking through the comments to see if the regression was expected.
8280

8381
Difficult cases: the merge was a rollup of multiple PRs.
84-
- Look through the PRs and try to determine which was the cause. Often you
85-
can easily tell that one or more PRs could not have caused the change, e.g.
86-
because they made trivial changes, documentation-only changes, etc.
87-
- If there are still PRs left over, look at the 'detailed-query' page on perf.rlo: often, there is a single timing pass that improved significantly, and the name may give you a hint. You can find the page by expanding the dropdown for the build with the greatest change, then clicking on the percent change.
88-
- If you can't narrow it down to a single PR, in the rollup PR ask all the
89-
authors who might be responsible.
90-
- Once you have narrowed it down to a single PR, treat it like an easy case,
91-
above.
82+
- Look through the PRs and try to determine which was the cause. You can start
83+
a perf. run for a single PR merged in the rollup using the "unrolled build"
84+
table (see e.g. [here](https://github.com/rust-lang/rust/pull/119313#issuecomment-1869441617)) with
85+
the `@rust-timer build $SHA` command.
86+
- Often you can easily tell that one or more PRs could not have caused the change, e.g.
87+
because they made trivial changes, documentation-only changes, etc., so start with the
88+
perf. runs for the most "suspicious" PRs.
89+
- Once you have narrowed it down to a single PR, treat it like a single PR case, see above.
9290
- You might want to remind the author to use "@bors rollup=never" for PRs
9391
that are likely to affect performance.
94-
- Add an entry to the triage log, as for the easy cases.
92+
- Add an entry to the triage log, as for the single PR cases.
9593

9694
### Add analysis and follow-ups to report
9795

98-
- For each entry in the report, include useful details, such as the size of the regression/improvement, and any promises of follow-up action
99-
from authors in the case of a regression.
96+
- For each entry in the report, include useful details, such as the size of the regression/improvement,
97+
and any promises of follow-up action from authors in the case of a regression.
10098

10199
### This Week in Rust
102100

103101
Once finished, file a PR adding a link to the log entry in [This Week in
104102
Rust](https://github.com/emberian/this-week-in-rust/).
105103
- See the previous This Week in Rust edition for how the log entry should be formatted.
106104

107-
If you have any questions, the `t-compiler/performance` stream on Zulip is the
108-
best place to ask.
109-
105+
After you have finished the triage, also post a short summary to the
106+
[`t-compiler/performance`](https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance)
107+
stream on Zulip. If you have any questions, you can ask around in that stream.

0 commit comments

Comments
 (0)