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.

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

Add perf triage for 2024-11-05 #2005

Merged
merged 2 commits into from
Nov 6, 2024
Merged
Changes from all commits
Commits
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
111 changes: 111 additions & 0 deletions triage/2024-11-05.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# 2024-11-05 Triage Log

A week dominated by one large improvement and one large regression where luckily the improvement had a larger impact. The regression seems to have been caused by a newly introduced lint that might have performance issues. The improvement was in building rustc with protected visibility which reduces the number of dynamic relocations needed leading to some nice performance gains. Across a large swath of the perf suite, the compiler is on average 1% faster after this week compared to last week.

Triage done by **@rylev**.
Revision range: [c8a8c820..27e38f8f](https://perf.rust-lang.org/?start=c8a8c82035439cb2404b8f24ca0bc18209d534ca&end=27e38f8fc7efc57b75e9a763d7a0ee44822cd5f7&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.8% | [0.1%, 2.0%] | 80 |
| Regressions ❌ <br /> (secondary) | 1.9% | [0.2%, 3.4%] | 45 |
| Improvements ✅ <br /> (primary) | -1.9% | [-31.6%, -0.1%] | 148 |
| Improvements ✅ <br /> (secondary) | -5.1% | [-27.8%, -0.1%] | 180 |
| All ❌✅ (primary) | -1.0% | [-31.6%, 2.0%] | 228 |


1 Regression, 1 Improvement, 5 Mixed; 3 of them in rollups
46 artifact comparisons made in total

#### Regressions

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

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 1.9%] | 104 |
| Regressions ❌ <br /> (secondary) | 1.5% | [0.1%, 4.4%] | 68 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.7% | [0.2%, 1.9%] | 104 |
- Fairly large regression caused by https://github.com/rust-lang/rust/pull/131984.
- Most likely due to a non-performant lint being run that wasn't previously.
- Checked with the author and reviewer.


#### Improvements

Use protected visibility when building rustc with LLD [#131634](https://github.com/rust-lang/rust/pull/131634) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a8e1186e3c14a54f7a38cc1183117dc7e77f4f82&end=9fa9ef385c0aad8f5d4c8f7d92dca474367943a3&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -1.8% | [-31.3%, -0.1%] | 174 |
| Improvements ✅ <br /> (secondary) | -4.2% | [-27.5%, -0.2%] | 224 |
| All ❌✅ (primary) | -1.8% | [-31.3%, -0.1%] | 174 |


#### Mixed

Rollup of 12 pull requests [#132317](https://github.com/rust-lang/rust/pull/132317) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c8a8c82035439cb2404b8f24ca0bc18209d534ca&end=2dece5bb62f234f5622a08289c5a3d1555cd7843&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.2% | [0.2%, 0.4%] | 4 |
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.2%] | 1 |
| Improvements ✅ <br /> (primary) | -0.2% | [-0.4%, -0.1%] | 6 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 |
| All ❌✅ (primary) | -0.0% | [-0.4%, 0.4%] | 10 |
- Looks like a bit more time spent in LLVM? Might have been caused by a few PRs in this rollup, but the total result is a wash, so I don't think it's needed to dig deeper into this.


Rc/Arc: don't leak the allocation if drop panics [#132231](https://github.com/rust-lang/rust/pull/132231) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=2dece5bb62f234f5622a08289c5a3d1555cd7843&end=e473783d90e2289b8a97575fa60d6315f0a318eb&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.6% | [0.6%, 0.6%] | 1 |
| Regressions ❌ <br /> (secondary) | 2.3% | [1.1%, 3.5%] | 6 |
| Improvements ✅ <br /> (primary) | -0.9% | [-1.4%, -0.3%] | 3 |
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.6%, -0.2%] | 5 |
| All ❌✅ (primary) | -0.5% | [-1.4%, 0.6%] | 4 |
- There probably isn't too much to be done regarding perf, all of the major regressions come from one small stress test crate.


Mark `simplify_aggregate_to_copy` mir-opt as unsound [#132356](https://github.com/rust-lang/rust/pull/132356) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=20c909ff9cdd88d33768a4ddb8952927a675b0ad&end=a0d98ff0e5b6e1f2c63fd26f68484792621b235c&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.7% | [0.3%, 1.9%] | 14 |
| Regressions ❌ <br /> (secondary) | 0.7% | [0.2%, 2.3%] | 16 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.3%] | 1 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.4%, -0.3%] | 5 |
| All ❌✅ (primary) | 0.7% | [-0.3%, 1.9%] | 15 |
- This deactivated an unsound MIR optimization until it is fixed.


add const_panic macro to make it easier to fall back to non-formatting panic in const [#132542](https://github.com/rust-lang/rust/pull/132542) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=7028d9318fadc20e5e3058d52e44d785d31a6aaa&end=e3a918ece026cec748fc64af5b4983095b46097e&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.8% | [0.2%, 1.8%] | 6 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.5% | [-0.9%, -0.1%] | 5 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.2% | [-0.9%, 1.8%] | 11 |
- Looks like most of the regressions are taking longer in LLVM related queries (e.g., LLVM_module_codegen_emit_obj) which doesn't make a ton of sense to me.


Rollup of 11 pull requests [#132626](https://github.com/rust-lang/rust/pull/132626) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=96477c55bcfa7121470ed225f602c1c639aeaa18&end=27e38f8fc7efc57b75e9a763d7a0ee44822cd5f7&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.6% | [0.5%, 0.9%] | 7 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 2 |
| All ❌✅ (primary) | - | - | 0 |
- It's not obvious to me which PR might be the culprit here and the regression is small and contained enough that I don't think it's worth investigating.

Loading