Skip to content

perf triage for this week. #1523

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 1 commit into from
Feb 7, 2023
Merged

perf triage for this week. #1523

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
248 changes: 248 additions & 0 deletions triage/2023-02-07.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
# 2023-02-07 Triage Log

Much noise in benchmarks this week, which makes it hard to tell what the real
improvements were and what they were due to. A query cache change (PR #107667)
is part of the story. In addition, much improvement was reaped from the change
to *not* deaggregate MIR (PR #107267). Finally, microoptimizing `fold_ty`
(PR #107627) yielded a small improvement to a broad set of benchmarks.

Triage done by **@pnkfelix**.
Revision range: [a64ef7d0..e4dd9edb](https://perf.rust-lang.org/?start=a64ef7d07d0411315be85a646586cb85eeb9c136&end=e4dd9edb76a34ecbca539967f9662b8c0cc9c7fb&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.4% | [0.3%, 18.8%] | 18 |
| Regressions ❌ <br /> (secondary) | 1.8% | [0.2%, 4.1%] | 21 |
| Improvements ✅ <br /> (primary) | -1.0% | [-3.2%, -0.3%] | 88 |
| Improvements ✅ <br /> (secondary) | -4.0% | [-13.1%, -0.1%] | 47 |
| All ❌✅ (primary) | -0.4% | [-3.2%, 18.8%] | 106 |


3 Regressions, 3 Improvements, 8 Mixed; 3 of them in rollups
41 artifact comparisons made in total

#### Regressions

Fix handling of items inside a `doc(hidden)` block [#107000](https://github.com/rust-lang/rust/pull/107000) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f3126500f25114ba4e0ac3e76694dd45a22de56d&end=6c991b07403a3234dd1ec0ac973b8ef97055e605&stat=instructions:u)

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

* a number of doc benchmarks regressed, but only doc benchmarks.
* the big hit was a 18.9% regression to doc on hyper.
* rustdoc developer says the issue cannot be resolved without *some* amount of regression, so marked as triaged.

don't point at nonexisting code beyond EOF when warning about delims [#107663](https://github.com/rust-lang/rust/pull/107663) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=319b88c463fe6f51bb6badbbd3bb97252a60f3a5&end=a67649675014546ce454d65bc8fe3ebd18e6a319&stat=instructions:u)

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

* already marked as triaged


Run `expand-yaml-anchors` in `x test tidy` [#107704](https://github.com/rust-lang/rust/pull/107704) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=dffea43fc1102bdfe16d88ed412c23d4f0f08d9d&end=e4dd9edb76a34ecbca539967f9662b8c0cc9c7fb&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.0% | [0.4%, 1.5%] | 3 |
| Regressions ❌ <br /> (secondary) | 3.7% | [3.4%, 4.3%] | 6 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 1.0% | [0.4%, 1.5%] | 3 |

* PR author says PR touched only CI code, and therefore this must be noise
* primary regressions are on cranelift variations; secondary are on keccak
* graphs for cranelift and keccak both have up-and-down swings that both begun with #107627; will check with nnethercote about that
* marked as triaged.

#### Improvements

emit `ConstEquate` in `TypeRelating<D>` [#107434](https://github.com/rust-lang/rust/pull/107434) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=50d3ba5bcbf5c7e13d4ce068d3339710701dd603&end=2a6ff729233c62d1d991da5ed4d01aa29e59d637&stat=instructions:u)

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


Recover form missing expression in `for` loop [#107526](https://github.com/rust-lang/rust/pull/107526) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a67649675014546ce454d65bc8fe3ebd18e6a319&end=75a0be98f25a4b9de5afa0e15eb016e7f9627032&stat=instructions:u)

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


Remove `OnHit` callback from query caches. [#107667](https://github.com/rust-lang/rust/pull/107667) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0c13c172507f01d921808107d2c4ec37b43b982d&end=e7813fee92c56621d08e8dbe83948d9f4a30a9ec&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.6% | [-1.7%, -0.2%] | 49 |
| Improvements ✅ <br /> (secondary) | -2.0% | [-5.9%, -0.3%] | 36 |
| All ❌✅ (primary) | -0.6% | [-1.7%, -0.2%] | 49 |

* as [already noted](https://github.com/rust-lang/rust/pull/107667#issuecomment-1419760041) by nnethercote, these results appear better than reality
* much of the delta is noise (namely inverse of #107627 (see below)).

#### Mixed

Don't generate unecessary `&&self.field` in deriving Debug [#107599](https://github.com/rust-lang/rust/pull/107599) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a94b9fd0ace1336a3dd93f51f1c0db6ca0fd7f92&end=9545094994f1ab45cab5799d5b45980871a9e97b&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.3%, 0.4%] | 2 |
| Regressions ❌ <br /> (secondary) | 1.0% | [0.2%, 4.9%] | 7 |
| Improvements ✅ <br /> (primary) | -1.0% | [-2.9%, -0.4%] | 9 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.8% | [-2.9%, 0.4%] | 11 |

* [already triaged](https://github.com/rust-lang/rust/pull/107599#issuecomment-1416528826) as noise

Rollup of 6 pull requests [#107642](https://github.com/rust-lang/rust/pull/107642) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9545094994f1ab45cab5799d5b45980871a9e97b&end=658fad6c5506f41c35b64fb1a22ceb0992697ff3&stat=instructions:u)

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

* sole regressions are to (secondary) deep-vector and match-stress.
* perf graph for deep-vector hints that this might not be noise, but it is also minor.
* marking as triaged.

Rollup of 8 pull requests [#107650](https://github.com/rust-lang/rust/pull/107650) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=658fad6c5506f41c35b64fb1a22ceb0992697ff3&end=886b2c3e005b153b3c8263f48193e0df7de0f5b3&stat=instructions:u)

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

* the vast bulk regressions are on six variations of secondary benchmarks match-stress
* and *all* of those appear to to have all been resolved by follow-on PR #107667 (see above)
* marking as triaged.

Do not deaggregate MIR [#107267](https://github.com/rust-lang/rust/pull/107267) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4aa6afa7f8a418a7dae5dbe4c95371d4f3bcc0e1&end=9dee4e4c42d23b0c5afd6d8fed025181f70fbe12&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 1.1%] | 51 |
| Regressions ❌ <br /> (secondary) | 0.8% | [0.1%, 2.0%] | 30 |
| Improvements ✅ <br /> (primary) | -1.4% | [-2.6%, -0.3%] | 25 |
| Improvements ✅ <br /> (secondary) | -6.0% | [-12.9%, -0.6%] | 27 |
| All ❌✅ (primary) | -0.1% | [-2.6%, 1.1%] | 76 |

* already marked as triaged (wins far outweigh losses)


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

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

* regressions are to secondary benchmark externs.
* whatever this regression was, it has since been more than recovered by #107667

Less import overhead for errors [#107679](https://github.com/rust-lang/rust/pull/107679) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=2a6ff729233c62d1d991da5ed4d01aa29e59d637&end=7f97aeaf73047268299ab55288b3dd886130be47&stat=instructions:u)

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

* already marked as triaged (its obvious noise)

rustdoc: change trait bound formatting [#102842](https://github.com/rust-lang/rust/pull/102842) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=7f97aeaf73047268299ab55288b3dd886130be47&end=319b88c463fe6f51bb6badbbd3bb97252a60f3a5&stat=instructions:u)

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

* already marked as triaged (and its noise)


Optimize `fold_ty` [#107627](https://github.com/rust-lang/rust/pull/107627) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=75a0be98f25a4b9de5afa0e15eb016e7f9627032&end=14ea63a7e005f9ca48bc13df6cb4fc5afe32febe&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.1% | [1.0%, 1.1%] | 2 |
| Regressions ❌ <br /> (secondary) | 2.3% | [0.6%, 4.2%] | 12 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.5%, -0.3%] | 18 |
| Improvements ✅ <br /> (secondary) | -0.7% | [-1.6%, -0.2%] | 30 |
| All ❌✅ (primary) | -0.3% | [-0.5%, 1.1%] | 20 |

* [already marked as triaged](https://github.com/rust-lang/rust/pull/107627#issuecomment-1419762439)

#### Untriaged Pull Requests

- [#107704 Run `expand-yaml-anchors` in `x test tidy`](https://github.com/rust-lang/rust/pull/107704)
- [#107672 Rollup of 3 pull requests](https://github.com/rust-lang/rust/pull/107672)
- [#107650 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/107650)
- [#107642 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/107642)
- [#107408 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/107408)
- [#107143 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/107143)
- [#107103 Use new solver in `evaluate_obligation` query (when new solver is enabled)](https://github.com/rust-lang/rust/pull/107103)
- [#107101 Filter param-env predicates for errors before calling `to_opt_poly_trait_pred`](https://github.com/rust-lang/rust/pull/107101)
- [#107000 Fix handling of items inside a `doc(hidden)` block](https://github.com/rust-lang/rust/pull/107000)
- [#106757 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/106757)
- [#105657 Guard ProjectionTy creation against passing the wrong number of substs](https://github.com/rust-lang/rust/pull/105657)
- [#105472 Make encode_info_for_trait_item use queries instead of accessing the HIR](https://github.com/rust-lang/rust/pull/105472)
- [#105426 Catch panics/unwinding in destruction of TLS values](https://github.com/rust-lang/rust/pull/105426)
- [#105378 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/105378)
- [#105323 Perform SimplifyLocals before ConstProp.](https://github.com/rust-lang/rust/pull/105323)
- [#105147 Allow unsafe through inline const](https://github.com/rust-lang/rust/pull/105147)
- [#104566 couple of clippy::perf fixes](https://github.com/rust-lang/rust/pull/104566)
- [#104533 Clean up and harden various methods around trait substs](https://github.com/rust-lang/rust/pull/104533)
- [#104017 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/104017)
- [#103998 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/103998)
- [#103975 Some tracing and comment cleanups](https://github.com/rust-lang/rust/pull/103975)
- [#103934 std: sync "Dependencies of the `backtrace` crate" with `backtrace`](https://github.com/rust-lang/rust/pull/103934)
- [#103880 Use non-ascribed type as field's type in mir](https://github.com/rust-lang/rust/pull/103880)
- [#103841 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/103841)
- [#103650 rustdoc: change `.src-line-numbers > span` to `.src-line-numbers > a`](https://github.com/rust-lang/rust/pull/103650)
- [#103562 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/103562)
- [#103439 Show note where the macro failed to match](https://github.com/rust-lang/rust/pull/103439)
- [#103295 ci: Bring back ninja for dist builders](https://github.com/rust-lang/rust/pull/103295)
- [#103071 Fix line numbers for MIR inlined code](https://github.com/rust-lang/rust/pull/103071)
- [#102975 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/102975)

#### Nags requiring follow up

TODO: Nags