-
Notifications
You must be signed in to change notification settings - Fork 156
Add triage-2020-12-15 #811
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
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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,65 @@ | ||
2020-12-15 Triage Log | ||
|
||
A week dominated by small regressions with only 1 modest yet clear performance gain. None of the regressions are large enough to cause concern, but there should be a followup to some to ensure that those regressions are at least examined. | ||
|
||
Triage done by **@rylevick**. | ||
Revision range: [4fd4a98d4788bc987d7f7add9df5f5ead6a1c15e..e1cce06e4ff5206daf397e1dcf91ed53653be171](https://perf.rust-lang.org/?start=4fd4a98d4788bc987d7f7add9df5f5ead6a1c15e&end=e1cce06e4ff5206daf397e1dcf91ed53653be171&absolute=false&stat=instructions%3Au) | ||
|
||
6 Regressions, 1 Improvements, 2 Mixed | ||
0 of them in rollups | ||
|
||
#### Regressions | ||
|
||
Also generate `StorageDead` in constants[#78679](https://github.com/rust-lang/rust/issues/78679) | ||
- Large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=c0bfe3485f97f267cc8adec724f109c56dab5526&end=cc03ee6702053ded253c3656cbd02f0bfdf25c73&stat=instructions:u) (up to 5.7% on `incr-patched: new row` builds of `tuple-stress-check`) | ||
- A removal of special casing of not marking statics and constants as `StorageDead` inside rustc_mir. | ||
- This was regression was [deemed as acceptable](https://github.com/rust-lang/rust/pull/78679#issuecomment-729030782) due to being more correct than the previous implementation. | ||
|
||
Properly re-use def path hash in incremental mode[#79721](https://github.com/rust-lang/rust/issues/79721) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=cc03ee6702053ded253c3656cbd02f0bfdf25c73&end=fa55f668e5ea5388ec98b9340969527252239151&stat=instructions:u) (up to 1.5% on `incr-unchanged` builds of `deeply-nested-opt`) | ||
- Fixes a p-high [ICE issue](https://github.com/rust-lang/rust/issues/79661), and so the perf regressions were deemed acceptable. | ||
- This is a fix for [#74967](https://github.com/rust-lang/rust/pull/74967) which saw [large perf gains](https://perf.rust-lang.org/compare.html?start=db79d2f63780613e700cb58b4339c48287555ae0&end=bf8e95436e60effbeb46a32e17df8ab7fcb0c6ad). | ||
|
||
Accept arbitrary expressions in key-value attributes at parse time[#78837](https://github.com/rust-lang/rust/issues/78837) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=1cc410710993d036730c11556039e40109f6ab41&end=58d2bad9f7ab0971495247b6c94978848760ca9d&stat=instructions:u) (up to 1.4% on `incr-unchanged` builds of `match-stress-exhaustive_patterns-check`) | ||
- [Pinged in the PR about this issue](https://github.com/rust-lang/rust/pull/78837#issuecomment-745380762) | ||
|
||
Capture precise paths in THIR and MIR[#79553](https://github.com/rust-lang/rust/issues/79553) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=9eb3a7ceafd1e2c1924177caa18c7cc0c25b413e&end=5bd9b60333b3dc0a51e7a5607cd1e0d537a9f718&stat=instructions:u) (up to 4.5% on `incr-unchanged` builds of `clap-rs-check`) | ||
- While this change powers a feature behind a feature flag (`capture_disjoint_fields`), it looks like it's still causing perf regressions in workloads not using this feature. | ||
- [Pinged in the PR about this issue](https://github.com/rust-lang/rust/pull/79553#issuecomment-745435558) | ||
|
||
Create `rustc_type_ir`[#79169](https://github.com/rust-lang/rust/issues/79169) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=c3ed6681ff8d446e68ce272be4bf66f4145f6e29&end=3f2088aa603d2cd3f43c20795872de9cd6ec7735&stat=instructions:u) (up to 3.1% on `full` builds of `ctfe-stress-4-check`) | ||
* This mainly seems to be moving code around so it might be an inlining issue. | ||
* [Pinged in the PR about this issue](https://github.com/rust-lang/rust/pull/79169#issuecomment-745388674) | ||
|
||
Update stdarch submodule[#79938](https://github.com/rust-lang/rust/issues/79938) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=fa416394275d2468d104b8f72ac31b1ddf7ee52e&end=8b3ee82eb68cb35030bb745c23f8aa76d9de5bee&stat=instructions:u) (up to 1.4% on `incr-unchanged` builds of `deeply-nested-debug`) | ||
- This was a wholesale update of the stdarch submodule. | ||
* [Pinged in the PR about this issue](https://github.com/rust-lang/rust/pull/79938#issuecomment-745393740) | ||
|
||
#### Improvements | ||
|
||
Compress RWU from at least 32 bits to 4 bits[#79727](https://github.com/rust-lang/rust/issues/79727) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=5e6e1e33a11d140a4d70f946730137f241224eb3&end=1700ca07c6dd7becff85678409a5df6ad4cf4f47&stat=instructions:u) (up to -4.8% on `full` builds of `clap-rs-check`) | ||
- This was explicitly an experiment to gain performance, and it seems to worked fairly well. Other bit representations were tested but the one chosen was the most efficient. | ||
|
||
#### Mixed | ||
|
||
Use `def_path_hash_to_def_id` when re-using a `RawDefId`[#79915](https://github.com/rust-lang/rust/issues/79915) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a2e29d67c26bdf8f278c98ee02d6cc77a279ed2e&end=19eb1c4c526071c430c05fffc64da71ac057a3d5&stat=instructions:u) (up to -3.5% on `incr-unchanged` builds of `clap-rs-check`) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=a2e29d67c26bdf8f278c98ee02d6cc77a279ed2e&end=19eb1c4c526071c430c05fffc64da71ac057a3d5&stat=instructions:u) (up to 2.7% on `incr-patched: dummy fn` builds of `unused-warnings-check`) | ||
- The amount of regressions outweighs the improvements (which were just in the clap benchmark). | ||
- This is a followup fix to [#79721](https://github.com/rust-lang/rust/issues/79721). Overall these regressions still represent a perf gain when compared to before the changes introduced in [#74967](https://github.com/rust-lang/rust/pull/74967). | ||
|
||
Lower `discriminant_value` intrinsic[#79922](https://github.com/rust-lang/rust/issues/79922) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=8b3ee82eb68cb35030bb745c23f8aa76d9de5bee&end=5d77fc8d0db3b69f3a3691d86eba23e4cdc390e1&stat=instructions:u) (up to -3.9% on `full` builds of `match-stress-enum-check`) | ||
- Smaller regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=8b3ee82eb68cb35030bb745c23f8aa76d9de5bee&end=5d77fc8d0db3b69f3a3691d86eba23e4cdc390e1&stat=instructions:u) (up to 1.3% on `incr-unchanged` builds of `clap-rs-check`) | ||
- The improvement outweighs the regression. | ||
|
||
#### Nags requiring follow up | ||
|
||
- Several regressions need followup investigations. See their respective entries above for the issue. | ||
- As mentioned last week, stdarch expansion causing a 40% libcore compile time regression is still not | ||
resolved, and resolution is unclear. |
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.
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.
Note: this PR was also made to reduce the max-rss with some success: perf run from the PR and e.g. a 85% reduction in this issue happening in real code