Should I ever use T: Fn() + Send + Sync?

help
1

Continuing the discussion from Async equivalent of rayon::iter::ParallelIterator:

Should I ever use a T: Fn() + Send + Sync bound? It seems like I can convert any Fn() + Sync into an Fn() + Send + Sync by borrowing:

pub fn foo<T: Fn() + Send + Sync>(_f: T) {}

pub fn bar<T: Fn() + Sync>(f: T) {
    foo(&f)
}

(Playground)

For example, rayon::iter::ParallelIterator demands the mapping closure to be F: Fn(Self::Item) -> + Sync + Send. But I guess it could be defined in such a way that it doesn't need Send.

Is there any performance downgrade when I borrow from a Fn() + Sync closure to make it Send? What's the idiomatic bound I should use for such closures?

2

I don’t think it can easily. The map function returns an owned ParallelIterator which has a Send supertrait bound, and since it’s owned, it cannot use a borrow internally, either, to gain Send from Sync alone.

I would suspect there isn’t much performance impact as the function calls are static calls, so if repeated dereferencing were to happen in a tight loop, I would suspect it could be optimized away. This is pure guesswork though.

I’m not sure about idiomatics. It feels somewhat analogue in a way to the discussion of Stream parameters requiring Unpin or not, as you can always gain Unpin by adding a level of &mut … reference, but with Unpin you get more convenient usage without needing to pin! anything.

That being said, there is an argument to be had of leaving creation of an indirection to the caller, so in the other thread, where map(move |arg| constructor(arg)) or map(constructor) would have added removed some indirection compared to map(|arg| constructor(arg)) or map(&constructor), but left it to the caller to add the &-borrow if necessary seems a little bit “nicer” to me. After all, there aren’t all that many Sync + !Send types that would require this anyways, so in most cases it’s a slight win?

Even if loops manage to get optimized properly (which is just a guess on something that might happen, as noted above), the extra indirection is real, and it will have to be dereferenced at least once at some point.

1 Like
3

The Send bound does have an effect. If the closure is not Send, then you must not run its destructor on any other thread than the one you got it on.

5 Likes
4

Interestingly enough for_each requires Send + Sync even though the implementation clearly only utilizes the latter. Maybe the idea there is future-proofing the API in case the implementation would change to do make use of that Send after all, though I haven’t found confirmation for the actual motivation yet.

Edit: Found this PR now that mentions

It's not strictly necessary in all cases, like for_each that doesn't create a parallel iterator itself, but it seems nicer to apply a consistent requirement.

1 Like
5

Since then, we've also added for_each_init and for_each_with which do use their Send bound internally as e.g. .map_init(init, op).collect(). We could do the same with for_each being .map(op).collect(), or we could go the other way and manually implement the init/with variants without needing Send closures. Keeping the bound in the public API allows that flexibility.

1 Like
6

Well, since for_each doesn’t return an iterator, in that case it could always fall back to using a borrow as in .map(&op).collect(), but one might want to avoid the double indirection, so I suppose it’s a valid argument :slight_smile:

1 Like