Skip to content

Commit 6cfbab3

Browse files
KobzolKobzol
authored
Merge pull request #1735 from rust-lang/triage-2023-10-18
Triage 2023 10 18
2 parents 04ef4d1 + 420012f commit 6cfbab3

File tree

1 file changed

+198
-0
lines changed

1 file changed

+198
-0
lines changed

‎triage/2023-10-18.md

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
# 2023-10-18 Triage Log
2+
3+
Overall an interesting week performance wise, with small improvements to a vast
4+
number of benchmarks seeming to outweigh an isolated set of (slightly) larger
5+
regressions. It included a number of PRs regressed instruction counts but did
6+
not matter for cycle times, plus one mysterious regression to `check_match` and
7+
`mir_borrowck` from reworking constructor splitting (see report on PR 116391 for
8+
details), and an awesome broad set of improvements from automatically inlining
9+
small functions across crates (see report on PR 116505 for details).
10+
11+
Triage done by **@pnkfelix**.
12+
Revision range: [84d44dd1..b9832e72](https://perf.rust-lang.org/?start=84d44dd1d8ec1e98fff94272ba4f96b2a1f044ca&end=b9832e72c9223f4e96049aa5911effd258b92591&absolute=false&stat=instructions%3Au)
13+
14+
**Summary**:
15+
16+
| (instructions:u) | mean | range | count |
17+
|:----------------------------------:|:-----:|:---------------:|:-----:|
18+
| Regressions ❌ <br /> (primary) | 3.0% | [0.3%, 12.2%] | 7 |
19+
| Regressions ❌ <br /> (secondary) | 0.7% | [0.3%, 1.2%] | 15 |
20+
| Improvements ✅ <br /> (primary) | -1.1% | [-17.9%, -0.2%] | 131 |
21+
| Improvements ✅ <br /> (secondary) | -2.4% | [-39.6%, -0.2%] | 121 |
22+
| All ❌✅ (primary) | -0.9% | [-17.9%, 12.2%] | 138 |
23+
24+
25+
4 Regressions, 1 Improvements, 4 Mixed; 3 of them in rollups
26+
84 artifact comparisons made in total
27+
28+
#### Regressions
29+
30+
Rollup of 7 pull requests [#116605](https://github.com/rust-lang/rust/pull/116605) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5b88d659f8c2428536589d4bd36b9099d53a6815&end=c30b28bdc17f1da73515afa0886f0d4f55c76e1f&stat=instructions:u)
31+
32+
| (instructions:u) | mean | range | count |
33+
|:----------------------------------:|:----:|:------------:|:-----:|
34+
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.6%] | 7 |
35+
| Regressions ❌ <br /> (secondary) | 0.3% | [0.3%, 0.4%] | 3 |
36+
| Improvements ✅ <br /> (primary) | - | - | 0 |
37+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
38+
| All ❌✅ (primary) | 0.4% | [0.2%, 0.6%] | 7 |
39+
40+
* solely rustdoc regression
41+
* believed to be caused by [PR 109422](https://github.com/rust-lang/rust/pull/109422) "rustdoc-search: add impl disambiguator to duplicate assoc items"
42+
* already marked as triaged
43+
44+
Optimize `librustc_driver.so` with BOLT [#116352](https://github.com/rust-lang/rust/pull/116352) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ee8c9d3c34719a129f280cd91ba5d324017bb02b&end=c543b6f3516767150af84d94c14a27b19d4b0291&stat=instructions:u)
45+
46+
| (instructions:u) | mean | range | count |
47+
|:----------------------------------:|:-----:|:--------------:|:-----:|
48+
| Regressions ❌ <br /> (primary) | 2.3% | [0.2%, 5.7%] | 10 |
49+
| Regressions ❌ <br /> (secondary) | 1.9% | [0.3%, 5.0%] | 60 |
50+
| Improvements ✅ <br /> (primary) | - | - | 0 |
51+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 4 |
52+
| All ❌✅ (primary) | 2.3% | [0.2%, 5.7%] | 10 |
53+
54+
* primary instruction-count regressions were restricted to helloworld and html5ever
55+
* As noted in comment by Kobzol, the instruction counts regressed for many benchmarks, but the [cycle counts](https://perf.rust-lang.org/compare.html?start=ee8c9d3c34719a129f280cd91ba5d324017bb02b&end=c543b6f3516767150af84d94c14a27b19d4b0291&stat=cycles:u) solely improved, significantly so, and bootstrap time improved (628.052s -> 623.517s (-0.72%)).
56+
* already marked as triaged
57+
58+
Rollup of 3 pull requests [#116742](https://github.com/rust-lang/rust/pull/116742) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c543b6f3516767150af84d94c14a27b19d4b0291&end=e292fec36880f48101bda4054be37097312e73c0&stat=instructions:u)
59+
60+
| (instructions:u) | mean | range | count |
61+
|:----------------------------------:|:----:|:------------:|:-----:|
62+
| Regressions ❌ <br /> (primary) | 0.3% | [0.3%, 0.4%] | 3 |
63+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
64+
| Improvements ✅ <br /> (primary) | - | - | 0 |
65+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
66+
| All ❌✅ (primary) | 0.3% | [0.3%, 0.4%] | 3 |
67+
68+
* Regressions are solely to bitmaps full scenarios.
69+
* Looks like a blip (i.e. noise) based on the graph over time.
70+
* marking as triaged.
71+
72+
don't UB on dangling ptr deref, instead check inbounds on projections [#114330](https://github.com/rust-lang/rust/pull/114330) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a00c09e9d80b763fb29206b47b04e1d99c3ace96&end=e7bdc5f9f869219e8d20060b42a09ea10a837851&stat=instructions:u)
73+
74+
| (instructions:u) | mean | range | count |
75+
|:----------------------------------:|:----:|:------------:|:-----:|
76+
| Regressions ❌ <br /> (primary) | - | - | 0 |
77+
| Regressions ❌ <br /> (secondary) | 0.7% | [0.5%, 1.0%] | 17 |
78+
| Improvements ✅ <br /> (primary) | - | - | 0 |
79+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
80+
| All ❌✅ (primary) | - | - | 0 |
81+
82+
* From skimming the PR, one can see that the PR author (RalfJung) iterated on this to identify a solution that would minimize regressions.
83+
* As noted by the PR author, only secondary benchmarks were affected.
84+
* Also, while instruction-counts regressed, the [cycle-counts](https://perf.rust-lang.org/compare.html?start=a00c09e9d80b763fb29206b47b04e1d99c3ace96&end=e7bdc5f9f869219e8d20060b42a09ea10a837851&stat=cycles%3Au)
85+
did not, at least not enough to pass our noise threshold.
86+
* marking as triaged.
87+
88+
#### Improvements
89+
90+
optimize zipping over array iterators [#115515](https://github.com/rust-lang/rust/pull/115515) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e292fec36880f48101bda4054be37097312e73c0&end=0d410be23c45e2f3567a6ec35985f690473f9176&stat=instructions:u)
91+
92+
| (instructions:u) | mean | range | count |
93+
|:----------------------------------:|:-----:|:--------------:|:-----:|
94+
| Regressions ❌ <br /> (primary) | - | - | 0 |
95+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
96+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.2%] | 3 |
97+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
98+
| All ❌✅ (primary) | -0.3% | [-0.4%, -0.2%] | 3 |
99+
100+
* A small win from a PR addressing user-filed performance regression, namely [issue #115339](https://github.com/rust-lang/rust/issues/115339), "Performance regression of array::IntoIter vs slice::Iter"
101+
102+
#### Mixed
103+
104+
Also consider call and yield as MIR SSA. [#113915](https://github.com/rust-lang/rust/pull/113915) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c30b28bdc17f1da73515afa0886f0d4f55c76e1f&end=d627cf07ce46d230a93732a4714d16f00df9466b&stat=instructions:u)
105+
106+
| (instructions:u) | mean | range | count |
107+
|:----------------------------------:|:-----:|:--------------:|:-----:|
108+
| Regressions ❌ <br /> (primary) | 3.9% | [3.9%, 3.9%] | 1 |
109+
| Regressions ❌ <br /> (secondary) | 0.1% | [0.1%, 0.1%] | 2 |
110+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.9%, -0.2%] | 26 |
111+
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.6%, -0.3%] | 5 |
112+
| All ❌✅ (primary) | -0.2% | [-0.9%, 3.9%] | 27 |
113+
114+
* The try perf run had sole primary regression of unicode-normalization-0.1.19 opt-full (1.19%), while the perf run against master had sole primary regression of exa-0.10.1 opt-full (3.90%).
115+
* The exa regression has persisted forward (i.e. it is not transient noise).
116+
* It was already been marked as triaged, as the performance changes were deemed a wash, apart from object code sizes which saw "small but clear" improvement.
117+
118+
Rollup of 5 pull requests [#116640](https://github.com/rust-lang/rust/pull/116640) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c1691db366c0f2e2341c60377c248ca2d9335076&end=475c71da0710fd1d40c046f9cee04b733b5b2b51&stat=instructions:u)
119+
120+
| (instructions:u) | mean | range | count |
121+
|:----------------------------------:|:-----:|:--------------:|:-----:|
122+
| Regressions ❌ <br /> (primary) | - | - | 0 |
123+
| Regressions ❌ <br /> (secondary) | 1.1% | [1.1%, 1.1%] | 1 |
124+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.2%] | 4 |
125+
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.5%, -0.4%] | 6 |
126+
| All ❌✅ (primary) | -0.3% | [-0.4%, -0.2%] | 4 |
127+
128+
* sole regression was to secondary benchmark coercions debug incr-patched: add static arr item
129+
* Looks like a blip (i.e. noise) based on the graph over time.
130+
* marking as triaged
131+
132+
exhaustiveness: Rework constructor splitting [#116391](https://github.com/rust-lang/rust/pull/116391) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=df4379b4eb5357263f0cf75475953f9b5c48c31f&end=e20cb7702117f1ad8127a16406ba9edd230c4f65&stat=instructions:u)
133+
134+
| (instructions:u) | mean | range | count |
135+
|:----------------------------------:|:-----:|:--------------:|:-----:|
136+
| Regressions ❌ <br /> (primary) | 0.2% | [0.2%, 0.3%] | 4 |
137+
| Regressions ❌ <br /> (secondary) | 3.9% | [0.5%, 5.8%] | 9 |
138+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.4%] | 1 |
139+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
140+
| All ❌✅ (primary) | 0.1% | [-0.4%, 0.3%] | 5 |
141+
142+
* the primary regressions were to cranelift-codegen-0.82.1 and cargo-0.60.0 in various incremental settings (mostly check builds)
143+
* the large (>5%) secondary regressions are all to match-stress.
144+
* the above cases were regressions for instruction-counts, but the cycle-counts didn't get marked as regressed in *any* of the same cases.
145+
* in all cases, the performance loss from these regressions was subsequently recovered (or masked) by [PR 116505](https://github.com/rust-lang/rust/pull/116505) "Automatically enable cross-crate inlining for small functions".
146+
(I don't know if that's actually related or just an awesome change that bought so much performance that it masked this problem).
147+
* Since the match-stress one was relatively large, I looked at the
148+
self-profile results in the [details](https://perf.rust-lang.org/detailed-query.html?commit=e20cb7702117f1ad8127a16406ba9edd230c4f65&benchmark=match-stress-check&scenario=full&base_commit=df4379b4eb5357263f0cf75475953f9b5c48c31f)
149+
which indicates a change in the delta(time) for match-stress might be due to new overheads in `check_match` and `mir_borrowck`.
150+
* But this is strange; I cannot tell how this PR could have affected codegen, which would be the only way I could imagine those functions being impacted.
151+
* Not marking as triaged for now; this mystery might be worth looking into a bit more. (But then again, the only significant regression was to a secondary stress test, so maybe its not worth spending time on.)
152+
153+
Automatically enable cross-crate inlining for small functions [#116505](https://github.com/rust-lang/rust/pull/116505) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ca89f732ec0f910fc92111a45dd7e6829baa9d4b&end=5d5edf0248d967baa6ac5cbea09b91c7c9947942&stat=instructions:u)
154+
155+
| (instructions:u) | mean | range | count |
156+
|:----------------------------------:|:-----:|:---------------:|:-----:|
157+
| Regressions ❌ <br /> (primary) | 2.3% | [0.3%, 13.0%] | 8 |
158+
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 0.8%] | 2 |
159+
| Improvements ✅ <br /> (primary) | -1.2% | [-18.1%, -0.1%] | 148 |
160+
| Improvements ✅ <br /> (secondary) | -2.2% | [-39.8%, -0.2%] | 209 |
161+
| All ❌✅ (primary) | -1.0% | [-18.1%, 13.0%] | 156 |
162+
163+
* Already marked as triaged
164+
* This was clearly awesome and amazing (all the more amazing if you review the history)
165+
* 'Nuff said.
166+
167+
#### Untriaged Pull Requests
168+
169+
- [#116742 Rollup of 3 pull requests](https://github.com/rust-lang/rust/pull/116742)
170+
- [#116640 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/116640)
171+
- [#116492 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/116492)
172+
- [#116391 exhaustiveness: Rework constructor splitting](https://github.com/rust-lang/rust/pull/116391)
173+
- [#116183 Always preserve DebugInfo in DeadStoreElimination.](https://github.com/rust-lang/rust/pull/116183)
174+
- [#115762 Explain revealing of opaque types in layout_of ParamEnv](https://github.com/rust-lang/rust/pull/115762)
175+
- [#115751 some inspect improvements](https://github.com/rust-lang/rust/pull/115751)
176+
- [#115740 Cache reachable_set on disk](https://github.com/rust-lang/rust/pull/115740)
177+
- [#115252 Represent MIR composite debuginfo as projections instead of aggregates](https://github.com/rust-lang/rust/pull/115252)
178+
- [#115082 Fix races conditions with `SyntaxContext` decoding](https://github.com/rust-lang/rust/pull/115082)
179+
- [#115025 Make subtyping explicit in MIR](https://github.com/rust-lang/rust/pull/115025)
180+
- [#114892 Remove conditional use of `Sharded` from query caches](https://github.com/rust-lang/rust/pull/114892)
181+
- [#114481 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/114481)
182+
- [#114459 Do not run ConstProp on mir_for_ctfe.](https://github.com/rust-lang/rust/pull/114459)
183+
- [#114330 don't UB on dangling ptr deref, instead check inbounds on projections](https://github.com/rust-lang/rust/pull/114330)
184+
- [#114321 get auto traits for parallel rustc](https://github.com/rust-lang/rust/pull/114321)
185+
- [#114023 Warn on inductive cycle in coherence leading to impls being considered not overlapping](https://github.com/rust-lang/rust/pull/114023)
186+
- [#114004 Add `riscv64gc-unknown-hermit` target](https://github.com/rust-lang/rust/pull/114004)
187+
- [#113858 Always const-prop scalars and scalar pairs](https://github.com/rust-lang/rust/pull/113858)
188+
- [#113758 Turn copy into moves during DSE.](https://github.com/rust-lang/rust/pull/113758)
189+
- [#113485 Bump version to 1.73](https://github.com/rust-lang/rust/pull/113485)
190+
- [#113370 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/113370)
191+
- [#113320 Add some extra information to opaque type cycle errors](https://github.com/rust-lang/rust/pull/113320)
192+
- [#113306 Update debuginfo test runner to provide more useful output](https://github.com/rust-lang/rust/pull/113306)
193+
- [#113304 Upgrade to indexmap 2.0.0](https://github.com/rust-lang/rust/pull/113304)
194+
- [#113270 perform TokenStream replacement in-place when possible in expand_macro](https://github.com/rust-lang/rust/pull/113270)
195+
- [#113057 Rollup of 2 pull requests](https://github.com/rust-lang/rust/pull/113057)
196+
- [#112963 Stop bubbling out hidden types from the eval obligation queries](https://github.com/rust-lang/rust/pull/112963)
197+
- [#112882 Rewrite `UnDerefer`](https://github.com/rust-lang/rust/pull/112882)
198+
- [#112420 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/112420)

0 commit comments

Comments
 (0)