Add a benchmark for exhaustive_patterns #801
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nadrieril
force-pushed
the
stress-exhaustive-patterns
branch
from
b85fbec to
30ba044
Compare
|
I don't know what's wrong with the failing CI test. I keep scrolling down without reaching the bottom of the logs |
|
Yeah, CI logs are a bit opaque. Looks like you're missing a Cargo.lock in the newly added test? |
|
This also needs to update the README in collector/benchmarks to note the newly added test (you can see some existing descriptions there). |
|
I also noted that this doesn't seem to pull in the syn license files, but we should be careful to do so -- I think the easiest thing which should be correct is to just duplicate the license files as is into the directory. |
|
Thanks! This should automatically deploy on the next run or two. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 12, 2021
Turn type inhabitedness into a query to fix `exhaustive_patterns` perf We measured in rust-lang#79394 that enabling the [`exhaustive_patterns` feature](rust-lang#51085) causes significant perf degradation. It was conjectured that the culprit is type inhabitedness checking, and [I hypothesized](rust-lang#79394 (comment)) that turning this computation into a query would solve most of the problem. This PR turns `tcx.is_ty_uninhabited_from` into a query, and I measured a 25% perf gain on the benchmark that stress-tests `exhaustiveness_patterns`. This more than compensates for the 30% perf hit I measured [when creating it](rust-lang/rustc-perf#801). We'll have to measure enabling the feature again, but I suspect this fixes the perf regression entirely. I'd like a perf run on this PR obviously. I made small atomic commits to help reviewing. The first one is just me discovering the "revisions" feature of the testing framework. I believe there's a push to move things out of `rustc_middle` because it's huge. I guess `inhabitedness/mod.rs` could be moved out, but it's quite small. `DefIdForest` might be movable somewhere too. I don't know what the policy is for that. Ping `@camelid` since you were interested in following along `@rustbot` modify labels: +A-exhaustiveness-checking
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
exhaustive_patternsfeature was measured to cause significant perf degradation. I minimized the perf regression into this new benchmark, which I hope will be useful when hacking on match checking. I named it afterexhaustive_patternsbecause I optimized for that (i.e. large nested structs and enums).This benchmark gains 30% on check-full when the
exhaustive_patternsfeature is disabled.Ping #792 and rust-lang/rust#51085.