dimforge / nphysics

2 and 3-dimensional rigid body physics engine for Rust.
https://nphysics.org
Apache License 2.0
1.63k stars 121 forks source link

Need overhaul for embedable multi-theading support #71

Closed ahmedaliadeel closed 6 years ago

ahmedaliadeel commented 8 years ago

Dear Authors, this project is valuable and has much better potential use if you provide and overhaul for putting the World and other stuff Arc/Mutex compatible.

Regards

pengowen123 commented 7 years ago

I am using nphysics in one of my projects that also uses specs, which is built around multithreading. The use of Rc in nphysics means I must make my physics system single-threaded and non-modular.

I would be glad to help switch to something thread-safe, but that would make nphysics slower for everyone that doesn't care about multithreading. Any ideas about the best way to add thread support without a cost for people who don't use it would be aappreciated.

pengowen123 commented 7 years ago

I decided the best way to add this is with a cargo feature and a module with a wrapper type for Rc/Arc depending on whether the feature is enabled. I have implemented this here. The examples seem to run fine with this, but I am doing more testing with my own project to make sure nothing breaks.

sebcrozet commented 7 years ago

@Ralith, @mrluc, I'm currently starting to think about which design changes I'd like to make on nphysics. And whether or not we want Arc/Mutex compatibilty would affect the design decision. So I would be interested to know why you downvoted this issue?

Ralith commented 7 years ago

It would be marginally convenient to be able to send a complete nphysics world between threads, but in practice I have a difficult time imagining a defensible case in which you actually need Mutex in all the nphysics handle types (which is not the only solution). I mainly just don't want to be paying runtime overhead on tons of routine nphysics operations for the sake of locking and/or atomicity that is never actually needed if your code is well engineered.

Kerollmops commented 7 years ago

Why did you don't let an only thread touch the nphysic world receiving it's instructions from a channel ?

mrluc commented 7 years ago

Thumbs-down is a bit too terse, I suppose. :)

In my case, it was just a reaction to something that I didn't see as being necessary - like @Kerollmops, I use channels to communicate with an nphysics world - and on the other hand, there are plenty of bad reasons to use Arc/Mutex. But I feel I can safely assume that if @sebcrozet is considering it, it's for a good reason.

sebcrozet commented 7 years ago

@Ralith, @Kerollmops, @mrluc Thanks for your inputs!

I wouldn's say I am actually considering this. Right now I am totally undecided and wanted to get some ideas about why people would or wouldn't need that, and if there were alternative. For the past couple of years some users complained about this issue, especially when attempting to use nphysics with an ECS.

The solution to use channels for communication seems to be solid and might be a good answer when users encourter this issues.

torkleyy commented 7 years ago

Hey there! I'm actively working on Specs and we really get this question pretty often. Unfortunately, sending instructions over a channel isn't enough. Many types of nphysics aren't thread-safe, thus cannot be used as components.

I have to admit that I never tried myself using both crates together, but others have tried and failed :( So without knowing nphysics, here's what I think is the main reason nphysics and specs don't play together very well:

The physics simulation is centered around a World struct, which you can add all your rigid bodies to. Now the problem is that this conflicts with the ECS concept, since both the physics simulation and Specs have their own world. Thus, one would even be required to synchronize them and add / remove rigid bodies based on what happens in the ECS world. So the conclusion I've come to is that using nphysics3d's World is too high-level. Probably one could use lower-level parts of nphysics (or just ncollide)?

@Ralith I'm sorry, what exactly do you understand by "well-engineered code"? Is it that you dislike ECS or do you know a better solution to this problem? I agree that wrapping everything in Mutex / RwLock isn't the solution, though.

Xaeroxe commented 7 years ago

I think I speak for everyone on Amethyst when I say we 110% support this motion. If you decide to go forward with it I'd love to help with the implementation.

Xaeroxe commented 7 years ago

We use shred to avoid excessive run time overhead for our parallel systems. It's a lock-free multi-threading solution. specs is mostly a more convenient front-end to shred. I admittedly don't have a ton of experience with physics engines so I'm not sure how useful parallel computation is to your solver, but I'm just providing these so they can be considered.

Ralith commented 7 years ago

I'm sorry, what exactly do you understand by "well-engineered code"?

To clarify, I mainly want to avoid replacing every Rc<RefCell<T>> with Arc<Mutex<T>> or something analogous. Such a change would only make sense in principle if there was a genuine need to access nphysics state from other threads simultaneous to stepping the simulation and possibly block the simulation in doing so, which sounds like poor design to me compared to simply giving nphysics a dedicated thread and communicating with it as necessary.

I recognize that the !Send nature of everything is inconvenient, and while I don't currently use an ECS myself, I don't have anything against them; if World could somehow be factored out to make things work better, that'd be neat!

One way to provide something much like the current interface would be to implement handles as indexes into (typically World-owned) storage, rather than true pointers; then World could be Send.

sebcrozet commented 7 years ago

Yes, handles-as-indexes is one way to get rid of pointers. The drawback is that the user will always have to call some method on the scene to modify a rigid body (e.g. instead of calling rigid_body_handle.borrow_mut().set_translation(new_pos), he wil have to use physical_world.set_translation(rigid_body_handle, new_pos). I'm wondering if the cleanest way of doing this could be to design nphysics to fit into an ECS pattern, define rigid bodies as components (or split it into multiple components), and let the ECS engine deal with the rigid-bodies <-> index association. I am not sure yet if this is possible or if this wouldn't have bad impacts on performances.

Xaeroxe commented 7 years ago

My plan for Amethyst before reading this issue was actually to make a specs powered physics engine so I'd be ecstatic if you were willing to adapt this into that. I think it should be possible, just based on a quick overview of your code base. ECS systems do come with very slight overhead but most ways of managing several structures like this come with the same overhead. As for performance, I have some tips: choosing the correct storage type for your components is essential to good performance. Storage types are not a "one size fits all" deal and picking the right one for each component can vastly improve performance. I also noticed this code base has a few for loops that might benefit from use of rayon. Let me know if you decide to go forward with this and then we can stick our heads together on the best way to make it happen.

Ralith commented 7 years ago

I also noticed this code base has a few for loops that might benefit from use of rayon

Libraries spawning threads internally is bad practice, IMO. Provide an interface that allows work to be executed with whatever parallelism the application has available, don't make a bunch of assumptions about resource availability and hardcode it.

Ralith commented 7 years ago

I've recently adopted specs in a project with nphysics. FWIW, integrating with today's nphysics is simple, thanks to specs' loosely-bound architecture. You don't need any dedicated threads or message passing at all; just use run_now to invoke your system, and copy position data into components after stepping the nalgebra simulation, rather than trying to store rigid body handles as components directly. The one downside is that you need to manually maintain a mapping between specs entities and nphysics bodies, which is extra indirection. nphysics also might still benefit from a more component-oriented layout itself vs. scattering data all over the heap, of course.

sebcrozet commented 6 years ago

Starting with the version 0.8.0, nphysics now have a Send+Sync world.