Skip to content

Commit e691ac3

Browse files
Adjustments and conclusion of triage
1 parent d9a27ad commit e691ac3

File tree

1 file changed

+35
-21
lines changed

1 file changed

+35
-21
lines changed

triage/2020-11-24.md

+35-21
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
1-
2020-11-24 Triage Log
1+
# 2020-11-24 Triage Log
22

3-
TODO: Summary
3+
This week saw landing of #79237 which by itself provides no wins but opens the
4+
door to support for split debuginfo on macOS. This'll eventually show huge wins
5+
as we can likely avoid re-collecting debuginfo while retaining support for
6+
lldb and Rust backtraces. #79361 tracks the stabilization of the rustc flag, but
7+
the precise rollout to stable users is not yet 100% clear.
48

5-
Triage done by **@jyn514**.
9+
Triage done by **@jyn514** and **@simulacrum**.
610
Revision range: [c919f490bbcd2b29b74016101f7ec71aaa24bdbb..25a691003cf6676259ee7d4bed05b43cb6283cea](https://perf.rust-lang.org/?start=c919f490bbcd2b29b74016101f7ec71aaa24bdbb&end=25a691003cf6676259ee7d4bed05b43cb6283cea&absolute=false&stat=instructions%3Au)
711

8-
5 Regressions, 5 Improvements, 4 Mixed
9-
??? of them in rollups
12+
4 regressions, 4 improvements, 2 mixed results.
13+
5 of them in rollups.
1014

1115
#### Regressions
1216

@@ -28,8 +32,8 @@ Revision range: [c919f490bbcd2b29b74016101f7ec71aaa24bdbb..25a691003cf6676259ee7
2832

2933
[#79273](https://github.com/rust-lang/rust/issues/79273): Rollup of 8 pull requests
3034
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=3adedb8f4c5bb71e9e8a21a047cf8ed121ce0e75&end=da384694807172f0ca40eca2e49a11688aba6e93&stat=instructions:u) (up to 1.8% on `full` builds of `coercions-debug`). **@Mark-Simulacrum** thinks this is a false positive, since there are no similar regressions in `-opt` or `-check` builds.
31-
<!--Most regressions are in LLVM/codegen, so likely due to [#79067](https://github.com/rust-lang/rust/pull/79067/): Refactor the abi handling code a bit.-->
3235
- Minor improvements in [instruction counts](https://perf.rust-lang.org/compare.html?start=3adedb8f4c5bb71e9e8a21a047cf8ed121ce0e75&end=da384694807172f0ca40eca2e49a11688aba6e93&stat=instructions:u) on `doc` builds (up to .4% on `unused-warnings-doc`). Likely due to [#79264](https://github.com/rust-lang/rust/pull/79264): Get rid of some doctree items.
36+
- Most regressions are in LLVM/codegen, so likely due to [#79067](https://github.com/rust-lang/rust/pull/79067/): Refactor the abi handling code a bit.
3337

3438
#### Improvements
3539

@@ -46,36 +50,46 @@ Revision range: [c919f490bbcd2b29b74016101f7ec71aaa24bdbb..25a691003cf6676259ee7
4650
[#78088](https://github.com/rust-lang/rust/issues/78088): Add lint for `panic!("{}")`
4751
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=4ec27e4b79891b0ebc2ad71a3c4ac94f67d93f93&end=74285eb3a83eac639f9c54ba8c4ccf9879b3b00a&stat=instructions:u) (up to -3.3% on `incr-full` builds of `futures-opt`)
4852
- The improvement is likely because the desugaring of `panic!` changed.
49-
- Not related to performance triage, but this is the second PR I've seen removing clippy lints altogether instead of going through the deprecation process. Maybe **@rust-lang/clippy** should get pinged automatically on changes to `src/tools/clippy`?
50-
51-
<!-- automated reports start here -->
52-
53-
[#79275](https://github.com/rust-lang/rust/issues/79275)
54-
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=8ca930aa262c04a898cf64155e40a6de3ec9ba9e&end=20328b532336017213ccb4095740955d81060ebc&stat=instructions:u) (up to -1.1% on `incr-unchanged` builds of `deeply-nested-async-opt`)
5553

5654
[#78343](https://github.com/rust-lang/rust/issues/78343)
5755
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=d9a105fdd46c926ae606777a46dd90e5b838f92f&end=f32a0cce2fd5aaf5f361192af59cf1f2afa5f0ac&stat=instructions:u) (up to -3.0% on `incr-full` builds of `wg-grammar-opt`)
56+
- The improvement is likely because the way `panic!` is expanded changed.
57+
58+
[#79319](https://github.com/rust-lang/rust/issues/79319)
59+
- Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a0d664bae6ca79c54cc054aa2403198e105190a2&end=32da90b431919eedb3e281a91caea063ba4edb77&stat=instructions:u) (up to -26.4% on `incr-patched: println` builds of `cargo-opt`)
60+
- Predominantly incremental perf getting better, likely due to
61+
#77697 Split each iterator adapter and source into individual modules
62+
which presumably shuffled CGU ordering in core/std, avoiding multiple LLVM
63+
module invalidations.
5864

5965
#### Mixed
6066

6167
[#78461](https://github.com/rust-lang/rust/issues/78461)
6268
- Very large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=da384694807172f0ca40eca2e49a11688aba6e93&end=a1a13b2bc4fa6370b9501135d97c5fe0bc401894&stat=instructions:u) (up to 36.6% on `incr-patched: println` builds of `clap-rs-debug`)
6369
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=da384694807172f0ca40eca2e49a11688aba6e93&end=a1a13b2bc4fa6370b9501135d97c5fe0bc401894&stat=instructions:u) (up to -2.6% on `incr-patched: Compiler new` builds of `regex-opt`)
64-
65-
[#79219](https://github.com/rust-lang/rust/issues/79219)
66-
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=c643dd2ec8fed2852f5eee8f776d657293a6a8f2&end=a0d664bae6ca79c54cc054aa2403198e105190a2&stat=instructions:u) (up to 2.3% on `incr-patched: println` builds of `syn-opt`)
67-
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=c643dd2ec8fed2852f5eee8f776d657293a6a8f2&end=a0d664bae6ca79c54cc054aa2403198e105190a2&stat=instructions:u) (up to -1.0% on `incr-unchanged` builds of `deeply-nested-async-opt`)
68-
69-
[#79319](https://github.com/rust-lang/rust/issues/79319)
70-
- Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a0d664bae6ca79c54cc054aa2403198e105190a2&end=32da90b431919eedb3e281a91caea063ba4edb77&stat=instructions:u) (up to -26.4% on `incr-patched: println` builds of `cargo-opt`)
71-
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=a0d664bae6ca79c54cc054aa2403198e105190a2&end=32da90b431919eedb3e281a91caea063ba4edb77&stat=instructions:u) (up to 3.5% on `full` builds of `cranelift-codegen-opt`)
70+
- Pretty much limited to just incremental builds, likely the addition of
71+
allocators to Vec is causing some problems in incremental caching.
72+
Potentially worth tracking down the specific cause.
7273

7374
[#79186](https://github.com/rust-lang/rust/issues/79186)
7475
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=068320b39e3e4839d832b3aa71fa910ba170673b&end=40cf72108edb9b8633a9d284b238988309204494&stat=instructions:u) (up to -4.5% on `full` builds of `regression-31157-opt`)
7576
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=068320b39e3e4839d832b3aa71fa910ba170673b&end=40cf72108edb9b8633a9d284b238988309204494&stat=instructions:u) (up to 4.4% on `full` builds of `deeply-nested-async-check`)
77+
- Seems to largely be an improvement due to less queries being run in some
78+
cases, but there is some upfront cost -- presumably the regressed test case
79+
didn't end up calling/using the now less needed queries, but paid the price in
80+
metadata decoding.
7681

7782
#### Nags requiring follow up
7883

7984
- [Left a comment](https://github.com/rust-lang/rust/pull/79167#issuecomment-733207145) nagging the author of the `LD_PRELOAD` PR.
8085
- [Left a comment](https://github.com/rust-lang/rust/pull/79273#issuecomment-733224830) asking why a codegen refactor could have regressed instruction count.
81-
- [Left a comment](https://github.com/rust-lang/rust/pull/79220#issuecomment-733248754) noting that we should keep an eye on performance for when the MIR validation is re-enabled.
86+
87+
#### Compiler team notes
88+
89+
* https://github.com/rust-lang/rust/pull/78461 regressed incremental performance
90+
on debug builds of clap (interestingly, not opt builds). It may be worth
91+
investigating why, as the pattern of adding a generic parameter with a default
92+
really should not be causing regressions in downstream code. Not all of the
93+
regression is in LLVM.
94+
See by-query breakdown:
95+
https://perf.rust-lang.org/detailed-query.html?commit=a1a13b2bc4fa6370b9501135d97c5fe0bc401894&base_commit=da384694807172f0ca40eca2e49a11688aba6e93&benchmark=clap-rs-debug&run_name=incr-patched:%20println.

0 commit comments

Comments
 (0)