Skip to content

Commit ac048ad

Browse files
authored
Merge pull request #1759 from pnkfelix/triage-2023-11-28
Triage 2023 11 28
2 parents b69afe7 + 20911b7 commit ac048ad

File tree

1 file changed

+174
-0
lines changed

1 file changed

+174
-0
lines changed

triage/2023-11-28.md

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
# 2023-11-28 Triage Log
2+
3+
A good week, despite a few PRs that pnkfelix opted not to mark as triaged. In
4+
particular, a broad set of primary benchmarks improved, due to improvements to
5+
resolve (PR #118188) and a one-pass rewrite of exhaustiveness (PR #117611).
6+
7+
Triage done by **@pnkfelix**.
8+
Revision range: [4f3da903..df0295f0](https://perf.rust-lang.org/?start=4f3da903a43f22ea33d2ca4435a24b42fc1f842a&end=df0295f07175acc7325ce3ca4152eb05752af1f2&absolute=false&stat=instructions%3Au)
9+
10+
**Summary**:
11+
12+
| (instructions:u) | mean | range | count |
13+
|:----------------------------------:|:-----:|:--------------:|:-----:|
14+
| Regressions ❌ <br /> (primary) | 0.6% | [0.1%, 1.5%] | 15 |
15+
| Regressions ❌ <br /> (secondary) | 1.3% | [0.2%, 2.4%] | 16 |
16+
| Improvements ✅ <br /> (primary) | -0.7% | [-2.1%, -0.3%] | 66 |
17+
| Improvements ✅ <br /> (secondary) | -1.7% | [-8.1%, -0.2%] | 43 |
18+
| All ❌✅ (primary) | -0.5% | [-2.1%, 1.5%] | 81 |
19+
20+
21+
1 Regressions, 5 Improvements, 5 Mixed; 2 of them in rollups
22+
84 artifact comparisons made in total
23+
24+
#### Regressions
25+
26+
Rollup of 4 pull requests [#118319](https://github.com/rust-lang/rust/pull/118319) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=3dbb4da04267b19bc8c403c0bb2b41c5b8010a61&end=3bb0171999a65b0650d9405a7b2e8e7dc3476dec&stat=instructions:u)
27+
28+
| (instructions:u) | mean | range | count |
29+
|:----------------------------------:|:----:|:------------:|:-----:|
30+
| Regressions ❌ <br /> (primary) | 0.4% | [0.1%, 0.8%] | 23 |
31+
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 1.0%] | 11 |
32+
| Improvements ✅ <br /> (primary) | - | - | 0 |
33+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
34+
| All ❌✅ (primary) | 0.4% | [0.1%, 0.8%] | 23 |
35+
36+
* The bulk (in this case > 0.31%) of the primary regressions are to bitmaps and libc, in a variety of incremental modes.
37+
* nnethercote noted that this seems like it must be PR #118311 ("merge DefKind::Coroutine into Defkind::Closure"), and confirmed it by benchmarking that specific commit.
38+
* follow-up PR's have been proposed, but we have not successfully found one that undoes the regression.
39+
* meanwhile, a follow-on PR, #118188, has landed that is coupled to #118311. This PR #118188 seems to have wide benefits. So it may not be worthwhile to spend time trying to figure out the regression injected by #118311.
40+
* not marking as triaged yet.
41+
42+
#### Improvements
43+
44+
Remove `PredicateKind::ClosureKind` [#118120](https://github.com/rust-lang/rust/pull/118120) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=06d1afe5189bc0830b9b5654fd0ba89e9829f4cd&end=1e9dda77b5b8e690c7e21871bbd2dcf182e1a841&stat=instructions:u)
45+
46+
| (instructions:u) | mean | range | count |
47+
|:----------------------------------:|:-----:|:--------------:|:-----:|
48+
| Regressions ❌ <br /> (primary) | - | - | 0 |
49+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
50+
| Improvements ✅ <br /> (primary) | -0.2% | [-0.3%, -0.2%] | 4 |
51+
| Improvements ✅ <br /> (secondary) | -3.8% | [-8.1%, -0.5%] | 14 |
52+
| All ❌✅ (primary) | -0.2% | [-0.3%, -0.2%] | 4 |
53+
54+
* slight improvements to clap check-{incr-full,full}, cargo check-full, and diesel doc-full
55+
56+
57+
Cache flags for `ty::Const` [#118189](https://github.com/rust-lang/rust/pull/118189) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=eab8c7d5fd335d673bb96bb4aef86c74006cef4b&end=41fe75ec6b824d51e5365098c4af9de45e5a2723&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.3% | [-0.3%, -0.2%] | 10 |
64+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.2%] | 3 |
65+
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.2%] | 10 |
66+
67+
* slight improvements to bitmaps {check-full,opt-full}, serde {check-full,debug-full}, diesel check-full
68+
* the remaining 5 are doc-full improvements.
69+
70+
71+
Indicate that multiplication in Layout::array cannot overflow [#118228](https://github.com/rust-lang/rust/pull/118228) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f74f700952f105536446e415b8df8061bddfb25e&end=b06258cde4b0dd131cdbf289349ebf51b3b6388a&stat=instructions:u)
72+
73+
| (instructions:u) | mean | range | count |
74+
|:----------------------------------:|:-----:|:--------------:|:-----:|
75+
| Regressions ❌ <br /> (primary) | - | - | 0 |
76+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
77+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.5%, -0.3%] | 3 |
78+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
79+
| All ❌✅ (primary) | -0.4% | [-0.5%, -0.3%] | 3 |
80+
81+
* switches to unsafe { element_size.unchecked_mul(n) } with a big ol' safety comment about why.
82+
* improved opt incr-patched:println for clap, image, and cargo benchmarks.
83+
84+
`AmbiguityCause` should not eagerly format strings [#118267](https://github.com/rust-lang/rust/pull/118267) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=3acb261e214cd13ae54346af30eae5807501ec37&end=0b8a61b235662d397721d1b88ddefdfc147ba39a&stat=instructions:u)
85+
86+
| (instructions:u) | mean | range | count |
87+
|:----------------------------------:|:-----:|:--------------:|:-----:|
88+
| Regressions ❌ <br /> (primary) | - | - | 0 |
89+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
90+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.8%, -0.2%] | 5 |
91+
| Improvements ✅ <br /> (secondary) | - | - | 0 |
92+
| All ❌✅ (primary) | -0.4% | [-0.8%, -0.2%] | 5 |
93+
94+
* improved check builds for clap {incr-full,full,incr-unchanged} and hyper {incr-full,full}
95+
96+
resolve: Feed the `def_kind` query immediately on `DefId` creation [#118188](https://github.com/rust-lang/rust/pull/118188) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=df0295f07175acc7325ce3ca4152eb05752af1f2&end=5facb422f8a5a61df515572fe79b02433639d565&stat=instructions:u)
97+
98+
| (instructions:u) | mean | range | count |
99+
|:----------------------------------:|:-----:|:--------------:|:-----:|
100+
| Regressions ❌ <br /> (primary) | - | - | 0 |
101+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
102+
| Improvements ✅ <br /> (primary) | -0.3% | [-0.5%, -0.2%] | 58 |
103+
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.0%, -0.1%] | 34 |
104+
| All ❌✅ (primary) | -0.3% | [-0.5%, -0.2%] | 58 |
105+
106+
* wide range of benchmarks improved on incr-unchanged and incr-patched variants: stm32f4, diesel, bitmaps, cranelift-codegen, syn, serde, et cetera.
107+
* as noted above with #118319, this is coupled with a PR (#118311) associated with some regressions.
108+
109+
#### Mixed
110+
111+
Refactor `binary_search_by` to use conditional moves [#117722](https://github.com/rust-lang/rust/pull/117722) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=41fe75ec6b824d51e5365098c4af9de45e5a2723&end=8abf920985368264ed4d46e62e1730232e161292&stat=instructions:u)
112+
113+
| (instructions:u) | mean | range | count |
114+
|:----------------------------------:|:-----:|:--------------:|:-----:|
115+
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.4%] | 1 |
116+
| Regressions ❌ <br /> (secondary) | 1.3% | [1.3%, 1.4%] | 2 |
117+
| Improvements ✅ <br /> (primary) | -1.4% | [-1.9%, -0.2%] | 5 |
118+
| Improvements ✅ <br /> (secondary) | -1.8% | [-2.6%, -1.3%] | 8 |
119+
| All ❌✅ (primary) | -1.1% | [-1.9%, 0.4%] | 6 |
120+
121+
* The single primary regression here seems to be a measurement blip, based on the 30-day history.
122+
* Even if it weren't, the improvements would outweigh the regression.
123+
* Marked as triaged.
124+
125+
Rewrite exhaustiveness in one pass [#117611](https://github.com/rust-lang/rust/pull/117611) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f5dc2653fdd8b5d177b2ccbd84057954340a89fc&end=ee80c8d0a8bc63b69f68216c5d37f9ab837eedd0&stat=instructions:u)
126+
127+
| (instructions:u) | mean | range | count |
128+
|:----------------------------------:|:-----:|:--------------:|:-----:|
129+
| Regressions ❌ <br /> (primary) | 1.1% | [1.0%, 1.1%] | 2 |
130+
| Regressions ❌ <br /> (secondary) | 1.6% | [0.3%, 2.4%] | 9 |
131+
| Improvements ✅ <br /> (primary) | -0.9% | [-2.0%, -0.2%] | 11 |
132+
| Improvements ✅ <br /> (secondary) | -0.2% | [-0.2%, -0.2%] | 1 |
133+
| All ❌✅ (primary) | -0.6% | [-2.0%, 1.1%] | 13 |
134+
135+
* primary improvements were to html5ever, cranelift-codegen, exa, and image.
136+
* unicode-normalization was the main primary regression, by up to 1.15% (check incr-full); but its worth noting that it was very close to the significance factor (1.13%) for that benchmark, so its borderline historically.
137+
* already marked as triaged by nnethercote
138+
139+
rustc: Make `def_kind` mandatory for all `DefId`s [#118250](https://github.com/rust-lang/rust/pull/118250) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=33f6af805257c6d462ad45c5de32da3fb38bfaf7&end=5c97719393b093997a03d7bb5d8a01d712c66c0e&stat=instructions:u)
140+
141+
| (instructions:u) | mean | range | count |
142+
|:----------------------------------:|:-----:|:--------------:|:-----:|
143+
| Regressions ❌ <br /> (primary) | - | - | 0 |
144+
| Regressions ❌ <br /> (secondary) | 0.5% | [0.5%, 0.5%] | 2 |
145+
| Improvements ✅ <br /> (primary) | -0.1% | [-0.1%, -0.1%] | 5 |
146+
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.5%, -0.2%] | 9 |
147+
| All ❌✅ (primary) | -0.1% | [-0.1%, -0.1%] | 5 |
148+
149+
* already marked as triaged by nnethercote. (regressions are confined to secondary match-stress benchmark).
150+
151+
Add `debug_assert_nounwind` and convert `assert_unsafe_precondition` [#110303](https://github.com/rust-lang/rust/pull/110303) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5c97719393b093997a03d7bb5d8a01d712c66c0e&end=9529a5d2655f6974c2ee16e91c5db548a3daea03&stat=instructions:u)
152+
153+
| (instructions:u) | mean | range | count |
154+
|:----------------------------------:|:-----:|:--------------:|:-----:|
155+
| Regressions ❌ <br /> (primary) | 0.5% | [0.4%, 0.6%] | 4 |
156+
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.3%] | 2 |
157+
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.4%] | 1 |
158+
| Improvements ✅ <br /> (secondary) | -0.6% | [-0.6%, -0.6%] | 2 |
159+
| All ❌✅ (primary) | 0.3% | [-0.4%, 0.6%] | 5 |
160+
161+
* already marked as triaged by nnethercote (hoped to be churn/noise).
162+
163+
Rollup of 7 pull requests [#118405](https://github.com/rust-lang/rust/pull/118405) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e06c94d6cb61ef2fa28370fb69a8d2e11b6678c4&end=46a24ed2f4b4bdfccca36fb20b1574a6164893d8&stat=instructions:u)
164+
165+
| (instructions:u) | mean | range | count |
166+
|:----------------------------------:|:-----:|:--------------:|:-----:|
167+
| Regressions ❌ <br /> (primary) | 0.5% | [0.4%, 0.6%] | 3 |
168+
| Regressions ❌ <br /> (secondary) | - | - | 0 |
169+
| Improvements ✅ <br /> (primary) | - | - | 0 |
170+
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.3%, -0.2%] | 4 |
171+
| All ❌✅ (primary) | 0.5% | [0.4%, 0.6%] | 3 |
172+
173+
* regressions are confined to clap opt {full,incr-full,incr-patched:println}
174+
* not marking as triaged

0 commit comments

Comments
 (0)