Skip to content

Commit

Permalink
Merge pull request #1735 from rust-lang/triage-2023-10-18
Browse files Browse the repository at this point in the history 
Triage 2023 10 18
  • Loading branch information
@Kobzol
Kobzol authored Oct 19, 2023
2 parents 04ef4d1 + 420012f commit 6cfbab3
Showing 1 changed file with 198 additions and 0 deletions.
198 changes: 198 additions & 0 deletions triage/2023-10-18.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
# 2023-10-18 Triage Log

Overall an interesting week performance wise, with small improvements to a vast
number of benchmarks seeming to outweigh an isolated set of (slightly) larger
regressions. It included a number of PRs regressed instruction counts but did
not matter for cycle times, plus one mysterious regression to `check_match` and
`mir_borrowck` from reworking constructor splitting (see report on PR 116391 for
details), and an awesome broad set of improvements from automatically inlining
small functions across crates (see report on PR 116505 for details).

Triage done by **@pnkfelix**.
Revision range: [84d44dd1..b9832e72](https://perf.rust-lang.org/?start=84d44dd1d8ec1e98fff94272ba4f96b2a1f044ca&end=b9832e72c9223f4e96049aa5911effd258b92591&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 3.0% | [0.3%, 12.2%] | 7 |
| Regressions ❌ <br /> (secondary) | 0.7% | [0.3%, 1.2%] | 15 |
| Improvements ✅ <br /> (primary) | -1.1% | [-17.9%, -0.2%] | 131 |
| Improvements ✅ <br /> (secondary) | -2.4% | [-39.6%, -0.2%] | 121 |
| All ❌✅ (primary) | -0.9% | [-17.9%, 12.2%] | 138 |


4 Regressions, 1 Improvements, 4 Mixed; 3 of them in rollups
84 artifact comparisons made in total

#### Regressions

Rollup of 7 pull requests [#116605](https://github.com/rust-lang/rust/pull/116605) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5b88d659f8c2428536589d4bd36b9099d53a6815&end=c30b28bdc17f1da73515afa0886f0d4f55c76e1f&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.6%] | 7 |
| Regressions ❌ <br /> (secondary) | 0.3% | [0.3%, 0.4%] | 3 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.4% | [0.2%, 0.6%] | 7 |

* solely rustdoc regression
* believed to be caused by [PR 109422](https://github.com/rust-lang/rust/pull/109422) "rustdoc-search: add impl disambiguator to duplicate assoc items"
* already marked as triaged

Optimize `librustc_driver.so` with BOLT [#116352](https://github.com/rust-lang/rust/pull/116352) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ee8c9d3c34719a129f280cd91ba5d324017bb02b&end=c543b6f3516767150af84d94c14a27b19d4b0291&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.3% | [0.2%, 5.7%] | 10 |
| Regressions ❌ <br /> (secondary) | 1.9% | [0.3%, 5.0%] | 60 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 4 |
| All ❌✅ (primary) | 2.3% | [0.2%, 5.7%] | 10 |

* primary instruction-count regressions were restricted to helloworld and html5ever
* As noted in comment by Kobzol, the instruction counts regressed for many benchmarks, but the [cycle counts](https://perf.rust-lang.org/compare.html?start=ee8c9d3c34719a129f280cd91ba5d324017bb02b&end=c543b6f3516767150af84d94c14a27b19d4b0291&stat=cycles:u) solely improved, significantly so, and bootstrap time improved (628.052s -> 623.517s (-0.72%)).
* already marked as triaged

Rollup of 3 pull requests [#116742](https://github.com/rust-lang/rust/pull/116742) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c543b6f3516767150af84d94c14a27b19d4b0291&end=e292fec36880f48101bda4054be37097312e73c0&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.3%, 0.4%] | 3 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.3% | [0.3%, 0.4%] | 3 |

* Regressions are solely to bitmaps full scenarios.
* Looks like a blip (i.e. noise) based on the graph over time.
* marking as triaged.

don't UB on dangling ptr deref, instead check inbounds on projections [#114330](https://github.com/rust-lang/rust/pull/114330) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a00c09e9d80b763fb29206b47b04e1d99c3ace96&end=e7bdc5f9f869219e8d20060b42a09ea10a837851&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.7% | [0.5%, 1.0%] | 17 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | - | - | 0 |

* From skimming the PR, one can see that the PR author (RalfJung) iterated on this to identify a solution that would minimize regressions.
* As noted by the PR author, only secondary benchmarks were affected.
* Also, while instruction-counts regressed, the [cycle-counts](https://perf.rust-lang.org/compare.html?start=a00c09e9d80b763fb29206b47b04e1d99c3ace96&end=e7bdc5f9f869219e8d20060b42a09ea10a837851&stat=cycles%3Au)
did not, at least not enough to pass our noise threshold.
* marking as triaged.

#### Improvements

optimize zipping over array iterators [#115515](https://github.com/rust-lang/rust/pull/115515) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e292fec36880f48101bda4054be37097312e73c0&end=0d410be23c45e2f3567a6ec35985f690473f9176&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.2%] | 3 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.3% | [-0.4%, -0.2%] | 3 |

* A small win from a PR addressing user-filed performance regression, namely [issue #115339](https://github.com/rust-lang/rust/issues/115339), "Performance regression of array::IntoIter vs slice::Iter"

#### Mixed

Also consider call and yield as MIR SSA. [#113915](https://github.com/rust-lang/rust/pull/113915) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c30b28bdc17f1da73515afa0886f0d4f55c76e1f&end=d627cf07ce46d230a93732a4714d16f00df9466b&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 3.9% | [3.9%, 3.9%] | 1 |
| Regressions ❌ <br /> (secondary) | 0.1% | [0.1%, 0.1%] | 2 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.9%, -0.2%] | 26 |
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.6%, -0.3%] | 5 |
| All ❌✅ (primary) | -0.2% | [-0.9%, 3.9%] | 27 |

* The try perf run had sole primary regression of unicode-normalization-0.1.19 opt-full (1.19%), while the perf run against master had sole primary regression of exa-0.10.1 opt-full (3.90%).
* The exa regression has persisted forward (i.e. it is not transient noise).
* It was already been marked as triaged, as the performance changes were deemed a wash, apart from object code sizes which saw "small but clear" improvement.

Rollup of 5 pull requests [#116640](https://github.com/rust-lang/rust/pull/116640) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c1691db366c0f2e2341c60377c248ca2d9335076&end=475c71da0710fd1d40c046f9cee04b733b5b2b51&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 1.1% | [1.1%, 1.1%] | 1 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.4%, -0.2%] | 4 |
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.5%, -0.4%] | 6 |
| All ❌✅ (primary) | -0.3% | [-0.4%, -0.2%] | 4 |

* sole regression was to secondary benchmark coercions debug incr-patched: add static arr item
* Looks like a blip (i.e. noise) based on the graph over time.
* marking as triaged

exhaustiveness: Rework constructor splitting [#116391](https://github.com/rust-lang/rust/pull/116391) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=df4379b4eb5357263f0cf75475953f9b5c48c31f&end=e20cb7702117f1ad8127a16406ba9edd230c4f65&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.2% | [0.2%, 0.3%] | 4 |
| Regressions ❌ <br /> (secondary) | 3.9% | [0.5%, 5.8%] | 9 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.4%] | 1 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.1% | [-0.4%, 0.3%] | 5 |

* the primary regressions were to cranelift-codegen-0.82.1 and cargo-0.60.0 in various incremental settings (mostly check builds)
* the large (>5%) secondary regressions are all to match-stress.
* the above cases were regressions for instruction-counts, but the cycle-counts didn't get marked as regressed in *any* of the same cases.
* in all cases, the performance loss from these regressions was subsequently recovered (or masked) by [PR 116505](https://github.com/rust-lang/rust/pull/116505) "Automatically enable cross-crate inlining for small functions".
(I don't know if that's actually related or just an awesome change that bought so much performance that it masked this problem).
* Since the match-stress one was relatively large, I looked at the
self-profile results in the [details](https://perf.rust-lang.org/detailed-query.html?commit=e20cb7702117f1ad8127a16406ba9edd230c4f65&benchmark=match-stress-check&scenario=full&base_commit=df4379b4eb5357263f0cf75475953f9b5c48c31f)
which indicates a change in the delta(time) for match-stress might be due to new overheads in `check_match` and `mir_borrowck`.
* But this is strange; I cannot tell how this PR could have affected codegen, which would be the only way I could imagine those functions being impacted.
* Not marking as triaged for now; this mystery might be worth looking into a bit more. (But then again, the only significant regression was to a secondary stress test, so maybe its not worth spending time on.)

Automatically enable cross-crate inlining for small functions [#116505](https://github.com/rust-lang/rust/pull/116505) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ca89f732ec0f910fc92111a45dd7e6829baa9d4b&end=5d5edf0248d967baa6ac5cbea09b91c7c9947942&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:---------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.3% | [0.3%, 13.0%] | 8 |
| Regressions ❌ <br /> (secondary) | 0.5% | [0.2%, 0.8%] | 2 |
| Improvements ✅ <br /> (primary) | -1.2% | [-18.1%, -0.1%] | 148 |
| Improvements ✅ <br /> (secondary) | -2.2% | [-39.8%, -0.2%] | 209 |
| All ❌✅ (primary) | -1.0% | [-18.1%, 13.0%] | 156 |

* Already marked as triaged
* This was clearly awesome and amazing (all the more amazing if you review the history)
* 'Nuff said.

#### Untriaged Pull Requests

- [#116742 Rollup of 3 pull requests](https://github.com/rust-lang/rust/pull/116742)
- [#116640 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/116640)
- [#116492 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/116492)
- [#116391 exhaustiveness: Rework constructor splitting](https://github.com/rust-lang/rust/pull/116391)
- [#116183 Always preserve DebugInfo in DeadStoreElimination.](https://github.com/rust-lang/rust/pull/116183)
- [#115762 Explain revealing of opaque types in layout_of ParamEnv](https://github.com/rust-lang/rust/pull/115762)
- [#115751 some inspect improvements](https://github.com/rust-lang/rust/pull/115751)
- [#115740 Cache reachable_set on disk](https://github.com/rust-lang/rust/pull/115740)
- [#115252 Represent MIR composite debuginfo as projections instead of aggregates](https://github.com/rust-lang/rust/pull/115252)
- [#115082 Fix races conditions with `SyntaxContext` decoding](https://github.com/rust-lang/rust/pull/115082)
- [#115025 Make subtyping explicit in MIR](https://github.com/rust-lang/rust/pull/115025)
- [#114892 Remove conditional use of `Sharded` from query caches](https://github.com/rust-lang/rust/pull/114892)
- [#114481 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/114481)
- [#114459 Do not run ConstProp on mir_for_ctfe.](https://github.com/rust-lang/rust/pull/114459)
- [#114330 don't UB on dangling ptr deref, instead check inbounds on projections](https://github.com/rust-lang/rust/pull/114330)
- [#114321 get auto traits for parallel rustc](https://github.com/rust-lang/rust/pull/114321)
- [#114023 Warn on inductive cycle in coherence leading to impls being considered not overlapping](https://github.com/rust-lang/rust/pull/114023)
- [#114004 Add `riscv64gc-unknown-hermit` target](https://github.com/rust-lang/rust/pull/114004)
- [#113858 Always const-prop scalars and scalar pairs](https://github.com/rust-lang/rust/pull/113858)
- [#113758 Turn copy into moves during DSE.](https://github.com/rust-lang/rust/pull/113758)
- [#113485 Bump version to 1.73](https://github.com/rust-lang/rust/pull/113485)
- [#113370 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/113370)
- [#113320 Add some extra information to opaque type cycle errors](https://github.com/rust-lang/rust/pull/113320)
- [#113306 Update debuginfo test runner to provide more useful output](https://github.com/rust-lang/rust/pull/113306)
- [#113304 Upgrade to indexmap 2.0.0](https://github.com/rust-lang/rust/pull/113304)
- [#113270 perform TokenStream replacement in-place when possible in expand_macro](https://github.com/rust-lang/rust/pull/113270)
- [#113057 Rollup of 2 pull requests](https://github.com/rust-lang/rust/pull/113057)
- [#112963 Stop bubbling out hidden types from the eval obligation queries](https://github.com/rust-lang/rust/pull/112963)
- [#112882 Rewrite `UnDerefer`](https://github.com/rust-lang/rust/pull/112882)
- [#112420 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/112420)

0 comments on commit 6cfbab3

Please sign in to comment.