Skip to content

Commit 07ee1a9

Browse files
authored
Merge pull request #1786 from Kobzol/triage-doc
Update perf. triage documentation
2 parents aa08c4c + 3547fd1 commit 07ee1a9

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,
@@ -48,6 +49,7 @@ are actually not regressions and have only been labeled as such due to noise.
4849
Look for significant changes (regressions or improvements) in the following:
4950
- instruction count
5051
- max rss
52+
- binary size
5153

5254
When working with graphs:
5355
- Click and drag a region of a graph to zoom in on it. This is useful when data
@@ -59,12 +61,11 @@ To view the code changes:
5961
open the page of commits in the merge.
6062

6163
Understanding the comparison page:
62-
- Each benchmark is listed with the min, max and the avg percentage change
64+
- Each benchmark is listed with the percentage change
6365
(red indicates regressions, green indicates improvements) across the various
6466
benchmarks run (e.g., full, incremental-full, incremental-unchanged, etc.).
65-
- Clicking on a specific benchmark will show the results for each of the various
66-
benchmarks. Clicking on the percentages will open a more specific detail view
67-
of timing for queries run during compilation.
67+
- Clicking on a specific benchmark run will show a detailed view of the results, including
68+
history chart and links to self-profile query timings.
6869

6970
### Interpreting results
7071

@@ -78,37 +79,34 @@ For help understanding how to interpret results, consult the [comparison analysi
7879
### Ping PR Author/Reviewer
7980

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

8886
Difficult cases: the merge was a rollup of multiple PRs.
89-
- Look through the PRs and try to determine which was the cause. Often you
90-
can easily tell that one or more PRs could not have caused the change, e.g.
91-
because they made trivial changes, documentation-only changes, etc.
92-
- 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.
93-
- If you can't narrow it down to a single PR, in the rollup PR ask all the
94-
authors who might be responsible.
95-
- Once you have narrowed it down to a single PR, treat it like an easy case,
96-
above.
87+
- Look through the PRs and try to determine which was the cause. You can start
88+
a perf. run for a single PR merged in the rollup using the "unrolled build"
89+
table (see e.g. [here](https://github.com/rust-lang/rust/pull/119313#issuecomment-1869441617)) with
90+
the `@rust-timer build $SHA` command.
91+
- Often you can easily tell that one or more PRs could not have caused the change, e.g.
92+
because they made trivial changes, documentation-only changes, etc., so start with the
93+
perf. runs for the most "suspicious" PRs.
94+
- Once you have narrowed it down to a single PR, treat it like a single PR case, see above.
9795
- You might want to remind the author to use "@bors rollup=never" for PRs
9896
that are likely to affect performance.
99-
- Add an entry to the triage log, as for the easy cases.
97+
- Add an entry to the triage log, as for the single PR cases.
10098

10199
### Add analysis and follow-ups to report
102100

103-
- For each entry in the report, include useful details, such as the size of the regression/improvement, and any promises of follow-up action
104-
from authors in the case of a regression.
101+
- For each entry in the report, include useful details, such as the size of the regression/improvement,
102+
and any promises of follow-up action from authors in the case of a regression.
105103

106104
### This Week in Rust
107105

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

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

0 commit comments

Comments
 (0)