Skip to content

Commit e755e59

Browse files
Merge pull request #824 from rylev/triage-2021-01-12
Add triage-2021-01-12
2 parents 3df7408 + 7915de0 commit e755e59

File tree

1 file changed

+53
-0
lines changed

1 file changed

+53
-0
lines changed

triage/2021-01-12.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
2021-01-12 Triage Log
2+
3+
Overall, a positive albeit quiet week. The largest change came from the incremental compilation working group which delivered large gains in performance caused by [changes](https://github.com/rust-lang/rust/issues/76896) in how inlining is handled in debug mode. Unfortunately, these changes may be reversed due to concerns.
4+
5+
Triage done by **@rylev**.
6+
Revision range: [f4b9d32ef53c0629732ee131b640920ae12d1edb..b6b461652a9bebfb4ddabcae896ee7237cf0962a](https://perf.rust-lang.org/?start=f4b9d32ef53c0629732ee131b640920ae12d1edb&end=b6b461652a9bebfb4ddabcae896ee7237cf0962a&absolute=false&stat=instructions%3Au)
7+
8+
1 Regressions, 2 Improvements, 3 Mixed
9+
2 of them in rollups
10+
11+
#### Regressions
12+
13+
Rollup of 9 pull requests [#80867](https://github.com/rust-lang/rust/issues/80867)
14+
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=7a193921a024e910262ff90bfb028074fddf20d0&end=34628e5b533d35840b61c5db0665cf7633ed3c5a&stat=instructions:u) (up to 1.7% on `incr-full` builds of `syn-opt`)
15+
- Minor regressions in mostly incremental builds (both opt and debug).
16+
- It's hard to tell which PR was responsible but [#79968](https://github.com/rust-lang/rust/pull/79968/) seems to be the most likely. We will investigate.
17+
- There was a change in the `impl_trait_ref` which may give hints as to the root cause.
18+
19+
#### Improvements
20+
21+
Rollup of 9 pull requests [#80928](https://github.com/rust-lang/rust/issues/80928)
22+
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=fe531d5a5f1404281e3fb237daaf87b8180bd13d&end=04064416644eba7351b1a457c1de27d28a750c95&stat=instructions:u) (up to -2.2% on `full` builds of `match-stress-enum-check`)
23+
- Moderate gain in a stress test. It's hard to tell which PR might be responsible.
24+
25+
Do not query the HIR directly in `opt_associated_item`. [#80889](https://github.com/rust-lang/rust/issues/80889)
26+
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a2cd91ceb0f156cb442d75e12dc77c3d064cdde4&end=6526e5c772f2da07db745c94ca6bb0a591a39ba4&stat=instructions:u) (up to -1.8% on `incr-patched: println` builds of `webrender-wrench-check`)
27+
- Simplifies getting a `AssocItem` given a `DefId` by not actually querying the HIR.
28+
- The biggest impact was to the `check_mod_privacy` query
29+
30+
#### Mixed
31+
32+
Do not make local copies of inline fns in debug mode [#76896](https://github.com/rust-lang/rust/issues/76896)
33+
- Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=937f629535f38c655267f1ed21ce6830f592f5df&end=ddf2cc7f8eb34f1a63b491d6a52e3e8208393c09&stat=instructions:u) (up to -46.8% on `incr-patched: sparse set` builds of `regex-debug`)
34+
- Large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=937f629535f38c655267f1ed21ce6830f592f5df&end=ddf2cc7f8eb34f1a63b491d6a52e3e8208393c09&stat=instructions:u) (up to 6.0% on `incr-patched: static str 6144` builds of `issue-46449-debug`)
35+
- The performance was already analyzed by the incremental compilation working group [here](https://hackmd.io/xT9WrXgOQLarYRGrV8Hqog).
36+
- This change largely effects how LLVM performs when users compile in debug mode. The improvements were largely in real world crates while regressions were in synthetic benchmarks.
37+
- This change may be reverted [due to some concerns](https://github.com/rust-lang/rust/pull/76896#issuecomment-758097233).
38+
39+
ast: Remove some indirection layers from values in key-value attributes [#80441](https://github.com/rust-lang/rust/issues/80441)
40+
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=6184f23950fb4aa14884ce310d948dc6fca269a3&end=f30733adb9150a9b965a6af1650d5213bec8ddf6&stat=instructions:u) (up to 1.3% on `incr-patched: println` builds of `coercions-debug`)
41+
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=6184f23950fb4aa14884ce310d948dc6fca269a3&end=f30733adb9150a9b965a6af1650d5213bec8ddf6&stat=instructions:u) (up to -1.1% on `incr-unchanged` builds of `issue-46449-check`)
42+
- An attempt to address some of the perf lost in [here](https://perf.rust-lang.org/compare.html?start=1cc410710993d036730c11556039e40109f6ab41&end=58d2bad9f7ab0971495247b6c94978848760ca9d&stat=instructions:u) in [#78837](https://github.com/rust-lang/rust/pull/78837), but the changes don't seem to be directly correlated.
43+
- The original [perf run](https://perf.rust-lang.org/compare.html?start=76aca6659a0eb3f5696541d0be518530cabdd963&end=e3c34b4b68e99fdff4c3f40e35e9df57d62b0085) only yielded positive results but it seems this change may invalidate the gains from another change introduced after that perf run.
44+
45+
Serialize incr comp structures to file via fixed-size buffer [#80463](https://github.com/rust-lang/rust/issues/80463)
46+
- Large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=467f5e99a541db94235f0c173bdffc8aeb177522&end=8234db5bc7b122dd9e39d738c30bcae005a96568&stat=instructions:u) (up to -8.9% on `incr-unchanged` builds of `ctfe-stress-4-check`)
47+
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=467f5e99a541db94235f0c173bdffc8aeb177522&end=8234db5bc7b122dd9e39d738c30bcae005a96568&stat=instructions:u) (up to 3.1% on `incr-unchanged` builds of `clap-rs-check`)
48+
- Improvement is in synthetic benchmarks while regressions are largely in real world crates.
49+
- This change was done largely to reduce memory footprint size and the perf impact [was deemed worth it](https://github.com/rust-lang/rust/pull/80463#issuecomment-758120283)
50+
51+
#### Nags requiring follow up
52+
53+
- Follow up needs to happen on the regressions, especially on the rollup [#80928](https://github.com/rust-lang/rust/issues/80928) and [#80441](https://github.com/rust-lang/rust/issues/80441) where the performance regressed from an early perf run done while the PR was still open.

0 commit comments

Comments
 (0)