Exhaustiveness: Improve complexity on some wide matches

[Nadrieril]

#118437 revealed an exponential case in exhaustiveness checking. While exponential cases are unavoidable, this one only showed up after my #117611 rewrite of the algorithm. I remember anticipating a case like this and dismissing it as unrealistic, but here we are :').

The tricky match is as follows:

match command {
    BaseCommand { field01: true, .. } => {}
    BaseCommand { field02: true, .. } => {}
    BaseCommand { field03: true, .. } => {}
    BaseCommand { field04: true, .. } => {}
    BaseCommand { field05: true, .. } => {}
    BaseCommand { field06: true, .. } => {}
    BaseCommand { field07: true, .. } => {}
    BaseCommand { field08: true, .. } => {}
    BaseCommand { field09: true, .. } => {}
    BaseCommand { field10: true, .. } => {}
    // ...20 more of the same

    _ => {}
}

To fix this, this PR formalizes a concept of "relevancy" (naming is hard) that was already used to decide what patterns to report. Now we track it for every row, which in wide matches like the above can drastically cut on the number of cases we explore. After this fix, the above match is checked with linear-many cases instead of exponentially-many.

Fixes #118437

r? @cjgillot

[lqd]

Out of curiosity, how much does this PR improve things on #118437, and how does that compare to the situation prior to #117611?

[Nadrieril]

On #118437, this goes from "takes so long that the playground timeouts" to "compiles instantly".

Compared to before the one-pass rewrite, my guess is that it's comparable in performance. Our benchmarks don't have a lot of variety on matches so it's hard to measure. The rewrite was more for code health than for performance honestly, the difference would only be noticeable for somewhat complex matches. Our benchmarks barely noticed.

[Nadrieril]

Let's see how this fares:

@bors try @rust-timer queue

[bors]

⌛ Trying commit 13a8e36 with merge 4abe247...

[lqd]

On #118437, this goes from "takes so long that the playground timeouts" to "compiles instantly".

So if I understand correctly, #118437 also compiled instantly prior to #117611.

[bors]

☀️ Try build successful - checks-actions
Build commit: 4abe247 (4abe2475bffdcf7fd207c66fbd3525f1443e7fe9)

[rust-timer]

Finished benchmarking commit (4abe247): 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
Regressions ❌ (secondary)	2.9%	[2.7%, 3.2%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
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.9%	[0.5%, 4.6%]	3
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

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)	3.3%	[2.9%, 3.8%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.935s -> 669.415s (-0.08%)
Artifact size: 314.15 MiB -> 314.10 MiB (-0.01%)

[Nadrieril]

So if I understand correctly, #118437 also compiled instantly prior to #117611.

Indeed! You can try it here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1f9de10106c7eec6d18773c9804e614f . On stable is the pre-#117611 behavior; on nightly is the current exponential behavior.

Between pre-#117611 and post-this-PR, the tradeoff is roughly: pre-#117611 can check some cases several times, which post-this-PR never does; conversely, there are cases that post-this-PR considers distinct but pre-#117611 does in one go.

[apiraino]

I'm going to pre-emptively nominate this for beta backport in case the review and merge make it to Dec, 21st.

Ref. Zulip thread where T-infra announces that the beta cut will happen a bit earlier in this release cycle

@rustbot label +beta-nominated

[cjgillot]

So in the example, the constructor North is irrelevant?

[Nadrieril]

That was poorly phrased, relevancy really depends on the rows. I reworked the explanation entirely

[cjgillot]

Thanks!
@bors r+

[bors]

📌 Commit efb04e6 has been approved by cjgillot

It is now in the queue for this repository.

[bors]

⌛ Testing commit efb04e6 with merge 1a086e4...

[bors]

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 1a086e4 to master...

[bors]

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 1a086e4 to master...

[rust-timer]

Finished benchmarking commit (1a086e4): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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)	2.9%	[2.6%, 3.1%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	-	-	0

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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	1
All ❌✅ (primary)	-	-	0

Bootstrap: 671.86s -> 670.392s (-0.22%)
Artifact size: 312.67 MiB -> 312.52 MiB (-0.05%)

[Nadrieril]

This needs to be backported onto 1.76.0, i.e. the beta that just branched off (or is about to be). So there's no rush, I'll wait for the current stable/beta releases to finish being released so we don't get confused about where to backport it

[apiraino]

@Nadrieril thank you 🙂 I see the PR has been merged with some minuses in the perf. report. Do these results need a comment? Will they be triaged during the usual weekly perf. triage?

[Nadrieril]

Perf feels ok to me; it's a stress-test and I'm not surprised it was affected by this change

[apiraino]

Awesome, then I'll mark this as perf. triaged (I think this is the correct process, in case it's not please anyone feel free to jump in and correct me 🙂 )

@rustbot label -perf-regression +perf-regression-triaged

[Kobzol]

@apiraino I think that normally we just apply the perf-regression-triaged label, without removing the perf-regression label (check the example command in the perf comment).

[apiraino]

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

[Nadrieril]

I seem to understand that someone will swoop in and backport this along with a bunch of other PRs at some point? Ping me if this doesn't backport cleanly.