Use custom wrap-around type instead of RangeInclusive

[bonega]

Two reasons:

1. More memory is allocated than necessary for valid_range in Scalar. The range is not used as an iterator and exhausted is never used.
2. contains, count etc. methods in RangeInclusive are doing very unhelpful(and dangerous!) things when used as a wrap-around range. - In general this PR wants to limit potentially confusing methods, that have a low probability of working.

Doing a local perf run, every metric shows improvement except for instructions.
Max-rss seem to have a very consistent improvement.

Sorry - newbie here, probably doing something wrong.

[rust-highfive]

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information.

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 1b4064f with merge b36ec6286bd82a4ef6fbe3af0c69fc7e9a8b316b...

[bors]

☀️ Try build successful - checks-actions
Build commit: b36ec6286bd82a4ef6fbe3af0c69fc7e9a8b316b (b36ec6286bd82a4ef6fbe3af0c69fc7e9a8b316b)

[rust-timer]

Queued b36ec6286bd82a4ef6fbe3af0c69fc7e9a8b316b with parent 91f9806, future comparison URL.

[rust-timer]

Finished benchmarking try commit (b36ec6286bd82a4ef6fbe3af0c69fc7e9a8b316b): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

• Small improvement in instruction counts (up to -0.4% on incr-patched: empty 3072 builds of issue-46449)
• Moderate regression in instruction counts (up to 0.7% on full builds of ctfe-stress-4)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

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 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-review -S-waiting-on-perf +perf-regression

[bonega]

Thanks for your quick review!
Unfortunately I must be doing something wrong with my local perf run, which showed clear improvements.
Especially surprised to see max-rss go up, local testing was 450 positive 22 negative or something.
Will analyze it and get back.

[RalfJung]

Big 👍 on making this a dedicated type! Even without perf improvements, this is a good change.

However, the name AllocationRange does not make sense to me. This range has nothing to do with our Allocation type, nor with any user use of the word "allocation" that I can think of. What about WrappingRange?

[bonega]

Big on making this a dedicated type! Even without perf improvements, this is a good change.

However, the name AllocationRange does not make sense to me. This range has nothing to do with our Allocation type, nor with any user use of the word "allocation" that I can think of. What about WrappingRange?

Originally I thought of AllocationRange to be part of reserve functionality for niches(not true), which would have meant specific considerations.
I will change it to WrappingRange which is much better.

[bors]

☀️ Try build successful - checks-actions
Build commit: fe0af6963339bbddcd972e30a71a118b6d996a93 (fe0af6963339bbddcd972e30a71a118b6d996a93)

[rust-timer]

Queued fe0af6963339bbddcd972e30a71a118b6d996a93 with parent f66e825, future comparison URL.

[rust-timer]

Finished benchmarking try commit (fe0af6963339bbddcd972e30a71a118b6d996a93): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

[bonega]

Thanks, unfortunately I think the picture is still a bit unclear.
Perf-run 2: fc1c134 had no parent 5998c2e, therefore it is somewhat problematic to judge the performance.(I posted a link, but I don't think we can use it to draw conclusions)

It would be nice to have a mean delta for every metric.
I only get confused when looking at the perfs.
number of changes vs magnitude..

[oli-obk]

The perf page is just broken for me since two days... It looks like an unsubstituted template page to me. Not sure what's going on

[oli-obk]

perf is clean, diff lgtm. Thanks for doing this!

@bors r+

[bors]

📌 Commit f17e384 has been approved by oli-obk

[bonega]

Thanks to both of you for the excellent newbie support!

[bors]

⌛ Testing commit f17e384 with merge e5484ce...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing e5484ce to master...