Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiThreading for Bullet 2.x #847

Merged
merged 1 commit into from
Nov 3, 2016

Conversation

lunkhound
Copy link
Contributor

@lunkhound lunkhound commented Oct 30, 2016

(an updated version of #390)

  • fixing various race conditions throughout (usage of static vars, etc)
  • addition of a few lightweight mutexes (which are compiled out by default)
  • slight code rearrangement in discreteDynamicsWorld to facilitate multithreading
  • PoolAllocator::allocate() can now be called when pool is full without
    crashing (null pointer returned)
  • PoolAllocator allocate and freeMemory, are OPTIONALLY threadsafe
    (default is un-threadsafe)
  • CollisionDispatcher no longer checks if the pool allocator is full
    before calling allocate(), instead it just calls allocate() and
    checks if the return is null -- this avoids a race condition
  • SequentialImpulseConstraintSolver OPTIONALLY uses different logic in
    getOrInitSolverBody() to avoid a race condition with kinematic bodies
  • addition of 2 classes which together allow simulation islands to be run
    in parallel:
    • btSimulationIslandManagerMt
    • btDiscreteDynamicsWorldMt
  • MultiThreadedDemo example in the example browser demonstrating use of
    OpenMP, Microsoft PPL, and Intel TBB
  • use multithreading for other demos
  • benchmark demo: add parallel raycasting

 - fixing various race conditions throughout (usage of static vars, etc)
 - addition of a few lightweight mutexes (which are compiled out by default)
 - slight code rearrangement in discreteDynamicsWorld to facilitate multithreading
 - PoolAllocator::allocate() can now be called when pool is full without
     crashing (null pointer returned)
 - PoolAllocator allocate and freeMemory, are OPTIONALLY threadsafe
     (default is un-threadsafe)
 - CollisionDispatcher no longer checks if the pool allocator is full
     before calling allocate(), instead it just calls allocate() and
     checks if the return is null -- this avoids a race condition
 - SequentialImpulseConstraintSolver OPTIONALLY uses different logic in
     getOrInitSolverBody() to avoid a race condition with kinematic bodies
 - addition of 2 classes which together allow simulation islands to be run
   in parallel:
    - btSimulationIslandManagerMt
    - btDiscreteDynamicsWorldMt
 - MultiThreadedDemo example in the example browser demonstrating use of
   OpenMP, Microsoft PPL, and Intel TBB
 - use multithreading for other demos
 - benchmark demo: add parallel raycasting
@lunkhound
Copy link
Contributor Author

I've tested this on Windows 7 with Cmake/Visual Studio 2013. I also used a virtual machine with Ubuntu 16 32-bit to test it with Cmake/GCC. I didn't add the threading options to premake but I tried to keep from breaking premake.
Using Cmake/Clang runs into an issue with not finding the omp.h header for OpenMP. Seems to be a known issue without a clean solution that I could find. The non-multithreading configuration works fine with Clang of course.

I added GUI buttons to the example browser so that for those demos that call CommonRigidBodyBase::createEmptyPhysicsWorld() a button appears which allows switching to a multi-threaded physics world (requires re-double-clicking on the demo).
Other buttons appear to allow switching between the supported threading APIs (OpenMP, Intel TBB, Microsoft PPL). And you get a slider widget to change the number of threads.

@erwincoumans
Copy link
Member

thanks a lot for updating this, I'll review it soon and either merge the pull request or get back to you . Thanks!

@erwincoumans
Copy link
Member

I'll accept the pull request but will reorganize some files afterwards: the 'CommonInterfaces' should just have header files, and I'll move some of the new classes out of that.
Thanks for all the work, as discussed here: http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?f=9&t=10232

@erwincoumans erwincoumans merged commit f64166d into bulletphysics:master Nov 3, 2016
@erwincoumans
Copy link
Member

Moved the CommonRigidBodyBase.cpp out of interfaces into MultiThreadedDemo/CommonRigidBodyBaseMT.cpp:
4235c61

@erwincoumans
Copy link
Member

I ran into another issue, BulletCollision is not allowed to depend in any way on BulletDynamics, this is violated in btSimulationIslandManagerMt.cpp:
#include "BulletDynamics/ConstraintSolver/btTypedConstraint.h"

@lunkhound
Copy link
Contributor Author

How about moving it into BulletDynamics?

On Thursday, November 10, 2016, erwincoumans notifications@github.com
wrote:

I ran into another issue, BulletCollision is not allowed to depend in any
way on BulletDynamics, this is violated in btSimulationIslandManagerMt.
cpp:
#include "BulletDynamics/ConstraintSolver/btTypedConstraint.h"


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#847 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGs0YYiQMEqE3g7NluIL2F1NW3f-UXOzks5q865ogaJpZM4KkfV0
.

@erwincoumans
Copy link
Member

Yes, I move it to BulletDynamics/Dynamics

@lunkhound
Copy link
Contributor Author

Yes that looks like a bug. I thought I had put something in to handle the removal case, but evidently I forgot. I can make a PR to fix that.

@erwincoumans
Copy link
Member

There is already a unique Id in each object through its broadphase handle. Given a btCollisionObject, you can access this UID using colObj->getBroadphaseHandle()->m_uniqueId;

@lunkhound
Copy link
Contributor Author

Actually the broadphase handle unique id is an always increasing value which is not quite what I need. What I need is the index of the body in the world's collisionObjects array (i.e. an id that is unique for each object existing at the current moment, but no larger than necessary).
"unique id" is not the best name for that field (in collisionObject), maybe it should be called something like "array index" or something?

@lunkhound
Copy link
Contributor Author

See PR #860

@erwincoumans
Copy link
Member

Apparently this pull request broke friction on kinematic animated objects, see #893
We should add a unit test for this (and many more) case, so it doesn't regress next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants