contain-rs / par-vec Goto Github PK

The current implementation does not check for the possibility of false-sharing, which is an unexpected bottleneck in parallel computation.

It might be a good feature to have additional construction method which errors if slices meant for two different threads might end up sharing a cache line. Unfortunately the alignment of the allocation which Vec takes isn't controllable at the moment, so this can't be a 100% guarantee AFAIK.

A thread panicking while operating on a ParSlice could leave an incomplete result, similar to what would happen if a thread panicked while holding a MutexGuard or a RWLockWriteGuard. This suggests that ParSlice should have similar poisoning behavior.

However, poisoning a whole vector just because a thread panicked while holding a subslice seems like overkill. As a compromise, unwrapping a ParVec could return two vectors: one with all the elements from healthy (unpoisoned) subslices, and one with the contaminated elements from poisoned subslices. This would add complexity to the implementation as subslices are currently only tracked via the strong refcount on Arc, and would also require additional allocations where the current API uses existing storage.

But I would also argue that ParVec should not poison altogether, because it does not allow actual concurrent (or mutually exclusive) access to its elements; each is only ever touched by either the originating thread, or the thread that its containing ParSlice was sent to, and never at the same time, mutably or otherwise. In this way, incomplete results seem more like a logic error than undefined behavior, and thus should be handled by the user.

It is clear some discussion is merited here. Is ParVec safe as it is, or is it hiding dangerous, undefined behavior behind a "safe" API?

cc @gankro, @huonw

'static is not necessary anymore; it is only an implementation detail of ParSlice that can (and should) be circumvented.

Similarly, Sync is only to satisfy the requirements of Arc; by design, ParVec does not provide concurrent aliases to data. Thus, the Sync bound can be removed with an inner wrapper implementing Sync manually.

I'm working on both of these in my personal fork. I will have a PR available within the hour.