Skip to content

Commit 703f3dd

Browse files
pnkfelixpnkfelix
committed
triage for 2024-01-16
1 parent cc74229 commit 703f3dd

File tree

1 file changed

+35
-40
lines changed

1 file changed

+35
-40
lines changed

‎triage/2024-01-16.md

Lines changed: 35 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
# 2024-01-16 Triage Log
22

3-
TODO: Summary
3+
This week had some small regressions that did not warrant further investigation,
4+
several of which were dismissed as being noise/blips in the data. There were
5+
also a number of gains. (Don't get exicited about that 20.6% improvement, its an
6+
measurement artifact from a temporary blip in the PR that immediately preceded
7+
this week's triage.)
48

5-
Triage done by **@???**.
9+
Triage done by **@pnkfelix**.
610
Revision range: [76101eec..f9c2421a](https://perf.rust-lang.org/?start=76101eecbe9aa80753664bbe637ad06d1925f315&end=f9c2421a2a6e34f3756900dd7d600704c08bfccb&absolute=false&stat=instructions%3Au)
711

812
**Summary**:
@@ -16,7 +20,7 @@ Revision range: [76101eec..f9c2421a](https://perf.rust-lang.org/?start=76101eecb
1620
| All ❌✅ (primary) | -1.2% | [-20.6%, 0.7%] | 135 |
1721

1822

19-
3 Regressions, 5 Improvements, 5 Mixed; ??? of them in rollups
23+
3 Regressions, 5 Improvements, 5 Mixed; 3 of them in rollups
2024
55 artifact comparisons made in total
2125

2226
#### Regressions
@@ -31,6 +35,9 @@ Rollup of 10 pull requests [#119754](https://github.com/rust-lang/rust/pull/1197
3135
| Improvements ✅ <br /> (secondary) | - | - | 0 |
3236
| All ❌✅ (primary) | - | - | 0 |
3337

38+
* The 2 regressing (and secondary) benchmarks are tt-muncher debug {incr-full, full}. Its not transient.
39+
* I've skimmed over the list of PR's in the rollup. None of them are obvious culprits here. I looked at the ones related to debuginfo (#118903) and to code-coverage (#119033 and #119681), but none of those seem likely to be to blame here
40+
* Since this only affects a secondary benchmark, and only the instruction count (e.g. not cpu-clock:u nor wall-time for these two benchmarks), I do not think its worth further investigation and I'm going to mark it as triaged.
3441

3542
Exhaustiveness: use an `Option` instead of allocating fictitious patterns [#119688](https://github.com/rust-lang/rust/pull/119688) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a2d9d73e608f1b24eba840c4fd2d68dbe3b65e01&end=0a8923361ec2a37fa341292c029ef7c6d0405d4b&stat=instructions:u)
3643

@@ -42,6 +49,10 @@ Exhaustiveness: use an `Option` instead of allocating fictitious patterns [#1196
4249
| Improvements ✅ <br /> (secondary) | - | - | 0 |
4350
| All ❌✅ (primary) | - | - | 0 |
4451

52+
* This impacted the 6 variants of match-stress {incr-full,full} x {check,debug,opt}
53+
* I think the impact on match-stress was probably well-anticipated, and within a reasonable range for a stress-test benchmark.
54+
* Note that #119688 was a precursor to some further cleanup code (namely to remove the use of a local-arena within exhaustiveness checking).
55+
* Marking as triaged.
4556

4657
never patterns: Check bindings wrt never patterns [#119610](https://github.com/rust-lang/rust/pull/119610) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=bfcc027a751595ba290c554f47907eaa3779f798&end=714b29a17ff5fa727c794bbb60bfd335f8e75d42&stat=instructions:u)
4758

@@ -53,6 +64,10 @@ never patterns: Check bindings wrt never patterns [#119610](https://github.com/r
5364
| Improvements ✅ <br /> (secondary) | - | - | 0 |
5465
| All ❌✅ (primary) | 0.3% | [0.3%, 0.4%] | 3 |
5566

67+
* This impacted 3 variants of unicode-normalization-0.1.19: debug incr-unchanged and check {incr-unchanged, incr-patched:println}.
68+
* Interestingly, during two different try runs, those three variants were found to have improved by similar amounts by this PR.
69+
* there's some weird interaction between that benchmark and the code paths impacted by this PR, and I do not think its worth investing effort in further investigation.
70+
* marking as triaged.
5671

5772
#### Improvements
5873

@@ -66,6 +81,7 @@ macro_rules: Add an expansion-local cache to span marker [#119693](https://githu
6681
| Improvements ✅ <br /> (secondary) | -0.8% | [-1.9%, -0.3%] | 16 |
6782
| All ❌✅ (primary) | -1.4% | [-20.5%, -0.2%] | 80 |
6883

84+
* the bitmaps changes (-20.5%, -17.9%, -13.1%) are all artifacts of returning to normal after a blip in the previous PR.
6985

7086
A more efficient slice comparison implementation for T: !BytewiseEq [#116846](https://github.com/rust-lang/rust/pull/116846) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ae9d24de80b00b4158d1a29a212a6b02aeda0e75&end=190f4c96116a3b59b7de4881cfec544be0246d84&stat=instructions:u)
7187

@@ -77,6 +93,7 @@ A more efficient slice comparison implementation for T: !BytewiseEq [#116846](ht
7793
| Improvements ✅ <br /> (secondary) | -0.6% | [-0.6%, -0.6%] | 1 |
7894
| All ❌✅ (primary) | -0.5% | [-0.9%, -0.2%] | 15 |
7995

96+
* it is too bad that work in PR #100124 stalled.
8097

8198
Remove a large amount of leb128-coded integers [#119791](https://github.com/rust-lang/rust/pull/119791) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d73bd3fb3ba312f3e6b5af4d56d1161d37b71620&end=68acb393c5d2cff049b41981e35217a7e630f63a&stat=instructions:u)
8299

@@ -88,6 +105,8 @@ Remove a large amount of leb128-coded integers [#119791](https://github.com/rust
88105
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.5%, -0.1%] | 12 |
89106
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.2%] | 5 |
90107

108+
* the 1.1% hit is to deep-vector debug full. It may be transient; the history is pretty up-and-down at the time of this PR, and has settled at a lower level than where it was when this PR landed.
109+
* in any case, the gains elsewhere, especially bootstrap, outweigh the loss to that one secondary benchmark. (Which ... I guess is what the rustc-perf bot now computes as well, since it categorized this as an Improvement rather than Mixed?)
91110

92111
Exhaustiveness: track overlapping ranges precisely [#119396](https://github.com/rust-lang/rust/pull/119396) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=bfd799f1a5a86d16e6b8caa2857bcb4aac6e0174&end=174e73a3f6df6f96ab453493796e461164dea94a&stat=instructions:u)
93112

@@ -123,6 +142,10 @@ Support async recursive calls (as long as they have indirection) [#117703](https
123142
| Improvements ✅ <br /> (secondary) | - | - | 0 |
124143
| All ❌✅ (primary) | -0.3% | [-0.4%, -0.3%] | 3 |
125144

145+
* this is weird, it looks like an inverse blip occurred on the preceding PR, where tt-muncher check incr-unchanged had a single point with -0.2% instruction-count, and then it preceding to "return to normal" on the succeeding PRs.
146+
* (Its harder for me to explain away "inverse blips" ...)
147+
* but at the same time, this does not seem like a significant regression by our usual metrics.
148+
* marking as triaged.
126149

127150
Rollup of 9 pull requests [#119767](https://github.com/rust-lang/rust/pull/119767) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=be00c5a9b89161b7f45ba80340f709e8e41122f9&end=5876c8cdfd3df742c334d6447d44d760c77103b6&stat=instructions:u)
128151

@@ -134,6 +157,9 @@ Rollup of 9 pull requests [#119767](https://github.com/rust-lang/rust/pull/11976
134157
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 |
135158
| All ❌✅ (primary) | 1.3% | [0.4%, 2.3%] | 2 |
136159

160+
* primary regressions: syn opt-full regressed by 2.3%, bitmaps check-incr-full by 0.35%. secondary regressions: coercions debug-full by 1.23%, ctfe-stress check-full by 0.51%
161+
* from the overall history, it seems like syn opt-full returned to "normal" with later PRs that don't necessarily seem like they would have affected syn (e.g. PR #117449). bitmap check-incr-full's trend is likewise downward after this point.
162+
* marking as triaged
137163

138164
Add assume into `NonZeroIntX::get` [#119452](https://github.com/rust-lang/rust/pull/119452) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ce1f2ccf5a5ac9343623bd115a05e4151d93af0d&end=2319be8e265dd19973574eb581d28297baf44b11&stat=instructions:u)
139165

@@ -145,6 +171,8 @@ Add assume into `NonZeroIntX::get` [#119452](https://github.com/rust-lang/rust/p
145171
| Improvements ✅ <br /> (secondary) | - | - | 0 |
146172
| All ❌✅ (primary) | 0.3% | [-0.7%, 0.9%] | 5 |
147173

174+
* scottmcm writes: "Instructions have a couple red in instruction counts for opt, but that's entirely reasonable for something intended to enable optimizations. Notably, the cycles are green, with no regressions. So I think this is fine."
175+
* marking as triaged
148176

149177
Avoid some redundant work in GVN [#119439](https://github.com/rust-lang/rust/pull/119439) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=714b29a17ff5fa727c794bbb60bfd335f8e75d42&end=f9c2421a2a6e34f3756900dd7d600704c08bfccb&stat=instructions:u)
150178

@@ -156,6 +184,9 @@ Avoid some redundant work in GVN [#119439](https://github.com/rust-lang/rust/pul
156184
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.4%, -0.4%] | 1 |
157185
| All ❌✅ (primary) | 0.0% | [-1.1%, 0.9%] | 8 |
158186

187+
* primary regressions are regex-1.5.5 debug-full, opt-incr-patched:Job, incr-full, and exa opt-full.
188+
* the exa regression looks like a blip. The regex ones were predicted during a try run for the PR. I assume they were deemed acceptable as they are offset improvements elsewhere (or dismissed as noise?)
189+
* marking as triaged.
159190

160191
Sandwich MIR optimizations between DSE. [#119672](https://github.com/rust-lang/rust/pull/119672) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f9c2421a2a6e34f3756900dd7d600704c08bfccb&end=fa0dc208d0a34027c1d3cca7d47975d8238bcfde&stat=instructions:u)
161192

@@ -167,41 +198,5 @@ Sandwich MIR optimizations between DSE. [#119672](https://github.com/rust-lang/r
167198
| Improvements ✅ <br /> (secondary) | -0.9% | [-2.2%, -0.2%] | 10 |
168199
| All ❌✅ (primary) | -0.4% | [-2.2%, 1.4%] | 45 |
169200

170-
171-
#### Untriaged Pull Requests
172-
173-
- [#119767 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/119767)
174-
- [#119754 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/119754)
175-
- [#119688 Exhaustiveness: use an `Option` instead of allocating fictitious patterns](https://github.com/rust-lang/rust/pull/119688)
176-
- [#119662 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/119662)
177-
- [#119610 never patterns: Check bindings wrt never patterns](https://github.com/rust-lang/rust/pull/119610)
178-
- [#119452 Add assume into `NonZeroIntX::get`](https://github.com/rust-lang/rust/pull/119452)
179-
- [#119439 Avoid some redundant work in GVN](https://github.com/rust-lang/rust/pull/119439)
180-
- [#119204 macro_rules: Less hacky heuristic for using `tt` metavariable spans](https://github.com/rust-lang/rust/pull/119204)
181-
- [#119002 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/119002)
182-
- [#118661 Restore `const PartialEq`](https://github.com/rust-lang/rust/pull/118661)
183-
- [#118473 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/118473)
184-
- [#118420 Introduce support for `async gen` blocks](https://github.com/rust-lang/rust/pull/118420)
185-
- [#118405 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/118405)
186-
- [#118319 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/118319)
187-
- [#118308 Don't warn an empty pattern unreachable if we're not sure the data is valid](https://github.com/rust-lang/rust/pull/118308)
188-
- [#117769 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/117769)
189-
- [#117736 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/117736)
190-
- [#117703 Support async recursive calls (as long as they have indirection)](https://github.com/rust-lang/rust/pull/117703)
191-
- [#117213 Reorder check_item_type diagnostics so they occur next to the corresponding `check_well_formed` diagnostics](https://github.com/rust-lang/rust/pull/117213)
192-
- [#117180 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/117180)
193-
- [#116940 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/116940)
194-
- [#116915 Add an assume that the index is inbounds to slice::get_unchecked](https://github.com/rust-lang/rust/pull/116915)
195-
- [#116889 Eat close paren if capture_cfg to avoid unbalanced parens](https://github.com/rust-lang/rust/pull/116889)
196-
- [#116492 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/116492)
197-
- [#116391 exhaustiveness: Rework constructor splitting](https://github.com/rust-lang/rust/pull/116391)
198-
- [#116183 Always preserve DebugInfo in DeadStoreElimination.](https://github.com/rust-lang/rust/pull/116183)
199-
- [#116033 report `unused_import` for empty reexports even it is pub](https://github.com/rust-lang/rust/pull/116033)
200-
- [#115762 Explain revealing of opaque types in layout_of ParamEnv](https://github.com/rust-lang/rust/pull/115762)
201-
- [#115751 some inspect improvements](https://github.com/rust-lang/rust/pull/115751)
202-
- [#115740 Cache reachable_set on disk](https://github.com/rust-lang/rust/pull/115740)
203-
204-
#### Nags requiring follow up
205-
206-
TODO: Nags
201+
* already marked as triaged by @lqd with the comment "As seen in the previous runs: some nice wins on bigger benchmarks, and overall gains outweigh the few losses."
207202

0 commit comments

Comments
 (0)