Skip to content

Commit 6fd05ce

Browse files
KobzolKobzol
authored
Merge pull request #1807 from Kobzol/triage-2024-01-23
Triage 2024 01 23
2 parents 0db7349 + 16f2ce4 commit 6fd05ce

File tree

1 file changed

+261
-0
lines changed

1 file changed

+261
-0
lines changed

‎triage/2024-01-23.md

Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
# 2024-01-23 Triage Log
2+
3+
This week saw a bunch of regressions caused by correctness fixes and in general doing more work
4+
in the compiler. These were offset by many improvements (especially around hashing in the compiler)
5+
that improved performance by ~2% across a large number of benchmarks. Don't get too excited about the
6+
large 45+% wins though, these were only for tiny benchmarks like helloworld. They were caused by a
7+
[change in Cargo](https://github.com/rust-lang/cargo/pull/13257) which introduces stripping of debug
8+
symbols from Rust release binaries by default, and in turn also improves compilation time for small
9+
crates.
10+
11+
Triage done by **@kobzol**.
12+
Revision range: [f9c2421a..d6b151fc](https://perf.rust-lang.org/?start=f9c2421a2a6e34f3756900dd7d600704c08bfccb&end=d6b151fc77e213bf637db0f12c1965ace3ffe255&absolute=false&stat=instructions%3Au)
13+
14+
**Summary**:
15+
16+
| (instructions:u) | mean | range | count |
17+
|:----------------------------------:|:-----:|:---------------:|:-----:|
18+
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 1.5%] | 11 |
19+
| Regressions ❌ <br /> (secondary) | 2.2% | [0.2%, 9.9%] | 26 |
20+
| Improvements ✅ <br /> (primary) | -3.2% | [-47.5%, -0.2%] | 191 |
21+
| Improvements ✅ <br /> (secondary) | -7.9% | [-46.5%, -0.1%] | 123 |
22+
| All ❌✅ (primary) | -3.0% | [-47.5%, 1.5%] | 202 |
23+
24+
25+
4 Regressions, 4 Improvements, 9 Mixed; 4 of them in rollups
26+
48 artifact comparisons made in total
27+
28+
#### Regressions
29+
30+
fix fn/const items implied bounds and wf check (rebase) [#120019](https://github.com/rust-lang/rust/pull/120019) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=6ed31aba1a889b5d5e5362f1cdde316fb0b571d1&end=6bf600bc98879bf1bf2ab0f3b3b37c965f5bdff6&stat=instructions:u)
31+
32+
| (instructions:u) | mean | range | count |
33+
|:----------------------------------:|:----:|:------------:|:-----:|
34+
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.5%] | 26 |
35+
| Regressions ❌ <br /> (secondary) | 1.3% | [0.3%, 1.6%] | 9 |
36+
| Improvements ✅ <br /> (primary) | - | - | 0 |
37+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
38+
| All ❌✅ (primary) | 0.3% | [0.2%, 0.5%] | 26 |
39+
40+
* This was a correctness fix that had an expected performance hit.
41+
42+
Rollup of 8 pull requests [#120187](https://github.com/rust-lang/rust/pull/120187) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=867d39cdf625e4db4b381faff993346582e598b4&end=cb25c5bc3d526a8fb931314cb3a7849115134b04&stat=instructions:u)
43+
44+
| (instructions:u) | mean | range | count |
45+
|:----------------------------------:|:----:|:------------:|:-----:|
46+
| Regressions ❌ <br /> (primary) | 0.5% | [0.3%, 1.0%] | 8 |
47+
| Regressions ❌ <br /> (secondary) | 0.9% | [0.3%, 1.8%] | 16 |
48+
| Improvements ✅ <br /> (primary) | - | - | 0 |
49+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
50+
| All ❌✅ (primary) | 0.5% | [0.3%, 1.0%] | 8 |
51+
52+
* This was a mixture of several PRs that had a perf. effect.
53+
* [#119461](https://github.com/rust-lang/rust/pull/119461) and [#118811](https://github.com/rust-lang/rust/pull/118811)
54+
slightly regressed the `deep-vector` benchmark, which was however quite noisy recently, so it was
55+
probably just a blip.
56+
* [#116090](https://github.com/rust-lang/rust/pull/116090) caused a real regression on `doc`
57+
benchmarks, which was caused simply by adding a bunch of new methods to the standard library.
58+
59+
Rollup of 8 pull requests [#120196](https://github.com/rust-lang/rust/pull/120196) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a58ec8ff03b3269b20104eb7eae407be48ab95a7&end=6fff796eac247c072ddb84f8202bedecc8e94f0d&stat=instructions:u)
60+
61+
| (instructions:u) | mean | range | count |
62+
|:----------------------------------:|:----:|:------------:|:-----:|
63+
| Regressions ❌ <br /> (primary) | 1.3% | [1.3%, 1.3%] | 1 |
64+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
65+
| Improvements ✅ <br /> (primary) | - | - | 0 |
66+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
67+
| All ❌✅ (primary) | 1.3% | [1.3%, 1.3%] | 1 |
68+
69+
* Caused by [#120145](https://github.com/rust-lang/rust/pull/120145), which was a correctness fix,
70+
so marked as triaged.
71+
72+
Rollup of 10 pull requests [#120242](https://github.com/rust-lang/rust/pull/120242) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=021861aea8de20c76c7411eb8ada7e8235e3d9b5&end=d5fd0997291ca0135401a39dff25c8a9c13b8961&stat=instructions:u)
73+
74+
| (instructions:u) | mean | range | count |
75+
|:----------------------------------:|:----:|:------------:|:-----:|
76+
| Regressions ❌ <br /> (primary) | - | - | 0 |
77+
| Regressions ❌ <br /> (secondary) | 3.1% | [3.1%, 3.1%] | 1 |
78+
| Improvements ✅ <br /> (primary) | - | - | 0 |
79+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
80+
| All ❌✅ (primary) | - | - | 0 |
81+
82+
* Caused by [#120137](https://github.com/rust-lang/rust/pull/120137), which added more validation
83+
to the compiler, and the perf. hit was deemed acceptable, so marked as triaged.
84+
85+
#### Improvements
86+
87+
Cache local DefId-keyed queries without hashing [#119977](https://github.com/rust-lang/rust/pull/119977) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=92f2e0aa62113a5f31076a9414daca55722556cf&end=098d4fd74c078b12bfc2e9438a2a04bc18b393bc&stat=instructions:u)
88+
89+
| (instructions:u) | mean | range | count |
90+
|:----------------------------------:|:-----:|:--------------:|:-----:|
91+
| Regressions ❌ <br /> (primary) | 0.7% | [0.7%, 0.7%] | 1 |
92+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
93+
| Improvements ✅ <br /> (primary) | -1.2% | [-8.2%, -0.2%] | 161 |
94+
| Improvements ✅ <br /> (secondary) | -1.5% | [-3.7%, -0.3%] | 64 |
95+
| All ❌✅ (primary) | -1.2% | [-8.2%, 0.7%] | 162 |
96+
97+
* Incredible wins across many benchmarks, caused by improving hashing in the compiler.
98+
* The wall-time results of this PR were even more [impressive](https://perf.rust-lang.org/compare.html?start=92f2e0aa62113a5f31076a9414daca55722556cf&end=098d4fd74c078b12bfc2e9438a2a04bc18b393bc&stat=wall-time), resulting in a ~4% mean improvement across more than a hundred benchmark
99+
configurations!
100+
101+
Get rid of the hir_owner query. [#120006](https://github.com/rust-lang/rust/pull/120006) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=25f8d01fd8bda339612d0c0a8844173a09205f7c&end=d3c9082a44f00b22152ebc9c92c129b10ccb7793&stat=instructions:u)
102+
103+
| (instructions:u) | mean | range | count |
104+
|:----------------------------------:|:-----:|:--------------:|:-----:|
105+
| Regressions ❌ <br /> (primary) | - | - | 0 |
106+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.2%, 0.4%] | 3 |
107+
| Improvements ✅ <br /> (primary) | -1.8% | [-5.5%, -0.3%] | 129 |
108+
| Improvements ✅ <br /> (secondary) | -2.1% | [-4.4%, -0.4%] | 46 |
109+
| All ❌✅ (primary) | -1.8% | [-5.5%, -0.3%] | 129 |
110+
111+
* Again, great wins across a lot of benchmarks.
112+
113+
Use UnhashMap for a few more maps [#120076](https://github.com/rust-lang/rust/pull/120076) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d3c9082a44f00b22152ebc9c92c129b10ccb7793&end=1bd42be8cb707aadaf2068d5ac186154970c4d80&stat=instructions:u)
114+
115+
| (instructions:u) | mean | range | count |
116+
|:----------------------------------:|:-----:|:--------------:|:-----:|
117+
| Regressions ❌ <br /> (primary) | - | - | 0 |
118+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
119+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.2%] | 13 |
120+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.4%, -0.2%] | 4 |
121+
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.2%] | 13 |
122+
123+
* Continuation of hashing improvements.
124+
125+
Always use RevealAll for const eval queries [#119821](https://github.com/rust-lang/rust/pull/119821) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=128148d4cf742c3056e9bfc65e7cc9531cb0f815&end=5378c1cf0713ab224eb6431327c28522182feb69&stat=instructions:u)
126+
127+
| (instructions:u) | mean | range | count |
128+
|:----------------------------------:|:-----:|:--------------:|:-----:|
129+
| Regressions ❌ <br /> (primary) | - | - | 0 |
130+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
131+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.3%] | 3 |
132+
| Improvements ✅ <br /> (secondary) | -1.4% | [-1.8%, -1.2%] | 7 |
133+
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.3%] | 3 |
134+
135+
* This PR improved mainly the performance of const evaluation in the compiler.
136+
137+
#### Mixed
138+
139+
Sandwich MIR optimizations between DSE. [#119672](https://github.com/rust-lang/rust/pull/119672) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f9c2421a2a6e34f3756900dd7d600704c08bfccb&end=fa0dc208d0a34027c1d3cca7d47975d8238bcfde&stat=instructions:u)
140+
141+
| (instructions:u) | mean | range | count |
142+
|:----------------------------------:|:-----:|:--------------:|:-----:|
143+
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 1.4%] | 14 |
144+
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 2.7%] | 14 |
145+
| Improvements ✅ <br /> (primary) | -1.0% | [-2.2%, -0.2%] | 31 |
146+
| Improvements ✅ <br /> (secondary) | -0.9% | [-2.2%, -0.2%] | 10 |
147+
| All ❌✅ (primary) | -0.4% | [-2.2%, 1.4%] | 45 |
148+
149+
* Several nice wins on larger benchmarks, overall the wins outweighed the losses.
150+
151+
large_assignments: Lint on specific large args passed to functions [#116520](https://github.com/rust-lang/rust/pull/116520) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e64f8495e73fbc3653b4bcb73268c58b9c4a0a0d&end=92f2e0aa62113a5f31076a9414daca55722556cf&stat=instructions:u)
152+
153+
| (instructions:u) | mean | range | count |
154+
|:----------------------------------:|:-----:|:--------------:|:-----:|
155+
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.8%] | 45 |
156+
| Regressions ❌ <br /> (secondary) | 0.6% | [0.2%, 2.1%] | 8 |
157+
| Improvements ✅ <br /> (primary) | - | - | 0 |
158+
| Improvements ✅ <br /> (secondary) | -6.0% | [-6.0%, -6.0%] | 1 |
159+
| All ❌✅ (primary) | 0.4% | [0.2%, 0.8%] | 45 |
160+
161+
* This new lint introduced more tracking of spans in the compiler, which expectedly
162+
slightly regressed compile times. It is expected that the new spans will be used
163+
by future lints/errors to provide better diagnostics.
164+
165+
Update cargo [#120036](https://github.com/rust-lang/rust/pull/120036) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=098d4fd74c078b12bfc2e9438a2a04bc18b393bc&end=6ed31aba1a889b5d5e5362f1cdde316fb0b571d1&stat=instructions:u)
166+
167+
| (instructions:u) | mean | range | count |
168+
|:----------------------------------:|:------:|:---------------:|:-----:|
169+
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.5%] | 2 |
170+
| Regressions ❌ <br /> (secondary) | 1.4% | [1.4%, 1.4%] | 1 |
171+
| Improvements ✅ <br /> (primary) | -16.8% | [-47.6%, -0.4%] | 13 |
172+
| Improvements ✅ <br /> (secondary) | -20.5% | [-46.9%, -2.2%] | 37 |
173+
| All ❌✅ (primary) | -14.5% | [-47.6%, 0.5%] | 15 |
174+
175+
* A few tiny regressions, but there's not much to be done about these since this
176+
was a Cargo update.
177+
* The large instruction count improvements came from https://github.com/rust-lang/cargo/pull/13257,
178+
which introduced stripping of `--release` binaries by default. This coincidentally also reduces
179+
linking time on Linux quite a lot for tiny programs, which caused these improvements on
180+
`helloworld`-like crates.
181+
182+
error on incorrect implied bounds in wfcheck except for Bevy dependents [#118553](https://github.com/rust-lang/rust/pull/118553) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c485ee71477a29041895c47cc441b364670f3772&end=a34faab155417481e191e8d467a5ea9bdd2174a0&stat=instructions:u)
183+
184+
| (instructions:u) | mean | range | count |
185+
|:----------------------------------:|:-----:|:--------------:|:-----:|
186+
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 1.6%] | 30 |
187+
| Regressions ❌ <br /> (secondary) | 0.4% | [0.2%, 0.6%] | 13 |
188+
| Improvements ✅ <br /> (primary) | - | - | 0 |
189+
| Improvements ✅ <br /> (secondary) | -1.0% | [-1.2%, -0.8%] | 6 |
190+
| All ❌✅ (primary) | 0.7% | [0.2%, 1.6%] | 30 |
191+
192+
* Regressions have outweighted the improvements here, however, this was a correctness fix,
193+
so we will have to take the hit. A part of the regressions reverted wins from
194+
[#120123](https://github.com/rust-lang/rust/pull/120123).
195+
196+
Rollup of 9 pull requests [#120112](https://github.com/rust-lang/rust/pull/120112) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=16fadb3f252bcfc5ee3f0be09472c9600a052202&end=92d727796be7c882d2efbc06e08bbf4743cf29dc&stat=instructions:u)
197+
198+
| (instructions:u) | mean | range | count |
199+
|:----------------------------------:|:-----:|:--------------:|:-----:|
200+
| Regressions ❌ <br /> (primary) | - | - | 0 |
201+
| Regressions ❌ <br /> (secondary) | 3.2% | [0.3%, 4.1%] | 7 |
202+
| Improvements ✅ <br /> (primary) | -0.7% | [-0.7%, -0.7%] | 1 |
203+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
204+
| All ❌✅ (primary) | -0.7% | [-0.7%, -0.7%] | 1 |
205+
206+
* This was caused by [#120037](https://github.com/rust-lang/rust/pull/120037), which
207+
seems to have just undone a previous perf. win on the same benchmarks caused by
208+
code being reshuffled around.
209+
210+
use implied bounds compat mode in MIR borrowck [#120123](https://github.com/rust-lang/rust/pull/120123) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=32ec40c68533f325a3c8fe787b77ef5c9e209b23&end=88189a71e4e4376eea82ac61db6a539612eb200a&stat=instructions:u)
211+
212+
| (instructions:u) | mean | range | count |
213+
|:----------------------------------:|:-----:|:--------------:|:-----:|
214+
| Regressions ❌ <br /> (primary) | - | - | 0 |
215+
| Regressions ❌ <br /> (secondary) | 0.9% | [0.9%, 1.0%] | 4 |
216+
| Improvements ✅ <br /> (primary) | -0.7% | [-0.9%, -0.6%] | 6 |
217+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
218+
| All ❌✅ (primary) | -0.7% | [-0.9%, -0.6%] | 6 |
219+
220+
* This PR had more wins than losses, however its effects were later mostly reverted in
221+
[#118553](https://github.com/rust-lang/rust/pull/118553).
222+
223+
LLVM 18 x86 data layout update [#116672](https://github.com/rust-lang/rust/pull/116672) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=88189a71e4e4376eea82ac61db6a539612eb200a&end=0547c41f906760ce117a55ca690820b44d8e7eef&stat=instructions:u)
224+
225+
| (instructions:u) | mean | range | count |
226+
|:----------------------------------:|:-----:|:--------------:|:-----:|
227+
| Regressions ❌ <br /> (primary) | 0.7% | [0.7%, 0.7%] | 1 |
228+
| Regressions ❌ <br /> (secondary) | 1.0% | [0.3%, 3.3%] | 6 |
229+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.6%, -0.3%] | 8 |
230+
| Improvements ✅ <br /> (secondary) | -0.6% | [-1.1%, -0.4%] | 11 |
231+
| All ❌✅ (primary) | -0.3% | [-0.6%, 0.7%] | 9 |
232+
233+
* The wins have outweighted the losses.
234+
* This change is required to update to LLVM 18.
235+
* It introduced a few Max-RSS regressions, which were later fixed in
236+
[#120080](https://github.com/rust-lang/rust/pull/120080).
237+
238+
Rollup of 6 pull requests [#120170](https://github.com/rust-lang/rust/pull/120170) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1828461982f37d1a0053d156cedad5522b0f8a97&end=038d115cd8659bbe4d5e9600839759f55dfcfb0c&stat=instructions:u)
239+
240+
| (instructions:u) | mean | range | count |
241+
|:----------------------------------:|:-----:|:--------------:|:-----:|
242+
| Regressions ❌ <br /> (primary) | 0.6% | [0.6%, 0.6%] | 1 |
243+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
244+
| Improvements ✅ <br /> (primary) | -0.9% | [-1.4%, -0.5%] | 2 |
245+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
246+
| All ❌✅ (primary) | -0.4% | [-1.4%, 0.6%] | 3 |
247+
248+
* Tiny regression on a single benchmark, marked as triaged.
249+
250+
Pack u128 in the compiler to mitigate new alignment [#120080](https://github.com/rust-lang/rust/pull/120080) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=366d112fa69164d79239ceeaa49e06497df5497f&end=30662530506ed29ea29191798cb2ab8aa1249023&stat=instructions:u)
251+
252+
| (instructions:u) | mean | range | count |
253+
|:----------------------------------:|:-----:|:--------------:|:-----:|
254+
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.5%] | 12 |
255+
| Regressions ❌ <br /> (secondary) | 1.2% | [0.4%, 2.1%] | 2 |
256+
| Improvements ✅ <br /> (primary) | -0.7% | [-0.7%, -0.7%] | 1 |
257+
| Improvements ✅ <br /> (secondary) | -0.6% | [-0.7%, -0.3%] | 5 |
258+
| All ❌✅ (primary) | 0.3% | [-0.7%, 0.5%] | 13 |
259+
260+
* This PR mitigates the Max-RSS hits caused by [#116672](https://github.com/rust-lang/rust/pull/116672),
261+
at the cost of a few small instruction count regressions.

0 commit comments

Comments
 (0)