[Do not merge] Experimental multithreaded physics #2070
Conversation
|
So awesome to see this being worked on, even though just for testing purposes so far. |
|
points finger in air Is this the panacea. |
|
I tested between the 0.45 RC and this artifact. I didn't measure any differences, but as I said on Discord, I've never been physics limited. I have a 4th gen i7 in Windows 10 for reference. Are there any settings I need to enable? Excited to see what comes out of this, even if it's not directly helping me! |
|
You need to increase the number of threads in the config. Also, it's likely the artifact won't work properly because its dependency may have not built with the cmake option toggled on. |
Does upstream Bullet know about these issues? They be able to help? In addition to this, are we sharing state or passing by reference what should be passed by value to the threads? |
|
Seems good to me, I'm not getting any errors. |
|
@psi29a I suppose you were actually trying to quote terabyte25 and not edit his post..? :) |
Did you set the value here: What value did you chose? If you just tested the branch as-is, you won't get any errors since it will remain in one thread. @lysol90 that was my intent, yes, thanks ;) |
|
@psi29a I used 2 threads, then spawned ~150 vivecs. I made sure to configure Bullet3 with the multithreaded option as well. |
|
I've run it with tsan and there are definitely a ton of races, here's one example: I think the problem is that while you lock the global mutex when updating Also, not sure why you're getting signal 7 (SIGBUS? maybe it's the same reason but you didn't build with aserts?), I'm however getting an abort at https://github.com/bulletphysics/bullet3/blob/8bc1c8e01b1b2b9284df08385da0e03241f4e6aa/src/LinearMath/btThreads.cpp#L234 which basically seems that every thread gets it own stack until it runs out (https://github.com/bulletphysics/bullet3/blob/6e4707df5fa1f9927109e89a7cd2a6d6a6ddd072/src/BulletCollision/BroadphaseCollision/btDbvtBroadphase.cpp#L242); and since we keep spawning threads and the counter never decrements, it runs out pretty fast. It seems they fixed the issue bulletphysics/bullet3@74223ce fairly recently, however I think bullet still expects you to pre-spawn threads and reuse them instead of spawning them every time |
That would be difficult, because actors can interact. That is, actors that can interact within a given frame must be simulated one after the other (so, on the same thread), not interacting with past versions of one another, or they'll end up inside one another. The way things are structured right now, interacting with the "new" version of an actor's state means that you have to update the collision world. You could do it if actors weren't part of the world. |
|
I wonder how TES3MP handles players in different cells. |
Interesting, how other games handle collisions? For example, Mount & Blade can handle several hundreds of actors in area without issues or FPS drop, even if they collide each other. I doubt it just moves actors one by one as OpenMW does. |
|
bulletphysics/bullet3#2004 (comment) |
|
Can not reproduce crashes with bullet 2.88, but it still would be nice to spawn threads only once somehow. |
| @@ -1310,12 +1353,10 @@ namespace MWPhysics | |||
| else if (heightDiff < 0) | |||
| stats.addToFallHeight(-heightDiff); | |||
|
|
|||
| mMutex.lock(); | |||
| mMovementResults.push_back(std::make_pair(iter->first, interpolated)); | |||
There was a problem hiding this comment.
It seems to me that mMovementResults updated only in this line (apart from cleaning before calculations) and newer used in calculations, so each thread may fill its own vector and then merge them in main thread. Then protection by a mutex not to be necessary
| standingCollisionTracker[ptr] = ptrHolder->getPtr(); | ||
| mutex.unlock(); |
There was a problem hiding this comment.
It is always better to use lock_guard to guarantee that the mutex will be always released, even when it seems that no exceptions will exist. C++ has long-range action, changes in one part of the code can easily affect another
| currentThread++; | ||
| } | ||
|
|
||
| for(auto&& thread : threads) |
There was a problem hiding this comment.
Here the managing thread will stand idle while other threads are engaged in work. It is possible to generate mNumThreads-1 of additional threads and do calculation as well in this thread
|
|
||
| void PhysicsSystem::solveTask(int offset, int module, int numSteps) | ||
| { | ||
| int i = 0; | ||
| const MWWorld::Ptr player = MWMechanics::getPlayer(); | ||
| const MWBase::World *world = MWBase::Environment::get().getWorld(); | ||
| PtrVelocityList::iterator iter = mMovementQueue.begin(); | ||
| for(;iter != mMovementQueue.end();++iter) |
There was a problem hiding this comment.
You protect mMovementResults with mutex, but not protect mMovementQueue though they have the same type, so are not thread-safe. Or this vector can not change during physics update? If so, I think it is better to explicit comment that here (or even better, as doxygen comment on mMovementQueue)
|
Closing this PR since is was designed as a proof of concept and I have no skills to implement it properly. |
Do not merge, just a proof of concept!
Requires a Bullet, configured with a -DBULLET2_USE_THREAD_LOCKS=ON.
Indeed improves performance a lot on physics-bounded setups.
Unfortunately, it seems the convexSweepTest() is not fully threadsafe - I get crashes somewhere inside the Bullet code (usually here or here) during intensive collisions (e.g. after spawn of many actors in the same position).
Also sometimes I get the
error without log.
Any help to track down these issues would be welcome.