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

Specialize "loops of next" #818

Merged

Conversation

Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented Dec 13, 2023

There are some cases where next/next_back is repeatedly called in a loop, where a specialized method would do the same job, except it might be faster if the iterator they adapt has some specialized faster method.
It was the case in #816.

cargo bench --bench specializations "(filter_ok|filter_map_ok|unique|unique_by)/next"

unique/next             time:   [18.728 µs 18.779 µs 18.843 µs]
unique/next             time:   [14.166 µs 14.274 µs 14.406 µs]
                        change: [-26.223% -25.048% -23.941%]

unique/next_back        time:   [18.523 µs 18.599 µs 18.698 µs]
unique/next_back        time:   [14.189 µs 14.259 µs 14.344 µs]
                        change: [-25.116% -24.300% -23.568%]

unique_by/next          time:   [20.766 µs 20.798 µs 20.833 µs]
unique_by/next          time:   [19.123 µs 19.174 µs 19.230 µs]
                        change: [-8.9607% -7.5864% -6.1789%]

unique_by/next_back     time:   [20.852 µs 20.915 µs 20.979 µs]
unique_by/next_back     time:   [20.740 µs 20.825 µs 20.919 µs]
                        change: [-1.1273% -0.6647% -0.1885%]

filter_ok/next          time:   [910.82 ns 915.08 ns 920.18 ns]
filter_ok/next          time:   [880.88 ns 883.96 ns 887.81 ns]
                        change: [-5.5278% -4.1578% -2.4871%]

filter_map_ok/next      time:   [863.55 ns 865.77 ns 868.14 ns]
filter_map_ok/next      time:   [849.20 ns 852.26 ns 855.36 ns]
                        change: [-5.0466% -3.7661% -2.6449%]

Those benchmarks are not that impressive, they are based on slices that roughly reimplement the while loop themselves. But if the iterator they adapt have a specialized try_[r]fold on which find/find_map/rfind usually rely, then it should be faster.

The code is shorter in multiple cases and I believe clearer.

The partition function did not have any benchmark, there is now one and there are no difference, both 330µs.

Copy link
Member

@phimuemue phimuemue left a comment

Choose a reason for hiding this comment

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

Thanks.

while let Some(v) = self.iter.iter.next_back() {
if let Entry::Vacant(entry) = self.iter.used.entry(v) {
let UniqueBy { iter, used, .. } = &mut self.iter;
iter.rev().find_map(|v| {
Copy link
Member

Choose a reason for hiding this comment

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

Off-topic: Is it strange that there's rfind, but no rfind_map?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not off-topic to me. I was wondering the same question while writing this.
Maybe there is no large performance gain to expect compare to .rev().method(). I searched a bit but did not find anything.
More generally, DoubleEndedIterator does not have a lot of methods.

@Philippe-Cholet Philippe-Cholet added this pull request to the merge queue Dec 14, 2023
Merged via the queue into rust-itertools:master with commit 451c72a Dec 14, 2023
8 checks passed
@Philippe-Cholet Philippe-Cholet deleted the specialize-looped-next branch December 14, 2023 12:39
@Philippe-Cholet Philippe-Cholet added this to the next milestone Dec 14, 2023
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

Successfully merging this pull request may close these issues.

2 participants