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.

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

performance of all_equal #282

Closed
RSabet opened this issue May 17, 2018 · 4 comments
Closed

performance of all_equal #282

RSabet opened this issue May 17, 2018 · 4 comments

Comments

@RSabet
Copy link

RSabet commented May 17, 2018

Here is a comparison of itertools::all_equal with two alternatives for a vector consisting of 5_000_000 zeroes followed by 5_000_000 ones on rust playground:

  • using itertools::all_equal: 607 msecs
  • using any: 487 msecs
  • using a simple for loop: 536 msecs

i would love if all_qual was as fast

playground

@bluss
Copy link
Member

bluss commented May 18, 2018

@RSabet That's a very reasonable concern. Note that we must use Release mode compiles to make meaningful performance comparisons. I'm not sure you'll have a different conclusion with that, but you'll have faster timings across the board, and the code will need to be fixed to not optimize out some parts of the benchmark.

@RSabet
Copy link
Author

RSabet commented May 19, 2018

You are right, trying to hide the values in the vector from the optimizer one can use:

let mut v: Vec<i32> = (0..5_000_000).map(|_| rng.gen_range(1, 2)).collect();
v.extend((0..5_000_000).map(|_| rng.gen_range(2, 3)) .collect::<Vec<i32>>());

again 5_000_000 ones followed by 5_000_000 twos

and now in release mode:

  • using itertools::all_equal: 5 msecs
  • using any: 3 msecs
  • using a simple for loop: 0 msecs (guess the compiler still found a way to optimize)

playground

@timvermeulen
Copy link

all_equal's implementation is currently suboptimal:

fn all_equal(&mut self) -> bool
    where Self::Item: PartialEq,
{
    self.dedup().nth(1).is_none()
}

This doesn't necessarily iterate the entire iterator, but in @RSabet's example code, it does evaluate all the 2s.

bors bot added a commit that referenced this issue Aug 20, 2019
342: make all_equal() faster r=jswrenn a=fyrchik

Hello!
This PR adresses #282 issue. Variant with `dedup` does not short circuit, but one with `all` does.
I have also added some benchmarks and test for an empty iterator.
```
test all_equal                                ... bench:     999,832 ns/iter (+/- 217,245)
test all_equal_default                        ... bench:   4,814,277 ns/iter (+/- 315,335)
test all_equal_for                            ... bench:   2,096,174 ns/iter (+/- 165,596)
```

Let me know, what do you think.

Co-authored-by: Evgenii <kraunid@gmail.com>
@Philippe-Cholet
Copy link
Member

Solved by #342.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants