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-2021-02-24 #844

Merged
merged 2 commits into from
Feb 25, 2021
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
43 changes: 43 additions & 0 deletions triage/2021-02-24.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# 2021-02-24 Triage Log

Overall, a positive week for compiler performance with only one moderate regression. The change that introduced the regression leads to significantly improved [bootstrap speed](https://github.com/rust-lang/rust/pull/70951#issuecomment-766292996) of the compiler as well as easier maintainability.

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

1 Regression, 5 Improvements, 0 Mixed
0 of them in rollups

#### Regressions

Move the query engine out of rustc_middle [#70951](https://github.com/rust-lang/rust/issues/70951)
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=e7c23ab933ebc1f205c3b59f4ebc85d40f67d404&end=83b30a639d5abd1270ade35d9bd92271f5a5ba18&stat=instructions:u) (up to 4.9% on `full` builds of `deeply-nested-check`)
- While this does somewhat hurt compiler performance, it is a huge gain in bootstrap speed. The performance impact was deemed acceptable, but perhaps an investigation in if the remaining performance regressions can be eliminated, should be looked into.
- Added a nag to [the pull request](https://github.com/rust-lang/rust/pull/70951#issuecomment-785044935).

#### Improvements

Only store a LocalDefId in some HIR nodes [#81611](https://github.com/rust-lang/rust/issues/81611)
- Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a143517d44cac50b20cbd3a0b579addab40dd399&end=8fe989dd768f5dfdb0fc90933f3f74fa4579fefd&stat=instructions:u) (up to -9.6% on `incr-unchanged` builds of `many-assoc-items-check`)
- Large improvements in the associated_item query which naturally led to improvements in the stress test related to associated items.

Inline try_get_cached [#82197](https://github.com/rust-lang/rust/issues/82197)
- Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=8fe989dd768f5dfdb0fc90933f3f74fa4579fefd&end=ee88f46bb5e27c4d9f30326e69ff2298dcbeecb1&stat=instructions:u) (up to -10.4% on `full` builds of `externs-debug`)
- This change shows improvements originally expected from [#81855](https://github.com/rust-lang/rust/issues/81855), which based query fast path on try_get_cached. In-between the initial successful perf run of #81855 and merging, another PR [#81892](https://github.com/rust-lang/rust/issues/81892) removed an inline hint from the function, which was soon to be on the fast path.

Reduce size of InterpErrorInfo to 8 bytes [#82116](https://github.com/rust-lang/rust/issues/82116)
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=ee88f46bb5e27c4d9f30326e69ff2298dcbeecb1&end=5ef21063f0c0fd5b973bfa8cb88c0b70982da977&stat=instructions:u) (up to -4.1% on `full` builds of `ctfe-stress-4-check`)
- Small improvements in ctfe stress tests. We'll take it though.

Make the `Query` enum a simple struct. [#80891](https://github.com/rust-lang/rust/issues/80891)
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=fe1bf8e05c39bdcc73fc09e246b7209444e389bc&end=301ad8a4fa3ea56fb980443b7997c8f9d72dd717&stat=instructions:u) (up to -1.5% on `full` builds of `externs-debug`)
- Largely a code quality refactoring which originally had poor perf results but was fine tuned to lead to a small improvement. Good job!

Improve assert_eq! and assert_ne! [#79100](https://github.com/rust-lang/rust/issues/79100)
- Large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a31c16212d70fcae3ad9d073b00d883951e573ee&end=ed58a2b03b6284b070fae2349898b16df98b7765&stat=instructions:u) (up to -8.4% on `full` builds of `packed-simd-check`)
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=a31c16212d70fcae3ad9d073b00d883951e573ee&end=ed58a2b03b6284b070fae2349898b16df98b7765&stat=instructions:u) (up to 1.2% on `incr-patched: b9b3e592dd cherry picked` builds of `style-servo-debug`)
- The regression is fairly small. It is servo which seems to have regressed in codegen queries. I imagine this would be the case if there are a lot of monomorphized instances of the new `assert_failed` function but this could was probably being generated before so I'm not entirely sure. The codegen query seems to be noisy this week and has caused other (clearly non-perf sensitive changes) to show changes in codegen times.

Nags requiring follow up

- One nag for the only regression for the week.