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

impl FromParallelIterator for tuple pairs #802

Merged
merged 2 commits into from
Apr 1, 2021

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Sep 17, 2020

Like #604 for ParallelExtend, implementing FromParallelIterator for tuple pairs opens up new possibilities for nesting collect. The possibility of short-circuiting into Result<(T, U), E> for #801 is particularly motivating.

  • FromParallelIterator<(A, B)> for (FromA, FromB) works like unzip
  • FromParallelIterator<Either<L, R>> for (A, B) works like partition_map

@nikomatsakis
Copy link
Member

@cuviper

The possibility of short-circuiting into Result<(T, U), E> for #801 is particularly motivating.

Can you explain this a bit more? Is that potential future work or did I miss something.

@cuviper
Copy link
Member Author

cuviper commented Oct 20, 2020

It's a composition. Result<X, E> implements a short-circuiting FromParallelIterator for any X: FromParallelIterator. With this PR, that X can be (T, U), so we now have a short-circuiting unzip/partition_map via collect::<Result<(T, U), E>>().

@nikomatsakis
Copy link
Member

I see.

@cuviper cuviper marked this pull request as ready for review March 26, 2021 21:26
@cuviper
Copy link
Member Author

cuviper commented Mar 26, 2021

I finally got around to adding doc-examples for this.

@nikomatsakis
Copy link
Member

I think this will not get accepted in the stdlib, right?

@cuviper
Copy link
Member Author

cuviper commented Mar 30, 2021

I think this will not get accepted in the stdlib, right?

I expect not, but I haven't even proposed it yet. I think given how IntoIterator rust-lang/rust#78204 went, it's unlikely that FromIterator would be accepted. But I also think it's unlikely that FromIterator for tuples would ever be added with different semantics, so I'm not worried about us being incompatible.

And I do think it's still justified here, because it's otherwise really hard to collect multiple things in parallel. With serial Iterator, it's trivial to just write a manual for loop that unpacks items and stuffs them into however many collections you want. With ParallelIterator, collection is much more complicated to do manually, probably best as fold+reduce, or try_fold+try_reduce if you want short-circuiting. Yet this PR is a relatively small addition that enables a lot of power easily.

@nikomatsakis
Copy link
Member

That makes sense.

@nikomatsakis
Copy link
Member

I'd be r+ on landing this.

@cuviper
Copy link
Member Author

cuviper commented Apr 1, 2021

bors r=nikomatsakis

@bors bors bot merged commit c571f8f into rayon-rs:master Apr 1, 2021
@cuviper cuviper deleted the collect-pairs branch February 25, 2023 17:58
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