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

More specialization benchmarks #806

Conversation

Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented Nov 23, 2023

I do not want us to add specialization benchmarks one by one in future pull requests so this is mostly done!
Not all of our iterators are benchmarked here:

  • group_by, chunks, tee, rciter just like in More specialization tests #799.
  • peeking_take_while and take_while_ref do not take ownership (see Ownership issues for Itertools methods #710) of the iterator they adapt and it's therefore problematic to add them to the bench_specializations macro.
  • process_results does not give an iterator but process one and we can't create a ProcessResults object outside of the library.
  • MapForGrouping used for GroupingMapBy but it's strictly internal to the library, we will need to benchmark a (non-iterator) method of GroupingMapBy to benchmark the future MapForGrouping::fold.
  • unfold, iterate: infinite iterators, some benchmarks would not end. We can avoid running them but maybe we should handle them differently later.

After this, we will be able to specialize most fold methods (see #755) faster because their specialization tests/benchmarks are written and we will just need to write the specialization in a commit, run tests, benchmark it before/after and review the PR.

I do not want to add them one by one in future pull requests so this is (mostly) done!
`tuple_windows, circular_tuple_windows, tuples, tuple_combinations, combinations, combinations_with_replacement, permutations` all generate elements of various fixed lengths.
Benchmark against those lengths (here 1 2 3 4) might provide more insight.
Copy link
Member

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

This is excellent!

@jswrenn
Copy link
Member

jswrenn commented Dec 7, 2023

Go ahead and click Merge if you're ready to do so.

@Philippe-Cholet Philippe-Cholet added this pull request to the merge queue Dec 7, 2023
Merged via the queue into rust-itertools:master with commit b559bb5 Dec 7, 2023
8 checks passed
@Philippe-Cholet Philippe-Cholet deleted the more-specialization-benchmarks branch December 7, 2023 15:24
@Philippe-Cholet Philippe-Cholet added this to the next milestone Dec 7, 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