Skip to content

Triage 2024 05 06 #1905

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

Merged
merged 2 commits into from
May 10, 2024
Merged

Triage 2024 05 06 #1905

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
283c645
Initial triage report template.
pnkfelix May 7, 2024
6774c87
triage for 2024-05-07.
pnkfelix May 7, 2024
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
180 changes: 180 additions & 0 deletions triage/2024-05-06.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
# 2024-05-07 Triage Log

Largely uneventful week; the most notable shifts were considered false-alarms
that arose from changes related to cfg-checking (either cargo enabling it, or
adding cfg's like `rustfmt` to the "well-known cfgs list").

Triage done by **@pnkfelix**.
Revision range: [c65b2dc9..69f53f5e](https://perf.rust-lang.org/?start=c65b2dc935c27c0c8c3997c6e8d8894718a2cb1a&end=69f53f5e5583381267298ac182eb02c7f1b5c1cd&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 3.0% | [0.2%, 19.5%] | 65 |
| Regressions ❌ <br /> (secondary) | 1.3% | [0.2%, 4.5%] | 103 |
| Improvements ✅ <br /> (primary) | -0.9% | [-2.2%, -0.2%] | 24 |
| Improvements ✅ <br /> (secondary) | -0.7% | [-1.4%, -0.4%] | 23 |
| All ❌✅ (primary) | 1.9% | [-2.2%, 19.5%] | 89 |


3 Regressions, 2 Improvements, 3 Mixed; 5 of them in rollups
54 artifact comparisons made in total

#### Regressions

Rollup of 7 pull requests [#124675](https://github.com/rust-lang/rust/pull/124675) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0d7b2fb797f214ea7514cfeaf2caef8178d8e3fc&end=befabbc9e5f6e82e659f9f52040ee0dd40593d8a&stat=instructions:u)

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

* all primary regressions are to doc-full scenarios, and the 1.2% is to helloworld.
* not worth teasing apart a rollup PR.
* marking as triaged.

Update cargo [#124684](https://github.com/rust-lang/rust/pull/124684) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=09cd00fea4aecaa6707f122d7e143196b8a12ee2&end=2c4bf249bd47f232de3c1e78ffe69b40c29bfcca&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:-------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.4% | [0.2%, 19.1%] | 83 |
| Regressions ❌ <br /> (secondary) | 1.6% | [0.2%, 5.7%] | 92 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 2.4% | [0.2%, 19.1%] | 83 |


* syn (mostly check builds, but also a debug incr-unchanged and opt incr-unchanged) had regressions ranging from 7.24% all the way up to 19.11%.
* The most plausible hypothesis is that this is due to an explosion in the number of warnings emitted for this benchmark. (The number of warnings went from ~200 up to 1800, according to Urgau's analysis).
* This means the code ends up becoming, at least in part, a benchmark of the lint machinery, regardless of whether that is our intent or not.
* see also [rustc-perf#1819](https://github.com/rust-lang/rustc-perf/issues/1819) "Consider passing -Awarnings (or similar) to avoid false alarms from lint *reporting*"
* marking as triaged.

Rollup of 3 pull requests [#124784](https://github.com/rust-lang/rust/pull/124784) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=96f1da82687f499dd3f57006ae71548714532382&end=d287f3e4eeaf680e8fe875f1ec75cca68f357d30&stat=instructions:u)

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

* all regressions were to syn, to various incr-unchanged and incr-patched:println scenarios.
* current hypothesis is that this is due to PR #124742, which adds `rustfmt` to the well-known cfgs list.
* that hypothesis implies that this is a (mostly-)false alarm, much like #124684.
* marking as triaged

#### Improvements

Rollup of 10 pull requests [#124646](https://github.com/rust-lang/rust/pull/124646) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=79734f1db8dbe322192dea32c0f6b80ab14c4c1d&end=561b5dea1e7e5c21f0fb550ca579229ceb878297&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -1.0% | [-2.8%, -0.2%] | 24 |
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.6%, -0.3%] | 9 |
| All ❌✅ (primary) | -1.0% | [-2.8%, -0.2%] | 24 |

* the bulk of the improvements are to variations of html5ever and serde_derive.
* skimming over the rollup list, I cannot identify an immediate root cause for improvement
* but for now will treat it like a happy accident

Some hir cleanups [#124401](https://github.com/rust-lang/rust/pull/124401) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d2d24e395a1e4fcee62ca17bf4cbddb1f903af97&end=09cd00fea4aecaa6707f122d7e143196b8a12ee2&stat=instructions:u)

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

* all improvements are to variations of typenum
* the hir cleanups in question are largely to store `AnonConst` (e.g. for array lengths) in the HIR arena, and then move the ConstArg span over to AnonConst span instead.
* inspection of typenum didn't show any particular cases that seemed like the would stress `AnonConst`; maybe the benefit comes more from the places where we now pass a span by value instead of passing a pointer to it.

#### Mixed

Account for immutably borrowed locals in MIR copy-prop and GVN [#123602](https://github.com/rust-lang/rust/pull/123602) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=befabbc9e5f6e82e659f9f52040ee0dd40593d8a&end=d2d24e395a1e4fcee62ca17bf4cbddb1f903af97&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.9%] | 10 |
| Regressions ❌ <br /> (secondary) | 0.8% | [0.2%, 2.6%] | 4 |
| Improvements ✅ <br /> (primary) | -0.5% | [-1.1%, -0.2%] | 6 |
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.0%, -0.3%] | 8 |
| All ❌✅ (primary) | 0.0% | [-1.1%, 0.9%] | 16 |

* html5ever opt-full regressed by 0.92%; libc in various incremental scenarios regressed by 0.30% to 0.39%.
* the libc changes were anticipated in the perf build prior to merge; html5ever opt-full was not predicted there.
* pnkfelix hypothesizes that this just reflects some extra-work from the compiler attempting to do the copy-propagation and global-value-numbering mir-optimizations on a larger set of immutably-borrowed locals, and is acceptable given the expected benefits.
* marking as triaged

Rollup of 8 pull requests [#124703](https://github.com/rust-lang/rust/pull/124703) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=7dd170fccb3be6b1737af5df14dd736b366236c1&end=d7ea27808deb5e10a0f7384e339e4e6165e33398&stat=instructions:u)

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

* image opt-full regressed by 0.63%; html5ever debug-{incr-full,full} by ~0.5%, html5ever opt-incr-unchaged by 0.21%
* already triaged by Kobzol, who hypothesizes that PR #124700 modified some inlining decisions.

Rollup of 4 pull requests [#124716](https://github.com/rust-lang/rust/pull/124716) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1a851da73cdeb02e2c62d301aa6bd98e515a50da&end=d568423a7a4ddb4b49323d96078a22f94df55fbd&stat=instructions:u)

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


* all regressions are secondary (specifically on unused-warnings benchmark)
* regression identified by Kobzol as caused by [PR #124584](https://github.com/rust-lang/rust/pull/124584) "Various improvements to entrypoint code"
* seems like noise to pnkfelix
* marked as triaged

#### Untriaged Pull Requests

- [#124784 Rollup of 3 pull requests](https://github.com/rust-lang/rust/pull/124784)
- [#124716 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/124716)
- [#124700 Remove an unnecessary cast](https://github.com/rust-lang/rust/pull/124700)
- [#124684 Update cargo](https://github.com/rust-lang/rust/pull/124684)
- [#124675 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/124675)
- [#124241 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/124241)
- [#123909 Stabilize `Utf8Chunks`](https://github.com/rust-lang/rust/pull/123909)
- [#123602 Account for immutably borrowed locals in MIR copy-prop and GVN](https://github.com/rust-lang/rust/pull/123602)
- [#123147 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/123147)
- [#122976 Remove len argument from RawVec::reserve_for_push](https://github.com/rust-lang/rust/pull/122976)
- [#122900 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/122900)
- [#122671 Codegen const panic messages as function calls](https://github.com/rust-lang/rust/pull/122671)
- [#122396 Less generic code for Vec allocations](https://github.com/rust-lang/rust/pull/122396)
- [#121955 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/121955)
- [#121804 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/121804)
- [#121752 Detect unused struct impls pub trait](https://github.com/rust-lang/rust/pull/121752)
- [#121462 Combine `Sub` and `Equate`](https://github.com/rust-lang/rust/pull/121462)
- [#121345 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/121345)
- [#120985 Update host LLVM on x64 Linux to LLVM 18](https://github.com/rust-lang/rust/pull/120985)
- [#120863 Use intrinsics::debug_assertions in debug_assert_nounwind](https://github.com/rust-lang/rust/pull/120863)
- [#120862 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/120862)
- [#120809 Use `transmute_unchecked` in `NonZero::new`.](https://github.com/rust-lang/rust/pull/120809)
- [#120588 wasm: Store rlib metadata in wasm object files](https://github.com/rust-lang/rust/pull/120588)
- [#120504 Vec::try_with_capacity](https://github.com/rust-lang/rust/pull/120504)
- [#120401 Rollup of 12 pull requests](https://github.com/rust-lang/rust/pull/120401)
- [#120335 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/120335)
- [#119662 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/119662)
- [#119204 macro_rules: Less hacky heuristic for using `tt` metavariable spans](https://github.com/rust-lang/rust/pull/119204)
- [#119002 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/119002)
- [#118661 Restore `const PartialEq`](https://github.com/rust-lang/rust/pull/118661)

Loading