Skip to content

Commit 376f07a

Browse files
rylevrylev
authored
Merge pull request #1505 from pnkfelix/triage-2022-12-06
Triage 2022 12 06
2 parents e6cd6e8 + cc0dd3c commit 376f07a

File tree

1 file changed

+247
-0
lines changed

1 file changed

+247
-0
lines changed

‎triage/2022-12-06.md

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
# 2022-12-06 Triage Log
2+
3+
A mixed bag of a week. 2 of the 3 regressions are connected to changes to the
4+
query system that underlies incremental compilation. The third regression is
5+
still being investigated. For all three, the impact on cycle counts that are
6+
within our noise tolerance levels There were several small-ish improvements,
7+
though PR #104963 is worth calling out: a well-factored change to
8+
how we lower the AST into HIR that had a broad positive impact. One final
9+
note: the summary-opt max-rss seems to gone up by 1.5% over the past month
10+
([perf](https://perf.rust-lang.org/?start=2022-11-06&end=2022-12-06&kind=percentfromfirst&stat=max-rss)),
11+
via a gradual climb; just something to keep our eyes on going forward.
12+
13+
14+
Triage done by **@pnkfelix**.
15+
Revision range: [8a09420a..9db224fc](https://perf.rust-lang.org/?start=8a09420ac48658cad726e0a6997687ceac4151e3&end=9db224fc908059986c179fc6ec433944e9cfce50&absolute=false&stat=instructions%3Au)
16+
17+
**Summary**:
18+
19+
| (instructions:u) | mean | range | count |
20+
|:----------------------------------:|:-----:|:--------------:|:-----:|
21+
| Regressions ❌ <br /> (primary) | 0.6% | [0.2%, 1.5%] | 52 |
22+
| Regressions ❌ <br /> (secondary) | 1.2% | [0.2%, 5.6%] | 80 |
23+
| Improvements ✅ <br /> (primary) | -0.8% | [-1.7%, -0.2%] | 34 |
24+
| Improvements ✅ <br /> (secondary) | -2.2% | [-4.8%, -0.2%] | 29 |
25+
| All ❌✅ (primary) | 0.0% | [-1.7%, 1.5%] | 86 |
26+
27+
28+
3 Regressions, 7 Improvements, 4 Mixed; 3 of them in rollups
29+
50 artifact comparisons made in total
30+
31+
#### Regressions
32+
33+
Allow to feed a value in another query's cache [#104940](https://github.com/rust-lang/rust/pull/104940) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=90711a86e5bdd4b0b65d293f0c2c48fd2db761b3&end=c97b539e408ea353f4fde2f9251d598291fec421&stat=instructions:u)
34+
35+
| (instructions:u) | mean | range | count |
36+
|:----------------------------------:|:----:|:------------:|:-----:|
37+
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.4%] | 44 |
38+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.2%, 0.4%] | 12 |
39+
| Improvements ✅ <br /> (primary) | - | - | 0 |
40+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
41+
| All ❌✅ (primary) | 0.3% | [0.2%, 0.4%] | 44 |
42+
43+
* Many primary benchmarks regressed, but the regression is contained solely to incremental builds, and the magnitude is also well-contained.
44+
* [oli-obk investigated](https://github.com/rust-lang/rust/pull/104940#issuecomment-1333435955) and determined that
45+
the bulk of the regression is time spent in try_mark_previous_green and incremental_verify_ich calls.
46+
* The impact on cycle-counts (rather than instruction counts) was well within noise tolerance levels.
47+
* Therefore, marked as triaged.
48+
49+
Reduce macro usage for lints [#104863](https://github.com/rust-lang/rust/pull/104863) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cef44f53034eac46be3a0e3eec7b2b3d4ef5140b&end=e960b5e7749e95c6a6b2fdec7250a48105664efb&stat=instructions:u)
50+
51+
| (instructions:u) | mean | range | count |
52+
|:----------------------------------:|:----:|:------------:|:-----:|
53+
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 1.4%] | 116 |
54+
| Regressions ❌ <br /> (secondary) | 0.9% | [0.1%, 2.3%] | 83 |
55+
| Improvements ✅ <br /> (primary) | - | - | 0 |
56+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
57+
| All ❌✅ (primary) | 0.5% | [0.2%, 1.4%] | 116 |
58+
59+
* [nnethercote is investigating](https://github.com/rust-lang/rust/pull/104863#issuecomment-1335853746)
60+
* they have already put up [PR 105291](https://github.com/rust-lang/rust/pull/105291) which fixed a few cases but the bulk remain
61+
* they have also [posted a separate idea](https://github.com/rust-lang/rust/pull/104863#issuecomment-1338224753), to avoid calling empty `check_*` methods.
62+
* not marking as triaged yet
63+
64+
feed resolver_for_lowering instead of storing it in a field [#105220](https://github.com/rust-lang/rust/pull/105220) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=226202d9026b0f24b4f7aad4de398bd8378774cd&end=ed61c139c2bc778ebb91f5dd6a5393aa20467f78&stat=instructions:u)
65+
66+
| (instructions:u) | mean | range | count |
67+
|:----------------------------------:|:-----:|:--------------:|:-----:|
68+
| Regressions ❌ <br /> (primary) | 0.2% | [0.2%, 0.3%] | 2 |
69+
| Regressions ❌ <br /> (secondary) | 0.5% | [0.1%, 0.8%] | 15 |
70+
| Improvements ✅ <br /> (primary) | - | - | 0 |
71+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 |
72+
| All ❌✅ (primary) | 0.2% | [0.2%, 0.3%] | 2 |
73+
74+
75+
* the original timer run said [this had no problem](https://github.com/rust-lang/rust/pull/105220#issuecomment-1336312355)
76+
* (though to be fair, I don't know what changes occurred to the commit series between that timer run and what landed)
77+
* oli-obj investigated and found [cachegrind shows this to be artifacts from inlining decisions](https://github.com/rust-lang/rust/pull/105220#issuecomment-1338942653)
78+
* The impact on cycle-counts (rather than instruction counts) was well within noise tolerance levels.
79+
* Therefore, marked as triaged.
80+
81+
#### Improvements
82+
83+
Also cache the stable hash of interned Predicates [#94487](https://github.com/rust-lang/rust/pull/94487) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e0098a5cc3a87d857e597af824d0ce1ed1ad85e0&end=bddad597feb997a4e5d2cd174a76c3b07a84e4d6&stat=instructions:u)
84+
85+
| (instructions:u) | mean | range | count |
86+
|:----------------------------------:|:-----:|:--------------:|:-----:|
87+
| Regressions ❌ <br /> (primary) | - | - | 0 |
88+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
89+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.6%, -0.2%] | 12 |
90+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
91+
| All ❌✅ (primary) | -0.4% | [-0.6%, -0.2%] | 12 |
92+
93+
94+
Some initial normalization method changes [#104905](https://github.com/rust-lang/rust/pull/104905) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a569a88f5500e8780c7cc65fa53fc8b098517674&end=24606deaf48e56f604a6125fa11fb9f70cf4f1f4&stat=instructions:u)
95+
96+
| (instructions:u) | mean | range | count |
97+
|:----------------------------------:|:-----:|:--------------:|:-----:|
98+
| Regressions ❌ <br /> (primary) | - | - | 0 |
99+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
100+
| Improvements ✅ <br /> (primary) | -0.9% | [-1.0%, -0.8%] | 2 |
101+
| Improvements ✅ <br /> (secondary) | -2.2% | [-2.8%, -0.3%] | 7 |
102+
| All ❌✅ (primary) | -0.9% | [-1.0%, -0.8%] | 2 |
103+
104+
105+
Attribute cleanups [#104861](https://github.com/rust-lang/rust/pull/104861) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=744a97b98c3f2722e1a43486c05203b04e499e21&end=d6c4de0fb22fe8f3cc5a27e94b6b6d88fb504a91&stat=instructions:u)
106+
107+
| (instructions:u) | mean | range | count |
108+
|:----------------------------------:|:-----:|:--------------:|:-----:|
109+
| Regressions ❌ <br /> (primary) | - | - | 0 |
110+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
111+
| Improvements ✅ <br /> (primary) | -0.2% | [-0.2%, -0.1%] | 3 |
112+
| Improvements ✅ <br /> (secondary) | -0.7% | [-0.9%, -0.5%] | 6 |
113+
| All ❌✅ (primary) | -0.2% | [-0.2%, -0.1%] | 3 |
114+
115+
116+
rustc_ast_lowering: Stop lowering imports into multiple items [#104963](https://github.com/rust-lang/rust/pull/104963) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=56c241c86212d84b9528dca1628bc06e32c742c1&end=11663b1b4857ffeafbd85a9a36c234d117373b76&stat=instructions:u)
117+
118+
| (instructions:u) | mean | range | count |
119+
|:----------------------------------:|:-----:|:--------------:|:-----:|
120+
| Regressions ❌ <br /> (primary) | - | - | 0 |
121+
| Regressions ❌ <br /> (secondary) | 0.6% | [0.6%, 0.6%] | 1 |
122+
| Improvements ✅ <br /> (primary) | -0.5% | [-1.2%, -0.3%] | 54 |
123+
| Improvements ✅ <br /> (secondary) | -4.2% | [-5.4%, -2.0%] | 13 |
124+
| All ❌✅ (primary) | -0.5% | [-1.2%, -0.3%] | 54 |
125+
126+
* Wow, I would not have expected this to have this significant of an impact.
127+
* bravo petrochenkov
128+
129+
Ensure query backtraces work for `DefId`s created after ast lowering [#105133](https://github.com/rust-lang/rust/pull/105133) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=24f2704e96c9d3a4f887d2d0a7d79ecffd79bd65&end=703d95e183fbb678249d8f61cabc732e46884e00&stat=instructions:u)
130+
131+
| (instructions:u) | mean | range | count |
132+
|:----------------------------------:|:-----:|:--------------:|:-----:|
133+
| Regressions ❌ <br /> (primary) | - | - | 0 |
134+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
135+
| Improvements ✅ <br /> (primary) | -0.2% | [-0.3%, -0.2%] | 5 |
136+
| Improvements ✅ <br /> (secondary) | -0.2% | [-0.3%, -0.2%] | 2 |
137+
| All ❌✅ (primary) | -0.2% | [-0.3%, -0.2%] | 5 |
138+
139+
140+
Rewrite LLVM's archive writer in Rust [#97485](https://github.com/rust-lang/rust/pull/97485) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4bb15759d7eb519be70c9a955dba9be09e13c06d&end=cab4fd678c5b148a330f2bf255bf28a67dfea0fc&stat=instructions:u)
141+
142+
| (instructions:u) | mean | range | count |
143+
|:----------------------------------:|:-----:|:--------------:|:-----:|
144+
| Regressions ❌ <br /> (primary) | - | - | 0 |
145+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
146+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.6%, -0.2%] | 12 |
147+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
148+
| All ❌✅ (primary) | -0.4% | [-0.6%, -0.2%] | 12 |
149+
150+
151+
Cheaper `dump_mir` take two [#105121](https://github.com/rust-lang/rust/pull/105121) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1195b672fb58364e4ca7022fbbe62a9a7a451b60&end=fd02567705326884ddb94535d2ab230a9595dc2a&stat=instructions:u)
152+
153+
| (instructions:u) | mean | range | count |
154+
|:----------------------------------:|:-----:|:--------------:|:-----:|
155+
| Regressions ❌ <br /> (primary) | - | - | 0 |
156+
| Regressions ❌ <br /> (secondary) | 0.9% | [0.9%, 0.9%] | 1 |
157+
| Improvements ✅ <br /> (primary) | -1.1% | [-1.9%, -0.6%] | 17 |
158+
| Improvements ✅ <br /> (secondary) | -1.4% | [-1.9%, -0.6%] | 9 |
159+
| All ❌✅ (primary) | -1.1% | [-1.9%, -0.6%] | 17 |
160+
161+
162+
#### Mixed
163+
164+
Rollup of 9 pull requests [#105017](https://github.com/rust-lang/rust/pull/105017) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8a09420ac48658cad726e0a6997687ceac4151e3&end=2585bcea0bc2a9c42a4be2c1eba5c61137f2b167&stat=instructions:u)
165+
166+
| (instructions:u) | mean | range | count |
167+
|:----------------------------------:|:-----:|:--------------:|:-----:|
168+
| Regressions ❌ <br /> (primary) | 0.9% | [0.9%, 1.0%] | 2 |
169+
| Regressions ❌ <br /> (secondary) | 3.7% | [2.2%, 5.6%] | 12 |
170+
| Improvements ✅ <br /> (primary) | -0.6% | [-0.8%, -0.4%] | 7 |
171+
| Improvements ✅ <br /> (secondary) | -1.1% | [-1.3%, -0.9%] | 4 |
172+
| All ❌✅ (primary) | -0.3% | [-0.8%, 1.0%] | 9 |
173+
174+
* nnethercote already [marked as triaged](https://github.com/rust-lang/rust/pull/105017#issuecomment-1333117104)
175+
176+
Rollup of 14 pull requests [#105070](https://github.com/rust-lang/rust/pull/105070) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=bddad597feb997a4e5d2cd174a76c3b07a84e4d6&end=d38a99078cd3c4dadac1ba8b729ea77e8d1d5a82&stat=instructions:u)
177+
178+
| (instructions:u) | mean | range | count |
179+
|:----------------------------------:|:-----:|:--------------:|:-----:|
180+
| Regressions ❌ <br /> (primary) | 0.6% | [0.4%, 0.9%] | 7 |
181+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.2%, 0.4%] | 7 |
182+
| Improvements ✅ <br /> (primary) | -0.9% | [-1.0%, -0.8%] | 2 |
183+
| Improvements ✅ <br /> (secondary) | -2.1% | [-2.7%, -1.4%] | 8 |
184+
| All ❌✅ (primary) | 0.3% | [-1.0%, 0.9%] | 9 |
185+
186+
* lqd [already investigated](https://github.com/rust-lang/rust/pull/105070#issuecomment-1332141661)
187+
* all the primary regressions were deemed noise.
188+
* marking as triaged.
189+
190+
Rollup of 5 pull requests [#105080](https://github.com/rust-lang/rust/pull/105080) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8de4b138455add55bde6de5553a933a2ab79b71f&end=a569a88f5500e8780c7cc65fa53fc8b098517674&stat=instructions:u)
191+
192+
| (instructions:u) | mean | range | count |
193+
|:----------------------------------:|:-----:|:--------------:|:-----:|
194+
| Regressions ❌ <br /> (primary) | 0.9% | [0.8%, 1.0%] | 2 |
195+
| Regressions ❌ <br /> (secondary) | 2.4% | [2.1%, 2.8%] | 6 |
196+
| Improvements ✅ <br /> (primary) | -0.7% | [-0.9%, -0.5%] | 7 |
197+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
198+
| All ❌✅ (primary) | -0.3% | [-0.9%, 1.0%] | 9 |
199+
200+
201+
° lqd already [marked as triaged](https://github.com/rust-lang/rust/pull/105080#issuecomment-1332124349)
202+
203+
Disable top down MIR inlining [#105119](https://github.com/rust-lang/rust/pull/105119) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8e440b03761f725d452120a992723c7e261822b2&end=226202d9026b0f24b4f7aad4de398bd8378774cd&stat=instructions:u)
204+
205+
| (instructions:u) | mean | range | count |
206+
|:----------------------------------:|:-----:|:--------------:|:-----:|
207+
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 1.0%] | 12 |
208+
| Regressions ❌ <br /> (secondary) | 1.0% | [0.2%, 2.1%] | 15 |
209+
| Improvements ✅ <br /> (primary) | -0.4% | [-1.0%, -0.2%] | 8 |
210+
| Improvements ✅ <br /> (secondary) | -0.7% | [-1.0%, -0.1%] | 18 |
211+
| All ❌✅ (primary) | 0.1% | [-1.0%, 1.0%] | 20 |
212+
213+
214+
* nnethercote already [marked as triaged](https://github.com/rust-lang/rust/pull/105119#issuecomment-1338814158)
215+
216+
#### Untriaged Pull Requests
217+
218+
- [#105220 feed resolver_for_lowering instead of storing it in a field](https://github.com/rust-lang/rust/pull/105220)
219+
- [#105070 Rollup of 14 pull requests](https://github.com/rust-lang/rust/pull/105070)
220+
- [#104940 Allow to feed a value in another query's cache](https://github.com/rust-lang/rust/pull/104940)
221+
- [#104863 Reduce macro usage for lints](https://github.com/rust-lang/rust/pull/104863)
222+
- [#104566 couple of clippy::perf fixes](https://github.com/rust-lang/rust/pull/104566)
223+
- [#104533 Clean up and harden various methods around trait substs](https://github.com/rust-lang/rust/pull/104533)
224+
- [#104017 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/104017)
225+
- [#103998 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/103998)
226+
- [#103975 Some tracing and comment cleanups](https://github.com/rust-lang/rust/pull/103975)
227+
- [#103934 std: sync "Dependencies of the `backtrace` crate" with `backtrace`](https://github.com/rust-lang/rust/pull/103934)
228+
- [#103841 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/103841)
229+
- [#103650 rustdoc: change `.src-line-numbers > span` to `.src-line-numbers > a`](https://github.com/rust-lang/rust/pull/103650)
230+
- [#103562 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/103562)
231+
- [#103439 Show note where the macro failed to match](https://github.com/rust-lang/rust/pull/103439)
232+
- [#103295 ci: Bring back ninja for dist builders](https://github.com/rust-lang/rust/pull/103295)
233+
- [#103071 Fix line numbers for MIR inlined code](https://github.com/rust-lang/rust/pull/103071)
234+
- [#102975 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/102975)
235+
- [#102915 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/102915)
236+
- [#102895 Get rid of `rustc_query_description!`](https://github.com/rust-lang/rust/pull/102895)
237+
- [#102867 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/102867)
238+
- [#102809 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/102809)
239+
- [#102570 Perform simple scalar replacement of aggregates (SROA) MIR opt](https://github.com/rust-lang/rust/pull/102570)
240+
- [#102548 Mark Cell::replace() as #[inline]](https://github.com/rust-lang/rust/pull/102548)
241+
- [#102026 Populate effective visibilities in 'rustc_resolve'](https://github.com/rust-lang/rust/pull/102026)
242+
- [#101858 derive various impls instead of hand-rolling them](https://github.com/rust-lang/rust/pull/101858)
243+
- [#101857 change `FnMutDelegate` to trait objects](https://github.com/rust-lang/rust/pull/101857)
244+
- [#101702 rustdoc: add hash to filename of toolchain files](https://github.com/rust-lang/rust/pull/101702)
245+
- [#101632 Remove `TypeckResults` from `InferCtxt`](https://github.com/rust-lang/rust/pull/101632)
246+
- [#101620 Compute lint levels by definition](https://github.com/rust-lang/rust/pull/101620)
247+
- [#101433 Emit a note that static bounds from HRTBs are a bug](https://github.com/rust-lang/rust/pull/101433)

0 commit comments

Comments
 (0)