Skip to content

Commit 6a76b61

Browse files
Mark-SimulacrumMark-Simulacrum
authored
Merge pull request #1342 from rylev/triage-2022-06-07
Add triage for 2022-06-07
2 parents 08c0319 + 1d5630d commit 6a76b61

File tree

1 file changed

+185
-0
lines changed

1 file changed

+185
-0
lines changed

‎triage/2022-06-07.md

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
# 2022-06-07 Triage Log
2+
3+
A busy week in compiler performance, but fortunately improvements outweighed regressions. The biggest improvements came from @nnethercote's work on making the decoding of `SourceFile::lines` lazy which significantly cuts the costs of decoding crate metadata. The biggest regressions came from the removal of json handling in `rustc_serialize` which has been a multi-month effort to improve the maintainability of json (de-)serialization in the compiler.
4+
5+
Triage done by **@rylev**.
6+
Revision range: [0a43923a..bb55bd](https://perf.rust-lang.org/?start=0a43923a86c3b8f11d005884871b152f59b746f7&end=bb55bd449e65e611da928560d948982d73e50027&absolute=false&stat=instructions%3Au)
7+
8+
**Summary**:
9+
10+
| | mean | max | count |
11+
|:----------:|:----:|:---:|:-----:|
12+
| Regressions 😿 <br /> (primary) | 0.5% | 3.2% | 36 |
13+
| Regressions 😿 <br /> (secondary) | 0.3% | 0.9% | 15 |
14+
| Improvements 🎉 <br /> (primary) | -1.3% | -15.1% | 124 |
15+
| Improvements 🎉 <br /> (secondary) | -2.7% | -13.5% | 182 |
16+
| All 😿🎉 (primary) | -0.9% | -15.1% | 160 |
17+
18+
19+
2 Regression, 6 Improvements, 5 Mixed; 4 of them in rollups
20+
48 artifact comparisons made in total
21+
22+
#### Regressions
23+
24+
rewrite error handling for unresolved inference vars [#89862](https://github.com/rust-lang/rust/pull/89862) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=72f7e3144a386c820c188350092d2d93a74889b8&end=e40d5e83dc133d093c22c7ff016b10daa4f40dcf&stat=instructions:u)
25+
26+
| | mean | max | count |
27+
|:----------:|:----:|:---:|:-----:|
28+
| Regressions 😿 <br /> (primary) | 0.2% | 0.3% | 7 |
29+
| Regressions 😿 <br /> (secondary) | 0.4% | 1.0% | 23 |
30+
| Improvements 🎉 <br /> (primary) | N/A | N/A | 0 |
31+
| Improvements 🎉 <br /> (secondary) | N/A | N/A | 0 |
32+
| All 😿🎉 (primary) | 0.2% | 0.3% | 7 |
33+
- Ran cache grind diff and saw that `ObligationForest::process_obligations` is getting called a lot more.
34+
- I'm very unfamiliar with trait resolution, so I'm unsure if this is a red herring or not.
35+
- In any case, [here](https://gist.github.com/rylev/8e5fd44e2ee0db7f3dd449eb0e88ccbd) is the full diff.
36+
- Left a comment as such [here](https://github.com/rust-lang/rust/pull/89862#issuecomment-1148733903)
37+
38+
39+
Remove all json handling from rustc_serialize [#85993](https://github.com/rust-lang/rust/pull/85993) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9a74608543d499bcc7dd505e195e8bfab9447315&end=7e9b92cb43a489b34e2bcb8d21f36198e02eedbc&stat=instructions:u)
40+
41+
| | mean | max | count |
42+
|:----------:|:----:|:---:|:-----:|
43+
| Regressions 😿 <br /> (primary) | 0.5% | 1.3% | 68 |
44+
| Regressions 😿 <br /> (secondary) | 0.9% | 3.5% | 40 |
45+
| Improvements 🎉 <br /> (primary) | -0.4% | -0.6% | 3 |
46+
| Improvements 🎉 <br /> (secondary) | -0.5% | -1.0% | 24 |
47+
| All 😿🎉 (primary) | 0.5% | 1.3% | 71 |
48+
- @nnethercote was not able to reproduce the regressions in a local build, and it seems the consensus is that the regressions are worth the hit.
49+
50+
51+
#### Improvements
52+
53+
Make params be SmallVec as originally was [#97670](https://github.com/rust-lang/rust/pull/97670) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a6b8c6954829669a5c4fa320c3e6132edf04fcfc&end=f15370b4e44988ad5d228b25e939650c0a6c2ec7&stat=instructions:u)
54+
55+
| | mean | max | count |
56+
|:----------:|:----:|:---:|:-----:|
57+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
58+
| Regressions 😿 <br /> (secondary) | N/A | N/A | 0 |
59+
| Improvements 🎉 <br /> (primary) | -0.2% | -0.2% | 3 |
60+
| Improvements 🎉 <br /> (secondary) | N/A | N/A | 0 |
61+
| All 😿🎉 (primary) | -0.2% | -0.2% | 3 |
62+
63+
64+
Add PID to LLVM PGO profile path [#97137](https://github.com/rust-lang/rust/pull/97137) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c7b0452ece11bf714f7cf2003747231931504d59&end=63641795406e1831a822f011242fdfb225fc8fbc&stat=instructions:u)
65+
66+
| | mean | max | count |
67+
|:----------:|:----:|:---:|:-----:|
68+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
69+
| Regressions 😿 <br /> (secondary) | N/A | N/A | 0 |
70+
| Improvements 🎉 <br /> (primary) | -0.6% | -0.8% | 20 |
71+
| Improvements 🎉 <br /> (secondary) | -0.7% | -1.0% | 8 |
72+
| All 😿🎉 (primary) | -0.6% | -0.8% | 20 |
73+
74+
75+
Rollup of 6 pull requests [#97742](https://github.com/rust-lang/rust/pull/97742) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4e725bad73747a4c93a3ac53106e4b4006edc665&end=43874a2ee749c2dd9f052172341f2f87fa36cd79&stat=instructions:u)
76+
77+
| | mean | max | count |
78+
|:----------:|:----:|:---:|:-----:|
79+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
80+
| Regressions 😿 <br /> (secondary) | N/A | N/A | 0 |
81+
| Improvements 🎉 <br /> (primary) | N/A | N/A | 0 |
82+
| Improvements 🎉 <br /> (secondary) | -1.9% | -2.6% | 12 |
83+
| All 😿🎉 (primary) | N/A | N/A | 0 |
84+
85+
86+
interpret: better control over whether we read data with provenance [#97684](https://github.com/rust-lang/rust/pull/97684) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=79b6bad406155cad4481150fc5dfa0da5394e3b6&end=9d20fd109809f20c049d6895a5be27a1fbd39daa&stat=instructions:u)
87+
88+
| | mean | max | count |
89+
|:----------:|:----:|:---:|:-----:|
90+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
91+
| Regressions 😿 <br /> (secondary) | N/A | N/A | 0 |
92+
| Improvements 🎉 <br /> (primary) | -0.7% | -1.9% | 8 |
93+
| Improvements 🎉 <br /> (secondary) | -5.5% | -10.5% | 12 |
94+
| All 😿🎉 (primary) | -0.7% | -1.9% | 8 |
95+
96+
97+
Remove migrate borrowck mode [#95565](https://github.com/rust-lang/rust/pull/95565) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9f7e997c8bc3cacd2ab4eb75e63cb5fa9279c7b0&end=bb55bd449e65e611da928560d948982d73e50027&stat=instructions:u)
98+
99+
| | mean | max | count |
100+
|:----------:|:----:|:---:|:-----:|
101+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
102+
| Regressions 😿 <br /> (secondary) | N/A | N/A | 0 |
103+
| Improvements 🎉 <br /> (primary) | -0.5% | -1.9% | 47 |
104+
| Improvements 🎉 <br /> (secondary) | -0.5% | -1.4% | 21 |
105+
| All 😿🎉 (primary) | -0.5% | -1.9% | 47 |
106+
107+
108+
Lazify `SourceFile::lines`. [#97575](https://github.com/rust-lang/rust/pull/97575) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=44e9516c8535c6f41f2d1c444657cd672b4ab81b&end=e71440575c930dcecac288b7c3536410d688b351&stat=instructions:u)
109+
110+
| | mean | max | count |
111+
|:----------:|:----:|:---:|:-----:|
112+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
113+
| Regressions 😿 <br /> (secondary) | 0.4% | 0.5% | 6 |
114+
| Improvements 🎉 <br /> (primary) | -1.8% | -15.3% | 52 |
115+
| Improvements 🎉 <br /> (secondary) | -2.9% | -13.8% | 124 |
116+
| All 😿🎉 (primary) | -1.8% | -15.3% | 52 |
117+
118+
119+
#### Mixed
120+
121+
Add `#[inline]` to `Vec`'s `Deref/DerefMut` [#97553](https://github.com/rust-lang/rust/pull/97553) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=02916c4c75912f70b651c0b20b501444ce2ca231&end=395a09c3dafe0c7838c9ca41d2b47bb5e79a5b6d&stat=instructions:u)
122+
123+
| | mean | max | count |
124+
|:----------:|:----:|:---:|:-----:|
125+
| Regressions 😿 <br /> (primary) | 0.5% | 0.8% | 6 |
126+
| Regressions 😿 <br /> (secondary) | 0.4% | 0.7% | 31 |
127+
| Improvements 🎉 <br /> (primary) | -1.2% | -1.7% | 10 |
128+
| Improvements 🎉 <br /> (secondary) | -1.1% | -1.9% | 10 |
129+
| All 😿🎉 (primary) | -0.5% | -1.7% | 16 |
130+
- As with any chance to inlining, performance is expected to change and to not always have a positive impact.
131+
- The improvements outweigh the regressions (especially in primary benchmarks), and so it doesn't seem worth it to dig too much more into this.
132+
133+
134+
Rollup of 6 pull requests [#97644](https://github.com/rust-lang/rust/pull/97644) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5e6bb83268518dcd74c96b5504f485b71e604e4c&end=9598b4b594c97dff66feb93522e22db500deea07&stat=instructions:u)
135+
136+
| | mean | max | count |
137+
|:----------:|:----:|:---:|:-----:|
138+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
139+
| Regressions 😿 <br /> (secondary) | 0.6% | 0.7% | 5 |
140+
| Improvements 🎉 <br /> (primary) | -0.2% | -0.3% | 2 |
141+
| Improvements 🎉 <br /> (secondary) | -0.3% | -0.5% | 11 |
142+
| All 😿🎉 (primary) | -0.2% | -0.3% | 2 |
143+
- The improvements outweigh the regressions (which are pretty small and contained in secondary benchmarks).
144+
- Given it's in a rollup, it's not worth the effort to investigate.
145+
146+
147+
Rollup of 5 pull requests [#97654](https://github.com/rust-lang/rust/pull/97654) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=20976bae5c426c738262db376eadbd8859aafc08&end=44e9516c8535c6f41f2d1c444657cd672b4ab81b&stat=instructions:u)
148+
149+
| | mean | max | count |
150+
|:----------:|:----:|:---:|:-----:|
151+
| Regressions 😿 <br /> (primary) | N/A | N/A | 0 |
152+
| Regressions 😿 <br /> (secondary) | 0.6% | 0.6% | 1 |
153+
| Improvements 🎉 <br /> (primary) | N/A | N/A | 0 |
154+
| Improvements 🎉 <br /> (secondary) | -0.6% | -0.8% | 8 |
155+
| All 😿🎉 (primary) | N/A | N/A | 0 |
156+
- The improvements outweigh the regressions (which are pretty small and contained in secondary benchmarks).
157+
- Given it's in a rollup, it's not worth the effort to investigate.
158+
159+
160+
Rollup of 3 pull requests [#97694](https://github.com/rust-lang/rust/pull/97694) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e40d5e83dc133d093c22c7ff016b10daa4f40dcf&end=9a74608543d499bcc7dd505e195e8bfab9447315&stat=instructions:u)
161+
162+
| | mean | max | count |
163+
|:----------:|:----:|:---:|:-----:|
164+
| Regressions 😿 <br /> (primary) | 0.2% | 0.2% | 1 |
165+
| Regressions 😿 <br /> (secondary) | 0.3% | 0.5% | 5 |
166+
| Improvements 🎉 <br /> (primary) | -0.4% | -0.6% | 12 |
167+
| Improvements 🎉 <br /> (secondary) | -0.3% | -0.5% | 13 |
168+
| All 😿🎉 (primary) | -0.4% | -0.6% | 13 |
169+
- The improvements outweigh the regressions (which are pretty small and almost completely contained in secondary benchmarks).
170+
- Given it's in a rollup, it's not worth the effort to investigate.
171+
172+
173+
Inline `bridge::Buffer` methods. [#97604](https://github.com/rust-lang/rust/pull/97604) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c3384ea35cafc3a8a6554a2ad524dbf70df4bbcd&end=cb0584f86b8cfa952dffad55f7d83bd90765120f&stat=instructions:u)
174+
175+
| | mean | max | count |
176+
|:----------:|:----:|:---:|:-----:|
177+
| Regressions 😿 <br /> (primary) | 3.8% | 3.8% | 1 |
178+
| Regressions 😿 <br /> (secondary) | N/A | N/A | 0 |
179+
| Improvements 🎉 <br /> (primary) | -0.3% | -0.3% | 1 |
180+
| Improvements 🎉 <br /> (secondary) | -2.1% | -2.1% | 3 |
181+
| All 😿🎉 (primary) | 1.8% | 3.8% | 2 |
182+
- I'm a bit confused as this PR was opened to address a performance regression, but it seems to be itself a partial perf regression (at least in instruction counts).
183+
- This causes a near 4% perf regression in [serde_derive-1.0.136](https://github.com/rust-lang/rustc-perf/tree/master/collector/benchmarks/serde_derive-1.0.136). Granted that particular benchmark has been somewhat noisy but not nearly to the level seen here.
184+
- Add a comment seeking [justification](https://github.com/rust-lang/rust/pull/97604#issuecomment-1148722741)
185+

0 commit comments

Comments
 (0)