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

Jump to bottom

Triage 2024 04 09 #1890

Merged
merged 4 commits into from Apr 10, 2024
Merged

Triage 2024 04 09 #1890

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eee4f66
Initial triage report template.
pnkfelix Apr 9, 2024
b58a81a
Triage report template with shortened commit ids
pnkfelix Apr 9, 2024
a894de1
initial round of fill-in for triage.
pnkfelix Apr 9, 2024
61ca7ad
wrote summary.
pnkfelix Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
184 changes: 184 additions & 0 deletions triage/2024-04-09.md
View file
Open in desktop
@@ -0,0 +1,184 @@
# 2024-04-09 Triage Log

A quiet week; all the outright regressions were already triaged (the one biggish one was #122077, which is justified as an important bug fix). There was a very nice set of improvements from PR #122070, which cleverly avoids a lot of unnecessary allocator calls when building an incremental dep graph by reusing the old edges from the previous graph.

Triage done by **@pnkfelix**.
Revision range: [3d5528c2..86b603cd](https://perf.rust-lang.org/?start=3d5528c287860b918e178a34f04ff903325571b3&end=86b603cd792b3f6172ba4f676d7b586c1af7630a&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.0% | [0.4%, 5.0%] | 83 |
| Regressions ❌ <br /> (secondary) | 2.1% | [0.5%, 4.8%] | 79 |
| Improvements ✅ <br /> (primary) | -1.5% | [-2.9%, -0.3%] | 121 |
| Improvements ✅ <br /> (secondary) | -1.4% | [-3.5%, -0.3%] | 67 |
| All ❌✅ (primary) | -0.1% | [-2.9%, 5.0%] | 204 |


3 Regressions, 3 Improvements, 7 Mixed; 1 of them in rollups
78 artifact comparisons made in total

#### Regressions

instantiate higher ranked goals outside of candidate selection [#119820](https://github.com/rust-lang/rust/pull/119820) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0accf4ec4c07d23aa86f6a97aeb8797941abc30e&end=43f4f2a3b1a3d3fb3dbbbe4fde33fb97c780ee98&stat=instructions:u)

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

* already triaged by Jakub as an expected small performance regresison

Pass list of defineable opaque types into canonical queries [#122077](https://github.com/rust-lang/rust/pull/122077) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ab5bda1aa70f707014e2e691e43bc37a8819252a&end=b234e449443a49ab19ef6b712bf56cc65927d98f&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.9% | [0.2%, 5.4%] | 101 |
| Regressions ❌ <br /> (secondary) | 2.3% | [0.3%, 4.7%] | 77 |

| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 1.8% | [-0.6%, 5.4%] | 102 |

* already triaged by oli, as an expected performance regression that is [justified](https://github.com/rust-lang/rust/pull/122077#issuecomment-1995694305) as an important bugfix

Replace some `CrateStore` trait methods with hooks. [#123099](https://github.com/rust-lang/rust/pull/123099) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=59c808fcd9eeb3c5528209d1cef3aaa5521edbd6&end=bd12986fd6659a3091cff7694b8225374f4a26fe&stat=instructions:u)

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

* already triaged by oli, categorized as noise.

#### Improvements

hir: Drop owner's own item-local id (zero) from parenting tables [#123415](https://github.com/rust-lang/rust/pull/123415) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=703dc9ce64d9b31a239a7280d9b5f9ddd85ffed6&end=98efd808e1b77cd70a097620aad6250727167a28&stat=instructions:u)

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


[perf] cache type info for ParamEnv [#123058](https://github.com/rust-lang/rust/pull/123058) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=6f837503aa120ca69c2985b6c9a474c00674cef1&end=087ae978a13013800c8a484cf17c8951ab0b6b0c&stat=instructions:u)

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


Remove debuginfo from rustc-demangle too [#123608](https://github.com/rust-lang/rust/pull/123608) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0e5f5207881066973486e6a480fa46cfa22947e9&end=75fd074338801fba74a8cf7f8c48c5c5be362d08&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.6% | [0.4%, 0.9%] | 4 |
| Improvements ✅ <br /> (primary) | -1.6% | [-2.7%, -0.3%] | 7 |
| Improvements ✅ <br /> (secondary) | -1.3% | [-2.4%, -0.3%] | 24 |
| All ❌✅ (primary) | -1.6% | [-2.7%, -0.3%] | 7 |


#### Mixed

Add `Ord::cmp` for primitives as a `BinOp` in MIR [#118310](https://github.com/rust-lang/rust/pull/118310) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=029cb1b13b6388b95e64e8996ec8b41a9f3cf16e&end=a77322c16f188402fa22a5e87100acce42433cbc&stat=instructions:u)

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

* The impact here is somewhat limited, and the graph indicates that the 0.69% instruction-count regression for image-0.24.1 was subsequently recovered.

Encode dep graph edges directly from the previous graph when promoting [#122070](https://github.com/rust-lang/rust/pull/122070) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d009f60b55fe4527e7ddf122bc4520f351d7b9d4&end=4563f70c3b599411836e285591479f4a3d364708&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.4% | [0.4%, 0.4%] | 1 |
| Improvements ✅ <br /> (primary) | -1.6% | [-3.1%, -0.2%] | 113 |
| Improvements ✅ <br /> (secondary) | -1.5% | [-3.6%, -0.3%] | 37 |
| All ❌✅ (primary) | -1.6% | [-3.1%, -0.2%] | 113 |

* already marked as triaged by Jakub

Implement T-types suggested logic for perfect non-local impl detection [#122747](https://github.com/rust-lang/rust/pull/122747) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ea40fa210b87a322d2259852c149ffa212a3a0da&end=9d79cd5f79e75bd0d2083260271307ce9acd9081&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.3% | [0.6%, 4.4%] | 12 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.2%] | 9 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 1.2% | [-0.4%, 4.4%] | 21 |

* already marked as triaged by Urgau, with the comment "The perf regressions in diesel are due to the lint being triggered and producing nearly 300 warnings (with 155 actually shown)"
* (This reminds me of rustc-perf#1819)

Remove sharding for VecCache [#123556](https://github.com/rust-lang/rust/pull/123556) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=087ae978a13013800c8a484cf17c8951ab0b6b0c&end=af2525317be950fdae635bcbb46b3e755d14ab49&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.4%] | 1 |
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 1.0%] | 5 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.1%, -0.7%] | 4 |
| All ❌✅ (primary) | 0.4% | [0.4%, 0.4%] | 1 |

* already marked as triaged by simulacrum, presumably because this is likely noise since it is "just" removing the sharded type whose feature was not in use.

Use unchecked_sub in str indexing [#123561](https://github.com/rust-lang/rust/pull/123561) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=fc1a4c5cc9308c4b5980c64a73fd344a59c10601&end=4e431fad67b46c480f1833119cd368fa33df95f7&stat=instructions:u)

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

* this is an improvement to the code for `str::get_unchecked` when overflow checks are enabled; its calling a compiler-intrinsic directly now.
* it really doesn't make any sense that it caused any regression at all. (Perhaps this change is causing a change to inlining decisions, at least for cargo?)
* marking as triaged.

Rollup of 9 pull requests [#123645](https://github.com/rust-lang/rust/pull/123645) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=211518e5fb1336de6a4aab45dc1c05f5d83ce856&end=ab5bda1aa70f707014e2e691e43bc37a8819252a&stat=instructions:u)

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

* Looks like a temporary spike. Marking as triaged.

Only collect mono items from reachable blocks [#123272](https://github.com/rust-lang/rust/pull/123272) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=86b603cd792b3f6172ba4f676d7b586c1af7630a&end=bb78dba64ca4158ef2f3488d0d41a82c75a504f2&stat=instructions:u)

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

* fixed an important bug.
* from the comment history, it looks like the minor restricted regressions were anticipated.
* marking as triaged.