[rustc_data_structures] Use partition_point to find slice range end.

[ttsugriy]

This PR uses approach introduced in #114152 to find
the end of the range. It's much easier to understand and reason about invariants of such
implementation.
Technically it's possible to make it even shorter by returning &[start..end] unconditionally
because even if searched item is not present in the slice, start and end would point at
the same index, so the range would be empty. The reason I decided not to use this shorter
implementation is because it would involve more comparisons in case there are no elements
in the slice with key equal to key.

Also, not that it matters much, but this implementation also improves perf according to the
benchmark below:
https://gist.github.com/ttsugriy/63c0ed39ae132b131931fa1f8a3dea55

The results on my M1 macbook air are:

Running benches/bin_search_slice_benchmark.rs (target/release/deps/bin_search_slice_benchmark-90fa6d68c3bd1298)
Benchmarking multiply add/binary_search_slice: Collecting 100 samples in estimated 5.0002 s (1
multiply add/binary_search_slice
                        time:   [44.719 ns 44.918 ns 45.158 ns]
                        No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe
Benchmarking multiply add/binary_search_slice_new: Collecting 100 samples in estimated 5.0001 
multiply add/binary_search_slice_new
                        time:   [36.955 ns 37.060 ns 37.221 ns]
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe

[rustbot]

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

[TaKO8Ki]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 9ced089 with merge cdca3430dc0d29742e4cff0188d837a46e9a1f3d...

[bors]

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

[rust-timer]

Finished benchmarking commit (cdca3430dc0d29742e4cff0188d837a46e9a1f3d): comparison URL.

Overall result: ❌✅ regressions and improvements - 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)	6.1%	[6.1%, 6.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.6%	[-0.7%, -0.4%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

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

Binary size

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

Bootstrap: 652.574s -> 650.842s (-0.27%)

[TaKO8Ki]

This pull request seems to have regressions. Do you have any ideas to improve this? @ttsugriy

[ttsugriy]

@TaKO8Ki , are you sure these regression numbers legit? Even if this PR had a regression, which I doubt, the magnitude seems very unlikely. Is there a way to rerun these benchmarks?

[TaKO8Ki]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 9ced089 with merge 539a718a86697c85050b140cfd17d0b168c1162d...

[bors]

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

[rust-timer]

Finished benchmarking commit (539a718a86697c85050b140cfd17d0b168c1162d): comparison URL.

Overall result: no relevant changes - no 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.

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

Instruction count

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

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%	[-3.3%, -2.3%]	4
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 649.854s -> 651.21s (0.21%)

[ttsugriy]

Thanks for rerunning the tests, @TaKO8Ki ! As you can see, as expected, the "regression" was just a measurement noise :)

[cjgillot]

One nit wrt comments. r=me afterwards.

[Dylan-DPC]

@ttsugriy any updates on this?

[ttsugriy]

@Dylan-DPC I don't see any comments below mine, so I guess there are no updates

[Dylan-DPC]

@ttsugriy you haven't made the changes with respect to this review which is all that's left to get this pr merged

[ttsugriy]

@Dylan-DPC the review comment didn't request any changes - it simply questioned the value of explicit invariants. I think that plain text comments are much less useful than invariants so I didn't consider that comment as a call for action. Since I no longer use Rust, I don't mind removing these useful invariants, so they are now gone.

[wesleywiser]

Thanks @ttsugriy!

@bors r=cjgillot

[bors]

📌 Commit 97bc528 has been approved by cjgillot

It is now in the queue for this repository.

[bors]

⌛ Testing commit 97bc528 with merge 25f8d01...

[bors]

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 25f8d01 to master...

[rust-timer]

Finished benchmarking commit (25f8d01): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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

Cycles

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

Binary size

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

Bootstrap: 665.206s -> 666.403s (0.18%)
Artifact size: 308.34 MiB -> 308.34 MiB (-0.00%)