Skip to content

Commit 547fa2b

Browse files
Mark-SimulacrumMark-Simulacrum
committed
Add triage for this week
1 parent e3bad9c commit 547fa2b

File tree

1 file changed

+159
-0
lines changed

1 file changed

+159
-0
lines changed

‎triage/2022-09-20.md

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
# 2022-09-20 Triage Log
2+
3+
This was a fairly negative week for compiler performance, with regressions
4+
overall up to 14% on some workloads (primarily incr-unchanged scenarios),
5+
largely caused by [#101620](https://github.com/rust-lang/rust/pull/101620). We
6+
are still chasing down either a revert or a fix for that regression, though a
7+
partial mitigation in [#101862](https://github.com/rust-lang/rust/pull/101862)
8+
has been applied. Hopefully the full fix or revert will be part of the next
9+
triage report.
10+
11+
We also saw a number of other regressions land, though most were much smaller in magnitude.
12+
13+
Triage done by **@simulacrum**.
14+
Revision range: [17cbdfd07178349d0a3cecb8e7dde8f915666ced..8fd6d03e22fba2930ad377b87299de6a37076074](https://perf.rust-lang.org/?start=17cbdfd07178349d0a3cecb8e7dde8f915666ced&end=8fd6d03e22fba2930ad377b87299de6a37076074&absolute=false&stat=instructions%3Au)
15+
16+
**Summary**:
17+
18+
| (instructions:u) | mean | range | count |
19+
|:----------------:|:----:|:-----:|:-----:|
20+
| Regressions ❌ <br /> (primary) | 3.8% | [0.3%, 14.0%] | 148 |
21+
| Regressions ❌ <br /> (secondary) | 4.3% | [0.3%, 23.6%] | 141 |
22+
| Improvements ✅ <br /> (primary) | -6.4% | [-24.8%, -0.5%] | 24 |
23+
| Improvements ✅ <br /> (secondary) | -2.1% | [-4.0%, -0.4%] | 12 |
24+
| All ❌✅ (primary) | 2.4% | [-24.8%, 14.0%] | 172 |
25+
26+
27+
1 Regressions, 2 Improvements, 6 Mixed; 2 of them in rollups
28+
43 artifact comparisons made in total
29+
30+
#### Regressions
31+
32+
Cleanup slice sort related closures in core and alloc [#101816](https://github.com/rust-lang/rust/pull/101816) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=65c16dc3a83e34de0360c4667a0dac7f0e217e2b&end=4c2e500788cb3875f90eedb0791b76bcbb91d758&stat=instructions:u)
33+
34+
| (instructions:u) | mean | range | count |
35+
|:----------------:|:----:|:-----:|:-----:|
36+
| Regressions ❌ <br /> (primary) | 1.1% | [1.1%, 1.1%] | 1 |
37+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
38+
| Improvements ✅ <br /> (primary) | - | - | 0 |
39+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
40+
| All ❌✅ (primary) | 1.1% | [1.1%, 1.1%] | 1 |
41+
42+
This regression occurred in a single benchmark and only in the incremental
43+
scenario, so it isn't worth follow up investigation at this time.
44+
45+
#### Improvements
46+
47+
Partially revert #101433 [#101902](https://github.com/rust-lang/rust/pull/101902) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=54f20bbb8a7aeab93da17c0019c1aaa10329245a&end=4d4e51e428ba7b1ece3c67d1c114e2b486dc85dd&stat=instructions:u)
48+
49+
| (instructions:u) | mean | range | count |
50+
|:----------------:|:----:|:-----:|:-----:|
51+
| Regressions ❌ <br /> (primary) | - | - | 0 |
52+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
53+
| Improvements ✅ <br /> (primary) | -0.5% | [-0.7%, -0.4%] | 6 |
54+
| Improvements ✅ <br /> (secondary) | -0.6% | [-0.6%, -0.6%] | 1 |
55+
| All ❌✅ (primary) | -0.5% | [-0.7%, -0.4%] | 6 |
56+
57+
Do not fetch HIR node when iterating to find lint. [#101862](https://github.com/rust-lang/rust/pull/101862) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5253b0a0a1f366fad0ebed57597fcf2703b9e893&end=bc7b17cfe3bf08b618d1c7b64838053faeb1f590&stat=instructions:u)
58+
59+
| (instructions:u) | mean | range | count |
60+
|:----------------:|:----:|:-----:|:-----:|
61+
| Regressions ❌ <br /> (primary) | - | - | 0 |
62+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
63+
| Improvements ✅ <br /> (primary) | -0.6% | [-2.3%, -0.2%] | 24 |
64+
| Improvements ✅ <br /> (secondary) | -13.1% | [-39.1%, -3.2%] | 6 |
65+
| All ❌✅ (primary) | -0.6% | [-2.3%, -0.2%] | 24 |
66+
67+
68+
#### Mixed
69+
70+
Simplify caching and storage for queries [#101307](https://github.com/rust-lang/rust/pull/101307) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=88a192257ce110e7fb1732aa2b65e481f811db7a&end=a5b58addae4d629734ebbfc9c69f4e0653b99569&stat=instructions:u)
71+
72+
| (instructions:u) | mean | range | count |
73+
|:----------------:|:----:|:-----:|:-----:|
74+
| Regressions ❌ <br /> (primary) | - | - | 0 |
75+
| Regressions ❌ <br /> (secondary) | 1.7% | [1.7%, 1.7%] | 1 |
76+
| Improvements ✅ <br /> (primary) | -0.2% | [-0.3%, -0.2%] | 7 |
77+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 3 |
78+
| All ❌✅ (primary) | -0.2% | [-0.3%, -0.2%] | 7 |
79+
80+
This change is either neutral or a slight improvement; the one regression
81+
occurs in a benchmark and only in one scenario, which is also prone to noise,
82+
so further investigation isn't necessary.
83+
84+
85+
Rollup of 6 pull requests [#101805](https://github.com/rust-lang/rust/pull/101805) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c97922dca563cb7f9385b18dbf7ca8c97f8e1597&end=a92669638461836f41f54f95e396f9082bb91391&stat=instructions:u)
86+
87+
| (instructions:u) | mean | range | count |
88+
|:----------------:|:----:|:-----:|:-----:|
89+
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.5%] | 2 |
90+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
91+
| Improvements ✅ <br /> (primary) | - | - | 0 |
92+
| Improvements ✅ <br /> (secondary) | -4.5% | [-5.0%, -4.1%] | 6 |
93+
| All ❌✅ (primary) | 0.4% | [0.4%, 0.5%] | 2 |
94+
95+
Using the newish rollup-introspection tooling, @nnethercote narrowed this down
96+
to [#101433](https://github.com/rust-lang/rust/pull/101433). Put a
97+
[comment](https://github.com/rust-lang/rust/pull/101433#issuecomment-1252309512)
98+
on that PR and marked it as a perf-regression.
99+
100+
Compute lint levels by definition [#101620](https://github.com/rust-lang/rust/pull/101620) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=750bd1a7ff3e010611b97ee75d30b7cbf5f3a03c&end=2cb9a65684dba47c52de8fa938febf97a73e70a9&stat=instructions:u)
101+
102+
| (instructions:u) | mean | range | count |
103+
|:----------------:|:----:|:-----:|:-----:|
104+
| Regressions ❌ <br /> (primary) | 4.1% | [0.4%, 14.5%] | 133 |
105+
| Regressions ❌ <br /> (secondary) | 5.2% | [0.2%, 64.1%] | 131 |
106+
| Improvements ✅ <br /> (primary) | -5.1% | [-24.3%, -0.4%] | 30 |
107+
| Improvements ✅ <br /> (secondary) | -1.0% | [-1.2%, -0.5%] | 5 |
108+
| All ❌✅ (primary) | 2.4% | [-24.3%, 14.5%] | 163 |
109+
110+
Regression is partially addressed by
111+
[#101862](https://github.com/rust-lang/rust/pull/101862), but not completely.
112+
Week/week diff remains negative. Left a comment asking us to pursue the revert
113+
for the time being, since the suggested fix for this regression didn't recover
114+
performance fully.
115+
116+
Derive various impls instead of hand-rolling them [#101858](https://github.com/rust-lang/rust/pull/101858) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=35a0407814a6b5a04f0929105631e9c69e293e9d&end=df34db9b032b15efd86df3544cc75e6d55dc492e&stat=instructions:u)
117+
118+
| (instructions:u) | mean | range | count |
119+
|:----------------:|:----:|:-----:|:-----:|
120+
| Regressions ❌ <br /> (primary) | 1.2% | [0.2%, 2.2%] | 11 |
121+
| Regressions ❌ <br /> (secondary) | 0.9% | [0.3%, 2.1%] | 18 |
122+
| Improvements ✅ <br /> (primary) | - | - | 0 |
123+
| Improvements ✅ <br /> (secondary) | -0.7% | [-0.9%, -0.5%] | 6 |
124+
| All ❌✅ (primary) | 1.2% | [0.2%, 2.2%] | 11 |
125+
126+
This was a fairly large regression in a number of benchmarks;
127+
[#101893](https://github.com/rust-lang/rust/pull/101893) is pursuing further
128+
investigation here and tracking either improvements or a possible revert. It
129+
seems like moving to the derive'd impls may have also been a (correct) behavior
130+
change in [some cases](https://github.com/rust-lang/rust/pull/101893#issuecomment-1250741413)
131+
so a straightforward revert may be undesirable.
132+
133+
Change `FnMutDelegate` to trait objects [#101857](https://github.com/rust-lang/rust/pull/101857) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=672831a5c890f51d3222511ab2575ca7a86c8e20&end=48de123d7a0753026c362a06109f9a9cebde2a2a&stat=instructions:u)
134+
135+
| (instructions:u) | mean | range | count |
136+
|:----------------:|:----:|:-----:|:-----:|
137+
| Regressions ❌ <br /> (primary) | - | - | 0 |
138+
| Regressions ❌ <br /> (secondary) | 1.2% | [0.9%, 1.6%] | 9 |
139+
| Improvements ✅ <br /> (primary) | -1.2% | [-1.2%, -1.2%] | 1 |
140+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
141+
| All ❌✅ (primary) | -1.2% | [-1.2%, -1.2%] | 1 |
142+
143+
Regressions are limited to secondary benchmarks and stress tests at that. The
144+
goal here is an improvement in bootstrap times (measured at ~1.5% earlier,
145+
though the actual PR merge was during a window where that measurement was
146+
failing). That improvement is worth the slight loss on stress tests.
147+
148+
Rollup of 7 pull requests [#101949](https://github.com/rust-lang/rust/pull/101949) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=98ad6a5519651af36e246c0335c964dd52c554ba&end=5253b0a0a1f366fad0ebed57597fcf2703b9e893&stat=instructions:u)
149+
150+
| (instructions:u) | mean | range | count |
151+
|:----------------:|:----:|:-----:|:-----:|
152+
| Regressions ❌ <br /> (primary) | 0.2% | [0.2%, 0.2%] | 3 |
153+
| Regressions ❌ <br /> (secondary) | 0.8% | [0.2%, 3.5%] | 15 |
154+
| Improvements ✅ <br /> (primary) | - | - | 0 |
155+
| Improvements ✅ <br /> (secondary) | -1.4% | [-1.6%, -1.3%] | 6 |
156+
| All ❌✅ (primary) | 0.2% | [0.2%, 0.2%] | 3 |
157+
158+
Still working on identifying the causes. Likely to be a minor enough delta that
159+
spending too much more time won't make sense.

0 commit comments

Comments
 (0)