Skip to content

Add triage for this week #1912

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 1 commit into from
May 27, 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
153 changes: 153 additions & 0 deletions triage/2024-05-27.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
# 2024-05-27 Triage Log

A relatively quiet week, with few large changes, the largest driven by further
increasing the scope of unsafe precondition checking.

Triage done by **@simulacrum**.
Revision range: [1d0e4afd..a59072ec](https://perf.rust-lang.org/?start=1d0e4afd4cac09078e12a232508c3e9f8d42535d&end=a59072ec4fb6824213df5e9de8cae4812fd4fe97&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.6% | [0.2%, 6.3%] | 84 |
| Regressions ❌ <br /> (secondary) | 0.9% | [0.1%, 3.8%] | 66 |
| Improvements ✅ <br /> (primary) | -0.4% | [-2.3%, -0.2%] | 37 |
| Improvements ✅ <br /> (secondary) | -1.7% | [-3.8%, -0.2%] | 22 |
| All ❌✅ (primary) | 1.0% | [-2.3%, 6.3%] | 121 |

2 Regressions, 3 Improvements, 5 Mixed; 3 of them in rollups
51 artifact comparisons made in total

#### Regressions

Rewrite native thread-local storage [#116123](https://github.com/rust-lang/rust/pull/116123) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ed172dbbaf1c702b99da54554b33b3fe65021da9&end=9c8a58fdb895204cb19eeb97472a78caa1c57c19&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 1.2%] | 10 |
| Regressions ❌ <br /> (secondary) | 1.0% | [0.7%, 1.6%] | 9 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.4% | [0.2%, 1.2%] | 10 |

Regressions deemed worth the overall change's contents (see
https://github.com/rust-lang/rust/pull/116123#issuecomment-2009408868). The TLS
state is now stored in a single thread-local object rather than two, which
should decrease costs of addressing it in general (modulo LLVM difficulties).

Rollup of 6 pull requests [#125463](https://github.com/rust-lang/rust/pull/125463) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=78dd504f2fd87c0cfabff7d9174253411caf2f80&end=7601adcc764d42c9f2984082b49948af652df986&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.4% | [1.1%, 2.0%] | 8 |
| Regressions ❌ <br /> (secondary) | 1.0% | [0.4%, 1.8%] | 23 |
| Improvements ✅ <br /> (primary) | -0.5% | [-0.5%, -0.5%] | 1 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 |
| All ❌✅ (primary) | 1.2% | [-0.5%, 2.0%] | 9 |

Likely related to fixing an issue around LLD discovery, see
https://github.com/rust-lang/rust/pull/125463#issuecomment-2129069901. Bugfix
warrants spending a bit more time.

#### Improvements

Move the checks for Arguments constructors to inline const [#125518](https://github.com/rust-lang/rust/pull/125518) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0a59f113629aafb6e5ee55ad04a2d451a11d8466&end=75e2c5dcd0ddce0fe0eb3d4a2195cd51073c729b&stat=instructions:u)

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


Remove `DefId` from `EarlyParamRegion` [#125468](https://github.com/rust-lang/rust/pull/125468) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cdc509f7c09361466d543fc8311ce7066b10cc4f&end=fec98b3bbc94b54a0b3085d004708aabcc48081a&stat=instructions:u)

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


[perf] Delay the construction of early lint diag structs [#125410](https://github.com/rust-lang/rust/pull/125410) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=fec98b3bbc94b54a0b3085d004708aabcc48081a&end=b582f807fae230b22ac126ff1d8a13262bb099ba&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.5% | [-0.7%, -0.3%] | 12 |
| Improvements ✅ <br /> (secondary) | -2.0% | [-2.8%, -1.2%] | 13 |
| All ❌✅ (primary) | -0.5% | [-0.7%, -0.3%] | 12 |


#### Mixed

Follow-up fixes to `report_return_mismatched_types` [#123812](https://github.com/rust-lang/rust/pull/123812) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1d0e4afd4cac09078e12a232508c3e9f8d42535d&end=e8753914580fb42554a79a4b5c5fb4cc98933231&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.1% | [0.6%, 1.8%] | 3 |
| Regressions ❌ <br /> (secondary) | 0.2% | [0.1%, 0.2%] | 6 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -1.1% | [-1.1%, -1.1%] | 2 |
| All ❌✅ (primary) | 1.1% | [0.6%, 1.8%] | 3 |

Seems to be limited primarily to one scenario in regex, with lots of new
metadata decoding. Asked for follow-up by PR author.

Make early lints translatable [#124417](https://github.com/rust-lang/rust/pull/124417) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=39e02f1bd1e53d009da382654139f7c0639172a8&end=791adf759cc065316f054961875052d5bc03e16c&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.5% | [0.2%, 6.3%] | 66 |
| Regressions ❌ <br /> (secondary) | 0.4% | [0.3%, 0.5%] | 7 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.7% | [-1.1%, -0.6%] | 12 |
| All ❌✅ (primary) | 1.5% | [0.2%, 6.3%] | 66 |

Regressions are partially mitigated by #125410 (see earlier in the report).
Further follow-up is expected to investigate closing remaining gap
(https://github.com/rust-lang/rust/pull/124417#issuecomment-2126056523).

Panic directly in Arguments::new* instead of recursing [#117804](https://github.com/rust-lang/rust/pull/117804) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9c8a58fdb895204cb19eeb97472a78caa1c57c19&end=606afbb617a2949a4e35c4b0258ff94c980b9451&stat=instructions:u)

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

Regressions are likely to be inliner noise, not meaningful changes.

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

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

Seems plausible that the regression is due to #124389 (since it affected derive
macros). But also seems not worth further investigation given it's a secondary
benchmark and minimal impact. Marked as triaged.

Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods [#121571](https://github.com/rust-lang/rust/pull/121571) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=14562ddf8c4302a52c72c9c71f5be6516fec5537&end=48f00110d0dae38b3046a9ac05d20ea321fd6637&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 1.5% | [0.2%, 3.4%] | 27 |
| Regressions ❌ <br /> (secondary) | 1.8% | [0.4%, 3.8%] | 5 |
| Improvements ✅ <br /> (primary) | -0.9% | [-2.5%, -0.3%] | 5 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 1.1% | [-2.5%, 3.4%] | 32 |

Regressions are likely expected as we're lowering more code that's late-removed.
Loading