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.

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

Add triage for 2023-10-26 #1737

Merged
merged 2 commits into from
Oct 31, 2023
Merged

Add triage for 2023-10-26 #1737

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
837dff5
Add triage for 2023-10-26
rylev Oct 27, 2023
789ec59
Add a few comments from review
Mark-Simulacrum Oct 31, 2023
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
227 changes: 227 additions & 0 deletions triage/2023-10-26.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
# 2023-10-26 Triage Log

A largely positive week with really only 1 true regression and 1 large improvement both by cjgillot proving that the compiler giveth and taketh away. The improvement, an improvement in the borrow checker, lead to a 0.7% improvement across ~80 benchmarks while the regression had the same magnitude but only impacted 18 benchmarks.

Triage done by **@rylev**.
Revision range: [b9832e7..698db85](https://perf.rust-lang.org/?start=b9832e72c9223f4e96049aa5911effd258b92591&end=698db856de0b67313ddcb96b6599598058489ea9&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 3.4%] | 17 |
| Regressions ❌ <br /> (secondary) | 1.2% | [0.9%, 1.7%] | 11 |
| Improvements ✅ <br /> (primary) | -0.8% | [-3.0%, -0.1%] | 85 |
| Improvements ✅ <br /> (secondary) | -1.0% | [-1.8%, -0.1%] | 31 |
| All ❌✅ (primary) | -0.6% | [-3.0%, 3.4%] | 102 |


7 Regressions, 6 Improvements, 4 Mixed; 5 of them in rollups
85 artifact comparisons made in total

#### Regressions

Bump `COINDUCTIVE_OVERLAP_IN_COHERENCE` to deny + warn in deps [#116493](https://github.com/rust-lang/rust/pull/116493) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cc705b801236d064260bb67b3a0a25e4747fa7ec&end=0039d739d40a076334e111488946441378d11cd7&stat=instructions:u)

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


Rollup of 7 pull requests [#116923](https://github.com/rust-lang/rust/pull/116923) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=020d00867a0129fe77d02fb7f07d6bc96a7033ff&end=36b61e5aa593cc60cc92fc548b0d351e414aee2a&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.3% | [0.2%, 0.4%] | 9 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
- While this very much does not look like noise, the regressions are small enough that I don't think this is really worth any investigation.


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

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.7% | [0.2%, 1.2%] | 18 |
| Regressions ❌ <br /> (secondary) | 1.0% | [0.7%, 1.2%] | 9 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -1.8% | [-1.8%, -1.8%] | 1 |
| All ❌✅ (primary) | 0.7% | [0.2%, 1.2%] | 18 |
- From lqd "I think this is all in mir transforms, likely the additional pass name calls and string finds in turn -- and since is all static data, maybe should be cached, and stored in a better-suited data structure."
- [#116896](https://github.com/rust-lang/rust/pull/116896) is the culprit. Following up with the author, cjgillot.


Lint `non_exhaustive_omitted_patterns` by columns [#116734](https://github.com/rust-lang/rust/pull/116734) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=6f97d838c644174b38413d920ed8d43b70cdc7db&end=786c94a4ebf8df6207a3ce146a687e66f3ced369&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 1.5% | [1.5%, 1.6%] | 7 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
- Given that the impacted benchmark is the stress test for the code in question, I think the small regression is fine.


Suggest removing redundant arguments in format!() [#115324](https://github.com/rust-lang/rust/pull/115324) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=913ceaee96581232cbb8eedb43224f442e0c5287&end=111adde7ed86b848e61d1bd3cb431f9c52584317&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:----:|:------------:|:-----:|
| Regressions ❌ <br /> (primary) | 2.1% | [0.3%, 3.9%] | 2 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 2.1% | [0.3%, 3.9%] | 2 |
- The perf regression is likely noise.


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

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


Improve android-ndk property interface [#116998](https://github.com/rust-lang/rust/pull/116998) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cd674d61790607dfb6faa9d754bd3adfa13aea7c&end=f1a5ce19f5aa0cf61ed7b9f75b30e610befeed72&stat=instructions:u)

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


#### Improvements

Panic when the global allocator tries to register a TLS destructor [#116402](https://github.com/rust-lang/rust/pull/116402) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=0039d739d40a076334e111488946441378d11cd7&end=89432aadcb3174a0d7611557fa9b1ef05c72b920&stat=instructions:u)

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


On Windows make `read_dir` error on the empty path [#116606](https://github.com/rust-lang/rust/pull/116606) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=62fae2305e5f3a959bd6ad6c20608c118e93648a&end=aec4741d42e10673e7d5fe0f1c96d407801884e8&stat=instructions:u)
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved

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

Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved
This is noise.

Separate move path tracking between borrowck and drop elaboration. [#116300](https://github.com/rust-lang/rust/pull/116300) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f654229c27267334023a22233795b88b75fc340e&end=cd674d61790607dfb6faa9d754bd3adfa13aea7c&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.2% | [0.1%, 0.2%] | 5 |
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.2%] | 2 |
| Improvements ✅ <br /> (primary) | -0.7% | [-3.6%, -0.2%] | 77 |
| Improvements ✅ <br /> (secondary) | -1.1% | [-1.8%, -0.4%] | 30 |
| All ❌✅ (primary) | -0.7% | [-3.6%, 0.2%] | 82 |


time: use clock_gettime on macos [#116238](https://github.com/rust-lang/rust/pull/116238) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f1a5ce19f5aa0cf61ed7b9f75b30e610befeed72&end=e918db897df677d9bbda64d82ebbb8f1bc9e557f&stat=instructions:u)
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved

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

Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved
This is noise.

Uplift `ClauseKind` and `PredicateKind` into `rustc_type_ir` [#116993](https://github.com/rust-lang/rust/pull/116993) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=2e4e2a8f288f642cafcc41fff211955ceddc453d&end=98108dc26cb3f73f29c07b03ab75540545f3b481&stat=instructions:u)

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


Refactor type visitor walking [#117076](https://github.com/rust-lang/rust/pull/117076) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c716f180e8c48b12bb055e6a7dea2a7f474cf66d&end=c2ef35161fc7477b38f2e556be2fd6d85d9f4905&stat=instructions:u)

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


#### Mixed

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

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.5%, 0.5%] | 1 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.5%, -0.2%] | 9 |
| Improvements ✅ <br /> (secondary) | -0.6% | [-0.9%, -0.3%] | 13 |
| All ❌✅ (primary) | -0.2% | [-0.5%, 0.5%] | 10 |
- Improvements outweigh regressions.


Update hashbrown to 0.14.2 [#116956](https://github.com/rust-lang/rust/pull/116956) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9e3f784eb2c7c847b6c3578b373c0e0bc9233ca3&end=cc3dce5bd07057818f3f69c3ce93bfd596fd8f0f&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.5%] | 7 |
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.3%] | 3 |
| Improvements ✅ <br /> (primary) | -0.6% | [-1.2%, -0.2%] | 5 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.0% | [-1.2%, 0.5%] | 12 |
- Mostly neutral - max-rss regression seems to have corrected itself in the meantime


report `unused_import` for empty reexports even it is pub [#116033](https://github.com/rust-lang/rust/pull/116033) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1322f9263410116cc565129bb517a35227c28479&end=41aa06ecf963cf3529146159f761d6e5b5f9cb48&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.3%, 0.5%] | 13 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -3.7% | [-3.7%, -3.7%] | 1 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.1% | [-3.7%, 0.5%] | 14 |
- Small regression - asked the author and reviewer if they feel like they had a quick idea how to address it.


Rollup of 7 pull requests [#117180](https://github.com/rust-lang/rust/pull/117180) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cf226e93dcb0e21e2daa7c26aca0c5b46ff1b646&end=ab5c841a1f3c09edc5ea07722519627c960aed17&stat=instructions:u)
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved

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

Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved
The one primary 3.7% regression looks real. The tiny secondary one looks either noise or has basically returned to its prior value. The small improvement also seems to have sustained at the new levels.
Loading