Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use internal iteration in Iterator comparison methods #100845

Merged
merged 1 commit into from
Sep 24, 2022
Merged

Use internal iteration in Iterator comparison methods #100845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions library/core/benches/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,13 @@ fn bench_partial_cmp(b: &mut Bencher) {
b.iter(|| (0..100000).map(black_box).partial_cmp((0..100000).map(black_box)))
}

#[bench]
fn bench_chain_partial_cmp(b: &mut Bencher) {
b.iter(|| {
(0..50000).chain(50000..100000).map(black_box).partial_cmp((0..100000).map(black_box))
})
}

#[bench]
fn bench_lt(b: &mut Bencher) {
b.iter(|| (0..100000).map(black_box).lt((0..100000).map(black_box)))
Expand Down
143 changes: 81 additions & 62 deletions library/core/src/iter/traits/iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3461,36 +3461,27 @@ pub trait Iterator {
/// assert_eq!(xs.iter().cmp_by(&ys, |&x, &y| (2 * x).cmp(&y)), Ordering::Greater);
/// ```
#[unstable(feature = "iter_order_by", issue = "64295")]
fn cmp_by<I, F>(mut self, other: I, mut cmp: F) -> Ordering
fn cmp_by<I, F>(self, other: I, cmp: F) -> Ordering
where
Self: Sized,
I: IntoIterator,
F: FnMut(Self::Item, I::Item) -> Ordering,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => {
if other.next().is_none() {
return Ordering::Equal;
} else {
return Ordering::Less;
}
}
Some(val) => val,
};

let y = match other.next() {
None => return Ordering::Greater,
Some(val) => val,
};

match cmp(x, y) {
Ordering::Equal => (),
non_eq => return non_eq,
#[inline]
fn compare<X, Y, F>(mut cmp: F) -> impl FnMut(X, Y) -> ControlFlow<Ordering>
where
F: FnMut(X, Y) -> Ordering,
{
move |x, y| match cmp(x, y) {
Ordering::Equal => ControlFlow::CONTINUE,
non_eq => ControlFlow::Break(non_eq),
}
}

match iter_compare(self, other.into_iter(), compare(cmp)) {
ControlFlow::Continue(ord) => ord,
ControlFlow::Break(ord) => ord,
}
}

/// [Lexicographically](Ord#lexicographical-comparison) compares the elements of this [`Iterator`] with those
Expand Down Expand Up @@ -3546,36 +3537,27 @@ pub trait Iterator {
/// );
/// ```
#[unstable(feature = "iter_order_by", issue = "64295")]
fn partial_cmp_by<I, F>(mut self, other: I, mut partial_cmp: F) -> Option<Ordering>
fn partial_cmp_by<I, F>(self, other: I, partial_cmp: F) -> Option<Ordering>
where
Self: Sized,
I: IntoIterator,
F: FnMut(Self::Item, I::Item) -> Option<Ordering>,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => {
if other.next().is_none() {
return Some(Ordering::Equal);
} else {
return Some(Ordering::Less);
}
}
Some(val) => val,
};

let y = match other.next() {
None => return Some(Ordering::Greater),
Some(val) => val,
};

match partial_cmp(x, y) {
Some(Ordering::Equal) => (),
non_eq => return non_eq,
#[inline]
fn compare<X, Y, F>(mut partial_cmp: F) -> impl FnMut(X, Y) -> ControlFlow<Option<Ordering>>
where
F: FnMut(X, Y) -> Option<Ordering>,
{
move |x, y| match partial_cmp(x, y) {
Some(Ordering::Equal) => ControlFlow::CONTINUE,
non_eq => ControlFlow::Break(non_eq),
}
}

match iter_compare(self, other.into_iter(), compare(partial_cmp)) {
ControlFlow::Continue(ord) => Some(ord),
ControlFlow::Break(ord) => ord,
}
}

/// Determines if the elements of this [`Iterator`] are equal to those of
Expand Down Expand Up @@ -3613,29 +3595,26 @@ pub trait Iterator {
/// assert!(xs.iter().eq_by(&ys, |&x, &y| x * x == y));
/// ```
#[unstable(feature = "iter_order_by", issue = "64295")]
fn eq_by<I, F>(mut self, other: I, mut eq: F) -> bool
fn eq_by<I, F>(self, other: I, eq: F) -> bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not this PR, but it might be interesting to experiment with a size_hint check for this one.

Something like

pub fn definitely_not_equal(a: &impl Iterator, b: &impl Iterator) -> bool {
    match (a.size_hint(), b.size_hint()) {
        ((a_lo, _), (_, Some(b_hi))) => b_hi < a_lo,
        ((_, Some(a_hi)), (b_lo, _)) => a_hi < b_lo,
        _ => false,
    }
}

Since that could instantly catch a bunch of cases where things have good size_hints in practice, while also optimizing away completely for things that always return (0, None) since that'd obviously always be false overall.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure! I almost included that in this PR, in fact. It's a behavior change, but hopefully one we feel like we're permitted to make.

where
Self: Sized,
I: IntoIterator,
F: FnMut(Self::Item, I::Item) -> bool,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_none(),
Some(val) => val,
};

let y = match other.next() {
None => return false,
Some(val) => val,
};

if !eq(x, y) {
return false;
#[inline]
fn compare<X, Y, F>(mut eq: F) -> impl FnMut(X, Y) -> ControlFlow<()>
where
F: FnMut(X, Y) -> bool,
{
move |x, y| {
if eq(x, y) { ControlFlow::CONTINUE } else { ControlFlow::BREAK }
}
}

match iter_compare(self, other.into_iter(), compare(eq)) {
ControlFlow::Continue(ord) => ord == Ordering::Equal,
ControlFlow::Break(()) => false,
}
}

/// Determines if the elements of this [`Iterator`] are unequal to those of
Expand Down Expand Up @@ -3860,6 +3839,46 @@ pub trait Iterator {
}
}

/// Compares two iterators element-wise using the given function.
///
/// If `ControlFlow::CONTINUE` is returned from the function, the comparison moves on to the next
/// elements of both iterators. Returning `ControlFlow::Break(x)` short-circuits the iteration and
/// returns `ControlFlow::Break(x)`. If one of the iterators runs out of elements,
/// `ControlFlow::Continue(ord)` is returned where `ord` is the result of comparing the lengths of
/// the iterators.
///
/// Isolates the logic shared by ['cmp_by'](Iterator::cmp_by),
/// ['partial_cmp_by'](Iterator::partial_cmp_by), and ['eq_by'](Iterator::eq_by).
#[inline]
fn iter_compare<A, B, F, T>(mut a: A, mut b: B, f: F) -> ControlFlow<T, Ordering>
where
A: Iterator,
B: Iterator,
F: FnMut(A::Item, B::Item) -> ControlFlow<T>,
{
#[inline]
fn compare<'a, B, X, T>(
b: &'a mut B,
mut f: impl FnMut(X, B::Item) -> ControlFlow<T> + 'a,
) -> impl FnMut(X) -> ControlFlow<ControlFlow<T, Ordering>> + 'a
where
B: Iterator,
{
move |x| match b.next() {
None => ControlFlow::Break(ControlFlow::Continue(Ordering::Greater)),
Some(y) => f(x, y).map_break(ControlFlow::Break),
}
}

match a.try_for_each(compare(&mut b, f)) {
ControlFlow::Continue(()) => ControlFlow::Continue(match b.next() {
None => Ordering::Equal,
Some(_) => Ordering::Less,
}),
ControlFlow::Break(x) => x,
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<I: Iterator + ?Sized> Iterator for &mut I {
type Item = I::Item;
Expand Down