Skip to content

Commit 75a44c5

Browse files
KobzolKobzol
authored
Merge pull request #1980 from pnkfelix/triage-2024-09-23
Triage 2024 09 23
2 parents bd9cb8e + 44fe248 commit 75a44c5

File tree

1 file changed

+203
-0
lines changed

1 file changed

+203
-0
lines changed

‎triage/2024-09-23.md

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
# 2024-09-23 Triage Log
2+
3+
Not too much happened this week. Most regressions of note were readily
4+
justified as removing sources of unpredictable/inconsistent behavior
5+
from code-generation. There was one notable improvement, from PR
6+
#130561: avoiding redoing a redundant normalization of the param-env
7+
ended up improving compile times for 93 primary benchmarks by -1.0% on
8+
average.
9+
10+
Triage done by **@pnkfelix**.
11+
Revision range: [170d6cb8..749f80ab](https://perf.rust-lang.org/?start=170d6cb845c8c3f0dcec5cdd4210df9ecf990244&end=749f80ab051aa0b3724b464130440b0e70a975ac&absolute=false&stat=instructions%3Au)
12+
Revision range: [506f22b4..4cadeda9](https://perf.rust-lang.org/?start=506f22b4663f3e756e1e6a4f66c6309fdc00819c&end=4cadeda932d5c261a9a0b1bbd25c4486e4e0a4c6&absolute=false&stat=instructions%3Au)
13+
14+
**Summary**:
15+
16+
Note: Due to issues in how the perf website generates its output and
17+
fails to deal with certain git histories, we have two different base
18+
summaries.
19+
20+
Base Revision range: [170d6cb8..749f80ab](https://perf.rust-lang.org/?start=170d6cb845c8c3f0dcec5cdd4210df9ecf990244&end=749f80ab051aa0b3724b464130440b0e70a975ac&absolute=false&stat=instructions%3Au)
21+
22+
| (instructions:u) | mean | range | count |
23+
|:----------------------------------:|:-----:|:--------------:|:-----:|
24+
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 2.5%] | 8 |
25+
| Regressions ❌ <br /> (secondary) | 1.2% | [0.1%, 3.2%] | 4 |
26+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.3%] | 9 |
27+
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.7%, -0.2%] | 23 |
28+
| All ❌✅ (primary) | 0.2% | [-0.4%, 2.5%] | 17 |
29+
30+
Base Revision range: [506f22b4..4cadeda9](https://perf.rust-lang.org/?start=506f22b4663f3e756e1e6a4f66c6309fdc00819c&end=4cadeda932d5c261a9a0b1bbd25c4486e4e0a4c6&absolute=false&stat=instructions%3Au)
31+
32+
| (instructions:u) | mean | range | count |
33+
|:----------------------------------:|:-----:|:--------------:|:-----:|
34+
| Regressions ❌ <br /> (primary) | 2.7% | [0.1%, 6.7%] | 16 |
35+
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.2%] | 6 |
36+
| Improvements ✅ <br /> (primary) | -1.1% | [-5.1%, -0.2%] | 80 |
37+
| Improvements ✅ <br /> (secondary) | -2.4% | [-5.2%, -0.2%] | 54 |
38+
| All ❌✅ (primary) | -0.5% | [-5.1%, 6.7%] | 96 |
39+
40+
Total:
41+
42+
2 Regression, 2 Improvements, 7 Mixed; 4 of them in rollups
43+
62 artifact comparisons made in total
44+
45+
#### Regressions
46+
47+
Remove semi-nondeterminism of `DefPathHash` ordering from inliner [#130455](https://github.com/rust-lang/rust/pull/130455) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=2e367d94f05f3c6170f4d49f5e387cfaa0c42c32&end=46b0f8bafcf11fa1d6b6c172f4f5c1b95c11ebdb&stat=instructions:u)
48+
49+
| (instructions:u) | mean | range | count |
50+
|:----------------------------------:|:----:|:------------:|:-----:|
51+
| Regressions ❌ <br /> (primary) | 0.8% | [0.3%, 2.5%] | 5 |
52+
| Regressions ❌ <br /> (secondary) | 0.6% | [0.1%, 1.0%] | 3 |
53+
| Improvements ✅ <br /> (primary) | - | - | 0 |
54+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
55+
| All ❌✅ (primary) | 0.8% | [0.3%, 2.5%] | 5 |
56+
57+
* primary regressions are all to opt-full for hyper (-2.54%), clap (-0.69%), html5ever (-0.36%), image (-0.31%), cargo (-0.27%)
58+
* from detailed results on hyper, looks like `mir_callgraph_reachable` doubled in instr-count; if I'm reading the table right,
59+
it seems like it is being called nearly twice as often as before? But this outcome seems like it might be expected, since
60+
this PR is deliberately removing some code that would bail out early from that call.
61+
* it looks like @**Ben Kimock (Saethlin)** had already
62+
[attempted to mark](https://github.com/rust-lang/rust/pull/130455#issuecomment-2354849189) this as triaged (due to it being
63+
acceptable overhead for the benefit of better de
64+
65+
Rollup of 6 pull requests [#130649](https://github.com/rust-lang/rust/pull/130649) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c0838c8ebec23fb87855bb6de3a287981cb1df98&end=74fd001cdae0321144a20133f2216ea8a97da476&stat=instructions:u)
66+
67+
| (instructions:u) | mean | range | count |
68+
|:----------------------------------:|:----:|:------------:|:-----:|
69+
| Regressions ❌ <br /> (primary) | - | - | 0 |
70+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.1%, 0.4%] | 13 |
71+
| Improvements ✅ <br /> (primary) | - | - | 0 |
72+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
73+
| All ❌✅ (primary) | - | - | 0 |
74+
75+
* This was flagged solely because of the *number* of secondary benchmarks that were affected.
76+
* Marking as triaged.
77+
78+
#### Improvements
79+
80+
[perf] skip normalizing param env if it is already normalized [#130561](https://github.com/rust-lang/rust/pull/130561) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=506f22b4663f3e756e1e6a4f66c6309fdc00819c&end=5793a9e90289382e447f5bc411cab2bc01c5836f&stat=instructions:u)
81+
82+
| (instructions:u) | mean | range | count |
83+
|:----------------------------------:|:-----:|:--------------:|:-----:|
84+
| Regressions ❌ <br /> (primary) | - | - | 0 |
85+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
86+
| Improvements ✅ <br /> (primary) | -1.0% | [-5.5%, -0.1%] | 93 |
87+
| Improvements ✅ <br /> (secondary) | -2.7% | [-6.5%, -0.2%] | 59 |
88+
| All ❌✅ (primary) | -1.0% | [-5.5%, -0.1%] | 93 |
89+
90+
* This is awesome. :)
91+
92+
Rollup of 7 pull requests [#130768](https://github.com/rust-lang/rust/pull/130768) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f5cd2c5888011d4d80311e5b771c6da507d860dd&end=4cadeda932d5c261a9a0b1bbd25c4486e4e0a4c6&stat=instructions:u)
93+
94+
| (instructions:u) | mean | range | count |
95+
|:----------------------------------:|:-----:|:--------------:|:-----:|
96+
| Regressions ❌ <br /> (primary) | - | - | 0 |
97+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
98+
| Improvements ✅ <br /> (primary) | - | - | 0 |
99+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.7%, -0.1%] | 14 |
100+
| All ❌✅ (primary) | - | - | 0 |
101+
102+
* This was flagged because of the *number* of secondary benchmarks that were affected.
103+
104+
#### Mixed
105+
106+
read_volatile __rust_no_alloc_shim_is_unstable in alloc_zeroed [#130497](https://github.com/rust-lang/rust/pull/130497) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=aaed38b2a631dfc593454abf471d75d84033773e&end=7fc70f870a1d11c3b104274d3a18112996cdec9a&stat=instructions:u)
107+
108+
| (instructions:u) | mean | range | count |
109+
|:----------------------------------:|:-----:|:--------------:|:-----:|
110+
| Regressions ❌ <br /> (primary) | 0.5% | [0.5%, 0.5%] | 1 |
111+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
112+
| Improvements ✅ <br /> (primary) | -0.5% | [-0.7%, -0.3%] | 2 |
113+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
114+
| All ❌✅ (primary) | -0.2% | [-0.7%, 0.5%] | 3 |
115+
116+
* cranelift-codegen opt-full-llvm regressed by -0.48%.
117+
* marking as triaged; this is bringing the two allocator paths into consistency with eachother (the question of whether we want these volatile reads at all, and how to otherwise achieve their effect on the end-to-end developer experience, is an [entirely separate question](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Different.20mechanism.20for.20__rust_no_alloc_shim_is_unstable/near/462025387)).
118+
119+
Rollup of 9 pull requests [#130534](https://github.com/rust-lang/rust/pull/130534) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f79a912d9edc3ad4db910c0e93672ed5c65133fa&end=a5cf8bbd4e1c8edeae08778c85c6f806dd00e853&stat=instructions:u)
120+
121+
| (instructions:u) | mean | range | count |
122+
|:----------------------------------:|:-----:|:--------------:|:-----:|
123+
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.3%] | 3 |
124+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
125+
| Improvements ✅ <br /> (primary) | - | - | 0 |
126+
| Improvements ✅ <br /> (secondary) | -1.1% | [-1.2%, -0.6%] | 4 |
127+
| All ❌✅ (primary) | 0.3% | [0.2%, 0.3%] | 3 |
128+
129+
* syn regressed on three variant profiles: check incr-unchanged (-0.28%), check incr-patched:println (-0.27%), and debug incr-unchanged (-0.23%)
130+
* [detailed perf results](https://perf.rust-lang.org/detailed-query.html?commit=a5cf8bbd4e1c8edeae08778c85c6f806dd00e853&benchmark=syn-1.0.89-check&scenario=incr-unchanged&base_commit=f79a912d9edc3ad4db910c0e93672ed5c65133fa&sort_idx=-11) indicates that incr_comp_persist_dep_graph went from 0.002 seconds to 0.14 seconds; a delta that accounts for the vast bulk of the difference here...
131+
* but from examining the PR's that follow, it doesn't seem like that result persists into the future. (Also, I'm realizing that the detailed results are presenting seconds, not instruction-counts, and therefore they are probably wildly unstable between runs...)
132+
* marking as triaged; this isn't worth investigating further.
133+
134+
Support `char::encode_utf8` in const scenarios. [#130511](https://github.com/rust-lang/rust/pull/130511) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=df7f77811c8806f85522a38878c57fde221138c9&end=f8192ba0d00bbbc58a10c45823ba010e98ea4474&stat=instructions:u)
135+
136+
| (instructions:u) | mean | range | count |
137+
|:----------------------------------:|:-----:|:--------------:|:-----:|
138+
| Regressions ❌ <br /> (primary) | 0.3% | [0.3%, 0.4%] | 3 |
139+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
140+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.5%, -0.3%] | 2 |
141+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
142+
| All ❌✅ (primary) | 0.0% | [-0.5%, 0.4%] | 5 |
143+
144+
* primary regressions to opt-full on clap (-0.38%), image (-0.33%), and webrender (-0.31%).
145+
* from inspection, it seems like this may just be a natural (and small, acceptable) artifact of the changes to which things get const-eval'ed.
146+
* marking as triaged
147+
148+
149+
Get rid of niche selection's dependence on fields's order [#130508](https://github.com/rust-lang/rust/pull/130508) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=749f80ab051aa0b3724b464130440b0e70a975ac&end=902f2956a627bd1bd44557b7fe6427a099bf8c48&stat=instructions:u)
150+
151+
| (instructions:u) | mean | range | count |
152+
|:----------------------------------:|:-----:|:--------------:|:-----:|
153+
| Regressions ❌ <br /> (primary) | 0.2% | [0.2%, 0.2%] | 1 |
154+
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 0.9%] | 3 |
155+
| Improvements ✅ <br /> (primary) | -0.6% | [-0.6%, -0.6%] | 1 |
156+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
157+
| All ❌✅ (primary) | -0.2% | [-0.6%, 0.2%] | 2 |
158+
159+
* primary regression to typenum doc-full (-0.23%)
160+
* marking as triaged; this is a very small (and probably semi-random) cost for a clear win IMO in terms of overall consistency of behavior.
161+
162+
Rollup of 7 pull requests [#130631](https://github.com/rust-lang/rust/pull/130631) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5ba6db1b648d93fbbab4ae0466e40db682fa45fc&end=da889684c80508036ff036db8c159ffdcf27648a&stat=instructions:u)
163+
164+
| (instructions:u) | mean | range | count |
165+
|:----------------------------------:|:-----:|:--------------:|:-----:|
166+
| Regressions ❌ <br /> (primary) | 0.6% | [0.6%, 0.6%] | 1 |
167+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.2%, 0.4%] | 5 |
168+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.3%] | 1 |
169+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
170+
| All ❌✅ (primary) | 0.1% | [-0.3%, 0.6%] | 2 |
171+
172+
* sole primary regression is to cargo opt-full by 0.6%.
173+
* no obvious smoking guns in the PR list for this rollup.
174+
* marking as triaged as "not being worth further investigation"
175+
176+
add unqualified_local_imports lint [#125645](https://github.com/rust-lang/rust/pull/125645) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9d6039ccae68a2f1930ed9c1542d387b2c0c0ba6&end=7042c269c166191cd5d8daf0409890903df7af57&stat=instructions:u)
177+
178+
| (instructions:u) | mean | range | count |
179+
|:----------------------------------:|:-----:|:--------------:|:-----:|
180+
| Regressions ❌ <br /> (primary) | - | - | 0 |
181+
| Regressions ❌ <br /> (secondary) | 1.5% | [0.4%, 2.0%] | 11 |
182+
| Improvements ✅ <br /> (primary) | - | - | 0 |
183+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.4%, -0.2%] | 6 |
184+
| All ❌✅ (primary) | - | - | 0 |
185+
186+
* There was some surprise here for the PR author because this was an allow-by-default lint and so they didn't expect it to have any actual perf impact,
187+
because they assumed that allowed-linted could be skipped (which is not actually the way they work today, but there are PR's in process that try
188+
to get that effect).
189+
* In any case, this PR was flagged solely because of the *number* of secondary benchmarks that were affected.
190+
* Marking as triaged.
191+
192+
Rework `non_local_definitions` lint to only use a syntactic heuristic [#127117](https://github.com/rust-lang/rust/pull/127117) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=35daf8b8aa95dee4296bad117c4cb7ecaea7bf4f&end=f5cd2c5888011d4d80311e5b771c6da507d860dd&stat=instructions:u)
193+
194+
| (instructions:u) | mean | range | count |
195+
|:----------------------------------:|:-----:|:--------------:|:-----:|
196+
| Regressions ❌ <br /> (primary) | 3.3% | [0.6%, 6.6%] | 13 |
197+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.1%, 0.4%] | 3 |
198+
| Improvements ✅ <br /> (primary) | -0.7% | [-1.0%, -0.2%] | 8 |
199+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 3 |
200+
| All ❌✅ (primary) | 1.8% | [-1.0%, 6.6%] | 21 |
201+
202+
* as noted by Urgau: "the version of diesel used by `rustc-perf` is too old and use named `const AA` items instead of un-named `const _` items, making the lint trigger many times (more than 150 times last time). We ruled in the past that given the regression where located in one benchmark only, and only a pathological case that the regressions where fine."
203+
* already marked as triaged.

0 commit comments

Comments
 (0)