-
Notifications
You must be signed in to change notification settings - Fork 156
Add triage for 2021-10-19 #1076
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
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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,96 @@ | ||
| # 2021-10-19 Triage Log | ||
|
|
||
| A week where improvements outweigh regressions. The highlight of the week is the change to split out LLVM profile guided optimization (PGO) and using clang 13 to compile LLVM which led to improvements in many real world crates (e.g., cargo) in the range of 10%. Most regressions were limited and at most in the less than 1% range. We are seeing more performance changes in rollups which are supposed to be performance neutral. We'll have to decide how to best address this. | ||
|
|
||
| Triage done by **@rylev**. | ||
| Revision range: [9475e609b8458fff9e444934a6017d2e590642cf..d45ed7502ad225739270a368528725930f54b7b6](https://perf.rust-lang.org/?start=9475e609b8458fff9e444934a6017d2e590642cf&end=d45ed7502ad225739270a368528725930f54b7b6&absolute=false&stat=instructions%3Au) | ||
|
|
||
| 3 Regressions, 4 Improvements, 2 Mixed; 2 of them in rollups | ||
| 34 comparisons made in total | ||
|
|
||
| #### Regressions | ||
|
|
||
| Rollup of 6 pull requests [#89858](https://github.com/rust-lang/rust/issues/89858) | ||
| - Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=dfc5add915e8bf4accbb7cf4de00351a7c6126a1&end=8c852bc15a058022c9d4175e8ed60da628960800&stat=instructions:u) (up to 0.7% on `incr-unchanged` builds of `cranelift-codegen`) | ||
| - This rollup is the kind that is very hard to diagnose because none of the individual PRs seem to be risky. | ||
| - Left a [comment](https://github.com/rust-lang/rust/pull/89858#issuecomment-947476063) about possible causes and a call out for more investigation. | ||
|
|
||
|
|
||
| add `slice::swap_unchecked` [#88540](https://github.com/rust-lang/rust/issues/88540) | ||
| - Small regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=72d66064e77281536588189a916af28a1819b313&end=1dafe6d1c328d2f0580763e8438a227e490deb10&stat=instructions:u) (up to 1.5% on `full` builds of `piston-image`) | ||
| - Seems like we're doing more bounds checking than is necessary when using `slice::swap` though this should only happen when debug assertions are on which is not the case in the compiler. | ||
| - It is interesting that this has predominately impacted full build scenarios, but it's not clear why this would be the case. | ||
| - It's already been suggested to revert this, and I've left a [comment](https://github.com/rust-lang/rust/pull/88540#issuecomment-946736079) as such in the PR. | ||
|
|
||
|
|
||
| Associated consts sidebar [#89815](https://github.com/rust-lang/rust/issues/89815) | ||
| - Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=8c852bc15a058022c9d4175e8ed60da628960800&end=7807a694c2f079fd3f395821bcc357eee8650071&stat=instructions:u) (up to 0.9% on `full` builds of `many-assoc-items`) | ||
| - Expected since we're doing more work and the benchmark in question is explicitly meant to test the worst case in this scenario. | ||
|
|
||
|
|
||
| #### Improvements | ||
|
|
||
| Remove textual span from diagnostic string [#89555](https://github.com/rust-lang/rust/issues/89555) | ||
| - Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=eeb16a2a892c2a29b1da3085e29f39efa3486e1c&end=dfc5add915e8bf4accbb7cf4de00351a7c6126a1&stat=instructions:u) (up to -0.9% on `full` builds of `diesel`) | ||
|
|
||
|
|
||
| polymorphization: shims and predicates [#89514](https://github.com/rust-lang/rust/issues/89514) | ||
| - Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=1d6f24210c4a8f46f9781a56f819a383e590cccf&end=6f53ddfa74ac3c10ceb63ad4a7a9c95e55853c87&stat=instructions:u) (up to -0.8% on `full` builds of `deeply-nested`) | ||
|
|
||
|
|
||
| Split out LLVM PGO step and use clang 13 to compile LLVM [#89499](https://github.com/rust-lang/rust/issues/89499) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=1f12ac87296ac61ec002e0243e7ad5a50364da35&end=5e02151318ddd431aea6d58e23948246c1446044&stat=instructions:u) (up to -13.6% on `incr-patched: println` builds of `cargo`) | ||
|
|
||
|
|
||
| Revert "Auto merge of #89709 - clemenswasser:apply_clippy_suggestions… [#89905](https://github.com/rust-lang/rust/issues/89905) | ||
| - Large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=ec724ac0753b7538668308a6aa5b78980b2931bb&end=cd8b56f528631b128f36605b28ae06e36377dc68&stat=instructions:u) (up to -3.3% on `full` builds of `inflate`) | ||
| - This directly addresses and improves the performance lost in [#89709](https://github.com/rust-lang/rust/pull/89709). | ||
|
|
||
|
|
||
| #### Mixed | ||
|
|
||
| Rollup of 10 pull requests [#89939](https://github.com/rust-lang/rust/issues/89939) | ||
| - Large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=6cc0a764e082d9c0abcf37a768d5889247ba13e2&end=7fbd4ce2768744b3bd2ddf8453b73f4f18dbe5bc&stat=instructions:u) (up to -1.7% on `full` builds of `inflate`) | ||
| - Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=6cc0a764e082d9c0abcf37a768d5889247ba13e2&end=7fbd4ce2768744b3bd2ddf8453b73f4f18dbe5bc&stat=instructions:u) (up to 1.0% on `full` builds of `diesel`) | ||
| - No obvious place where the regressions or improvements are coming from. | ||
| - Most likely culprit is [#89915](https://github.com/rust-lang/rust/pull/89915) as most other PRs in the rollup seem to be pretty low risk. | ||
| - Left a [comment](https://github.com/rust-lang/rust/pull/89939#issuecomment-946747553) saying as much. | ||
|
|
||
|
|
||
| Index and hash HIR as part of lowering [#89124](https://github.com/rust-lang/rust/issues/89124) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=5dab47dcd8267b8769421b46532414ec36d625e3&end=bd41e09da334697c0f993b36685cb599061d9faa&stat=instructions:u) (up to -5.4% on `full` builds of `cranelift-codegen`) | ||
| - Very large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=5dab47dcd8267b8769421b46532414ec36d625e3&end=bd41e09da334697c0f993b36685cb599061d9faa&stat=instructions:u) (up to 5.3% on `full` builds of `unused-warnings`) | ||
| - Definitely looks related to the change in question (`unused-warnings` shows larges increases in HIR lowering). | ||
| - Left a [comment](https://github.com/rust-lang/rust/pull/89124#issuecomment-947444364) asking for clarification of next steps. | ||
|
|
||
|
|
||
| #### Untriaged Pull Requests | ||
|
|
||
| - [#89939 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/89939) | ||
| - [#89858 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/89858) | ||
| - [#89695 Move top part of print_item to Tera templates](https://github.com/rust-lang/rust/pull/89695) | ||
| - [#89608 Rollup of 12 pull requests](https://github.com/rust-lang/rust/pull/89608) | ||
| - [#89534 Introduce `tcx.get_diagnostic_name`](https://github.com/rust-lang/rust/pull/89534) | ||
| - [#89495 Add two inline annotations for hot functions](https://github.com/rust-lang/rust/pull/89495) | ||
| - [#89435 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/89435) | ||
| - [#89405 Fix clippy lints](https://github.com/rust-lang/rust/pull/89405) | ||
| - [#89263 Suggest both of immutable and mutable trait implementations](https://github.com/rust-lang/rust/pull/89263) | ||
| - [#89165 Fix read_to_end to not grow an exact size buffer](https://github.com/rust-lang/rust/pull/89165) | ||
| - [#89125 Don't use projection cache or candidate cache in intercrate mode](https://github.com/rust-lang/rust/pull/89125) | ||
| - [#89124 Index and hash HIR as part of lowering](https://github.com/rust-lang/rust/pull/89124) | ||
| - [#89103 Migrate in-tree crates to 2021](https://github.com/rust-lang/rust/pull/89103) | ||
| - [#89047 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/89047) | ||
| - [#89030 Introduce `Rvalue::ShallowInitBox`](https://github.com/rust-lang/rust/pull/89030) | ||
| - [#88945 Remove concept of 'completion' from the projection cache](https://github.com/rust-lang/rust/pull/88945) | ||
| - [#88880 Rework HIR API to make invocations of the hir_crate query harder.](https://github.com/rust-lang/rust/pull/88880) | ||
| - [#88824 Rollup of 15 pull requests](https://github.com/rust-lang/rust/pull/88824) | ||
| - [#88804 Revise never type fallback algorithm](https://github.com/rust-lang/rust/pull/88804) | ||
| - [#88719 Point at argument instead of call for their obligations](https://github.com/rust-lang/rust/pull/88719) | ||
| - [#88703 Gather module items after lowering.](https://github.com/rust-lang/rust/pull/88703) | ||
| - [#88627 Do not preallocate HirIds](https://github.com/rust-lang/rust/pull/88627) | ||
| - [#88575 Querify `FnAbi::of_{fn_ptr,instance}` as `fn_abi_of_{fn_ptr,instance}`.](https://github.com/rust-lang/rust/pull/88575) | ||
| - [#88540 add `slice::swap_unchecked`](https://github.com/rust-lang/rust/pull/88540) | ||
| - [#88308 Morph `layout_raw` query into `layout_of`.](https://github.com/rust-lang/rust/pull/88308) | ||
| - [#87781 Remove box syntax from compiler and tools](https://github.com/rust-lang/rust/pull/87781) | ||
| - [#87064 Support `#[track_caller]` on closures and generators](https://github.com/rust-lang/rust/pull/87064) | ||
| - [#83302 Get piece unchecked in `write`](https://github.com/rust-lang/rust/pull/83302) | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW the splitting out of LLVM PGO is performance-neutral, it's purely the bump of bootstrap C++ compiler to clang 13 that brings the large wins here.