Skip to content

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 2 commits into from
Dec 15, 2020
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
65 changes: 65 additions & 0 deletions triage/2020-12-15.md
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)
Copy link
Member

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

- 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.