settle on one reduce, one fold about rayon HOT 10 CLOSED

Just had a random thought about fold. It is unfortunate to take 3 closures. I wonder if it would be possible to have an operation that has this signature:

Fn() -> U
Fn(U, T) -> U

and basically transforms an iterator over T into an iterator over U. This is then the first part of fold -- it's sort of like a "partial reduce" that randomly picks chunks of your iterator to fold up, so if you have an iterator over T, you wind up with an iterator over some number of U's, but you don't know how many. You can then combine with a reduce to finish everything.

Hence nbody would become:

iterator
    .fold(
        || (0, 0)
        |(diff1, diff2), item| ...)
    .reduce(|(diff1, diff2), (diff1b, diff2b)| ...)

from rayon.

I like how that fold-reduce looks, but will that interim object be a full ParallelIterator itself?

iterator
  .filter(|&x| x > 0)
  .fold(Vec::new, |v, x| { v.push(x); v })
  .filter(|v| v.len() > 42)
  .flat_map(|v| v)
  .sum()

That's contrived nonsense, of course, but perhaps there's some real use to true generality. And it may be a little confusing if this FoldIter::reduce is different than ParallelIterator::reduce.

from rayon.

The general point about having one reduce to rule them all is great, but I really wish it could be the single-closure form like reduce_with. I don't know how you'd implement this without Option though. It might look more like Iterator's internal select_fold1, but that doesn't fit the force-fed Consumer model.

(tangent) I've also been thinking about short-circuiting ops like find, any, etc., and these also don't fit well in the current Consumer model. Maybe there's a better push-pull balance we could find here.

from rayon.

@cuviper I have assumed that short-circuiting ops would use a shared Arc<AtomicBool> to signal when they should stop (and it'd be somewhat undefined which one you get, of course).

(But I've also wondered if we ought to make find guarantee that you get the first one -- I guess that kind of stinks though.)

from rayon.

I tend to think it's ok for it to take two closures. After all, fold takes two arguments.

I can't decide whether it's ok to use method names like fold (or find) which overlap seq iters but give them slightly different semantics (e.g., fold doesn't go L->R).

from rayon.

@cuviper

I like how that fold-reduce looks, but will that interim object be a full ParallelIterator itself?

I figured it would be, yes, but I haven't tried to implement it or anything.

from rayon.

I tend to think it's ok for it to take two closures. After all, fold takes two arguments.

Yeah, but fold has to create the base item at some point, whereas reduce could just use the first item it encounters. Two-closure reduce also means you have to repeat yourself:

iterator
    .fold(
        || (0, 0),
        |(diff1, diff2), item| ...)
    .reduce(
        || (0, 0),
        |(diff1, diff2), (diff1b, diff2b)| ...)

from rayon.

@cuviper

Two-closure reduce also means you have to repeat yourself...whereas reduce could just use the first item it encounters

Well, if you used one-closure reduce, you would get back an option, so you'd probably wind up writing:

iterator
    .fold(
        || (0, 0),
        |(diff1, diff2), item| ...)
    .reduce(
        |(diff1, diff2), (diff1b, diff2b)| ...)
    .unwrap_or((0, 0));

I think the repeating is due to separating fold out into a distinct operation. It doesn't really bother me much though.

from rayon.

I implemented the proposed reduce method in this branch.

I'll try some experiments around fold. I'm not even sure if what I proposed can really be implemented nicely.

from rayon.

OK, I've implemented my proposed fold too. I'm quite a fan of it, I have to admit. =)

The performance seems to be ever so slightly slower than the old fold, but basically the same:

lunch-box. cargo bench -- nbody
running 4 tests
test nbody::bench::nbody_par                ... bench:  13,558,854 ns/iter (+/- 5,578,199)
test nbody::bench::nbody_parreduce          ... bench:  21,083,144 ns/iter (+/- 9,391,178)
test nbody::bench::nbody_parreduce_master   ... bench:  20,730,507 ns/iter (+/- 10,288,526)
test nbody::bench::nbody_seq                ... bench: 135,046,865 ns/iter (+/- 39,235,146)

Here nbody_parreduce is the new fold and nbody_parreduce_master is the old one. You can see there is a 1.7% difference. I suspect this could be recovered with a bit of tuning and/or rustc optimization, and anyway the new version is certainly more flexible (e.g., you could collect those results into a vector, not just reduce them).

from rayon.