Skip to content

triage report for this week. #1508

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.

Sign up for GitHub

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 1 commit into from
Dec 27, 2022
Merged

triage report for this week. #1508

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
140 changes: 140 additions & 0 deletions triage/2022-12-27.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# 2022-12-27 Triage Log

A light week with few performance changes, apart from one PR that added some
necessary extra work to rustdoc and so we observed a corresponding hit to some
doc benchmarks.

Triage done by **@pnkfelix**.
Revision range: [8a746f4a..b38a6d37](https://perf.rust-lang.org/?start=8a746f4ac3a489efb724cde813607f3b96c2df7b&end=b38a6d373cb254697411147c0e49cd2e84864258&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.8% | [0.2%, 18.4%] | 14 |
| Regressions ❌ <br /> (secondary) | 1.3% | [0.2%, 2.6%] | 24 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.7% | [-1.1%, -0.3%] | 10 |
| All ❌✅ (primary) | 2.8% | [0.2%, 18.4%] | 14 |


3 Regressions, 2 Improvements, 1 Mixed; 1 of them in rollups
44 artifact comparisons made in total

#### Regressions

Rollup of 7 pull requests [#105951](https://github.com/rust-lang/rust/pull/105951) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8a746f4ac3a489efb724cde813607f3b96c2df7b&end=65bd2a6a73d6a74fb1266a1d96b23de8810a5fb2&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.1% | [1.0%, 1.2%] | 2 |
| Regressions ❌ <br /> (secondary) | 2.3% | [2.1%, 2.7%] | 6 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 1.1% | [1.0%, 1.2%] | 2 |

* [Already triaged; "these benchmarks are currently noisy"](https://github.com/rust-lang/rust/pull/105951#issuecomment-1359970022)

Fix impl block in const expr [#104889](https://github.com/rust-lang/rust/pull/104889) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=75f4ee8b4427278d7a35b7025ea72e02c55ae8f1&end=cce9e72c55994335f8d1dac892cca755b65c8f43&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:-------------:|:-----:|
| Regressions ❌ <br /> (primary) | 4.9% | [0.5%, 18.4%] | 7 |
| Regressions ❌ <br /> (secondary) | 1.6% | [1.5%, 1.6%] | 3 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 4.9% | [0.5%, 18.4%] | 7 |

* Regressions are all to rustdoc benchmarks; these were all expected, because this PR is inherently making rustdoc do extra work that it should have been doing all along (IIUC).
* Marked as triaged.

Stop promoting all the things [#105085](https://github.com/rust-lang/rust/pull/105085) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8766bbdc30a297aaa249193f5513fb261ccef17c&end=f5c3dfdbbf06d5997079ac7339de5002f7ced2a3&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.9% | [0.8%, 1.0%] | 3 |
| Regressions ❌ <br /> (secondary) | 1.6% | [0.9%, 4.1%] | 6 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.9% | [0.8%, 1.0%] | 3 |

* The three primary regressions are all to unicode-normalization {check-full, debug-full, check-incr-full}, by 1% or less.
* The secondary regressions are to coercions-debug-full (by 4%), and to variations of ucd (by 1.25% or less).
* In my judgement, the semantic problem this addresses more than offsets the cost that this PR is paying. Marking as triaged.

#### Improvements

Implement va_list and va_arg for s390x FFI [#105381](https://github.com/rust-lang/rust/pull/105381) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=65bd2a6a73d6a74fb1266a1d96b23de8810a5fb2&end=d6da428f343ab811b2b132364360ba13ff05830c&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -1.1% | [-1.2%, -1.0%] | 2 |
| Improvements ✅ <br /> (secondary) | -2.3% | [-2.6%, -2.0%] | 6 |
| All ❌✅ (primary) | -1.1% | [-1.2%, -1.0%] | 2 |

* I'm assuming this is due to [measurement bias](https://users.cs.northwestern.edu/~robby/courses/322-2013-spring/mytkowicz-wrong-data.pdf) or other kinds of noise.

Allow .. to be parsed as let initializer [#105701](https://github.com/rust-lang/rust/pull/105701) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d9ee0f468f8d07e92da94fe991db91e95822d721&end=300aa907a682dfa492f4eb394d27f5331fba0a64&stat=instructions:u)

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

* I'm assuming this is either noise or [measurement bias](https://users.cs.northwestern.edu/~robby/courses/322-2013-spring/mytkowicz-wrong-data.pdf)

#### Mixed

Use `DepKind` instead of `&'static str` in `QueryStackFrame` [#105550](https://github.com/rust-lang/rust/pull/105550) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=62cc86924520f23091976655dab93b54a4c5ba21&end=c2ff8ad035deebde575235db310eb27afb3af7a8&stat=instructions:u)

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

* The primary effects here are to various incremental scenarios, by relatively small amounts in either direction.
* The PR reviewer already has [posted some notes](https://github.com/rust-lang/rust/pull/105550#issuecomment-1364347419) summarizing their investigation into the performance delta.
* This PR is adding a type parameter to several methods that did not have one before, which I expect to change code layout of the compiler itself.
* i.e. I would expect this kind of change to have broad but shallow effects on rustc performance, which is consistent with the timer report.
* Marking as triaged.

#### Untriaged Pull Requests

- [#105657 Guard ProjectionTy creation against passing the wrong number of substs](https://github.com/rust-lang/rust/pull/105657)
- [#105550 Use `DepKind` instead of `&'static str` in `QueryStackFrame`](https://github.com/rust-lang/rust/pull/105550)
- [#105472 Make encode_info_for_trait_item use queries instead of accessing the HIR](https://github.com/rust-lang/rust/pull/105472)
- [#105378 Rollup of 9 pull requests](https://github.com/rust-lang/rust/pull/105378)
- [#105147 Allow unsafe through inline const](https://github.com/rust-lang/rust/pull/105147)
- [#105085 Stop promoting all the things](https://github.com/rust-lang/rust/pull/105085)
- [#104889 Fix impl block in const expr](https://github.com/rust-lang/rust/pull/104889)
- [#104566 couple of clippy::perf fixes](https://github.com/rust-lang/rust/pull/104566)
- [#104533 Clean up and harden various methods around trait substs](https://github.com/rust-lang/rust/pull/104533)
- [#104017 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/104017)
- [#103998 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/103998)
- [#103975 Some tracing and comment cleanups](https://github.com/rust-lang/rust/pull/103975)
- [#103934 std: sync "Dependencies of the `backtrace` crate" with `backtrace`](https://github.com/rust-lang/rust/pull/103934)
- [#103880 Use non-ascribed type as field's type in mir](https://github.com/rust-lang/rust/pull/103880)
- [#103841 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/103841)
- [#103650 rustdoc: change `.src-line-numbers > span` to `.src-line-numbers > a`](https://github.com/rust-lang/rust/pull/103650)
- [#103562 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/103562)
- [#103439 Show note where the macro failed to match](https://github.com/rust-lang/rust/pull/103439)
- [#103295 ci: Bring back ninja for dist builders](https://github.com/rust-lang/rust/pull/103295)
- [#103071 Fix line numbers for MIR inlined code](https://github.com/rust-lang/rust/pull/103071)
- [#102975 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/102975)
- [#102915 Rollup of 7 pull requests](https://github.com/rust-lang/rust/pull/102915)
- [#102895 Get rid of `rustc_query_description!`](https://github.com/rust-lang/rust/pull/102895)
- [#102867 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/102867)
- [#102809 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/102809)
- [#102570 Perform simple scalar replacement of aggregates (SROA) MIR opt](https://github.com/rust-lang/rust/pull/102570)
- [#102548 Mark Cell::replace() as #[inline]](https://github.com/rust-lang/rust/pull/102548)
- [#102026 Populate effective visibilities in 'rustc_resolve'](https://github.com/rust-lang/rust/pull/102026)
- [#101858 derive various impls instead of hand-rolling them](https://github.com/rust-lang/rust/pull/101858)
- [#101857 change `FnMutDelegate` to trait objects](https://github.com/rust-lang/rust/pull/101857)