Skip to content

Commit 9179446

Browse files
Mark-SimulacrumMark-Simulacrum
authored
Merge pull request #1336 from Mark-Simulacrum/triage
Add triage for this week
2 parents 87d51ce + 1bfb6d4 commit 9179446

File tree

1 file changed

+163
-0
lines changed

1 file changed

+163
-0
lines changed

‎triage/2022-05-24.md

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
# 2022-05-24 Triage Log
2+
3+
Overall a positive week for non-incremental performance (roughly 0.5% faster),
4+
with >1% regressions on incremental benchmarks, primarily due to
5+
[#95563](https://github.com/rust-lang/rust/pull/95563), which will hopefully be
6+
investigated in the coming weeks.
7+
8+
Triage done by **@simulacrum**.
9+
Revision range: [7355d971a954ed63293e4191f6677f60c1bc07d9..43d9f3859e0204e764161ee085a360274b5f3e9a](https://perf.rust-lang.org/?start=7355d971a954ed63293e4191f6677f60c1bc07d9&end=43d9f3859e0204e764161ee085a360274b5f3e9a&absolute=false&stat=instructions%3Au)
10+
11+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
12+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:-------------------------:|
13+
| count | 69 | 58 | 76 | 52 | 145 |
14+
| mean | 1.5% | 2.2% | -1.0% | -3.3% | 0.2% |
15+
| max | 5.1% | 7.3% | -2.9% | -36.9% | 5.1% |
16+
17+
2 Regressions, 5 Improvements, 4 Mixed; 0 of them in rollups
18+
57 artifact comparisons made in total
19+
20+
#### Regressions
21+
22+
Transition to valtrees pt1 [#97019](https://github.com/rust-lang/rust/pull/97019)
23+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=07ae142d77f56dd48f2736605e0a6c015f2aecc6&end=cd282d7f75da9080fda0f1740a729516e7fbec68&stat=instructions:u)
24+
25+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
26+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
27+
| count | 4 | 5 | 0 | 0 | 4 |
28+
| mean | 1.0% | 0.1% | N/A | N/A | 1.0% |
29+
| max | 1.1% | 0.1% | N/A | N/A | 1.1% |
30+
31+
@oli-obk has suggested applying some targeted inlining as a likely resolution to
32+
the regressions; work to follow up needs to be done. It seems like the
33+
regression is likely to be resolved (or at least effort applied to do so).
34+
35+
Make write/print macros eagerly drop temporaries [#96455](https://github.com/rust-lang/rust/pull/96455)
36+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d12557407c7c34f1599cfba79666954291810859&end=c186f7c07912064c352f12d8b0aa9d5e5975450e&stat=instructions:u)
37+
38+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
39+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
40+
| count | 3 | 0 | 0 | 0 | 3 |
41+
| mean | 0.4% | N/A | N/A | N/A | 0.4% |
42+
| max | 0.6% | N/A | N/A | N/A | 0.6% |
43+
44+
Change is relatively minor and limited to just a single benchmark, so marking
45+
the regression as triaged; further investigation is not warranted at this time.
46+
47+
#### Improvements
48+
49+
Add PID to PGO profile data filename [#97110](https://github.com/rust-lang/rust/pull/97110)
50+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a084b7ad35adb508bd2e053fc2a1b9a53df9536c&end=e5732a21711e7cefa6eb22e1790406b269d6197a&stat=instructions:u)
51+
52+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
53+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
54+
| count | 0 | 0 | 94 | 54 | 94 |
55+
| mean | N/A | N/A | -0.3% | -0.4% | -0.3% |
56+
| max | N/A | N/A | -0.5% | -1.2% | -0.5% |
57+
58+
An expected improvement in our PGO scripts, with a small magnitude effect across
59+
many benchmarks.
60+
61+
--remap-path-prefix: Fix duplicated path components in debuginfo [#96867](https://github.com/rust-lang/rust/pull/96867)
62+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e5732a21711e7cefa6eb22e1790406b269d6197a&end=936eba3b348e65b658b60218cc9237f02abdbeb4&stat=instructions:u)
63+
64+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
65+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
66+
| count | 0 | 0 | 2 | 6 | 2 |
67+
| mean | N/A | N/A | -0.4% | -4.1% | -0.4% |
68+
| max | N/A | N/A | -0.4% | -4.7% | -0.4% |
69+
70+
Improvements likely to be primarily driven by reducing the amount of codegen due
71+
to simpler/smaller debuginfo.
72+
73+
74+
rustdoc: Reduce clean::Type size [#93963](https://github.com/rust-lang/rust/pull/93963)
75+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4f372b14dea58cbff1dd76bb651f9c035d3f6e7b&end=5f33adce1bfe01e4c0c8754400bf782a95af88c9&stat=instructions:u)
76+
77+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
78+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
79+
| count | 0 | 0 | 12 | 20 | 12 |
80+
| mean | N/A | N/A | -0.4% | -0.6% | -0.4% |
81+
| max | N/A | N/A | -0.8% | -0.8% | -0.8% |
82+
83+
84+
rustc_parse: Move AST -> TokenStream conversion logic to rustc_ast [#97251](https://github.com/rust-lang/rust/pull/97251)
85+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=653463731a7f01f519cf85f444869def27f00395&end=4bb4dc4672b4d80509db4c0572ae8ceeaf7d4a1c&stat=instructions:u)
86+
87+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
88+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
89+
| count | 0 | 0 | 6 | 7 | 6 |
90+
| mean | N/A | N/A | -0.3% | -0.5% | -0.3% |
91+
| max | N/A | N/A | -0.4% | -0.6% | -0.4% |
92+
93+
94+
Refactor call terminator to always include destination place [#96098](https://github.com/rust-lang/rust/pull/96098)
95+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=acb5c16fa8acf7fd3b48fc218881f006577bab1a&end=43d9f3859e0204e764161ee085a360274b5f3e9a&stat=instructions:u)
96+
97+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
98+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
99+
| count | 0 | 0 | 5 | 17 | 5 |
100+
| mean | N/A | N/A | -0.3% | -0.5% | -0.3% |
101+
| max | N/A | N/A | -0.4% | -1.1% | -0.4% |
102+
103+
104+
#### Mixed
105+
106+
Add a query for checking whether a function is an intrinsic. [#97012](https://github.com/rust-lang/rust/pull/97012)
107+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=7355d971a954ed63293e4191f6677f60c1bc07d9&end=735efc0c703812343a5e5d19b600dac73b8a89f0&stat=instructions:u)
108+
109+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
110+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
111+
| count | 7 | 9 | 16 | 2 | 23 |
112+
| mean | 0.3% | 0.3% | -0.3% | -0.5% | -0.1% |
113+
| max | 0.3% | 0.4% | -0.5% | -0.5% | -0.5% |
114+
115+
Mixed results, but overall looks to be pretty neutral. The preparation for
116+
future maintainability outweighs the possible small cost to keeping this change.
117+
Regression was already marked as triaged by @oli-obk (thank you!).
118+
119+
Cache more queries on disk [#95418](https://github.com/rust-lang/rust/pull/95418)
120+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=536020c5f97883aa9f2a90897a5adb520486d2e1&end=e6a4afc3af2d2a53f91fc8a77bdfe94bea375b29&stat=instructions:u)
121+
122+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
123+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
124+
| count | 33 | 32 | 65 | 28 | 98 |
125+
| mean | 0.8% | 1.6% | -1.0% | -4.4% | -0.4% |
126+
| max | 2.1% | 4.0% | -3.6% | -33.0% | -3.6% |
127+
128+
This is overall an improvement, with regressions primarily limited to secondary
129+
benchmarks. Mixed results are fairly expected from a change like this; marking
130+
the regression as triaged. @cjgillot (PR author) also posted a good summary:
131+
132+
> This PR caches many more queries' results on disk. The queries have been
133+
> chosen to be: (1) outputs of typechecking, and (2) parts of metadata. This
134+
> caching allows for wide and large compile time savings, up to 36%. The
135+
> tradeoff is a compile time increase (4%) when these caches cannot be usefully
136+
> used, and a increased on-disk size around 30%.
137+
138+
Move the extended lifetime resolution into typeck context [#95563](https://github.com/rust-lang/rust/pull/95563)
139+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=acfd327fd4e3a302ebb0a077f422a527a7935333&end=653463731a7f01f519cf85f444869def27f00395&stat=instructions:u)
140+
141+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
142+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
143+
| count | 97 | 42 | 42 | 34 | 139 |
144+
| mean | 1.8% | 3.0% | -0.6% | -0.5% | 1.1% |
145+
| max | 5.8% | 8.9% | -1.9% | -1.4% | 5.8% |
146+
147+
Looks like a fairly large regression across a number of benchmarks, though
148+
limited to the incremental scenarios. Non-incremental benchmarks show an
149+
improvement (hence the mixed results). Left a
150+
[comment](https://github.com/rust-lang/rust/pull/95563#issuecomment-1135994637)
151+
on the PR asking for clarification and further investigation.
152+
153+
Update `rustc` PGO benchmark list [#97120](https://github.com/rust-lang/rust/pull/97120)
154+
[(Comparison Link)](https://perf.rust-lang.org/compare.html?start=222c5724ecc922fe67815f428c19f82c129d9386&end=ee160f2f5e73b6f5954bc33f059c316d9e8582c4&stat=instructions:u)
155+
156+
| | Regressions 😿 <br />(primary) | Regressions 😿 <br />(secondary) | Improvements 🎉 <br />(primary) | Improvements 🎉 <br />(secondary) | All 😿 🎉 <br />(primary) |
157+
|:----------:|:------------------------------:|:--------------------------------:|:-------------------------------:|:---------------------------------:|:------------------------:|
158+
| count | 3 | 18 | 83 | 53 | 86 |
159+
| mean | 0.3% | 0.8% | -0.5% | -0.5% | -0.5% |
160+
| max | 0.4% | 1.8% | -1.3% | -1.1% | -1.3% |
161+
162+
Overall an improvement, some slight regressions are expected from a larger
163+
change like this which bumps PGO inputs.

0 commit comments

Comments
 (0)