rust
rust copied to clipboard
rust-lang
Extend SCC construction to enable extra functionality
Do YOU feel like your SCC construction doesn't do enough? Then I have a patch for you! SCCs can now do everything! Well, almost.
This patch has been extracted from #123720. It specifically enhances
Sccs to allow tracking arbitrary commutative properties (think min/max mappings on nodes vs arbitrary closures) of strongly connected components, including
- reachable values (max/min)
- SCC-internal values (max/min)
This helps with among other things universe computation. We can now identify SCC universes as a reasonably straightforward "find max/min" operation during SCC construction. This is also included in this patch.
It's also more or less zero-cost; don't use the new features, don't pay for them.
This commit also vastly extends the documentation of the SCCs module, which I had a very hard time following. It may or may not have gotten easier to read for someone else.
I believe this logic can also be used in leak check, but haven't checked. Ha. ha. Ha.
r? @pnkfelix
rustbot has assigned @pnkfelix. 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
As discussed on zulip, until I have more time let's also temporarily redirect to niko for review or reassignment, and discuss at this week's meeting: r? nikomatsakis
:hourglass: Trying commit 609a202928a8ad7a29f7ea26c4101c2c176b6b1d with merge 6c7f0a6914780a8bd5240c26e668f7b0625b66dc...
:sunny: Try build successful - checks-actions
Build commit: 6c7f0a6914780a8bd5240c26e668f7b0625b66dc (6c7f0a6914780a8bd5240c26e668f7b0625b66dc)
Queued 6c7f0a6914780a8bd5240c26e668f7b0625b66dc with parent 6be7b0c7d2b085474f9f2f9323c2266f4df105d8, future comparison URL. There are currently 0 preceding artifacts in the queue. It will probably take at least ~1.2 hours until the benchmark run finishes.
Finished benchmarking commit (6c7f0a6914780a8bd5240c26e668f7b0625b66dc): comparison URL.
Overall result: ❌ regressions - 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.
Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.
@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.5% | [0.2%, 1.6%] | 45 |
| Regressions ❌ (secondary) |
1.4% | [0.1%, 4.0%] | 32 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 0.5% | [0.2%, 1.6%] | 45 |
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) |
2.8% | [2.4%, 3.2%] | 2 |
| Regressions ❌ (secondary) |
2.2% | [2.2%, 2.2%] | 2 |
| Improvements ✅ (primary) |
-2.6% | [-2.6%, -2.6%] | 1 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 1.0% | [-2.6%, 3.2%] | 3 |
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) |
1.9% | [1.6%, 2.1%] | 3 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-2.9% | [-2.9%, -2.9%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 675.291s -> 676.941s (0.24%) Artifact size: 315.98 MiB -> 316.29 MiB (0.10%)
:hourglass: Trying commit c510e73513a47bc17bc52971e425cb3c929af4fb with merge 1805d8ae3b6d155ffdc3ee3371389de5042fb85e...
My guess is 0.3% primary regressions and 0.84% in secondary, let's see how wrong I am!
Also, you can't edit on GitHub so there is no way for me to remove this if I'm enormously off. Too bad!
Queued 1805d8ae3b6d155ffdc3ee3371389de5042fb85e with parent a5c37eea5aa922f4d6b543f2d35bdbd892fea2a8, future comparison URL. There is currently 1 preceding artifact in the queue. It will probably take at least ~2.0 hours until the benchmark run finishes.
Finished benchmarking commit (1805d8ae3b6d155ffdc3ee3371389de5042fb85e): comparison URL.
Overall result: ❌ regressions - 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.
Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.
@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.3% | [0.2%, 0.8%] | 23 |
| Regressions ❌ (secondary) |
1.5% | [0.1%, 3.9%] | 20 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 0.3% | [0.2%, 0.8%] | 23 |
Max RSS (memory usage)
Results (primary -0.4%)
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) |
1.3% | [1.3%, 1.3%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-1.3% | [-2.4%, -0.2%] | 2 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | -0.4% | [-2.4%, 1.3%] | 3 |
Cycles
Results (primary -0.2%, secondary -1.0%)
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.2% | [7.2%, 7.2%] | 1 |
| Regressions ❌ (secondary) |
2.0% | [1.8%, 2.2%] | 5 |
| Improvements ✅ (primary) |
-2.1% | [-4.3%, -0.8%] | 4 |
| Improvements ✅ (secondary) |
-2.6% | [-3.8%, -2.0%] | 10 |
| All ❌✅ (primary) | -0.2% | [-4.3%, 7.2%] | 5 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 669.422s -> 671.224s (0.27%) Artifact size: 316.04 MiB -> 316.26 MiB (0.07%)
:hourglass: Trying commit b453158e337396ddbf0439079ba26c831086feb1 with merge f02f9b5f83a84a26d250b683819145c63ef1004c...
:hourglass: Trying commit b453158e337396ddbf0439079ba26c831086feb1 with merge f3129a502c9cb5f245ac39b47c8a169b68fba662...
Hmm, looks like we started two try runs in a row, but the previous one was not cancelled. That looks like a CI config bug (?).
:sunny: Try build successful - checks-actions
Build commit: f3129a502c9cb5f245ac39b47c8a169b68fba662 (f3129a502c9cb5f245ac39b47c8a169b68fba662)
Queued f3129a502c9cb5f245ac39b47c8a169b68fba662 with parent e8ada6ab253b510ac88edda131021d9878f2984f, future comparison URL. There is currently 1 preceding artifact in the queue. It will probably take at least ~1.8 hours until the benchmark run finishes.
Finished benchmarking commit (f3129a502c9cb5f245ac39b47c8a169b68fba662): comparison URL.
Overall result: ❌ regressions - 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.
Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.
@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.3% | [0.2%, 0.7%] | 26 |
| Regressions ❌ (secondary) |
1.3% | [0.1%, 3.9%] | 21 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 0.3% | [0.2%, 0.7%] | 26 |
Max RSS (memory usage)
Results (primary -0.1%)
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) |
2.4% | [2.4%, 2.4%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-2.6% | [-2.6%, -2.6%] | 1 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | -0.1% | [-2.6%, 2.4%] | 2 |
Cycles
Results (primary 2.4%, secondary 1.2%)
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) |
2.4% | [2.4%, 2.4%] | 1 |
| Regressions ❌ (secondary) |
2.2% | [2.1%, 2.5%] | 8 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-3.0% | [-3.9%, -2.0%] | 2 |
| All ❌✅ (primary) | 2.4% | [2.4%, 2.4%] | 1 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 670.321s -> 670.92s (0.09%) Artifact size: 316.18 MiB -> 316.07 MiB (-0.03%)
:hourglass: Trying commit 58ccdd09cbe0cdfc953ea81113b941105601c899 with merge 4580462219447558f8ed6b6c8e1baa94d6fc24f8...
:sunny: Try build successful - checks-actions
Build commit: 4580462219447558f8ed6b6c8e1baa94d6fc24f8 (4580462219447558f8ed6b6c8e1baa94d6fc24f8)