-
Notifications
You must be signed in to change notification settings - Fork 153
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1509 from rylev/triage-2023-01-03
Add triage for 2023-01-03
- Loading branch information
Showing
1 changed file
with
121 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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. |