From 431501e23a3b144edeb5b9dab936c88d56599d1f Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 7 Aug 2021 11:25:01 +0200 Subject: [PATCH 1/3] Remove redundant job distribution between threads Instead don't take jobs from queue until job for the same tile is processing. --- .../detournavigator/asyncnavmeshupdater.cpp | 100 +++++------------- .../detournavigator/asyncnavmeshupdater.hpp | 5 +- 2 files changed, 31 insertions(+), 74 deletions(-) diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 562420955d..20105aba06 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -102,7 +102,6 @@ namespace DetourNavigator { mShouldStop = true; std::unique_lock lock(mMutex); - mThreadsQueues.clear(); mWaiting.clear(); mHasJob.notify_all(); lock.unlock(); @@ -340,64 +339,38 @@ namespace DetourNavigator { std::unique_lock lock(mMutex); - const auto threadId = std::this_thread::get_id(); - auto& threadQueue = mThreadsQueues[threadId]; - - while (true) + bool shouldStop = false; + const auto hasJob = [&] { - bool shouldStop = false; + shouldStop = mShouldStop; + return shouldStop + || (!mWaiting.empty() && mWaiting.front()->mProcessTime <= std::chrono::steady_clock::now()); + }; - const auto hasJob = [&] { - shouldStop = mShouldStop; - return shouldStop - || (!mWaiting.empty() && mWaiting.front()->mProcessTime <= std::chrono::steady_clock::now()) - || !threadQueue.empty(); - }; - - if (!mHasJob.wait_for(lock, std::chrono::milliseconds(10), hasJob)) - { - if (mJobs.empty()) - mDone.notify_all(); - return mJobs.end(); - } - - if (shouldStop) - return mJobs.end(); - - Log(Debug::Debug) << "Got " << mJobs.size() << " navigator jobs and " - << threadQueue.size() << " thread jobs by thread=" << std::this_thread::get_id(); - - const JobIt job = threadQueue.empty() - ? getJob(mWaiting, true) - : getJob(threadQueue, false); - - if (job == mJobs.end()) - continue; - - const auto owner = lockTile(job->mAgentHalfExtents, job->mChangedTile); - - if (owner == threadId) - { - mPushed.erase(getAgentAndTile(*job)); - return job; - } - - postThreadJob(job, mThreadsQueues[owner]); + if (!mHasJob.wait_for(lock, std::chrono::milliseconds(10), hasJob)) + { + if (mJobs.empty()) + mDone.notify_all(); + return mJobs.end(); } - } - JobIt AsyncNavMeshUpdater::getJob(std::deque& jobs, bool changeLastUpdate) - { - const auto now = std::chrono::steady_clock::now(); - JobIt job = jobs.front(); - - if (job->mProcessTime > now) + if (shouldStop) return mJobs.end(); - jobs.pop_front(); + const JobIt job = mWaiting.front(); - if (changeLastUpdate && job->mChangeType == ChangeType::update) - mLastUpdates[getAgentAndTile(*job)] = now; + mWaiting.pop_front(); + + if (!lockTile(job->mAgentHalfExtents, job->mChangedTile)) + { + ++job->mTryNumber; + insertPrioritizedJob(job, mWaiting); + return mJobs.end(); + } + + if (job->mChangeType == ChangeType::update) + mLastUpdates[getAgentAndTile(*job)] = std::chrono::steady_clock::now(); + mPushed.erase(getAgentAndTile(*job)); return job; } @@ -435,7 +408,7 @@ namespace DetourNavigator if (mPushed.emplace(job->mAgentHalfExtents, job->mChangedTile).second) { ++job->mTryNumber; - mWaiting.push_back(job); + insertPrioritizedJob(job, mWaiting); mHasJob.notify_all(); return; } @@ -443,26 +416,11 @@ namespace DetourNavigator mJobs.erase(job); } - void AsyncNavMeshUpdater::postThreadJob(JobIt job, std::deque& queue) - { - queue.push_back(job); - mHasJob.notify_all(); - } - - std::thread::id AsyncNavMeshUpdater::lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile) + bool AsyncNavMeshUpdater::lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile) { if (mSettings.get().mAsyncNavMeshUpdaterThreads <= 1) - return std::this_thread::get_id(); - - auto locked = mProcessingTiles.lock(); - const auto tile = locked->find(std::make_tuple(agentHalfExtents, changedTile)); - if (tile == locked->end()) - { - const auto threadId = std::this_thread::get_id(); - locked->emplace(std::tie(agentHalfExtents, changedTile), threadId); - return threadId; - } - return tile->second; + return true; + return mProcessingTiles.lock()->emplace(agentHalfExtents, changedTile).second; } void AsyncNavMeshUpdater::unlockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile) diff --git a/components/detournavigator/asyncnavmeshupdater.hpp b/components/detournavigator/asyncnavmeshupdater.hpp index b770230673..9e521ad509 100644 --- a/components/detournavigator/asyncnavmeshupdater.hpp +++ b/components/detournavigator/asyncnavmeshupdater.hpp @@ -99,10 +99,9 @@ namespace DetourNavigator std::set> mPushed; Misc::ScopeGuarded mPlayerTile; NavMeshTilesCache mNavMeshTilesCache; - Misc::ScopeGuarded, std::thread::id>> mProcessingTiles; + Misc::ScopeGuarded>> mProcessingTiles; std::map, std::chrono::steady_clock::time_point> mLastUpdates; std::set> mPresentTiles; - std::map> mThreadsQueues; std::vector mThreads; void process() noexcept; @@ -119,7 +118,7 @@ namespace DetourNavigator void repost(JobIt job); - std::thread::id lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile); + bool lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile); void unlockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile); From 0f11acf709643fd829683e1ef4f6d0906950f349 Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 6 Aug 2021 19:50:29 +0200 Subject: [PATCH 2/3] Report more stats from AsyncNavMeshUpdater --- components/detournavigator/asyncnavmeshupdater.cpp | 9 ++++++++- components/resource/stats.cpp | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 20105aba06..c6db079ce8 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -234,13 +234,20 @@ namespace DetourNavigator void AsyncNavMeshUpdater::reportStats(unsigned int frameNumber, osg::Stats& stats) const { std::size_t jobs = 0; + std::size_t waiting = 0; + std::size_t pushed = 0; { const std::lock_guard lock(mMutex); jobs = mJobs.size(); + waiting = mWaiting.size(); + pushed = mPushed.size(); } - stats.setAttribute(frameNumber, "NavMesh UpdateJobs", jobs); + stats.setAttribute(frameNumber, "NavMesh Jobs", jobs); + stats.setAttribute(frameNumber, "NavMesh Waiting", waiting); + stats.setAttribute(frameNumber, "NavMesh Pushed", pushed); + stats.setAttribute(frameNumber, "NavMesh Processing", mProcessingTiles.lockConst()->size()); mNavMeshTilesCache.reportStats(frameNumber, stats); } diff --git a/components/resource/stats.cpp b/components/resource/stats.cpp index b5975e28e3..9881d0458b 100644 --- a/components/resource/stats.cpp +++ b/components/resource/stats.cpp @@ -390,7 +390,10 @@ void StatsHandler::setUpScene(osgViewer::ViewerBase *viewer) "Land", "Composite", "", - "NavMesh UpdateJobs", + "NavMesh Jobs", + "NavMesh Waiting", + "NavMesh Pushed", + "NavMesh Processing", "NavMesh CacheSize", "NavMesh UsedTiles", "NavMesh CachedTiles", From 0066c446f828570e029aa4267bbdb29fb9aa1642 Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 17 Aug 2021 12:10:22 +0200 Subject: [PATCH 3/3] Remove navmesh tiles outside allowed range first * Change job change type to remove when tile is outside allowed range. * Swap try number and change type in job priority. To make sure remove jobs always processed before any other. --- components/detournavigator/asyncnavmeshupdater.cpp | 12 +++++++++++- components/detournavigator/asyncnavmeshupdater.hpp | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index c6db079ce8..5c88d0cfa2 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -8,6 +8,8 @@ #include #include +#include + #include #include @@ -48,7 +50,7 @@ namespace auto getPriority(const Job& job) noexcept { - return std::make_tuple(job.mProcessTime, job.mTryNumber, job.mChangeType, job.mDistanceToPlayer, job.mDistanceToOrigin); + return std::make_tuple(job.mProcessTime, job.mChangeType, job.mTryNumber, job.mDistanceToPlayer, job.mDistanceToOrigin); } struct LessByJobPriority @@ -123,11 +125,19 @@ namespace DetourNavigator if (!playerTileChanged && changedTiles.empty()) return; + const dtNavMeshParams params = *navMeshCacheItem->lockConst()->getImpl().getParams(); + const std::lock_guard lock(mMutex); if (playerTileChanged) + { for (JobIt job : mWaiting) + { job->mDistanceToPlayer = getManhattanDistance(job->mChangedTile, playerTile); + if (!shouldAddTile(job->mChangedTile, playerTile, std::min(mSettings.get().mMaxTilesNumber, params.maxTiles))) + job->mChangeType = ChangeType::remove; + } + } for (const auto& [changedTile, changeType] : changedTiles) { diff --git a/components/detournavigator/asyncnavmeshupdater.hpp b/components/detournavigator/asyncnavmeshupdater.hpp index 9e521ad509..2d915ad434 100644 --- a/components/detournavigator/asyncnavmeshupdater.hpp +++ b/components/detournavigator/asyncnavmeshupdater.hpp @@ -60,7 +60,7 @@ namespace DetourNavigator const TilePosition mChangedTile; const std::chrono::steady_clock::time_point mProcessTime; unsigned mTryNumber = 0; - const ChangeType mChangeType; + ChangeType mChangeType; int mDistanceToPlayer; const int mDistanceToOrigin;