[experiment] use unreachable_unchecked() everywhere #53307
Closed
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
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I feel like if this looks to be a good idea then in the hotspots we should still have a debug fallback: You could probably instead just have this in scope where you want it, do reduce the diff, but that's probably an unnecessary amount of non-local effect: Can you override |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors delegate+ |
|
✌️ @ljedrz can now approve this pull request |
|
@ljedrz |
petrochenkov
added
S-waiting-on-author
ljedrz
force-pushed
the
experiment_unreachable_unchecked
branch
from
1b3a8a4 to
3b7fe5b
Compare
|
@petrochenkov thanks; I'd run all the tests on my own, but unfortunately on Windoze I can only get to And the module-level tests are performed only after those; do you know if there is any way I can fix these issues, skip edit: I just found a |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
You can disable the debuginfo test suite by commenting it out in this list https://github.com/rust-lang/rust/blob/master/src/bootstrap/builder.rs#L377-L429 |
|
@petrochenkov thanks, that allowed me to go a lot further; I'm still getting some spurious-looking errors by the end, but hopefully I've covered all the relevant issues by now. |
|
@bors try |
|
⌛ Trying commit 3400023 with merge 17a91f559ea8afdd4b2413b71b21348f4fe80286... |
|
☀️ Test successful - status-travis |
|
@rust-timer build 17a91f5 |
|
Insufficient permissions to issue commands to rust-timer. |
|
@petrochenkov I guess delegating doesn't extend to perf runs; can you help here? |
|
@rust-timer build 17a91f559ea8afdd4b2413b71b21348f4fe80286 |
|
Success: Queued 17a91f559ea8afdd4b2413b71b21348f4fe80286 with parent 5bb9239, comparison URL. |
kennytm
added
the
S-waiting-on-perf
|
Not much effect. |
|
Well that's fantastic news right? That means we get to keep the checked unreachables without leaving performance on the table. |
|
Yeah, it looks like there isn't much to gain here; I think we can close it along with the related issue. |
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.
This is an experimental commit designed to measure if changing
unreachable!()to its faster unsafe counterpart can result in measurable performance gains (see #52945).We don't want to perform this change everywhere, only consider it in hot spots under infallible conditions. With any luck a perf run could point us into the right direction; this approach has worked with
assert!(#53025).cc @dtolnay