Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Triage 2023 11 28 #1759

Merged
merged 3 commits into from
Dec 1, 2023
Merged

Triage 2023 11 28 #1759

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01d9a53
Initial triage report template.
pnkfelix Nov 28, 2023
8391cfe
Triage report template with shortened commit ids
pnkfelix Nov 28, 2023
20911b7
rustc-perf Triage for 2023-11-29.
pnkfelix Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 174 additions & 0 deletions triage/2023-11-28.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
# 2023-11-28 Triage Log

A good week, despite a few PRs that pnkfelix opted not to mark as triaged. In
particular, a broad set of primary benchmarks improved, due to improvements to
resolve (PR #118188) and a one-pass rewrite of exhaustiveness (PR #117611).

Triage done by **@pnkfelix**.
Revision range: [4f3da903..df0295f0](https://perf.rust-lang.org/?start=4f3da903a43f22ea33d2ca4435a24b42fc1f842a&end=df0295f07175acc7325ce3ca4152eb05752af1f2&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.6% | [0.1%, 1.5%] | 15 |
| Regressions ❌ <br /> (secondary) | 1.3% | [0.2%, 2.4%] | 16 |
| Improvements ✅ <br /> (primary) | -0.7% | [-2.1%, -0.3%] | 66 |
| Improvements ✅ <br /> (secondary) | -1.7% | [-8.1%, -0.2%] | 43 |
| All ❌✅ (primary) | -0.5% | [-2.1%, 1.5%] | 81 |


1 Regressions, 5 Improvements, 5 Mixed; 2 of them in rollups
84 artifact comparisons made in total

#### Regressions

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.1%, 0.8%] | 23 |
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 1.0%] | 11 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.4% | [0.1%, 0.8%] | 23 |

* The bulk (in this case > 0.31%) of the primary regressions are to bitmaps and libc, in a variety of incremental modes.
* 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.
* follow-up PR's have been proposed, but we have not successfully found one that undoes the regression.
* 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.
* not marking as triaged yet.

#### Improvements

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.2% | [-0.3%, -0.2%] | 4 |
| Improvements ✅ <br /> (secondary) | -3.8% | [-8.1%, -0.5%] | 14 |
| All ❌✅ (primary) | -0.2% | [-0.3%, -0.2%] | 4 |

* slight improvements to clap check-{incr-full,full}, cargo check-full, and diesel doc-full


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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.2%] | 10 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.2%] | 3 |
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.2%] | 10 |

* slight improvements to bitmaps {check-full,opt-full}, serde {check-full,debug-full}, diesel check-full
* the remaining 5 are doc-full improvements.


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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.5%, -0.3%] | 3 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.4% | [-0.5%, -0.3%] | 3 |

* switches to unsafe { element_size.unchecked_mul(n) } with a big ol' safety comment about why.
* improved opt incr-patched:println for clap, image, and cargo benchmarks.

`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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.8%, -0.2%] | 5 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.4% | [-0.8%, -0.2%] | 5 |

* improved check builds for clap {incr-full,full,incr-unchanged} and hyper {incr-full,full}

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.5%, -0.2%] | 58 |
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.0%, -0.1%] | 34 |
| All ❌✅ (primary) | -0.3% | [-0.5%, -0.2%] | 58 |

* wide range of benchmarks improved on incr-unchanged and incr-patched variants: stm32f4, diesel, bitmaps, cranelift-codegen, syn, serde, et cetera.
* as noted above with #118319, this is coupled with a PR (#118311) associated with some regressions.

#### Mixed

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.4%] | 1 |
| Regressions ❌ <br /> (secondary) | 1.3% | [1.3%, 1.4%] | 2 |
| Improvements ✅ <br /> (primary) | -1.4% | [-1.9%, -0.2%] | 5 |
| Improvements ✅ <br /> (secondary) | -1.8% | [-2.6%, -1.3%] | 8 |
| All ❌✅ (primary) | -1.1% | [-1.9%, 0.4%] | 6 |

* The single primary regression here seems to be a measurement blip, based on the 30-day history.
* Even if it weren't, the improvements would outweigh the regression.
* Marked as triaged.

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.1% | [1.0%, 1.1%] | 2 |
| Regressions ❌ <br /> (secondary) | 1.6% | [0.3%, 2.4%] | 9 |
| Improvements ✅ <br /> (primary) | -0.9% | [-2.0%, -0.2%] | 11 |
| Improvements ✅ <br /> (secondary) | -0.2% | [-0.2%, -0.2%] | 1 |
| All ❌✅ (primary) | -0.6% | [-2.0%, 1.1%] | 13 |

* primary improvements were to html5ever, cranelift-codegen, exa, and image.
* 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.
* already marked as triaged by nnethercote

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.5% | [0.5%, 0.5%] | 2 |
| Improvements ✅ <br /> (primary) | -0.1% | [-0.1%, -0.1%] | 5 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.5%, -0.2%] | 9 |
| All ❌✅ (primary) | -0.1% | [-0.1%, -0.1%] | 5 |

* already marked as triaged by nnethercote. (regressions are confined to secondary match-stress benchmark).

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.4%, 0.6%] | 4 |
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.3%] | 2 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.4%] | 1 |
| Improvements ✅ <br /> (secondary) | -0.6% | [-0.6%, -0.6%] | 2 |
| All ❌✅ (primary) | 0.3% | [-0.4%, 0.6%] | 5 |

* already marked as triaged by nnethercote (hoped to be churn/noise).

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)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.4%, 0.6%] | 3 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.3%, -0.2%] | 4 |
| All ❌✅ (primary) | 0.5% | [0.4%, 0.6%] | 3 |

* regressions are confined to clap opt {full,incr-full,incr-patched:println}
* not marking as triaged
Loading