From 9d61c494789749f1b3cda43f008920af6bf2177a Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 10 Mar 2019 20:28:33 +0300 Subject: [PATCH 1/2] Store key by reference in tiles map --- components/detournavigator/navmeshtilescache.cpp | 10 +++++----- components/detournavigator/navmeshtilescache.hpp | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/components/detournavigator/navmeshtilescache.cpp b/components/detournavigator/navmeshtilescache.cpp index 418e69e824..51450cdbd1 100644 --- a/components/detournavigator/navmeshtilescache.cpp +++ b/components/detournavigator/navmeshtilescache.cpp @@ -85,7 +85,7 @@ namespace DetourNavigator if (navMeshSize > mFreeNavMeshDataSize + (mMaxNavMeshDataSize - mUsedNavMeshDataSize)) return Value(); - const auto navMeshKey = makeNavMeshKey(recastMesh, offMeshConnections); + auto navMeshKey = makeNavMeshKey(recastMesh, offMeshConnections); const auto itemSize = navMeshSize + 2 * navMeshKey.size(); if (itemSize > mFreeNavMeshDataSize + (mMaxNavMeshDataSize - mUsedNavMeshDataSize)) @@ -94,9 +94,8 @@ namespace DetourNavigator while (!mFreeItems.empty() && mUsedNavMeshDataSize + itemSize > mMaxNavMeshDataSize) removeLeastRecentlyUsed(); - const auto iterator = mFreeItems.emplace(mFreeItems.end(), agentHalfExtents, changedTile, navMeshKey); - // TODO: use std::string_view or some alternative to avoid navMeshKey copy into both mFreeItems and mValues - const auto emplaced = mValues[agentHalfExtents][changedTile].mMap.emplace(navMeshKey, iterator); + const auto iterator = mFreeItems.emplace(mFreeItems.end(), agentHalfExtents, changedTile, std::move(navMeshKey)); + const auto emplaced = mValues[agentHalfExtents][changedTile].mMap.emplace(iterator->mNavMeshKey, iterator); if (!emplaced.second) { @@ -131,9 +130,10 @@ namespace DetourNavigator mUsedNavMeshDataSize -= getSize(item); mFreeNavMeshDataSize -= getSize(item); - mFreeItems.pop_back(); tileValues->second.mMap.erase(value); + mFreeItems.pop_back(); + if (!tileValues->second.mMap.empty()) return; diff --git a/components/detournavigator/navmeshtilescache.hpp b/components/detournavigator/navmeshtilescache.hpp index e244cda9d7..6c57f7563c 100644 --- a/components/detournavigator/navmeshtilescache.hpp +++ b/components/detournavigator/navmeshtilescache.hpp @@ -105,10 +105,24 @@ namespace DetourNavigator NavMeshData&& value); private: + class KeyView + { + public: + KeyView(const std::string& value) + : mValue(value) {} + + friend bool operator <(const KeyView& lhs, const KeyView& rhs) + { + return lhs.mValue.get() < rhs.mValue.get(); + } + + private: + std::reference_wrapper mValue; + }; struct TileMap { - std::map mMap; + std::map mMap; }; std::mutex mMutex; From 68948bc847f51a314b873bc41c60a731e7fe61b5 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 10 Mar 2019 22:04:41 +0300 Subject: [PATCH 2/2] Avoid key allocation to find tile in cache --- .../detournavigator/navmeshtilescache.cpp | 59 ++++++++++++++++++- .../detournavigator/navmeshtilescache.hpp | 45 +++++++++++++- 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/components/detournavigator/navmeshtilescache.cpp b/components/detournavigator/navmeshtilescache.cpp index 51450cdbd1..76060981f4 100644 --- a/components/detournavigator/navmeshtilescache.cpp +++ b/components/detournavigator/navmeshtilescache.cpp @@ -1,6 +1,8 @@ #include "navmeshtilescache.hpp" #include "exceptions.hpp" +#include + namespace DetourNavigator { namespace @@ -61,8 +63,7 @@ namespace DetourNavigator if (tileValues == agentValues->second.end()) return Value(); - // TODO: use different function to make key to avoid unnecessary std::string allocation - const auto tile = tileValues->second.mMap.find(makeNavMeshKey(recastMesh, offMeshConnections)); + const auto tile = tileValues->second.mMap.find(RecastMeshKeyView(recastMesh, offMeshConnections)); if (tile == tileValues->second.mMap.end()) return Value(); @@ -163,4 +164,58 @@ namespace DetourNavigator mFreeItems.splice(mFreeItems.begin(), mBusyItems, iterator); mFreeNavMeshDataSize += getSize(*iterator); } + + namespace + { + struct CompareBytes + { + const char* mRhsIt; + const char* mRhsEnd; + + template + int operator ()(const std::vector& lhs) + { + const auto lhsBegin = reinterpret_cast(lhs.data()); + const auto lhsEnd = reinterpret_cast(lhs.data() + lhs.size()); + const auto lhsSize = static_cast(lhsEnd - lhsBegin); + const auto rhsSize = static_cast(mRhsEnd - mRhsIt); + const auto size = std::min(lhsSize, rhsSize); + + if (const auto result = std::memcmp(lhsBegin, mRhsIt, size)) + return result; + + if (lhsSize > rhsSize) + return 1; + + mRhsIt += size; + + return 0; + } + }; + } + + int NavMeshTilesCache::RecastMeshKeyView::compare(const std::string& other) const + { + CompareBytes compareBytes {other.data(), other.data() + other.size()}; + + if (const auto result = compareBytes(mRecastMesh.get().getIndices())) + return result; + + if (const auto result = compareBytes(mRecastMesh.get().getVertices())) + return result; + + if (const auto result = compareBytes(mRecastMesh.get().getAreaTypes())) + return result; + + if (const auto result = compareBytes(mRecastMesh.get().getWater())) + return result; + + if (const auto result = compareBytes(mOffMeshConnections.get())) + return result; + + if (compareBytes.mRhsIt < compareBytes.mRhsEnd) + return -1; + + return 0; + } } diff --git a/components/detournavigator/navmeshtilescache.hpp b/components/detournavigator/navmeshtilescache.hpp index 6c57f7563c..3f2e0cc019 100644 --- a/components/detournavigator/navmeshtilescache.hpp +++ b/components/detournavigator/navmeshtilescache.hpp @@ -10,6 +10,7 @@ #include #include #include +#include namespace DetourNavigator { @@ -108,16 +109,54 @@ namespace DetourNavigator class KeyView { public: + KeyView() = default; + KeyView(const std::string& value) - : mValue(value) {} + : mValue(&value) {} + + const std::string& getValue() const + { + assert(mValue); + return *mValue; + } + + virtual int compare(const std::string& other) const + { + assert(mValue); + return mValue->compare(other); + } + + virtual bool isLess(const KeyView& other) const + { + assert(mValue); + return other.compare(*mValue) > 0; + } friend bool operator <(const KeyView& lhs, const KeyView& rhs) { - return lhs.mValue.get() < rhs.mValue.get(); + return lhs.isLess(rhs); } private: - std::reference_wrapper mValue; + const std::string* mValue = nullptr; + }; + + class RecastMeshKeyView : public KeyView + { + public: + RecastMeshKeyView(const RecastMesh& recastMesh, const std::vector& offMeshConnections) + : mRecastMesh(recastMesh), mOffMeshConnections(offMeshConnections) {} + + int compare(const std::string& other) const override; + + bool isLess(const KeyView& other) const override + { + return compare(other.getValue()) < 0; + } + + private: + std::reference_wrapper mRecastMesh; + std::reference_wrapper> mOffMeshConnections; }; struct TileMap