tim-harding / soa-rs

An SoA library for Rust
MIT License
113 stars 2 forks source link

Support Soa vector to be Sync, Send #7

Closed PPakalns closed 7 months ago

PPakalns commented 7 months ago

Useful when Soa is accessed from multiple threads.

Maybe this could be implemented in more safe way.

tim-harding commented 7 months ago

I agree that Soa should be Send and Sync where possible. It was at one point, but it looks like I forgot to add it back during a big safety refactor. However, I would choose to add it back to Slice, which Soa will inherit, and condition it on T's traits rather than the traits of its fields. I don't think SoaRaw is an appropriate place to implement these traits for the same reasons that raw pointers shouldn't implement them. Just because an f32 is shareable across threads doesn't mean that *mut f32 is, unless there is a safe interface around it. Since Soa and Slice semantically contain values of T, we should also respect manual trait impls or negative impls for that type. A two-liner in slice.rs should do the trick if you want the contribution.

PPakalns commented 7 months ago

I agree that Soa should be Send and Sync where possible. It was at one point, but it looks like I forgot to add it back during a big safety refactor. However, I would choose to add it back to Slice, which Soa will inherit, and condition it on T's traits rather than the traits of its fields. I don't think SoaRaw is an appropriate place to implement these traits for the same reasons that raw pointers shouldn't implement them. Just because an f32 is shareable across threads doesn't mean that *mut f32 is, unless there is a safe interface around it. Since Soa and Slice semantically contain values of T, we should also respect manual trait impls or negative impls for that type. A two-liner in slice.rs should do the trick if you want the contribution.

Good observation, needed to get it with Sync and Send and just implemented Sync and Send for type that compiler mentioned.

Implemented your suggestion.

tim-harding commented 7 months ago

Looks good to me!