Skip to content
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

Add triage for this week #1965

Merged
merged 1 commit into from
Aug 21, 2024
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
97 changes: 97 additions & 0 deletions triage/2024-08-19.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# 2024-08-19 Triage Log

A fairly noisy week (though most of that has been dropped from this report).
Overall we saw several improvements, and ended the week on a net positive.
Memory usage is down around 1.5-3% over the course of the week, primarily due
to [RawVec polymorphization](https://github.com/rust-lang/rust/pull/126793) and
[CloneToUninit impl expansion](https://github.com/rust-lang/rust/pull/126877).

Triage done by **@simulacrum**.
Revision range: [9cb1998e..4fe1e2bd](https://perf.rust-lang.org/?start=9cb1998ea15e179482504e07cad8fa121e169a32&end=4fe1e2bd5bf5a6f1cb245f161a5e9d315766f103&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 1.7%] | 124 |
| Regressions ❌ <br /> (secondary) | 0.5% | [0.1%, 1.4%] | 103 |
| Improvements ✅ <br /> (primary) | -1.3% | [-4.3%, -0.2%] | 50 |
| Improvements ✅ <br /> (secondary) | -1.4% | [-3.3%, -0.2%] | 15 |
| All ❌✅ (primary) | -0.1% | [-4.3%, 1.7%] | 174 |

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

#### Regressions

Fix problems with assoc expr token collection [#128725](https://github.com/rust-lang/rust/pull/128725) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4b7d074d766d00a32fa8b2e4fa3813dd7d4d64e6&end=be0ea0c99a00bc365d9f80f1c90f747967a2e152&stat=instructions:u)

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

Appears to be a real change in behavior (pre-merge showed fewer regressions)
but this is a correctness fix, so accepting them.

#### Improvements

Rework MIR inlining debuginfo so function parameters show up in debuggers. [#128861](https://github.com/rust-lang/rust/pull/128861) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=026e9ed3f0c5b3ee6233fd23b5e497cb94cf6434&end=3139ff09e9d07f7700f8d15ed25a231e29c43627&stat=instructions:u)

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

An improvement on many current benchmarks; the one regression is a spurious change.

#### Mixed

Apply "polymorphization at home" to RawVec [#126793](https://github.com/rust-lang/rust/pull/126793) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=41dd149fd6a6a06795fc6b9f54cb49af2f61775f&end=13f8a57cfb57d527abe797230beabb64758bb888&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 1.5%] | 89 |
| Regressions ❌ <br /> (secondary) | 0.4% | [0.1%, 1.3%] | 96 |
| Improvements ✅ <br /> (primary) | -1.1% | [-3.8%, -0.2%] | 54 |
| Improvements ✅ <br /> (secondary) | -1.2% | [-2.9%, -0.3%] | 13 |
| All ❌✅ (primary) | -0.1% | [-3.8%, 1.5%] | 143 |

An improvement on many current benchmarks, though some regressions. Overall a
net positive. Also reduced memory usage by about 1% for many of our benchmarks,
which is a great win!

Support reading thin archives in ArArchiveBuilder [#128936](https://github.com/rust-lang/rust/pull/128936) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=3139ff09e9d07f7700f8d15ed25a231e29c43627&end=d2b5aa6552c8acf67f38a2ad92062a32ec542f08&stat=instructions:u)

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

Regressions looks genuine, though relatively rare in our benchmark suite. It
might be worth doing some profiling of the Rust archive writer to see if
there's opportunities for optimization since it presumably hasn't received much
attention so far.

Rollup of 6 pull requests [#129202](https://github.com/rust-lang/rust/pull/129202) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9b318d2e93ce35e7ba32d8cfa96a1dbe63a7bed1&end=0f26ee4fd95a1c046582dfb18892f520788e2c2c&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.6% | [0.1%, 2.2%] | 5 |
| Improvements ✅ <br /> (primary) | -0.9% | [-1.7%, -0.3%] | 4 |
| Improvements ✅ <br /> (secondary) | -1.4% | [-2.2%, -0.6%] | 2 |
| All ❌✅ (primary) | -0.9% | [-1.7%, -0.3%] | 4 |

Regression on match-stress looks genuine but likely doesn't merit deeper
investigation given that this is in a rollup (and I don't see obvious
match-stress-implicating PRs in the list).
Loading