Skip to content

Commit b00915b

Browse files
KobzolKobzol
authored
Merge pull request #1848 from pnkfelix/triage-2024-03-11
Triage 2024 03 11
2 parents efa4367 + 69fb50d commit b00915b

File tree

1 file changed

+246
-0
lines changed

1 file changed

+246
-0
lines changed

‎triage/2024-03-11.md

+246
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
# 2024-03-11 Triage Log
2+
3+
A mixed week, with a vast number of improvements (in large part due to PR
4+
#122010, which undoes a prior regression; PR #120985, a host LLVM update).
5+
But also three admittedly small-ish regressions which seemed unanticipated and
6+
were still large enough that I did not feel comfortable rubber-stamping them
7+
with a perf-regression-triaged marking.
8+
9+
Triage done by **@pnkfelix**.
10+
Revision range: [41d97c8a..e919669d](https://perf.rust-lang.org/?start=41d97c8a5dea2731b0e56fe97cd7cb79e21cff79&end=e919669d42dfb8950866d4cb268c5359eb3f7c54&absolute=false&stat=instructions%3Au)
11+
12+
**Summary**:
13+
14+
| (instructions:u) | mean | range | count |
15+
|:----------------------------------:|:-----:|:--------------:|:-----:|
16+
| Regressions ❌ <br /> (primary) | 0.6% | [0.2%, 1.4%] | 38 |
17+
| Regressions ❌ <br /> (secondary) | 1.1% | [0.2%, 4.9%] | 50 |
18+
| Improvements ✅ <br /> (primary) | -1.0% | [-4.8%, -0.2%] | 119 |
19+
| Improvements ✅ <br /> (secondary) | -0.8% | [-2.2%, -0.4%] | 36 |
20+
| All ❌✅ (primary) | -0.6% | [-4.8%, 1.4%] | 157 |
21+
22+
23+
2 Regressions, 5 Improvements, 9 Mixed; 5 of them in rollups
24+
54 artifact comparisons made in total
25+
26+
#### Regressions
27+
28+
interpret: avoid a long-lived PlaceTy in stack frames [#121985](https://github.com/rust-lang/rust/pull/121985) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8c9a75b3238b66592779d6b240dbf78eacefebb8&end=52f8aec14c616387c5f793687f2d9026de6c78ca&stat=instructions:u)
29+
30+
| (instructions:u) | mean | range | count |
31+
|:----------------------------------:|:----:|:------------:|:-----:|
32+
| Regressions ❌ <br /> (primary) | 0.3% | [0.3%, 0.3%] | 1 |
33+
| Regressions ❌ <br /> (secondary) | 3.0% | [0.2%, 4.5%] | 8 |
34+
| Improvements ✅ <br /> (primary) | - | - | 0 |
35+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
36+
| All ❌✅ (primary) | 0.3% | [0.3%, 0.3%] | 1 |
37+
38+
* primary regression was to html5ever doc-full; was anctipated during development and is presumed spurious.
39+
* marking as triaged.
40+
41+
Detect unused struct impls pub trait [#121752](https://github.com/rust-lang/rust/pull/121752) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cd81f5b27ee00b49d413db50b5e6af871cebcf23&end=c69fda7dc664e62f8920a02a4e55d6207b212c24&stat=instructions:u)
42+
43+
| (instructions:u) | mean | range | count |
44+
|:----------------------------------:|:----:|:------------:|:-----:|
45+
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.5%] | 6 |
46+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
47+
| Improvements ✅ <br /> (primary) | - | - | 0 |
48+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
49+
| All ❌✅ (primary) | 0.4% | [0.2%, 0.5%] | 6 |
50+
51+
* primary regressions are all to serde and cranelift codegen for various profiles of incr-patched:println.
52+
* the cycles measurement didn't observe any change at all, but that could be due to the difference being swamped by overall variance
53+
* the [perf diff](https://perf.rust-lang.org/detailed-query.html?commit=c69fda7dc664e62f8920a02a4e55d6207b212c24&benchmark=serde-1.0.136-check&scenario=incr-patched%3A+println&base_commit=cd81f5b27ee00b49d413db50b5e6af871cebcf23&sort_idx=-11) highlights that the query `live_symbols_and_ignored_derived_traits` is the source of the perf regression, which is consistent with the idea that this lint has become more expensive since that's where we see the call to the newly-added `solve_rest_impl_items` (a worklist algorithm from the PR).
54+
* leaving a note for the author about this; not marking as triaged.
55+
56+
#### Improvements
57+
58+
Rollup of 7 pull requests [#122111](https://github.com/rust-lang/rust/pull/122111) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=bfe762e0ed2e95041cc12c02c5565c4368f2cc9f&end=7d3702e472b99be0f5de6608dd87af1df8f99428&stat=instructions:u)
59+
60+
| (instructions:u) | mean | range | count |
61+
|:----------------------------------:|:-----:|:--------------:|:-----:|
62+
| Regressions ❌ <br /> (primary) | - | - | 0 |
63+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
64+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.3%] | 7 |
65+
| Improvements ✅ <br /> (secondary) | -0.5% | [-0.6%, -0.3%] | 5 |
66+
| All ❌✅ (primary) | -0.4% | [-0.4%, -0.3%] | 7 |
67+
68+
* all 7 primary improvements are to html5ever.
69+
* all 5 secondary improvements are to tt-muncher.
70+
71+
Rollup of 8 pull requests [#122117](https://github.com/rust-lang/rust/pull/122117) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=7d3702e472b99be0f5de6608dd87af1df8f99428&end=d03b986db1f4146b58078c9dde5b0fa6d808f031&stat=instructions:u)
72+
73+
| (instructions:u) | mean | range | count |
74+
|:----------------------------------:|:-----:|:--------------:|:-----:|
75+
| Regressions ❌ <br /> (primary) | - | - | 0 |
76+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
77+
| Improvements ✅ <br /> (primary) | -2.1% | [-3.9%, -0.4%] | 12 |
78+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
79+
| All ❌✅ (primary) | -2.1% | [-3.9%, -0.4%] | 12 |
80+
81+
* all 12 primary improvements are to diesel
82+
* this is because of PR #122107, which made the `non_local_definitions` lint allow-by-default
83+
* this effectively had the reverse effect of PR #120393 (which added the aforementioned lint and caused regressions to 12 variations of diesel).
84+
85+
Merge `collect_mod_item_types` query into `check_well_formed` [#121500](https://github.com/rust-lang/rust/pull/121500) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=42825768b103c28b10ce0407749acb21d32abeec&end=74acabe9b042ea8c42862ee29aca2a8b7d333644&stat=instructions:u)
86+
87+
| (instructions:u) | mean | range | count |
88+
|:----------------------------------:|:-----:|:--------------:|:-----:|
89+
| Regressions ❌ <br /> (primary) | - | - | 0 |
90+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
91+
| Improvements ✅ <br /> (primary) | -0.6% | [-0.8%, -0.2%] | 4 |
92+
| Improvements ✅ <br /> (secondary) | -0.5% | [-0.6%, -0.5%] | 2 |
93+
| All ❌✅ (primary) | -0.6% | [-0.8%, -0.2%] | 4 |
94+
95+
* 3 significant primary improvements are to libc incr-patched:clone, and 1 less significant to bitmaps check incr-unchanged.
96+
97+
Avoid invoking the `intrinsic` query for DefKinds other than `Fn` or `AssocFn` [#122010](https://github.com/rust-lang/rust/pull/122010) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=01d73d4041969cde4a79bf9793521ef323248a24&end=4d4bb491b65c300835442f6cb4f34fc9a5685c26&stat=instructions:u)
98+
99+
| (instructions:u) | mean | range | count |
100+
|:----------------------------------:|:-----:|:--------------:|:-----:|
101+
| Regressions ❌ <br /> (primary) | - | - | 0 |
102+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
103+
| Improvements ✅ <br /> (primary) | -0.5% | [-1.0%, -0.2%] | 74 |
104+
| Improvements ✅ <br /> (secondary) | -0.7% | [-2.1%, -0.2%] | 26 |
105+
| All ❌✅ (primary) | -0.5% | [-1.0%, -0.2%] | 74 |
106+
107+
* undoes the vast bulk of the broad perf regression injected by PR #120675
108+
109+
Dep node encoding cleanups [#122064](https://github.com/rust-lang/rust/pull/122064) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5bc7b9ac8ace5312e1d2cdc2722715cf58d4f926&end=094a6204f590e6b4770b5f26359dd17a07897adf&stat=instructions:u)
110+
111+
| (instructions:u) | mean | range | count |
112+
|:----------------------------------:|:-----:|:--------------:|:-----:|
113+
| Regressions ❌ <br /> (primary) | - | - | 0 |
114+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
115+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.2%] | 19 |
116+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.4%, -0.2%] | 12 |
117+
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.2%] | 19 |
118+
119+
120+
#### Mixed
121+
122+
Optimize write with as_const_str for shorter code [#122059](https://github.com/rust-lang/rust/pull/122059) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=79d246112dc95bbd67848f7546f3fd1aca516b82&end=9fb91aa2e70bfcc1c0adaad79711f0321ea81ece&stat=instructions:u)
123+
124+
| (instructions:u) | mean | range | count |
125+
|:----------------------------------:|:-----:|:--------------:|:-----:|
126+
| Regressions ❌ <br /> (primary) | - | - | 0 |
127+
| Regressions ❌ <br /> (secondary) | 1.1% | [0.3%, 1.9%] | 2 |
128+
| Improvements ✅ <br /> (primary) | -0.8% | [-1.2%, -0.4%] | 2 |
129+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 |
130+
| All ❌✅ (primary) | -0.8% | [-1.2%, -0.4%] | 2 |
131+
132+
* (secondary) regressions are to deep-vector debug-full (which may be spurious based on the graph) and wg-grammar debug-incr-unchanged
133+
* since @nnethercote was already involved in related efforts here (e.g. PR #121001) and this resulting refinement, I'm going to mark this as triaged.
134+
135+
Replace the default branch with an unreachable branch If it is the last variant [#120268](https://github.com/rust-lang/rust/pull/120268) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9fb91aa2e70bfcc1c0adaad79711f0321ea81ece&end=14fbc3c00525b41a3a3ee2c90e9ab6fd3b05274f&stat=instructions:u)
136+
137+
| (instructions:u) | mean | range | count |
138+
|:----------------------------------:|:-----:|:--------------:|:-----:|
139+
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 1.8%] | 4 |
140+
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.3%] | 7 |
141+
| Improvements ✅ <br /> (primary) | -0.8% | [-1.2%, -0.3%] | 5 |
142+
| Improvements ✅ <br /> (secondary) | -0.9% | [-2.2%, -0.3%] | 3 |
143+
| All ❌✅ (primary) | -0.1% | [-1.2%, 1.8%] | 9 |
144+
145+
* primary regressions are to cargo opt-full (1.76%), cargo debug-incr-patched:println (0.40%), libc doc-full (0.50%), hyper doc-full (0.19%).
146+
* the regression to cargo opt-full [was anticipated](https://github.com/rust-lang/rust/pull/120268#issuecomment-1943896894) by rust-timer runs during development; but other follow-up rust-timer runs did not always include the same regression.
147+
* the PR itself notes that there are known performance problems in LLVM with unreachable branches (see e.g. [llvm#78578](https://github.com/llvm/llvm-project/issues/78578)). It is not clear to me whether the above regressions are related to that.
148+
* posted comment [asking](https://github.com/rust-lang/rust/pull/120268#issuecomment-1992182505) PR author for more info. Not marking as triaged.
149+
150+
Rollup of 8 pull requests [#122182](https://github.com/rust-lang/rust/pull/122182) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=14fbc3c00525b41a3a3ee2c90e9ab6fd3b05274f&end=1b2c53a15dba7962cfc284c3b6d61a0341ffa27a&stat=instructions:u)
151+
152+
| (instructions:u) | mean | range | count |
153+
|:----------------------------------:|:-----:|:--------------:|:-----:|
154+
| Regressions ❌ <br /> (primary) | 1.3% | [1.3%, 1.3%] | 1 |
155+
| Regressions ❌ <br /> (secondary) | 0.8% | [0.2%, 1.3%] | 2 |
156+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.7%, -0.2%] | 17 |
157+
| Improvements ✅ <br /> (secondary) | -0.5% | [-0.9%, -0.2%] | 15 |
158+
| All ❌✅ (primary) | -0.3% | [-0.7%, 1.3%] | 18 |
159+
160+
* sole primary regression is to image opt-full
161+
* improvements obviously outweigh the regressions
162+
* ... but oh, there is also a bootstrap regression that may be of concern: "Bootstrap: 648.483s -> 652.903s (0.68%)"
163+
* kobzol has hypothesized that PR #122099 may be the cause of the bootstrap regression.
164+
* after some discussion on the rollup PR, decided to mark as triaged; the author
165+
may well choose to do some followup, but we will not hound them about it. :)
166+
167+
Replace `TypeWalker` usage with `TypeVisitor` in `wf.rs` [#122150](https://github.com/rust-lang/rust/pull/122150) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9d272a1b0501f96da0ed10caa1b2eb6dbb653686&end=b054da815501bafb24a08284151d32862f7a3a13&stat=instructions:u)
168+
169+
| (instructions:u) | mean | range | count |
170+
|:----------------------------------:|:-----:|:--------------:|:-----:|
171+
| Regressions ❌ <br /> (primary) | - | - | 0 |
172+
| Regressions ❌ <br /> (secondary) | 0.7% | [0.3%, 1.3%] | 6 |
173+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.2%] | 6 |
174+
| Improvements ✅ <br /> (secondary) | -0.2% | [-0.3%, -0.2%] | 3 |
175+
| All ❌✅ (primary) | -0.3% | [-0.4%, -0.2%] | 6 |
176+
177+
178+
* six (secondary) regressions (to variants of unify-linearly and regression-31157) were anticipated during development
179+
* we were also expecting a broader set of 34 primary improvements. But the mean primary improvement we expected was -0.3%, which was unchanged.
180+
* marking as triaged
181+
182+
Rollup of 12 pull requests [#122241](https://github.com/rust-lang/rust/pull/122241) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=b054da815501bafb24a08284151d32862f7a3a13&end=8401645716b26a8b4c6974dc0680e55e81e9e8a1&stat=instructions:u)
183+
184+
| (instructions:u) | mean | range | count |
185+
|:----------------------------------:|:-----:|:--------------:|:-----:|
186+
| Regressions ❌ <br /> (primary) | - | - | 0 |
187+
| Regressions ❌ <br /> (secondary) | 0.9% | [0.2%, 1.6%] | 2 |
188+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.3%] | 2 |
189+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 |
190+
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.3%] | 2 |
191+
192+
* sole regressions are to (secondary) regression-31157 debug-incr-full (1.56%), which might be spurious based on the graph, and tt-muncher opt-incr-unchanged (0.19%), which I don't consider worth getting worried about.
193+
* marking as triaged.
194+
195+
Update host LLVM on x64 Linux to LLVM 18 [#120985](https://github.com/rust-lang/rust/pull/120985) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8401645716b26a8b4c6974dc0680e55e81e9e8a1&end=25ee3c6a2f429a97ff4ad239e3f42409cd71fe0a&stat=instructions:u)
196+
197+
| (instructions:u) | mean | range | count |
198+
|:----------------------------------:|:-----:|:--------------:|:-----:|
199+
| Regressions ❌ <br /> (primary) | 0.3% | [0.1%, 1.0%] | 88 |
200+
| Regressions ❌ <br /> (secondary) | 0.4% | [0.1%, 0.5%] | 21 |
201+
| Improvements ✅ <br /> (primary) | -1.0% | [-1.6%, -0.4%] | 54 |
202+
| Improvements ✅ <br /> (secondary) | -0.8% | [-1.2%, -0.6%] | 8 |
203+
| All ❌✅ (primary) | -0.2% | [-1.6%, 1.0%] | 142 |
204+
205+
* These performance changes were anticipated, and the improvements outweigh the regressions.
206+
* Marking as triaged.
207+
208+
Rollup of 8 pull requests [#122256](https://github.com/rust-lang/rust/pull/122256) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=25ee3c6a2f429a97ff4ad239e3f42409cd71fe0a&end=2d24fe591f30386d6d5fc2bb941c78d7266bf10f&stat=instructions:u)
209+
210+
| (instructions:u) | mean | range | count |
211+
|:----------------------------------:|:-----:|:--------------:|:-----:|
212+
| Regressions ❌ <br /> (primary) | 0.6% | [0.2%, 1.6%] | 21 |
213+
| Regressions ❌ <br /> (secondary) | 0.6% | [0.3%, 1.6%] | 7 |
214+
| Improvements ✅ <br /> (primary) | -0.7% | [-1.0%, -0.5%] | 3 |
215+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
216+
| All ❌✅ (primary) | 0.4% | [-1.0%, 1.6%] | 24 |
217+
218+
* Nadrieril has isolated this to the rolled up PR #120504 and has reported it on that PR.
219+
* (already marked as triaged)
220+
* It might be addressed by follow-up PR #122396 (which is undergoing evaluation now).
221+
222+
Distinguish between library and lang UB in assert_unsafe_precondition [#121662](https://github.com/rust-lang/rust/pull/121662) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=2d24fe591f30386d6d5fc2bb941c78d7266bf10f&end=768408af123a455fb27ad8af8055becd5b95d36f&stat=instructions:u)
223+
224+
| (instructions:u) | mean | range | count |
225+
|:----------------------------------:|:-----:|:--------------:|:-----:|
226+
| Regressions ❌ <br /> (primary) | 1.1% | [0.3%, 1.7%] | 4 |
227+
| Regressions ❌ <br /> (secondary) | 1.1% | [1.1%, 1.1%] | 1 |
228+
| Improvements ✅ <br /> (primary) | -0.9% | [-1.7%, -0.4%] | 4 |
229+
| Improvements ✅ <br /> (secondary) | -0.2% | [-0.2%, -0.2%] | 1 |
230+
| All ❌✅ (primary) | 0.1% | [-1.7%, 1.7%] | 8 |
231+
232+
* primary regressions are to cranelift-codegen opt-full (1.74%), cargo opt-full (1.33%), clap opt-full (1.18%), and exa debug-incr-unchanged (0.28%).
233+
* the [rust-timer run](https://github.com/rust-lang/rust/pull/121662#issuecomment-1966015546) was "only" expected to regress 5 benchmarks, largely a *different* set, by a mean of 0.5%, not the 1.1% reported above.
234+
* not marking as triaged
235+
236+
Stop using LLVM struct types for byval/sret [#122050](https://github.com/rust-lang/rust/pull/122050) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c69fda7dc664e62f8920a02a4e55d6207b212c24&end=a6d93acf5fdeb020ab86cc0d30d5672c23a7dba6&stat=instructions:u)
237+
238+
| (instructions:u) | mean | range | count |
239+
|:----------------------------------:|:-----:|:--------------:|:-----:|
240+
| Regressions ❌ <br /> (primary) | - | - | 0 |
241+
| Regressions ❌ <br /> (secondary) | 1.9% | [0.5%, 3.3%] | 2 |
242+
| Improvements ✅ <br /> (primary) | -2.1% | [-2.4%, -1.9%] | 2 |
243+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
244+
| All ❌✅ (primary) | -2.1% | [-2.4%, -1.9%] | 2 |
245+
246+
* already marked as triaged as the reported regressions are clearly spurious based on the performance graph

0 commit comments

Comments
 (0)