Add method fold_with to Producer and UnindexedProducer

[bluss]

This method allows the producers to customize the inner loop. This is
useful for iterators that can provide a more efficient inner loop than
the default for loop.

Example: multidimensional arrays.

[bluss]

This allows parallel producers to customize the inner loop. This is a temporary measure until iterators gain this natively (one proposal is the fold_ok method).

This is marked WIP because I haven't been able to use this on UnidexedProducer parallel iterators to make a real difference.

This is because for very lightweight operations, I need to set the weight very low; but the unindexed producer has no way of communicating the weight to the splitter (?), so it always splits the producer down to the limit used by fn can_split().

I do have success using ParallelIterator with UnindexedProducer if I do something heavy enough. For example: if the lower limit for can_split is 128 elements and the operation is f64::exp it's a win.

Replacing that expensive function call with something that autovectorizes has big gains:

fn fastexp(x: f64) -> f64 {
    let x = 1. + x/1024.;
    x.powi(1024)
}

However, only if the can split threshold is hardcoded to a larger limit. (Something like at least 10 000 elements at least, it turns out, for “fastexp”).

[bluss]

As an example of potential gains, the PR enables fastexp to be autovectorized in ndarray (with the parallel traits implemented, not yet integrated into ndarray), for a case like this:

    let mut a = Array2::<f64>::zeros((M, N));
    let mut a = a.slice_mut(s![.., ..-1]);
    a.view_mut().into_par_iter().for_each(|x| *x = fastexp(*x));

The inner loop customization is needed because the array is not contiguous (due to the slicing), yet each row is contiguous, so the inner loop can still be subject to loop optimizations if we circumvent the general Iterator::next.

.weight() has no effect here it seems? I think a way to impact the unindexedproducer's splitting is needed.

The WIP parallel iterator implementation is here: https://github.com/bluss/rust-ndarray/compare/par-iter...par-iter-elements?expand=1

[nikomatsakis]

Hi @bluss -- very cool! I will attempt to take a look soon. :)

[cuviper]

I think it may be more useful to leave complete out of this. At least for ChainProducer, that would let it call fold_using separately on its two parts, recursively if there are multiple levels of chains, then just let the bridge functions complete it at the end.

[bluss]

Neat. I know that idea :D rust-lang/rust#37315

[cuviper]

I knew I had seen it somewhere! 😄

[cuviper]

I might bikeshed this to something like fold_with, not sure, but anyway I like the idea. It's also interesting that the full concept could be applied to the short-circuiting fold_ok -- very nice!

As for tuning parameters, note that we're trying to move away from weight -- see #111. This is why I didn't bother adding the cost function to the new UnindexedProducer at all. But we probably do want some kind of tunables, and I think a minimum split size would address what you're wanting here. My rough idea is that we need both a minimum (jobs should be at least this big) and a maximum (jobs should be no bigger, e.g. used to fit the working size in cache).

so it always splits the producer down to the limit used by fn can_split().

This shouldn't always happen unless your workload is very small. The "thief" splitter works by first splitting roughly to the number of job threads, and then each time a job is stolen that gets split to the number of threads again. Towards the end of task, there probably will be a lot of eager stealing and thus splitting, maybe down to the minimum, but for most of a busy workload they should stay relatively large.

Automatic tuning is hard -- if you have input on manual tuning parameters, please chime in!

[bluss]

One important piece of tuning turns out to be (maybe not surprisingly) using an explicit configuration to use one thread per core (so not counting hyperthreading).

[cuviper]

I'm happy with this, but I'll leave it open for @nikomatsakis to take a look.

[bluss]

Sure. I'll squash this. I removed the WIP tag since it is an improvement when using a thread per core.

[bluss]

I'll squash when you think it's ready, I mean.

[nikomatsakis]

At some point, we discussed whether we should have Producer: UnindexedProducer, but we didn't have a good reason for it (even though it logically makes sense). The existence of this duplicate code might provide such a reason.

[bluss]

I'm new to the internals, maybe split_at can be removed and only split is left?

bridge_producer_consumer (indexed case) is this today:

            let mid = len / 2;
            let (left_producer, right_producer) = producer.split_at(mid);
            let (left_consumer, right_consumer, reducer) = consumer.split_at(mid);

It could change to something like:

            let (left_producer, right_producer) = producer.split();
            let mid = left_producer.len();
            let (left_consumer, right_consumer, reducer) = consumer.split_at(mid);

so the difference is that an indexed producer can tell its length (and that it has a meaningful length or before / after ordering of the halves).

ndarray's unindexed producer can tell the length of each part perfectly. What sets it apart is that (1) it can't split exactly at len / 2 since it must split in chunks of some axis' size (2) It doesn't want to split so that it has a "before" and "after" half. It prefers an unordered split for performance.

[bluss]

About order:

Take a 3 × 4 array

[[1, 2, 3, 4],
 [5, 6, 7, 8],
 [9, 0, 1, 2]];

The logical order of the regular .iter() is defined to be "logical" row major, which means the elements in the order we read them: 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2. The memory order of the array is separated from the logical order of .iter().

For parallelism, we guarantee no order and the unindexed producer for an array view will pick either of these two splits, depending on which preserves memory locality:

[[1, 2, | 3, 4],
 - - - - - - - - - v.split_at(Axis(0), 1) // best for row major memory order
 [5, 6, | 7, 8],
 [9, 0, | 1, 2]];
        \ v.split_at(Axis(1), 2)  // best for column major memory order

[nikomatsakis]

No, you need split_at in order to handle zip.

[nikomatsakis]

(That said, I did want to consider changing the structure so that the producer can offer a suggestion about where to split instead. But I haven't decided on the best way to do it.)

[nikomatsakis]

So the key distinctions are:

• can split at an arbitrary point
  • needed for zip
• knows exactly how many elements it will produce
  • needed for precise consumers

Today we tie those together, but they could be separated. Something like:

• UnindexedProducer
  • can split itself at some point
• KnownLengthProducer: UnindexedProducer
  • can tell you the number of items it will produce
• Producer: KnownLengthProducer
  • can further be split at an arbitrary point

we might then have two bridge methods:

• one takes a KnownLengthProducer and a Consumer
  • this is the one that today requires a Producer
  • it would instead invoke producer.split(), get resulting length, and invoke consumer.split_at(), as you suggested
• one takes a UnindexedProducer and a UnindexedConsumer
  • same as today

We would probably have to tweak the ParallelIterator hierarchy a bit too, but the idea is that zip() would require that its sub-iterators can produce a full Producer (or, potentially, that the LHS can produce a KnownLengthProducer and the RHS can produce a full Producer). It's split() implementation can then call left.split() and right.split_at(left_split.len())`.

Make sense?

[bluss]

Yes that makes sense. I'm happy with the two traits that are now; since the UnindexedProducer here didn't want to participate in zip in any way even if it can tell its exact length, the reason is that it doesn't produce elements in any particular order.

[cuviper]

At some point, we discussed whether we should have Producer: UnindexedProducer, but we didn't have a good reason for it (even though it logically makes sense). The existence of this duplicate code might provide such a reason.

Yes, although IIRC we were also wanting impl<T: Producer> UnindexedProducer for T to avoid duplicating can_split/split boilerplate, but that would really want specialization. Plus that boilerplate would never actually be invoked.

In the case of just fold_while, perhaps we could make a BaseProducer common to both, so the boilerplate is a one-line empty impl in most cases. Or we could extract this body to a shared free function just to avoid the duplication. But this is only 6 lines, let's not overthink it. :)

[bluss]

I think I'd vote for not more trivial traits to implement, unless they do something noticeable.

[cuviper]

Yet another option is to add something like:

trait Folder {
    // ...
    fn consume_iter<I:IntoIterator>(self, iterator: I) -> Self {
        // ...
    }
}

... which might be useful to folks specializing fold_with too.

[bluss]

Squashed commits together.

[nikomatsakis]

OK, I propose we merge this as is, but experiment with the producer hierarchy a bit.

[bluss]

Will this be in a point release, like 0.6.1 ?

[nikomatsakis]

@bluss presumably

[bluss]

Rayon is making big strides anyway, and that's exciting.

[nikomatsakis]

@bluss I'll push a new release ASAP, no reason to wait I guess