Skip to content

Add triage for 2023-01-03 #1509

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

Merged
merged 1 commit into from
Jan 4, 2023
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
121 changes: 121 additions & 0 deletions triage/2023-01-03.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# 2023-01-03 Triage Log

Fairly busy week with some massive performance improvements at the expense of some significant albeit smaller regressions. The main wins came in a long-standing PR from @cjgillot to enable encoding spans in metadata relative to their enclosing item. This causes more work in full compilation which causes some regressions up to 5% but can lead to very large wins in incremental compilation scenarios (up to ~70%). For example, the clap crate compiles 68% faster after a small 1 line change than it did previously.

Triage done by **@rylev**.
Revision range: [b38a6d..b43596](https://perf.rust-lang.org/?start=b38a6d373cb254697411147c0e49cd2e84864258&end=b435960c4cfd3975651c7051be56d7f5d6c201ab&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.6% | [0.3%, 4.6%] | 97 |
| Regressions ❌ <br /> (secondary) | 1.8% | [0.2%, 7.6%] | 60 |
| Improvements ✅ <br /> (primary) | -9.7% | [-68.7%, -0.2%] | 53 |
| Improvements ✅ <br /> (secondary) | -1.7% | [-15.3%, -0.1%] | 62 |
| All ❌✅ (primary) | -2.4% | [-68.7%, 4.6%] | 150 |

1 Regressions, 1 Improvements, 4 Mixed; 1 of them in rollups
47 artifact comparisons made in total

#### Regressions

Rollup of 8 pull requests [#106228](https://github.com/rust-lang/rust/pull/106228) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=270c94e484e19764a2832ef918c95224eb3f17c7&end=9709a438721d679572f0f7c389cd1fa60922fbef&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.6%] | 7 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.4% | [0.2%, 0.6%] | 7 |
- Kicked off a perf run on the only likely culprit. The regression is small enough that I don't think it's a big deal if we don't figure this out.

Allow trait method paths to satisfy const Fn bounds [#106210](https://github.com/rust-lang/rust/pull/106210) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=2c7536eaae59b44ff5259183ca98384129d69dd0&end=973a4db8d590d1ddfa4d07bd216aeffd28ba354e&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.5%] | 16 |
| Regressions ❌ <br /> (secondary) | 0.4% | [0.2%, 0.7%] | 16 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -1.4% | [-1.4%, -1.4%] | 1 |
| All ❌✅ (primary) | 0.3% | [0.2%, 0.5%] | 16 |
- Massive amount of noise reversed in [#106268](https://github.com/rust-lang/rust/pull/106268). No idea where this noise comes from.


#### Improvements


fix comment for `TokenCursor::desugar` [#106268](https://github.com/rust-lang/rust/pull/106268) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=808be91da07c91e4d6006cca0ad78385785a741e&end=e5e5fcb0b758fcf7f149cc9206155dcfa18ec909&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.5%, -0.2%] | 17 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.7%, -0.2%] | 15 |
| All ❌✅ (primary) | -0.3% | [-0.5%, -0.2%] | 17 |
- Massive amount of noise coming in [#106210](https://github.com/rust-lang/rust/pull/106210) reversed here. No idea where this noise comes from.


Abolish `QueryVTable` in favour of more assoc items on `QueryConfig` [#106307](https://github.com/rust-lang/rust/pull/106307) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=23b1cc197a3a6d0c1ab78a5c6e7cdad17681f1c9&end=d6f99e535a301a421dfee52a7c25bb4bdf420344&stat=instructions:u)

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


#### Mixed

Catch panics/unwinding in destruction of TLS values [#105426](https://github.com/rust-lang/rust/pull/105426) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f6cc345be403d481b4518e151218118c2a9eb4bb&end=7c991868c60a4afc1ee6334b912ea96061a2c98d&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.7% | [0.3%, 1.9%] | 11 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.6%, -0.2%] | 41 |
| Improvements ✅ <br /> (secondary) | -1.8% | [-14.6%, -0.2%] | 31 |
| All ❌✅ (primary) | -0.1% | [-0.6%, 1.9%] | 52 |
- Very much unsure where the performance changes are coming from. Some show LLVM regressions but not all. LLVM also does not explain the improvements.
- Ask [in the PR](https://github.com/rust-lang/rust/pull/105426#issuecomment-1370051574) for thoughts.


Use some more `const_eval_select` in pointer methods for compile times [#106275](https://github.com/rust-lang/rust/pull/106275) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=726bbfc8f0d6eb431dfa620c1c8fc3848020cd3c&end=dd01a163c1cff9461ddd85e7b3c2c0109657658f&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.4%] | 1 |
| Regressions ❌ <br /> (secondary) | 0.7% | [0.6%, 0.9%] | 2 |
| Improvements ✅ <br /> (primary) | -0.5% | [-0.9%, -0.2%] | 28 |
| Improvements ✅ <br /> (secondary) | -0.8% | [-0.8%, -0.7%] | 3 |
| All ❌✅ (primary) | -0.4% | [-0.9%, 0.4%] | 29 |
- Author investigated why this might have caused a regression in `regex` and found nothing. The gains much outweigh the regressions so we should be fine keeping it.


Encode spans relative to the enclosing item -- enable on nightly [#84762](https://github.com/rust-lang/rust/pull/84762) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f89003eda8917ff99f8ee3fb5c812310a58c014b&end=fb9dfa8ceffb985105be3176d7ed2f99515ea377&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:------:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.7% | [0.2%, 5.0%] | 108 |
| Regressions ❌ <br /> (secondary) | 1.8% | [0.2%, 7.6%] | 90 |
| Improvements ✅ <br /> (primary) | -20.0% | [-68.9%, -0.6%] | 25 |
| Improvements ✅ <br /> (secondary) | -2.7% | [-4.8%, -1.5%] | 11 |
| All ❌✅ (primary) | -2.4% | [-68.9%, 5.0%] | 133 |
- There was an [FCP](https://github.com/rust-lang/rust/pull/84762#issuecomment-1327518372) to decide whether the trade off of some regressions for some huge incremental performance wins was worth it.
- This FCP passed and so the PR was merged.


Reenable limited top-down MIR inlining [#106364](https://github.com/rust-lang/rust/pull/106364) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d6f99e535a301a421dfee52a7c25bb4bdf420344&end=67d16171d7eebc08057a46aeed887252e7cc286b&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.3%, 1.1%] | 4 |
| Regressions ❌ <br /> (secondary) | 0.6% | [0.5%, 0.6%] | 4 |
| Improvements ✅ <br /> (primary) | -0.6% | [-1.2%, -0.2%] | 9 |
| Improvements ✅ <br /> (secondary) | -0.9% | [-2.4%, -0.2%] | 18 |
| All ❌✅ (primary) | -0.2% | [-1.2%, 1.1%] | 13 |
- As pointed out [here](https://github.com/rust-lang/rust/pull/106364#issuecomment-1369111291) this was effectively a performance wash with regressions and improvements canceling themselves out.