Simplify parallel iteration methods

[JoJoJet]

Objective

The QueryParIter::for_each_mut function is required when doing parallel iteration with mutable queries.
This results in an unfortunate stutter: query.par_iter_mut().par_for_each_mut() ('mut' is repeated).

Solution

• Make for_each compatible with mutable queries, and deprecate for_each_mut. In order to prevent for_each from being called multiple times in parallel, we take ownership of the QueryParIter.

---

Changelog

• QueryParIter::for_each is now compatible with mutable queries. for_each_mut has been deprecated as it is now redundant.

Migration Guide

The method QueryParIter::for_each_mut has been deprecated and is no longer functional. Use for_each instead, which now supports mutable queries.

// Before:
query.par_iter_mut().for_each_mut(|x| ...);

// After:
query.par_iter_mut().for_each(|x| ...);

The method QueryParIter::for_each now takes ownership of the QueryParIter, rather than taking a shared reference.

// Before:
let par_iter = my_query.par_iter().batching_strategy(my_batching_strategy);
par_iter.for_each(|x| {
    // ...Do stuff with x...
    par_iter.for_each(|y| {
        // ...Do nested stuff with y...
    });
});

// After:
my_query.par_iter().batching_strategy(my_batching_strategy).for_each(|x| {
    // ...Do stuff with x...
    my_query.par_iter().batching_strategy(my_batching_strategy).for_each(|y| {
        // ...Do nested stuff with y...
    });
});

[JoJoJet]

Marking this as complex since there is a decent chance that I've missed some unsoundness.

[SkiFire13]

In order to prevent for_each from being called multiple times in parallel, we ensure that QueryParIter is !Sync for mutable queries.

Couldn't it just consume self instead? Iterator::for_each and rayon::ParallelIterator::for_each both consume self and it makes sense because after that you probably don't care about the iterator anymore. And if you do you can just create a new one since it's very cheap.

[JoJoJet]

Couldn't it just consume self instead? Iterator::for_each and rayon::ParallelIterator::for_each both consume self and it makes sense because after that you probably don't care about the iterator anymore. And if you do you can just create a new one since it's very cheap.

Yes, that is another option.

[james7132]

I'm much more comfortable having take ownership of QueryParIter over any borrow for the sake of soundness. This ensures there's no potential aliasing going on, and constructing a parallel iterator is by far the least expensive part of the operation, so there's very likely no significant perf blockers by doing so.

Note: you may also need to change the other builder methods like QueryParIter::batching_strategy to pass by ownership.

[SkiFire13]

In order to prevent for_each from being called multiple times in parallel, we ensure that QueryParIter is !Sync for mutable queries.

Thinking more about it, this doesn't prevent for_each to be called multiple times reentrantly, which should be possible as long as this is done on the same thread that owns the QueryParIter (e.g. by using something like fragile).

[JoJoJet]

I've modified for_each to take ownership, to fix the unsoundness that @SkiFire13 found.

[mockersf]

@JoJoJet could you fix the conflict and the comment above?

[JoJoJet]

I've addressed James' comment and fixed the merge conflict.