From e23ff0520b0f78ffb39d1f09931cd97f94a33c55 Mon Sep 17 00:00:00 2001 From: Saleel Kudchadker Date: Wed, 23 Oct 2024 02:09:03 +0000 Subject: [PATCH] SWDEV-491375 - Improve MemObjMap perf - Create bins each with its own map and lock. This would help cases where the hash of a VA is differnet than ther one which falls in different bin, and there is no lock contention - Use STL shared mutexes, that way we can unique_lock for map updates vs simple reads which can use shared_lock Change-Id: I118818be65c6373700f5e511045babb6a398938a --- rocclr/device/device.cpp | 121 ++++++++++++++++++++------------------- rocclr/device/device.hpp | 53 +++++++++++------ 2 files changed, 96 insertions(+), 78 deletions(-) diff --git a/rocclr/device/device.cpp b/rocclr/device/device.cpp index 7661f3e9d3..d179d69ca0 100644 --- a/rocclr/device/device.cpp +++ b/rocclr/device/device.cpp @@ -305,18 +305,13 @@ Context* Device::glb_ctx_ = nullptr; Monitor Device::p2p_stage_ops_(true); Memory* Device::p2p_stage_ = nullptr; -Monitor MemObjMap::AllocatedLock_ ROCCLR_INIT_PRIORITY(101) ("Guards MemObjMap allocation list"); -std::map MemObjMap::MemObjMap_ ROCCLR_INIT_PRIORITY(101); -std::map MemObjMap::VirtualMemObjMap_ ROCCLR_INIT_PRIORITY(101); - -size_t MemObjMap::size() { - amd::ScopedLock lock(AllocatedLock_); - return MemObjMap_.size(); -} +std::array MemObjMap::MemObjBin_ ROCCLR_INIT_PRIORITY(101) = {} ; +std::array MemObjMap::VirtualMemObjBin_ ROCCLR_INIT_PRIORITY(101) = {} ; void MemObjMap::AddMemObj(const void* k, amd::Memory* v) { - amd::ScopedLock lock(AllocatedLock_); - auto rval = MemObjMap_.insert({ reinterpret_cast(k), v }); + auto& bin = getMapBin(k, MemObjBin_); + std::unique_lock lock(bin.AllocatedLock_); + auto rval = bin.map_.insert({ reinterpret_cast(k), v }); if (!rval.second) { DevLogPrintfError("Memobj map already has an entry for ptr: 0x%x", reinterpret_cast(k)); @@ -324,17 +319,19 @@ void MemObjMap::AddMemObj(const void* k, amd::Memory* v) { } void MemObjMap::RemoveMemObj(const void* k) { - amd::ScopedLock lock(AllocatedLock_); - auto rval = MemObjMap_.erase(reinterpret_cast(k)); + auto& bin = getMapBin(k, MemObjBin_); + std::unique_lock lock(bin.AllocatedLock_); + auto rval = bin.map_.erase(reinterpret_cast(k)); guarantee(rval == 1, "Memobj map does not have ptr: 0x%x", reinterpret_cast(k)); } amd::Memory* MemObjMap::FindMemObj(const void* k, size_t* offset) { - amd::ScopedLock lock(AllocatedLock_); + auto& bin = getMapBin(k, MemObjBin_); + std::shared_lock lock(bin.AllocatedLock_); uintptr_t key = reinterpret_cast(k); - auto it = MemObjMap_.upper_bound(key); - if (it == MemObjMap_.begin()) { + auto it = bin.map_.upper_bound(key); + if (it == bin.map_.begin()) { return nullptr; } @@ -350,9 +347,50 @@ amd::Memory* MemObjMap::FindMemObj(const void* k, size_t* offset) { return nullptr; } } + +void MemObjMap::UpdateAccess(amd::Device *peerDev) { + if (peerDev == nullptr) { + return; + } + // Provides access to all memory allocated on peerDev but + // hsa_amd_agents_allow_access was not called because there was no peer + for (auto& bin : MemObjBin_) { + std::shared_lock lock(bin.AllocatedLock_); + for (auto it : bin.map_) { + const std::vector& devices = it.second->getContext().devices(); + if (devices.size() == 1 && devices[0] == peerDev) { + device::Memory* devMem = it.second->getDeviceMemory(*devices[0]); + if (!devMem->getAllowedPeerAccess()) { + peerDev->deviceAllowAccess(reinterpret_cast(it.first)); + devMem->setAllowedPeerAccess(true); + } + } + } + } +} + +void MemObjMap::Purge(amd::Device* dev) { + assert(dev != nullptr); + for (auto& bin : MemObjBin_) { + std::unique_lock lock(bin.AllocatedLock_); + for (auto it = bin.map_.cbegin(); it != bin.map_.cend(); ) { + amd::Memory* memObj = it->second; + unsigned int flags = memObj->getMemFlags(); + const std::vector& devices = memObj->getContext().devices(); + if (devices.size() == 1 && devices[0] == dev && !(flags & ROCCLR_MEM_INTERNAL_MEMORY)) { + memObj->release(); + it = bin.map_.erase(it); + } else { + ++it; + } + } + } +} + void MemObjMap::AddVirtualMemObj(const void* k, amd::Memory* v) { - amd::ScopedLock lock(AllocatedLock_); - auto rval = VirtualMemObjMap_.insert({ reinterpret_cast(k), v }); + auto& bin = getMapBin(k, VirtualMemObjBin_); + std::unique_lock lock(bin.AllocatedLock_); + auto rval = bin.map_.insert({ reinterpret_cast(k), v }); if (!rval.second) { DevLogPrintfError("Virtual Memobj map already has an entry for ptr: 0x%x", reinterpret_cast(k)); @@ -360,17 +398,19 @@ void MemObjMap::AddVirtualMemObj(const void* k, amd::Memory* v) { } void MemObjMap::RemoveVirtualMemObj(const void* k) { - amd::ScopedLock lock(AllocatedLock_); - auto rval = VirtualMemObjMap_.erase(reinterpret_cast(k)); + auto& bin = getMapBin(k, VirtualMemObjBin_); + std::unique_lock lock(bin.AllocatedLock_); + auto rval = bin.map_.erase(reinterpret_cast(k)); guarantee(rval == 1, "Virtual Memobj map does not have ptr: 0x%x", reinterpret_cast(k)); } amd::Memory* MemObjMap::FindVirtualMemObj(const void* k) { - amd::ScopedLock lock(AllocatedLock_); + auto& bin = getMapBin(k, VirtualMemObjBin_); + std::shared_lock lock(bin.AllocatedLock_); uintptr_t key = reinterpret_cast(k); - auto it = VirtualMemObjMap_.upper_bound(key); - if (it == VirtualMemObjMap_.begin()) { + auto it = bin.map_.upper_bound(key); + if (it == bin.map_.begin()) { return nullptr; } @@ -492,43 +532,6 @@ bool Device::DestroyVirtualBuffer(amd::Memory* vaddr_mem_obj) { return true; } -void MemObjMap::UpdateAccess(amd::Device *peerDev) { - if (peerDev == nullptr) { - return; - } - - // Provides access to all memory allocated on peerDev but - // hsa_amd_agents_allow_access was not called because there was no peer - amd::ScopedLock lock(AllocatedLock_); - for (auto it : MemObjMap_) { - const std::vector& devices = it.second->getContext().devices(); - if (devices.size() == 1 && devices[0] == peerDev) { - device::Memory* devMem = it.second->getDeviceMemory(*devices[0]); - if (!devMem->getAllowedPeerAccess()) { - peerDev->deviceAllowAccess(reinterpret_cast(it.first)); - devMem->setAllowedPeerAccess(true); - } - } - } -} - -void MemObjMap::Purge(amd::Device* dev) { - assert(dev != nullptr); - - amd::ScopedLock lock(AllocatedLock_); - for (auto it = MemObjMap_.cbegin(); it != MemObjMap_.cend(); ) { - amd::Memory* memObj = it->second; - unsigned int flags = memObj->getMemFlags(); - const std::vector& devices = memObj->getContext().devices(); - if (devices.size() == 1 && devices[0] == dev && !(flags & ROCCLR_MEM_INTERNAL_MEMORY)) { - memObj->release(); - it = MemObjMap_.erase(it); - } else { - ++it; - } - } -} - Device::BlitProgram::~BlitProgram() { if (program_ != nullptr) { program_->release(); diff --git a/rocclr/device/device.hpp b/rocclr/device/device.hpp index 933ef7cd49..224fb3ea17 100644 --- a/rocclr/device/device.hpp +++ b/rocclr/device/device.hpp @@ -57,6 +57,8 @@ #include #include #include +#include +#include namespace amd { class Command; @@ -1360,27 +1362,40 @@ namespace amd { //! MemoryObject map lookup class class MemObjMap : public AllStatic { public: - static size_t size(); //!< obtain the size of the container - static void AddMemObj(const void* k, - amd::Memory* v); //!< add the host mem pointer and buffer in the container - static void RemoveMemObj(const void* k); //!< Remove an entry of mem object from the container - static amd::Memory* FindMemObj( - const void* k, //!< find the mem object based on the input pointer - size_t* offset = nullptr); //!< Offset in the memory location - static void UpdateAccess(amd::Device *peerDev); - static void Purge(amd::Device* dev); //!< Purge all user allocated memories on the given device + //!< add the host mem pointer and buffer in the container + static void AddMemObj(const void* k, amd::Memory* v); - static void AddVirtualMemObj(const void* k, - amd::Memory* v); //!< Same as AddMemObj but for virtual addressing - static void RemoveVirtualMemObj(const void* k); //!< Same as RemoveMemObj but for virtual addressing - static amd::Memory* FindVirtualMemObj( - const void* k); //!< Same as FindMemObj but for virtual addressing + //!< Remove an entry of mem object from the container + static void RemoveMemObj(const void* k); + + //!< Find the mem object based on the input pointer, outputs the offset + static amd::Memory* FindMemObj( const void* k, size_t* offset = nullptr); + static void UpdateAccess(amd::Device *peerDev); + //!< Purge all user allocated memories on the given device + static void Purge(amd::Device* dev); + //!< Same as AddMemObj but for virtual addressing + static void AddVirtualMemObj(const void* k, amd::Memory* v); + + //!< Same as RemoveMemObj but for virtual addressing + static void RemoveVirtualMemObj(const void* k); + //!< Same as FindMemObj but for virtual addressing + static amd::Memory* FindVirtualMemObj(const void* k); + struct Bin { + std::shared_mutex AllocatedLock_; + std::map map_; + }; + static constexpr size_t kNumBins = 16; private: - static std::map - MemObjMap_; //!< the mem object<->hostptr information container - static std::map - VirtualMemObjMap_; //!< the virtual mem object<->hostptr information container - static amd::Monitor AllocatedLock_; //!< amd monitor locker + + //!< the mem object<->hostptr information container + static std::array MemObjBin_; + //!< the virtual mem object<->hostptr information container + static std::array VirtualMemObjBin_; + + static Bin& getMapBin(const void* k, std::array& bin) { + size_t index = std::hash{}(k) % kNumBins; + return bin.at(index); + } }; /// @brief Instruction Set Architecture properties.