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

join_slice() #402

Open
Nokel81 opened this issue Aug 22, 2017 · 7 comments · May be fixed by #406
Open

join_slice() #402

Nokel81 opened this issue Aug 22, 2017 · 7 comments · May be fixed by #406

Comments

@Nokel81
Copy link

Nokel81 commented Aug 22, 2017

It would be nice if it was possible to directly create additional items on the "queue".

What I am proposing is a new function that takes a slice of closures (until varadic parameters are a thing) which will add all additional closures that won't be immediately completed onto the queue for work stealing.

p.s. if this is a thing that should be added I am good with giving it a crack

@cuviper
Copy link
Member

cuviper commented Aug 22, 2017

join returns a tuple for the pair of return values, which may have different types. How would you return values from an arbitrary number of closures?

One possibility would be to implement this as a join! macro built around the regular join. Then calling join!(a, b, c) would expand something like join(a, || join(b, c)), probably also flattening the result to a 3-tuple.

@Nokel81
Copy link
Author

Nokel81 commented Aug 22, 2017

We could return some sort of heterogeneous list

@cuviper
Copy link
Member

cuviper commented Aug 22, 2017

Yes, well, the devil is in that detail. 🙂 A tuple is the closest thing I know to a heterogeneous list, and Rust can't deal with varying tuple lengths generically. (except through macros)

And actually, this is a problem for the "slice of closures" too, that slices need a homogeneous type. At least in this case we could use trait objects and homogeneous return values, like:

fn join_slice<R: Send>(funcs: &[&FnOnce() -> R + Send]) -> Vec<R>

If you have other ideas, please propose the actual function signature, as writing that out is a great way to figure out what's possible.

@fitzgen
Copy link

fitzgen commented Aug 25, 2017

One possibility would be to implement this as a join! macro built around the regular join. Then calling join!(a, b, c) would expand something like join(a, || join(b, c)), probably also flattening the result to a 3-tuple.

I actually implemented such a macro:

        macro_rules! join {
            ( $( self . $lhs:ident = $rhs:expr ; )* ) => {
                // Fork-join all of the expressions.
                let join!( < $( $lhs , )* ) = join!( > $( $rhs , )* );

                // And then assign them to self after they've all been computed.
                $(
                    self.$lhs = $lhs;
                )*
            };

            // Left hand side recursion to nest idents into tuple patterns like
            // `(x, (y, (z, ...)))`.
            ( < ) => {
                ()
            };
            ( < $x:ident , $( $xs:ident , )* ) => {
                ( $x , join!( < $( $xs , )* ) )
            };

            // Right hand side recursion to nest exprs into rayon fork-joins
            // like:
            //
            //     rayon::join(
            //         || x,
            //         || rayon::join(
            //             || y,
            //             || rayon::join(
            //                 || z,
            //                 || ...)))
            ( > ) => {
                ()
            };
            ( > $x:expr , $( $xs:expr , )* ) => {
                rayon::join( || $x , || join!( > $( $xs , )* ) )
            }
        }

And then usage looks like:

        join!(
            self.used_template_parameters = Some(immut_self.find_used_template_parameters());
            self.cannot_derive_copy = Some(immut_self.compute_cannot_derive_copy());
            self.has_type_param_in_array = Some(immut_self.compute_has_type_param_in_array());
            self.have_vtable = Some(immut_self.compute_has_vtable());
            self.cannot_derive_debug = immut_self.compute_cannot_derive_debug();
            self.cannot_derive_hash = immut_self.compute_cannot_derive_hash();
            self.cannot_derive_partialeq = immut_self.compute_cannot_derive_partialeq();
       );

Is this something you're interested in merging? If so, I can make a PR with sanity test coverage.

(The macro as it is right now is a little specific to my needs at the time, eg assigning to self, but it should be easy to generalize to either a bunch of lets or to any patterns.)

@Nokel81
Copy link
Author

Nokel81 commented Aug 25, 2017

Good job, I think that generalizing it would be a good idea to it can take any closure. Which I believe is an expr that we can just pipe into the join function

@cuviper
Copy link
Member

cuviper commented Sep 2, 2017

@fitzgen Yes I'd like to see that PR! And I agree with @Nokel81 that it should take general exprs. One could use it like join!(foo, || bar(), move || baz()). But that probably needs to assign temporary values before entering recursion to get the capture/move semantics right. Tricky...

I do realize that makes your self.foo = bar use case hard, since closures will try to capture your &mut self. There was a discussion about capturing more specific borrows. Maybe that just means you'd still be left with your own macro for your specific case.

@fitzgen fitzgen linked a pull request Sep 3, 2017 that will close this issue
@Nokel81
Copy link
Author

Nokel81 commented Nov 12, 2018

Would it be possible to use a builder pattern to get something like this to work?

Something like the following?

let mut res = vec![];
rayon::gate(&mut res)
            .add(|| x)
            .add(|| y)
            ...
            .wait();

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

Successfully merging a pull request may close this issue.

3 participants