Skip to content

Commit

Permalink
Merge pull request #1834 from Mark-Simulacrum/triage
Browse files Browse the repository at this point in the history
Add triage for this week
  • Loading branch information
Mark-Simulacrum committed Mar 5, 2024
2 parents 1e43daf + 35c0398 commit 9580e15
Showing 1 changed file with 191 additions and 0 deletions.
191 changes: 191 additions & 0 deletions triage/2024-03-05.md
@@ -0,0 +1,191 @@
# 2024-03-05 Triage Log

A bunch of noise this week which has been dropped from the report (but may be
present in the summary figures). As a result, the week is pretty busy in amount
of changes, but the net effect is nearly neutral to a slight regression for
most workloads.

Triage done by **@simulacrum**.
Revision range: [71ffdf7ff7ac6df5f9f64de7e780b8345797e8a0..41d97c8a5dea2731b0e56fe97cd7cb79e21cff79](https://perf.rust-lang.org/?start=71ffdf7ff7ac6df5f9f64de7e780b8345797e8a0&end=41d97c8a5dea2731b0e56fe97cd7cb79e21cff79&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.6% | [0.1%, 2.0%] | 136 |
| Regressions ❌ <br /> (secondary) | 0.8% | [0.2%, 2.6%] | 78 |
| Improvements ✅ <br /> (primary) | -0.6% | [-1.2%, -0.3%] | 9 |
| Improvements ✅ <br /> (secondary) | -0.6% | [-1.0%, -0.2%] | 14 |
| All ❌✅ (primary) | 0.6% | [-1.2%, 2.0%] | 145 |


2 Regressions, 0 Improvements, 10 Mixed; 4 of them in rollups
51 artifact comparisons made in total

#### Regressions

Weekly `cargo update` [#112865](https://github.com/rust-lang/rust/pull/112865) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0decdac390cfeedcd7f2f44c45f72c59c70d8143&end=da02fff3b6e4e27156054dcdda6675fe2a2591a6&stat=instructions:u)

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

Doesn't appear to be entirely noise (though some of the delta is likely due to
bimodality). However marking this regression as triaged since investigating is
likely to be painful and the regressions are predominantly in secondary
benchmarks.

Add a scheme for moving away from `extern "rust-intrinsic"` entirely [#120675](https://github.com/rust-lang/rust/pull/120675) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=50e77f133f8eb1f745e05681163a0143d6c4dd7d&end=2eeff462b762ed00f9d557d8c6ad7a3d562f692a&stat=instructions:u)

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

This regression is being addressed in a followup PR:
https://github.com/rust-lang/rust/pull/122010

#### Mixed

syms for legacy numeric constants diag items [#121667](https://github.com/rust-lang/rust/pull/121667) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=91cae1dcdcf1a31bd8a92e4a63793d65cfe289bb&end=1c28a2c1b0b82b525262e6ccc7675cab61ed040c&stat=instructions:u)

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

Regression seems likely to be noise (bimodality) or not significant enough to
investigate further. Marking as triaged. Some of the improvements seem genuine.

Diagnostic renaming [#121489](https://github.com/rust-lang/rust/pull/121489) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=bf9c7a64ad222b85397573668b39e6d1ab9f4a72&end=c475e2303b551d726307c646181e0677af1e0069&stat=instructions:u)

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

Regression is essentially guaranteed to be bimodality, not a real change. Marking as triaged.

Rollup of 10 pull requests [#121790](https://github.com/rust-lang/rust/pull/121790) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d3d145ea1cae47ad392173f890577788117da3d9&end=71a7b66f20c551f640f2f382bc7e7923ba0a5dab&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.4% | [-0.6%, -0.3%] | 2 |
| Improvements ✅ <br /> (secondary) | -1.9% | [-2.4%, -1.5%] | 2 |
| All ❌✅ (primary) | -0.1% | [-0.6%, 0.6%] | 3 |

The single primary benchmark regression (Cargo) change looks to be both above
any noise floor and does not show signs of reverting. However, it's not
significant enough (given limit to just Cargo) to investigate further.

Rollup of 7 pull requests [#121804](https://github.com/rust-lang/rust/pull/121804) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=384d26fc7e3bdd7687cc17b2662b091f6017ec2a&end=1a1876c9790f168fb51afa335a7ba3e6fc267d75&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.6%] | 7 |
| Regressions ❌ <br /> (secondary) | 1.8% | [1.7%, 1.9%] | 6 |
| Improvements ✅ <br /> (primary) | -0.8% | [-0.9%, -0.8%] | 4 |
| Improvements ✅ <br /> (secondary) | -0.5% | [-0.8%, -0.2%] | 8 |
| All ❌✅ (primary) | -0.1% | [-0.9%, 0.6%] | 11 |

Trying to track down the cause of the regression. Suspecting #121000 based on
the regressed benchmarks.

Combine `Sub` and `Equate` [#121462](https://github.com/rust-lang/rust/pull/121462) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=6cbf0926d54c80ea6d15df333be9281f65bbeb36&end=b0696a5160711c068cb1f01b7437db7990d15750&stat=instructions:u)

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

Future PRs may improve perf here, but this is targeting clean up and preparing for other refactoring first:

> Some ideas how to reduce the perf impact. Even if they don't completely
> remove it, this will allow significant improvements and cleanups going
> forward and generally simplifies the core type system. I would merge this
> even with these regressions.
(https://github.com/rust-lang/rust/pull/121462#issuecomment-1973282815)

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

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.9% | [0.5%, 3.3%] | 2 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.8% | [-1.1%, -0.5%] | 2 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.6% | [-1.1%, 3.3%] | 4 |

Cargo regression looks real but otherwise this looks like noise to me. I'm
going to mark as triaged, I don't think it merits digging through individual
PRs to try and isolate it.

Don't grab variances in `TypeRelating` relation if we're invariant [#121864](https://github.com/rust-lang/rust/pull/121864) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4cdd20584ccb75890d7d9bfae266054abfae5d46&end=2e3581bca93fdcce474e17cd43430b594a7955a0&stat=instructions:u)

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

Regression is likely real, but is in a secondary benchmark and incr-unchanged
scenario. Not worth further investigation, particularly given the improvements.

Always generate GEP i8 / ptradd for struct offsets [#121665](https://github.com/rust-lang/rust/pull/121665) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=516b6162a2ea8e66678c09e8243ebd83e4b8eeea&end=70aa0b86c066e721012852a9851fdf8586117823&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.4%] | 3 |
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 1.0%] | 12 |
| Improvements ✅ <br /> (primary) | -0.7% | [-1.6%, -0.3%] | 14 |
| Improvements ✅ <br /> (secondary) | -0.5% | [-1.0%, -0.3%] | 12 |
| All ❌✅ (primary) | -0.5% | [-1.6%, 0.4%] | 17 |

Changes are expected since this changed a lot of our codegen. In practice
though mostly not meaningful (i.e., some improvements, some regressions).

Rollup of 5 pull requests [#121955](https://github.com/rust-lang/rust/pull/121955) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=70aa0b86c066e721012852a9851fdf8586117823&end=89b78304e82dc5114e3b2faa0fbec747a28a2b37&stat=instructions:u)

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

Likely to be caused by a correctness fix: https://github.com/rust-lang/rust/pull/121955#issuecomment-1975993842

perf: improve write_fmt to handle simple strings [#121001](https://github.com/rust-lang/rust/pull/121001) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1547c076bfec8abb819d6a81e1e4095d267bd5b4&end=5a1e5449c8f4cb6b12b4f64238e3c058767ebf02&stat=instructions:u)

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

Improvements to runtime (or at least assembly quality), at the cost of a bit
more time in LLVM.

0 comments on commit 9580e15

Please sign in to comment.