From 076e772e3d99e00c4dbe6961d46c4c292f483526 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 12 Feb 2023 14:51:46 +0100 Subject: [PATCH] Use shared locks in physics system when using multithreaded bullet --- apps/openmw/mwphysics/mtphysics.cpp | 261 ++++++++++++++++------------ apps/openmw/mwphysics/mtphysics.hpp | 8 + 2 files changed, 155 insertions(+), 114 deletions(-) diff --git a/apps/openmw/mwphysics/mtphysics.cpp b/apps/openmw/mwphysics/mtphysics.cpp index 0ef69084ba..190c0726f3 100644 --- a/apps/openmw/mwphysics/mtphysics.cpp +++ b/apps/openmw/mwphysics/mtphysics.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -34,83 +35,96 @@ #include "physicssystem.hpp" #include "projectile.hpp" +namespace MWPhysics +{ + namespace + { + template + std::optional> makeExclusiveLock(Mutex& mutex, LockingPolicy lockingPolicy) + { + if (lockingPolicy == LockingPolicy::NoLocks) + return {}; + return std::unique_lock(mutex); + } + + /// @brief A scoped lock that is either exclusive or inexistent depending on configuration + template + class MaybeExclusiveLock + { + public: + /// @param mutex a mutex + /// @param threadCount decide wether the excluse lock will be taken + explicit MaybeExclusiveLock(Mutex& mutex, LockingPolicy lockingPolicy) + : mImpl(makeExclusiveLock(mutex, lockingPolicy)) + { + } + + private: + std::optional> mImpl; + }; + + template + std::optional> makeSharedLock(Mutex& mutex, LockingPolicy lockingPolicy) + { + if (lockingPolicy == LockingPolicy::NoLocks) + return {}; + return std::shared_lock(mutex); + } + + /// @brief A scoped lock that is either shared or inexistent depending on configuration + template + class MaybeSharedLock + { + public: + /// @param mutex a shared mutex + /// @param threadCount decide wether the shared lock will be taken + explicit MaybeSharedLock(Mutex& mutex, LockingPolicy lockingPolicy) + : mImpl(makeSharedLock(mutex, lockingPolicy)) + { + } + + private: + std::optional> mImpl; + }; + + template + std::variant, std::shared_lock> makeLock( + Mutex& mutex, LockingPolicy lockingPolicy) + { + switch (lockingPolicy) + { + case LockingPolicy::NoLocks: + return std::monostate{}; + case LockingPolicy::ExclusiveLocksOnly: + return std::unique_lock(mutex); + case LockingPolicy::AllowSharedLocks: + return std::shared_lock(mutex); + }; + + throw std::runtime_error("Unsupported LockingPolicy: " + + std::to_string(static_cast>(lockingPolicy))); + } + + /// @brief A scoped lock that is either shared, exclusive or inexistent depending on configuration + template + class MaybeLock + { + public: + /// @param mutex a shared mutex + /// @param threadCount decide wether the lock will be shared, exclusive or inexistent + explicit MaybeLock(Mutex& mutex, LockingPolicy lockingPolicy) + : mImpl(makeLock(mutex, lockingPolicy)) + { + } + + private: + std::variant, std::shared_lock> mImpl; + }; + } +} + namespace { - template - std::optional> makeExclusiveLock(Mutex& mutex, unsigned threadCount) - { - if (threadCount > 0) - return std::unique_lock(mutex); - return {}; - } - - /// @brief A scoped lock that is either exclusive or inexistent depending on configuration - template - class MaybeExclusiveLock - { - public: - /// @param mutex a mutex - /// @param threadCount decide wether the excluse lock will be taken - explicit MaybeExclusiveLock(Mutex& mutex, unsigned threadCount) - : mImpl(makeExclusiveLock(mutex, threadCount)) - { - } - - private: - std::optional> mImpl; - }; - - template - std::optional> makeSharedLock(Mutex& mutex, unsigned threadCount) - { - if (threadCount > 0) - return std::shared_lock(mutex); - return {}; - } - - /// @brief A scoped lock that is either shared or inexistent depending on configuration - template - class MaybeSharedLock - { - public: - /// @param mutex a shared mutex - /// @param threadCount decide wether the shared lock will be taken - explicit MaybeSharedLock(Mutex& mutex, unsigned threadCount) - : mImpl(makeSharedLock(mutex, threadCount)) - { - } - - private: - std::optional> mImpl; - }; - - template - std::variant, std::shared_lock> makeLock( - Mutex& mutex, unsigned threadCount) - { - if (threadCount > 1) - return std::shared_lock(mutex); - if (threadCount == 1) - return std::unique_lock(mutex); - return std::monostate{}; - } - - /// @brief A scoped lock that is either shared, exclusive or inexistent depending on configuration - template - class MaybeLock - { - public: - /// @param mutex a shared mutex - /// @param threadCount decide wether the lock will be shared, exclusive or inexistent - explicit MaybeLock(Mutex& mutex, unsigned threadCount) - : mImpl(makeLock(mutex, threadCount)) - { - } - - private: - std::variant, std::shared_lock> mImpl; - }; - bool isUnderWater(const MWPhysics::ActorFrameData& actorData) { return actorData.mPosition.z() < actorData.mSwimLevel; @@ -134,7 +148,7 @@ namespace { const Impl& mImpl; std::shared_mutex& mCollisionWorldMutex; - const unsigned mNumThreads; + const MWPhysics::LockingPolicy mLockingPolicy; template void operator()(MWPhysics::SimulationImpl& sim) const @@ -146,7 +160,7 @@ namespace // Locked shared_ptr has to be destructed after releasing mCollisionWorldMutex to avoid // possible deadlock. Ptr destructor also acquires mCollisionWorldMutex. const std::pair arg(std::move(ptr), frameData); - const Lock lock(mCollisionWorldMutex, mNumThreads); + const Lock lock(mCollisionWorldMutex, mLockingPolicy); mImpl(arg); } }; @@ -285,25 +299,43 @@ namespace } }; } +} - namespace Config +namespace MWPhysics +{ + namespace { - /// @return either the number of thread as configured by the user, or 1 if Bullet doesn't support multithreading - /// and user requested more than 1 background threads - unsigned computeNumThreads() + int getMaxBulletSupportedThreads() { - int wantedThread = Settings::Manager::getInt("async num threads", "Physics"); - auto broad = std::make_unique(); - auto maxSupportedThreads = broad->m_rayTestStacks.size(); - auto threadSafeBullet = (maxSupportedThreads > 1); - if (!threadSafeBullet && wantedThread > 1) + return broad->m_rayTestStacks.size(); + } + + LockingPolicy detectLockingPolicy() + { + if (Settings::Manager::getInt("async num threads", "Physics") < 1) + return LockingPolicy::NoLocks; + if (getMaxBulletSupportedThreads() > 1) + return LockingPolicy::AllowSharedLocks; + Log(Debug::Warning) << "Bullet was not compiled with multithreading support, 1 async thread will be used"; + return LockingPolicy::ExclusiveLocksOnly; + } + + unsigned getNumThreads(LockingPolicy lockingPolicy) + { + switch (lockingPolicy) { - Log(Debug::Warning) - << "Bullet was not compiled with multithreading support, 1 async thread will be used"; - return 1; + case LockingPolicy::NoLocks: + return 0; + case LockingPolicy::ExclusiveLocksOnly: + return 1; + case LockingPolicy::AllowSharedLocks: + return static_cast(std::max( + getMaxBulletSupportedThreads(), Settings::Manager::getInt("async num threads", "Physics"))); } - return static_cast(std::max(0, wantedThread)); + + throw std::runtime_error("Unsupported LockingPolicy: " + + std::to_string(static_cast>(lockingPolicy))); } } } @@ -317,7 +349,8 @@ namespace MWPhysics , mTimeAccum(0.f) , mCollisionWorld(collisionWorld) , mDebugDrawer(debugDrawer) - , mNumThreads(Config::computeNumThreads()) + , mLockingPolicy(detectLockingPolicy()) + , mNumThreads(getNumThreads(mLockingPolicy)) , mNumJobs(0) , mRemainingSteps(0) , mLOSCacheExpiry(Settings::Manager::getInt("lineofsight keep inactive cache", "Physics")) @@ -358,7 +391,7 @@ namespace MWPhysics { waitForWorkers(); { - MaybeExclusiveLock lock(mSimulationMutex, mNumThreads); + MaybeExclusiveLock lock(mSimulationMutex, mLockingPolicy); mQuit = true; mNumJobs = 0; mRemainingSteps = 0; @@ -424,7 +457,7 @@ namespace MWPhysics // This function run in the main thread. // While the mSimulationMutex is held, background physics threads can't run. - MaybeExclusiveLock lock(mSimulationMutex, mNumThreads); + MaybeExclusiveLock lock(mSimulationMutex, mLockingPolicy); double timeStart = mTimer->tick(); @@ -482,7 +515,7 @@ namespace MWPhysics void PhysicsTaskScheduler::resetSimulation(const ActorMap& actors) { waitForWorkers(); - MaybeExclusiveLock lock(mSimulationMutex, mNumThreads); + MaybeExclusiveLock lock(mSimulationMutex, mLockingPolicy); mBudget.reset(mDefaultPhysicsDt); mAsyncBudget.reset(0.0f); if (mSimulations != nullptr) @@ -500,27 +533,27 @@ namespace MWPhysics void PhysicsTaskScheduler::rayTest(const btVector3& rayFromWorld, const btVector3& rayToWorld, btCollisionWorld::RayResultCallback& resultCallback) const { - MaybeLock lock(mCollisionWorldMutex, mNumThreads); + MaybeLock lock(mCollisionWorldMutex, mLockingPolicy); mCollisionWorld->rayTest(rayFromWorld, rayToWorld, resultCallback); } void PhysicsTaskScheduler::convexSweepTest(const btConvexShape* castShape, const btTransform& from, const btTransform& to, btCollisionWorld::ConvexResultCallback& resultCallback) const { - MaybeLock lock(mCollisionWorldMutex, mNumThreads); + MaybeLock lock(mCollisionWorldMutex, mLockingPolicy); mCollisionWorld->convexSweepTest(castShape, from, to, resultCallback); } void PhysicsTaskScheduler::contactTest( btCollisionObject* colObj, btCollisionWorld::ContactResultCallback& resultCallback) { - MaybeSharedLock lock(mCollisionWorldMutex, mNumThreads); + MaybeSharedLock lock(mCollisionWorldMutex, mLockingPolicy); ContactTestWrapper::contactTest(mCollisionWorld, colObj, resultCallback); } std::optional PhysicsTaskScheduler::getHitPoint(const btTransform& from, btCollisionObject* target) { - MaybeLock lock(mCollisionWorldMutex, mNumThreads); + MaybeLock lock(mCollisionWorldMutex, mLockingPolicy); // target the collision object's world origin, this should be the center of the collision object btTransform rayTo; rayTo.setIdentity(); @@ -539,19 +572,19 @@ namespace MWPhysics void PhysicsTaskScheduler::aabbTest( const btVector3& aabbMin, const btVector3& aabbMax, btBroadphaseAabbCallback& callback) { - MaybeSharedLock lock(mCollisionWorldMutex, mNumThreads); + MaybeSharedLock lock(mCollisionWorldMutex, mLockingPolicy); mCollisionWorld->getBroadphase()->aabbTest(aabbMin, aabbMax, callback); } void PhysicsTaskScheduler::getAabb(const btCollisionObject* obj, btVector3& min, btVector3& max) { - MaybeSharedLock lock(mCollisionWorldMutex, mNumThreads); + MaybeSharedLock lock(mCollisionWorldMutex, mLockingPolicy); obj->getCollisionShape()->getAabb(obj->getWorldTransform(), min, max); } void PhysicsTaskScheduler::setCollisionFilterMask(btCollisionObject* collisionObject, int collisionFilterMask) { - MaybeExclusiveLock lock(mCollisionWorldMutex, mNumThreads); + MaybeExclusiveLock lock(mCollisionWorldMutex, mLockingPolicy); collisionObject->getBroadphaseHandle()->m_collisionFilterMask = collisionFilterMask; } @@ -559,14 +592,14 @@ namespace MWPhysics btCollisionObject* collisionObject, int collisionFilterGroup, int collisionFilterMask) { mCollisionObjects.insert(collisionObject); - MaybeExclusiveLock lock(mCollisionWorldMutex, mNumThreads); + MaybeExclusiveLock lock(mCollisionWorldMutex, mLockingPolicy); mCollisionWorld->addCollisionObject(collisionObject, collisionFilterGroup, collisionFilterMask); } void PhysicsTaskScheduler::removeCollisionObject(btCollisionObject* collisionObject) { mCollisionObjects.erase(collisionObject); - MaybeExclusiveLock lock(mCollisionWorldMutex, mNumThreads); + MaybeExclusiveLock lock(mCollisionWorldMutex, mLockingPolicy); mCollisionWorld->removeCollisionObject(collisionObject); } @@ -578,7 +611,7 @@ namespace MWPhysics } else { - MaybeExclusiveLock lock(mUpdateAabbMutex, mNumThreads); + MaybeExclusiveLock lock(mUpdateAabbMutex, mLockingPolicy); mUpdateAabb.insert(ptr); } } @@ -586,7 +619,7 @@ namespace MWPhysics bool PhysicsTaskScheduler::getLineOfSight( const std::shared_ptr& actor1, const std::shared_ptr& actor2) { - MaybeExclusiveLock lock(mLOSCacheMutex, mNumThreads); + MaybeExclusiveLock lock(mLOSCacheMutex, mLockingPolicy); auto req = LOSRequest(actor1, actor2); auto result = std::find(mLOSCache.begin(), mLOSCache.end(), req); @@ -602,7 +635,7 @@ namespace MWPhysics void PhysicsTaskScheduler::refreshLOSCache() { - MaybeSharedLock lock(mLOSCacheMutex, mNumThreads); + MaybeSharedLock lock(mLOSCacheMutex, mLockingPolicy); int job = 0; int numLOS = mLOSCache.size(); while ((job = mNextLOS.fetch_add(1, std::memory_order_relaxed)) < numLOS) @@ -620,7 +653,7 @@ namespace MWPhysics void PhysicsTaskScheduler::updateAabbs() { - MaybeExclusiveLock lock(mUpdateAabbMutex, mNumThreads); + MaybeExclusiveLock lock(mUpdateAabbMutex, mLockingPolicy); std::for_each(mUpdateAabb.begin(), mUpdateAabb.end(), [this](const std::weak_ptr& ptr) { auto p = ptr.lock(); if (p != nullptr) @@ -631,7 +664,7 @@ namespace MWPhysics void PhysicsTaskScheduler::updatePtrAabb(const std::shared_ptr& ptr) { - MaybeExclusiveLock lock(mCollisionWorldMutex, mNumThreads); + MaybeExclusiveLock lock(mCollisionWorldMutex, mLockingPolicy); if (const auto actor = std::dynamic_pointer_cast(ptr)) { actor->updateCollisionObjectPosition(); @@ -669,7 +702,7 @@ namespace MWPhysics { const Visitors::UpdatePosition impl{ mCollisionWorld }; const Visitors::WithLockedPtr vis{ impl, mCollisionWorldMutex, - mNumThreads }; + mLockingPolicy }; for (Simulation& sim : *mSimulations) std::visit(vis, sim); } @@ -685,7 +718,7 @@ namespace MWPhysics resultCallback.m_collisionFilterGroup = CollisionType_AnyPhysical; resultCallback.m_collisionFilterMask = CollisionType_World | CollisionType_HeightMap | CollisionType_Door; - MaybeLock lockColWorld(mCollisionWorldMutex, mNumThreads); + MaybeLock lockColWorld(mCollisionWorldMutex, mLockingPolicy); mCollisionWorld->rayTest(pos1, pos2, resultCallback); return !resultCallback.hasHit(); @@ -698,7 +731,7 @@ namespace MWPhysics mPreStepBarrier->wait([this] { afterPreStep(); }); int job = 0; const Visitors::Move impl{ mPhysicsDt, mCollisionWorld, *mWorldFrameData }; - const Visitors::WithLockedPtr vis{ impl, mCollisionWorldMutex, mNumThreads }; + const Visitors::WithLockedPtr vis{ impl, mCollisionWorldMutex, mLockingPolicy }; while ((job = mNextJob.fetch_add(1, std::memory_order_relaxed)) < mNumJobs) std::visit(vis, (*mSimulations)[job]); @@ -726,7 +759,7 @@ namespace MWPhysics void PhysicsTaskScheduler::debugDraw() { - MaybeSharedLock lock(mCollisionWorldMutex, mNumThreads); + MaybeSharedLock lock(mCollisionWorldMutex, mLockingPolicy); mDebugDrawer->step(); } @@ -757,7 +790,7 @@ namespace MWPhysics return; const Visitors::PreStep impl{ mCollisionWorld }; const Visitors::WithLockedPtr vis{ impl, mCollisionWorldMutex, - mNumThreads }; + mLockingPolicy }; for (auto& sim : *mSimulations) std::visit(vis, sim); } @@ -775,7 +808,7 @@ namespace MWPhysics void PhysicsTaskScheduler::afterPostSim() { { - MaybeExclusiveLock lock(mLOSCacheMutex, mNumThreads); + MaybeExclusiveLock lock(mLOSCacheMutex, mLockingPolicy); mLOSCache.erase( std::remove_if(mLOSCache.begin(), mLOSCache.end(), [](const LOSRequest& req) { return req.mStale; }), mLOSCache.end()); diff --git a/apps/openmw/mwphysics/mtphysics.hpp b/apps/openmw/mwphysics/mtphysics.hpp index ab0f768345..3721007762 100644 --- a/apps/openmw/mwphysics/mtphysics.hpp +++ b/apps/openmw/mwphysics/mtphysics.hpp @@ -28,6 +28,13 @@ namespace MWRender namespace MWPhysics { + enum class LockingPolicy + { + NoLocks, + ExclusiveLocksOnly, + AllowSharedLocks, + }; + class PhysicsTaskScheduler { public: @@ -95,6 +102,7 @@ namespace MWPhysics std::unique_ptr mPostStepBarrier; std::unique_ptr mPostSimBarrier; + LockingPolicy mLockingPolicy; unsigned mNumThreads; int mNumJobs; int mRemainingSteps;