While this definitely helps sometimes (particularly for trivial closures), it's also a pessimization sometimes, so it's better to leave this to (hypothetical) future LLVM improvements instead of forcing this on everyone.
I think it's better for the advice to be that sometimes you need to unroll manually than you sometimes need to not-unroll manually (like #64545).
squash of all commits for nth_back on ChunksMut
wip nth_back for chunks_mut
working chunksmut
fixed nth_back for chunksmut
Signed-off-by: wizAmit <amitforfriends_dns@yahoo.com>
r? @timvermeulen
r? @scottmcm
core: check for pointer equality when comparing Eq slices
Because `Eq` types must be reflexively equal, an equal-length slice to the same memory location must be equal.
This is related to #33892 (and #32699) answering this comment from that PR:
> Great! One more easy question: why does this optimization not apply in the non-BytewiseEquality implementation directly above?
Because slices of non-reflexively equal types (like `f64`) are not equal even if it's the same slice. But if the types are `Eq`, we can use this same-address optimization, which this PR implements. Obviously this changes behavior if types violate the reflexivity condition of `Eq`, because their impls of `PartialEq` will no longer be called per-item, but 🤷♂ .
It's not clear how often this optimization comes up in the real world outside of the same-`&str` case covered by #33892, so **I'm requesting a perf run** (on MacOS today, so can't run `rustc_perf` myself). I'm going ahead and making the PR on the basis of being surprised things didn't already work this way.
This is my first time hacking rust itself, so as a perf sanity check I ran `./x.py bench --stage 0 src/lib{std,alloc}`, but the differences were noisy.
To make the existing specialization for `BytewiseEquality` explicit, it's now a supertrait of `Eq + Copy`. `Eq` should be sufficient, but `Copy` was included for clarity.
Only call the closure parameter of Iterator::is_sorted_by_key once per item
See https://github.com/rust-lang/rust/issues/53485#issuecomment-472314004.
This changes `Iterator::is_sorted_by_key` to only call the given closure once for each item, which allows us to pass the items to the closure by value instead of by reference.
**Important**: `is_sorted_by_key` for slices and slice iterators is now no longer implemented in terms of the custom `slice::Iter::is_sorted_by` implementation. It's a trade-off: we could forward `slice::Iter::is_sorted_by_key` to it directly for potential SIMD benefits, but that would mean that the closure is potentially called twice for (almost) every element of the slice.
Implement nth_back for slice::{Iter, IterMut}
Part of #54054.
I implemented `nth_back` as straightforwardly as I could, and then slightly changed `nth` to match `nth_back`. I believe I did so correctly, but please double-check 🙂
I also added the helper methods `zst_shrink`, `next_unchecked`, and `next_back_unchecked` to get rid of some duplicated code. These changes hopefully make this code easier to understand for new contributors like me.
I noticed the `is_empty!` and `len!` macros which sole purpose seems to be inlining, according to the comment right above them, but the `is_empty` and `len` methods are already marked with `#[inline(always)]`. Does that mean we could replace these macros with method calls, without affecting anything? I'd love to get rid of them.
Implement nth_back for RChunks(Exact)(Mut)
Part of #54054.
These implementations may not be optimal because of the use of `self.len()`, but it's quite cheap and simplifies the code a lot.
There's quite some duplication going on here, I wouldn't mind cleaning this up later. A good next step would probably be to add private `split_off_up_to`/`split_off_from` helper methods for slices since their behavior is commonly useful throughout the `Chunks` types.
r? @scottmcm
as_ptr returns a read-only pointer
Add comments to `as_ptr` methods to warn that these are read-only pointers, and writing to them is UB.
[It was pointed out](https://internals.rust-lang.org/t/as-ptr-vs-as-mut-ptr/9940) that `CStr` does not even have an `as_mut_ptr`. I originally was going to add one, but there is no method at all that would mutate a `CStr`. Was that a deliberate choice or should I add an `as_mut_ptr` (similar to [what I did for `str`](https://github.com/rust-lang/rust/pull/58200))?
Add implementations of last in terms of next_back on a bunch of DoubleEndedIterators
Provided a `DoubleEndedIterator` has finite length, `Iterator::last` is equivalent to `DoubleEndedIterator::next_back`. But searching forwards through the iterator when it's unnecessary is obviously not good for performance. I ran into this on one of the collection iterators.
I tried adding appropriate overloads for a bunch of the iterator adapters like filter, map, etc, but I ran into a lot of type inference failures after doing so.
The other interesting case is what to do with `Repeat`. Do we consider it part of the contract that `Iterator::last` will loop forever on it? The docs do say that the iterator will be evaluated until it returns None. This is also relevant for the adapters, it's trivially easy to observe whether a `Map` adapter invoked its closure a zillion times or just once for the last element.
Fix small errors in docs for `rchunks_exact` and `rchunks_exact_mut`.
The documentation for `rchunks_exact` said it started at the beginning
of the slice, bit it actually starts at the end of the slice.
In addition, there were a couple of "of the slice of the slice"
duplicate phrases going on for `rchunks_exact` and `rchunks_exact_mut`.
This fixes#60068.