-
Notifications
You must be signed in to change notification settings - Fork 310
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 TupleCombinations::fold
#775
Specialize TupleCombinations::fold
#775
Conversation
25fceae
to
0e15566
Compare
`I::Item` and `A` are the same so the "where condition" is not really changed, but it is apparently needed here. We fold `c` then for each item of `iter`, we fold the cloned `iter`. The `while let` loop should probably be converted to `fold` somehow later (if possible) but the core logic is done and it is already really faster.
0e15566
to
21004ed
Compare
Maybe I should merge EDIT: It would be 6% slower. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Philippe-Cholet, sorry for the delay. Nice that's is already faster. Feel free to merge if you want.
Is this specialization already tested in our specialization tests?
As mentionned in the too big message, the specialization test was previously added (by me). |
Related to #755.
This is so much faster that I'm wandering if I'm missing something here!
I previously added the specialization test: a9aaeb4
I'm unsure we can fold the
while let
loop as we need to cloneiter
at every step of it.Both attempts below successfully pass the tests but are not faster.
Maybe there is a solution involving
std::cell::???
but I'm not familiar enough with this, are you?I tried with
itertools::rciter
but it was 60% slower (andRc
means theuse_alloc
feature). And with a basicRcIter::fold
specialization, I even had an (expected) error at runtime.I eventually found a way to rely on
iter.fold
usingunsafe
only (to hold&mut I
and&I
at the same time) but it's the same performance as the currentfold
specialization.I'm not familiar with unsafe Rust so it might just be dumb in the first place.