rust
rust copied to clipboard
Stop walking the bodies of statics for reachability, and evaluate them instead
cc @saethlin @RalfJung
cc #119214
This reuses the DefIdVisitor from rustc_privacy, because they basically try to do the same thing.
This PR's changes can probably be extended to constants, too, but let's tackle that separately, it's likely more involved.
r? @fee1-dead
rustbot has assigned @fee1-dead. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
@bors try @rust-timer queue
These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
If this was unintentional then you should revert the changes before this PR is merged. Otherwise, you can ignore this comment.
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
:hourglass: Trying commit 7b7b6b53ee704996ee9038944991855353da73eb with merge 1a609b9da1e44fc0d4e3b1a85563596147865afb...
Not familiar with this code, sorry.
r? compiler
I can't really comment on this PR, as I know neither the intended semantics of this "reachable" pass nor the way it is used.
It would be good to have some place that documents the intended semantics -- does that exist currently? Does it need updating with this PR?
:sunny: Try build successful - checks-actions
Build commit: 1a609b9da1e44fc0d4e3b1a85563596147865afb (1a609b9da1e44fc0d4e3b1a85563596147865afb)
Queued 1a609b9da1e44fc0d4e3b1a85563596147865afb with parent b0170b693ef91460c97ff9ea5e360888466611dd, future comparison URL. There are currently 2 preceding artifacts in the queue. It will probably take at least ~3.0 hours until the benchmark run finishes.
I'm pretty sure fmease doesn't know this part of the compiler either. I'll take a careful look over this later, but I think @tmiasko is a good reviewer for this based on past conversations.
r? tmiasko
Finished benchmarking commit (1a609b9da1e44fc0d4e3b1a85563596147865afb): comparison URL.
Overall result: ✅ improvements - no action needed
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 may lead to changes in compiler perf.
@bors rollup=never @rustbot label: -S-waiting-on-perf -perf-regression
Instruction count
This is a highly reliable metric that was used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-1.3% | [-1.4%, -1.2%] | 2 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
7.0% | [7.0%, 7.0%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-1.0% | [-1.0%, -1.0%] | 1 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 3.0% | [-1.0%, 7.0%] | 2 |
Cycles
This benchmark run did not return any relevant results for this metric.
Binary size
Results
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
0.0% | [0.0%, 0.0%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-0.1% | [-0.1%, -0.1%] | 6 |
| Improvements ✅ (secondary) |
-0.7% | [-3.7%, -0.0%] | 6 |
| All ❌✅ (primary) | -0.1% | [-0.1%, 0.0%] | 7 |
Bootstrap: 670.966s -> 671.851s (0.13%) Artifact size: 310.09 MiB -> 310.02 MiB (-0.02%)
Could you also add a negative test case demonstrating that calling a function from an initializer of a reachable static no longer makes the function reachable?
Added a test, and showed how it changes in the second commit
@bors r+
:pushpin: Commit 746e4eff263527ba8960552ea62db11ab9821644 has been approved by tmiasko
It is now in the queue for this repository.
:hourglass: Testing commit 746e4eff263527ba8960552ea62db11ab9821644 with merge c563f2ee799b285067124b516ce99f26063f8351...
:sunny: Test successful - checks-actions Approved by: tmiasko Pushing c563f2ee799b285067124b516ce99f26063f8351 to master...
Finished benchmarking commit (c563f2ee799b285067124b516ce99f26063f8351): comparison URL.
Overall result: ✅ improvements - no action needed
@rustbot label: -perf-regression
Instruction count
This is a highly reliable metric that was used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-1.0% | [-1.4%, -0.2%] | 3 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
2.6% | [2.6%, 2.6%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
Cycles
Results
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
0.9% | [0.9%, 0.9%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
Binary size
Results
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-0.0% | [-0.1%, -0.0%] | 6 |
| Improvements ✅ (secondary) |
-0.7% | [-3.7%, -0.0%] | 6 |
| All ❌✅ (primary) | -0.0% | [-0.1%, -0.0%] | 6 |
Bootstrap: 670.952s -> 670.527s (-0.06%) Artifact size: 311.63 MiB -> 311.59 MiB (-0.01%)