Defer extra liveness calculation for Polonius Alpha - #161938
Conversation
|
@bors try @rust-time queue |
This comment has been minimized.
This comment has been minimized.
Defer extra liveness calculation for Polonius Alpha
|
@rust-timer queue |
This comment has been minimized.
This comment has been minimized.
|
Disclosure: An LLM was used for experiments here, but all code here is written fresh. |
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 254d008 failed: CI. Failed job:
|
This comment has been minimized.
This comment has been minimized.
ef46fa3 to
29b9558
Compare
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Defer extra liveness calculation for Polonius Alpha
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f904a04): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -2.0%, secondary 1.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -3.6%, secondary -5.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 475.673s -> 475.822s (0.03%) |
29b9558 to
fead75b
Compare
This comment has been minimized.
This comment has been minimized.
|
@craterbot check As discussed today, this adds more region constraints for points that have drop locations but are not drop live - which could have some implications on things that use sccs. |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
808923f to
e24813e
Compare
This comment has been minimized.
This comment has been minimized.
e24813e to
5166f9c
Compare
|
@craterbot abort We no longer register region constraints without checking if there are drop-live points. |
|
🗑️ Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
I’ve already read the PR but the force pushes make it hard to see this change. What code changed from our last discussion about this? |
| // them, but *only if the local is drop-live*. | ||
| // It doesn't really make sense to only check drop-liveness but defer use-liveness, | ||
| // so we just treat this as eager. | ||
| if drop_data.region_constraint_data.is_some() { |
There was a problem hiding this comment.
If there are region constraints to push, we compute everything eagerly (which gates pushing them on drop-liveness).
|
☔ The latest upstream changes (presumably #162488) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
View all comments
Best reviewed by commit.
This moves liveness calculation of NLL-boring/Polonius-relevant locals to be lazy. This allows us to skip unnecessary work.
r? lqd