Skip to content

Commit 90415c8

Browse files
KobzolKobzol
authored
Merge pull request #1905 from pnkfelix/triage-2024-05-06
Triage 2024 05 06
2 parents c982583 + 6774c87 commit 90415c8

File tree

1 file changed

+180
-0
lines changed

1 file changed

+180
-0
lines changed

‎triage/2024-05-06.md

+180
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
# 2024-05-07 Triage Log
2+
3+
Largely uneventful week; the most notable shifts were considered false-alarms
4+
that arose from changes related to cfg-checking (either cargo enabling it, or
5+
adding cfg's like `rustfmt` to the "well-known cfgs list").
6+
7+
Triage done by **@pnkfelix**.
8+
Revision range: [c65b2dc9..69f53f5e](https://perf.rust-lang.org/?start=c65b2dc935c27c0c8c3997c6e8d8894718a2cb1a&end=69f53f5e5583381267298ac182eb02c7f1b5c1cd&absolute=false&stat=instructions%3Au)
9+
10+
**Summary**:
11+
12+
| (instructions:u) | mean | range | count |
13+
|:----------------------------------:|:-----:|:--------------:|:-----:|
14+
| Regressions ❌ <br /> (primary) | 3.0% | [0.2%, 19.5%] | 65 |
15+
| Regressions ❌ <br /> (secondary) | 1.3% | [0.2%, 4.5%] | 103 |
16+
| Improvements ✅ <br /> (primary) | -0.9% | [-2.2%, -0.2%] | 24 |
17+
| Improvements ✅ <br /> (secondary) | -0.7% | [-1.4%, -0.4%] | 23 |
18+
| All ❌✅ (primary) | 1.9% | [-2.2%, 19.5%] | 89 |
19+
20+
21+
3 Regressions, 2 Improvements, 3 Mixed; 5 of them in rollups
22+
54 artifact comparisons made in total
23+
24+
#### Regressions
25+
26+
Rollup of 7 pull requests [#124675](https://github.com/rust-lang/rust/pull/124675) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0d7b2fb797f214ea7514cfeaf2caef8178d8e3fc&end=befabbc9e5f6e82e659f9f52040ee0dd40593d8a&stat=instructions:u)
27+
28+
| (instructions:u) | mean | range | count |
29+
|:----------------------------------:|:----:|:------------:|:-----:|
30+
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 1.2%] | 11 |
31+
| Regressions ❌ <br /> (secondary) | 0.8% | [0.4%, 1.3%] | 17 |
32+
| Improvements ✅ <br /> (primary) | - | - | 0 |
33+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
34+
| All ❌✅ (primary) | 0.5% | [0.2%, 1.2%] | 11 |
35+
36+
* all primary regressions are to doc-full scenarios, and the 1.2% is to helloworld.
37+
* not worth teasing apart a rollup PR.
38+
* marking as triaged.
39+
40+
Update cargo [#124684](https://github.com/rust-lang/rust/pull/124684) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=09cd00fea4aecaa6707f122d7e143196b8a12ee2&end=2c4bf249bd47f232de3c1e78ffe69b40c29bfcca&stat=instructions:u)
41+
42+
| (instructions:u) | mean | range | count |
43+
|:----------------------------------:|:----:|:-------------:|:-----:|
44+
| Regressions ❌ <br /> (primary) | 2.4% | [0.2%, 19.1%] | 83 |
45+
| Regressions ❌ <br /> (secondary) | 1.6% | [0.2%, 5.7%] | 92 |
46+
| Improvements ✅ <br /> (primary) | - | - | 0 |
47+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
48+
| All ❌✅ (primary) | 2.4% | [0.2%, 19.1%] | 83 |
49+
50+
51+
* syn (mostly check builds, but also a debug incr-unchanged and opt incr-unchanged) had regressions ranging from 7.24% all the way up to 19.11%.
52+
* The most plausible hypothesis is that this is due to an explosion in the number of warnings emitted for this benchmark. (The number of warnings went from ~200 up to 1800, according to Urgau's analysis).
53+
* This means the code ends up becoming, at least in part, a benchmark of the lint machinery, regardless of whether that is our intent or not.
54+
* see also [rustc-perf#1819](https://github.com/rust-lang/rustc-perf/issues/1819) "Consider passing -Awarnings (or similar) to avoid false alarms from lint *reporting*"
55+
* marking as triaged.
56+
57+
Rollup of 3 pull requests [#124784](https://github.com/rust-lang/rust/pull/124784) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=96f1da82687f499dd3f57006ae71548714532382&end=d287f3e4eeaf680e8fe875f1ec75cca68f357d30&stat=instructions:u)
58+
59+
| (instructions:u) | mean | range | count |
60+
|:----------------------------------:|:----:|:------------:|:-----:|
61+
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.4%] | 5 |
62+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
63+
| Improvements ✅ <br /> (primary) | - | - | 0 |
64+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
65+
| All ❌✅ (primary) | 0.3% | [0.2%, 0.4%] | 5 |
66+
67+
* all regressions were to syn, to various incr-unchanged and incr-patched:println scenarios.
68+
* current hypothesis is that this is due to PR #124742, which adds `rustfmt` to the well-known cfgs list.
69+
* that hypothesis implies that this is a (mostly-)false alarm, much like #124684.
70+
* marking as triaged
71+
72+
#### Improvements
73+
74+
Rollup of 10 pull requests [#124646](https://github.com/rust-lang/rust/pull/124646) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=79734f1db8dbe322192dea32c0f6b80ab14c4c1d&end=561b5dea1e7e5c21f0fb550ca579229ceb878297&stat=instructions:u)
75+
76+
| (instructions:u) | mean | range | count |
77+
|:----------------------------------:|:-----:|:--------------:|:-----:|
78+
| Regressions ❌ <br /> (primary) | - | - | 0 |
79+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
80+
| Improvements ✅ <br /> (primary) | -1.0% | [-2.8%, -0.2%] | 24 |
81+
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.6%, -0.3%] | 9 |
82+
| All ❌✅ (primary) | -1.0% | [-2.8%, -0.2%] | 24 |
83+
84+
* the bulk of the improvements are to variations of html5ever and serde_derive.
85+
* skimming over the rollup list, I cannot identify an immediate root cause for improvement
86+
* but for now will treat it like a happy accident
87+
88+
Some hir cleanups [#124401](https://github.com/rust-lang/rust/pull/124401) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d2d24e395a1e4fcee62ca17bf4cbddb1f903af97&end=09cd00fea4aecaa6707f122d7e143196b8a12ee2&stat=instructions:u)
89+
90+
| (instructions:u) | mean | range | count |
91+
|:----------------------------------:|:-----:|:--------------:|:-----:|
92+
| Regressions ❌ <br /> (primary) | - | - | 0 |
93+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
94+
| Improvements ✅ <br /> (primary) | -0.1% | [-0.2%, -0.1%] | 3 |
95+
| Improvements ✅ <br /> (secondary) | -1.1% | [-2.0%, -0.2%] | 2 |
96+
| All ❌✅ (primary) | -0.1% | [-0.2%, -0.1%] | 3 |
97+
98+
* all improvements are to variations of typenum
99+
* the hir cleanups in question are largely to store `AnonConst` (e.g. for array lengths) in the HIR arena, and then move the ConstArg span over to AnonConst span instead.
100+
* inspection of typenum didn't show any particular cases that seemed like the would stress `AnonConst`; maybe the benefit comes more from the places where we now pass a span by value instead of passing a pointer to it.
101+
102+
#### Mixed
103+
104+
Account for immutably borrowed locals in MIR copy-prop and GVN [#123602](https://github.com/rust-lang/rust/pull/123602) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=befabbc9e5f6e82e659f9f52040ee0dd40593d8a&end=d2d24e395a1e4fcee62ca17bf4cbddb1f903af97&stat=instructions:u)
105+
106+
| (instructions:u) | mean | range | count |
107+
|:----------------------------------:|:-----:|:--------------:|:-----:|
108+
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.9%] | 10 |
109+
| Regressions ❌ <br /> (secondary) | 0.8% | [0.2%, 2.6%] | 4 |
110+
| Improvements ✅ <br /> (primary) | -0.5% | [-1.1%, -0.2%] | 6 |
111+
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.0%, -0.3%] | 8 |
112+
| All ❌✅ (primary) | 0.0% | [-1.1%, 0.9%] | 16 |
113+
114+
* html5ever opt-full regressed by 0.92%; libc in various incremental scenarios regressed by 0.30% to 0.39%.
115+
* the libc changes were anticipated in the perf build prior to merge; html5ever opt-full was not predicted there.
116+
* pnkfelix hypothesizes that this just reflects some extra-work from the compiler attempting to do the copy-propagation and global-value-numbering mir-optimizations on a larger set of immutably-borrowed locals, and is acceptable given the expected benefits.
117+
* marking as triaged
118+
119+
Rollup of 8 pull requests [#124703](https://github.com/rust-lang/rust/pull/124703) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=7dd170fccb3be6b1737af5df14dd736b366236c1&end=d7ea27808deb5e10a0f7384e339e4e6165e33398&stat=instructions:u)
120+
121+
| (instructions:u) | mean | range | count |
122+
|:----------------------------------:|:-----:|:--------------:|:-----:|
123+
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 0.6%] | 4 |
124+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
125+
| Improvements ✅ <br /> (primary) | - | - | 0 |
126+
| Improvements ✅ <br /> (secondary) | -1.0% | [-1.5%, -0.5%] | 4 |
127+
| All ❌✅ (primary) | 0.5% | [0.2%, 0.6%] | 4 |
128+
129+
* image opt-full regressed by 0.63%; html5ever debug-{incr-full,full} by ~0.5%, html5ever opt-incr-unchaged by 0.21%
130+
* already triaged by Kobzol, who hypothesizes that PR #124700 modified some inlining decisions.
131+
132+
Rollup of 4 pull requests [#124716](https://github.com/rust-lang/rust/pull/124716) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1a851da73cdeb02e2c62d301aa6bd98e515a50da&end=d568423a7a4ddb4b49323d96078a22f94df55fbd&stat=instructions:u)
133+
134+
| (instructions:u) | mean | range | count |
135+
|:----------------------------------:|:-----:|:--------------:|:-----:|
136+
| Regressions ❌ <br /> (primary) | - | - | 0 |
137+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.3%, 0.5%] | 6 |
138+
| Improvements ✅ <br /> (primary) | -0.8% | [-0.8%, -0.8%] | 1 |
139+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
140+
| All ❌✅ (primary) | -0.8% | [-0.8%, -0.8%] | 1 |
141+
142+
143+
* all regressions are secondary (specifically on unused-warnings benchmark)
144+
* regression identified by Kobzol as caused by [PR #124584](https://github.com/rust-lang/rust/pull/124584) "Various improvements to entrypoint code"
145+
* seems like noise to pnkfelix
146+
* marked as triaged
147+
148+
#### Untriaged Pull Requests
149+
150+
- [#124784 Rollup of 3 pull requests](https://github.com/rust-lang/rust/pull/124784)
151+
- [#124716 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/124716)
152+
- [#124700 Remove an unnecessary cast](https://github.com/rust-lang/rust/pull/124700)
153+
- [#124684 Update cargo](https://github.com/rust-lang/rust/pull/124684)
154+
- [#124675 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/124675)
155+
- [#124241 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/124241)
156+
- [#123909 Stabilize `Utf8Chunks`](https://github.com/rust-lang/rust/pull/123909)
157+
- [#123602 Account for immutably borrowed locals in MIR copy-prop and GVN](https://github.com/rust-lang/rust/pull/123602)
158+
- [#123147 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/123147)
159+
- [#122976 Remove len argument from RawVec::reserve_for_push](https://github.com/rust-lang/rust/pull/122976)
160+
- [#122900 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/122900)
161+
- [#122671 Codegen const panic messages as function calls](https://github.com/rust-lang/rust/pull/122671)
162+
- [#122396 Less generic code for Vec allocations](https://github.com/rust-lang/rust/pull/122396)
163+
- [#121955 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/121955)
164+
- [#121804 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/121804)
165+
- [#121752 Detect unused struct impls pub trait](https://github.com/rust-lang/rust/pull/121752)
166+
- [#121462 Combine `Sub` and `Equate`](https://github.com/rust-lang/rust/pull/121462)
167+
- [#121345 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/121345)
168+
- [#120985 Update host LLVM on x64 Linux to LLVM 18](https://github.com/rust-lang/rust/pull/120985)
169+
- [#120863 Use intrinsics::debug_assertions in debug_assert_nounwind](https://github.com/rust-lang/rust/pull/120863)
170+
- [#120862 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/120862)
171+
- [#120809 Use `transmute_unchecked` in `NonZero::new`.](https://github.com/rust-lang/rust/pull/120809)
172+
- [#120588 wasm: Store rlib metadata in wasm object files](https://github.com/rust-lang/rust/pull/120588)
173+
- [#120504 Vec::try_with_capacity](https://github.com/rust-lang/rust/pull/120504)
174+
- [#120401 Rollup of 12 pull requests](https://github.com/rust-lang/rust/pull/120401)
175+
- [#120335 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/120335)
176+
- [#119662 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/119662)
177+
- [#119204 macro_rules: Less hacky heuristic for using `tt` metavariable spans](https://github.com/rust-lang/rust/pull/119204)
178+
- [#119002 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/119002)
179+
- [#118661 Restore `const PartialEq`](https://github.com/rust-lang/rust/pull/118661)
180+

0 commit comments

Comments
 (0)