Disable drop range analysis #93284
Disable drop range analysis #93284
Conversation
The previous PR, rust-lang#93165, still performed the drop range analysis despite ignoring the results. Unfortunately, there were ICEs in the analysis as well, so some packages failed to build (see the issue rust-lang#93197 for an example). This change further disables the analysis and just provides dummy results in that case.
rustbot
added
the
T-compiler
|
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I've confirmed that eww builds with this change (see #93197). |
|
@RalfJung - you saw the previous change, so feel free to review this one too. |
Is it possible to add a testcase? |
|
@bors try |
|
⌛ Trying commit f4d7d09 with merge 51e7fdb3816eebad439712e3bbc700b0408d2468... |
|
☀️ Try build successful - checks-actions |
|
Actually, since the previous PR sometimes still ran the drop range analysis IIUC, let's see if that still had a noticeable impact on perf (in addition to the changes in the soft revert PR), by checking if this new way to disable it has noticeable perf changes. @rust-timer build 51e7fdb3816eebad439712e3bbc700b0408d2468 |
|
Queued 51e7fdb3816eebad439712e3bbc700b0408d2468 with parent 51126be, future comparison URL. |
|
Finished benchmarking commit (51e7fdb3816eebad439712e3bbc700b0408d2468): comparison url. Summary: This change led to moderate relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
Yes, I just included the one provided by @hellow554. |
|
@bors r+ |
|
📌 Commit 520bd69 has been approved by |
bors
added
S-waiting-on-bors
|
It looks like bors merged all but the last commit already, so I'll close this PR and fold the last commit into #93180. |
|
Oh wait, I didn't look closely enough. This didn't actually get merged... |
|
@bors r=pnkfelix |
|
📌 Commit 9f9d82a has been approved by |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (686663a): comparison url. Summary: This benchmark run shows 8 relevant improvements 🎉 to instruction counts.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
The previous PR, #93165, still performed the drop range analysis despite ignoring the results. Unfortunately, there were ICEs in the analysis as well, so some packages failed to build (see the issue #93197 for an example). This change further disables the analysis and just provides dummy results in that case.