From ff8f38646e08473681aa3d10d11190a3a7d5bc22 Mon Sep 17 00:00:00 2001 From: Kristi Belcher Date: Fri, 10 Oct 2025 11:50:26 -0700 Subject: [PATCH 1/2] WIP, trying to figure out the right data structure --- src/umpire/strategy/ResourceAwarePool.cpp | 30 ++++++++++++----------- src/umpire/strategy/ResourceAwarePool.hpp | 27 ++++++++++++++++++-- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/umpire/strategy/ResourceAwarePool.cpp b/src/umpire/strategy/ResourceAwarePool.cpp index ad240b363..41df1581e 100644 --- a/src/umpire/strategy/ResourceAwarePool.cpp +++ b/src/umpire/strategy/ResourceAwarePool.cpp @@ -57,18 +57,20 @@ void* ResourceAwarePool::allocate_resource(std::size_t bytes, camp::resources::R const std::size_t rounded_bytes{aligned_round_up(bytes)}; Chunk* chunk{nullptr}; - if (!m_pending_list.empty()) { - for (auto it = m_pending_list.begin(); it != m_pending_list.end(); it++) { + if (!m_pending_map.empty()) { + for (auto it = m_pending_map.begin(); it != m_pending_map.end(); it++) { auto pending_chunk = (*it); if (pending_chunk->size >= rounded_bytes && pending_chunk->resource == r) { // reusing chunk with same resource chunk = pending_chunk; chunk->free = false; - m_pending_list.erase(it); + m_pending_map.erase(it); break; } } } + + const auto& best = m_free_map.lower_bound(rounded_bytes); if (chunk == nullptr) { @@ -179,10 +181,10 @@ void ResourceAwarePool::do_deallocate(Chunk* chunk, void* ptr) noexcept chunk->free = true; // Removing chunk from pending - for (auto it = m_pending_list.begin(); it != m_pending_list.end();) { + for (auto it = m_pending_map.begin(); it != m_pending_map.end();) { auto my_chunk = (*it); if (my_chunk == chunk) { - it = m_pending_list.erase(it); + it = m_pending_map.erase(it); } else { it++; } @@ -274,8 +276,8 @@ void ResourceAwarePool::deallocate_resource(void* ptr, camp::resources::Resource if (chunk->event.check()) { do_deallocate(chunk, ptr); } else { - // Chunk is now pending, add to list - m_pending_list.push_back(chunk); + // Chunk is now pending, add to pending map + m_pending_map.push_back(chunk); } std::size_t suggested_size{m_should_coalesce(*this)}; @@ -293,7 +295,7 @@ void ResourceAwarePool::release() std::size_t prev_size{m_actual_bytes}; #endif - for (auto it = m_pending_list.begin(); it != m_pending_list.end();) { + for (auto it = m_pending_map.begin(); it != m_pending_map.end();) { auto chunk = (*it); if (m_is_destructing) { // If we are destructing, wait for all deallocations to occur chunk->event.wait(); @@ -302,7 +304,7 @@ void ResourceAwarePool::release() chunk->event.check()) { // Otherwise, move all finished pending chunks to free map to be released m_free_map.insert(std::make_pair(chunk->size, chunk)); chunk->free = true; - it = m_pending_list.erase(it); + it = m_pending_map.erase(it); } else { it++; } @@ -362,7 +364,7 @@ std::size_t ResourceAwarePool::getTotalBlocks() const noexcept std::size_t ResourceAwarePool::getNumPending() const noexcept { - return m_pending_list.size(); + return m_pending_map.size(); } std::size_t ResourceAwarePool::getActualSize() const noexcept @@ -397,7 +399,7 @@ Platform ResourceAwarePool::getPlatform() noexcept camp::resources::Resource ResourceAwarePool::getResource(void* ptr) const { - for (auto& chunk : m_pending_list) { // check pending chunks + for (auto& chunk : m_pending_map) { // check pending chunks if (chunk->data == ptr) { return chunk->resource; } @@ -438,7 +440,7 @@ bool ResourceAwarePool::tracksMemoryUse() const noexcept std::size_t ResourceAwarePool::getBlocksInPool() const noexcept { - return m_used_map.size() + m_free_map.size() + m_pending_list.size(); + return m_used_map.size() + m_free_map.size() + m_pending_map.size(); } std::size_t ResourceAwarePool::getLargestAvailableBlock() noexcept @@ -457,8 +459,8 @@ void ResourceAwarePool::coalesce() noexcept event.name("coalesce").category(event::category::operation).tag("allocator_name", getName()).tag("replay", "true"); }); - if (!m_pending_list.empty()) { - for (auto it = m_pending_list.begin(); it != m_pending_list.end(); it++) { + if (!m_pending_map.empty()) { + for (auto it = m_pending_map.begin(); it != m_pending_map.end(); it++) { auto pending_chunk = (*it); if (pending_chunk->free == false && pending_chunk->event.check()) { // a pending chunk is finished... do_deallocate(pending_chunk, pending_chunk->data); diff --git a/src/umpire/strategy/ResourceAwarePool.hpp b/src/umpire/strategy/ResourceAwarePool.hpp index 708a9993b..ecd0a9c38 100644 --- a/src/umpire/strategy/ResourceAwarePool.hpp +++ b/src/umpire/strategy/ResourceAwarePool.hpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include "camp/camp.hpp" #include "camp/resource.hpp" @@ -25,6 +27,22 @@ using Resource = camp::resources::Resource; using Event = camp::resources::Event; +namespace std { + // Hash function for variant Resource + template<> + struct hash> { + size_t operator()(const std::variant& key) const { + if (std::holds_alternative(key)) { + return 0; // Hash for no resource + } + // If there is a Resource, get it + const auto& resource = std::get(key); + return std::hash{}(&resource); // use the address of the resource object + } + }; + // End of PendingMap definitions +} + namespace umpire { class Allocator; @@ -183,7 +201,11 @@ class ResourceAwarePool : public AllocationStrategy, private mixins::AlignedAllo }; using PointerMap = std::unordered_map; - using PendingList = std::list; + + // PendingMap definitions + using ResourceKey = std::variant; + using PendingMap = std::unordered_multimap; + using SizeMap = std::multimap, pool_allocator>>; @@ -214,6 +236,7 @@ class ResourceAwarePool : public AllocationStrategy, private mixins::AlignedAllo Chunk* prev{nullptr}; Chunk* next{nullptr}; SizeMap::iterator size_map_it; + PendingMap::iterator pending_map_it; Resource resource; Event event; }; @@ -221,7 +244,7 @@ class ResourceAwarePool : public AllocationStrategy, private mixins::AlignedAllo private: PointerMap m_used_map{}; SizeMap m_free_map{}; - PendingList m_pending_list{}; + PendingMap m_pending_map{}; util::FixedMallocPool m_chunk_pool{sizeof(Chunk)}; From 0251cd74dbec1626bb5f080e8f874c3fa124f2fc Mon Sep 17 00:00:00 2001 From: Kristi Belcher Date: Tue, 21 Oct 2025 08:42:46 -0700 Subject: [PATCH 2/2] pulling in temporary camp commit, updating RAP pending map --- src/tpl/umpire/camp | 2 +- src/umpire/strategy/ResourceAwarePool.cpp | 106 ++++++++++++++-------- src/umpire/strategy/ResourceAwarePool.hpp | 22 +---- 3 files changed, 72 insertions(+), 58 deletions(-) diff --git a/src/tpl/umpire/camp b/src/tpl/umpire/camp index 4070ce93a..07c707523 160000 --- a/src/tpl/umpire/camp +++ b/src/tpl/umpire/camp @@ -1 +1 @@ -Subproject commit 4070ce93a802849d61037310a87c50cc24c9e498 +Subproject commit 07c7075239524c724cf88ecc491e853c00b1168d diff --git a/src/umpire/strategy/ResourceAwarePool.cpp b/src/umpire/strategy/ResourceAwarePool.cpp index 41df1581e..a2d86631c 100644 --- a/src/umpire/strategy/ResourceAwarePool.cpp +++ b/src/umpire/strategy/ResourceAwarePool.cpp @@ -57,19 +57,20 @@ void* ResourceAwarePool::allocate_resource(std::size_t bytes, camp::resources::R const std::size_t rounded_bytes{aligned_round_up(bytes)}; Chunk* chunk{nullptr}; - if (!m_pending_map.empty()) { - for (auto it = m_pending_map.begin(); it != m_pending_map.end(); it++) { - auto pending_chunk = (*it); - if (pending_chunk->size >= rounded_bytes && pending_chunk->resource == r) { // reusing chunk with same resource + auto range = m_pending_map.equal_range(std::optional(r)); + for (auto it = range.first; it != range.second; ++it) { + auto pending_chunk = it->second; + if (pending_chunk->size >= rounded_bytes) { + // reuse chunk with same resource chunk = pending_chunk; chunk->free = false; + + // delete from pending map and invalidate the iterator m_pending_map.erase(it); + chunk->pending_map_it = m_pending_map.end(); break; - } } - } - - + } const auto& best = m_free_map.lower_bound(rounded_bytes); @@ -180,14 +181,10 @@ void ResourceAwarePool::do_deallocate(Chunk* chunk, void* ptr) noexcept UMPIRE_USE_VAR(ptr); chunk->free = true; - // Removing chunk from pending - for (auto it = m_pending_map.begin(); it != m_pending_map.end();) { - auto my_chunk = (*it); - if (my_chunk == chunk) { - it = m_pending_map.erase(it); - } else { - it++; - } + // Remove chunk from pending + if (chunk->pending_map_it != m_pending_map.end()) { + m_pending_map.erase(chunk->pending_map_it); + chunk->pending_map_it = m_pending_map.end(); } UMPIRE_LOG(Debug, "In the do_deallocate function. Deallocating data held by " << chunk); @@ -277,7 +274,8 @@ void ResourceAwarePool::deallocate_resource(void* ptr, camp::resources::Resource do_deallocate(chunk, ptr); } else { // Chunk is now pending, add to pending map - m_pending_map.push_back(chunk); + auto it = m_pending_map.insert({std::optional(chunk->resource), chunk}); + chunk->pending_map_it = it; } std::size_t suggested_size{m_should_coalesce(*this)}; @@ -295,18 +293,51 @@ void ResourceAwarePool::release() std::size_t prev_size{m_actual_bytes}; #endif - for (auto it = m_pending_map.begin(); it != m_pending_map.end();) { - auto chunk = (*it); - if (m_is_destructing) { // If we are destructing, wait for all deallocations to occur +//TODO: double check this + auto it = m_pending_map.begin(); + while (it != m_pending_map.end()) { + auto chunk = it->second; + UMPIRE_LOG(Debug, "Found chunk @ " << chunk->data); + + // If we are destructing, wait for all deallocations to occur + if (m_is_destructing) { chunk->event.wait(); } - if (chunk != nullptr && chunk->free == false && - chunk->event.check()) { // Otherwise, move all finished pending chunks to free map to be released - m_free_map.insert(std::make_pair(chunk->size, chunk)); - chunk->free = true; - it = m_pending_map.erase(it); + + // Otherwise, free all finished pending chunks + if (chunk->event.check()) { + if (chunk->size == chunk->chunk_size) { + UMPIRE_LOG(Debug, "Releasing chunk " << chunk->data); + + m_actual_bytes -= chunk->chunk_size; + m_releasable_bytes -= chunk->chunk_size; + m_releasable_blocks--; + m_total_blocks--; + + try { + aligned_deallocate(chunk->data); + } catch (...) { + if (m_is_destructing) { + // + // Ignore error in case the underlying vendor API has already shutdown + // + UMPIRE_LOG(Error, "Pool is destructing, runtime_error Ignored"); + } else { + throw; + } + } + + chunk->~Chunk(); // manually call destructor + m_chunk_pool.deallocate(chunk); + it = m_pending_map.erase(it); + chunk->pending_map_it = m_pending_map.end(); + } else { + it = m_pending_map.erase(it); + chunk->pending_map_it = m_pending_map.end(); + do_deallocate(chunk, chunk->data); + } } else { - it++; + ++it; } } @@ -399,9 +430,10 @@ Platform ResourceAwarePool::getPlatform() noexcept camp::resources::Resource ResourceAwarePool::getResource(void* ptr) const { - for (auto& chunk : m_pending_map) { // check pending chunks + for (auto& [resource, chunk] : m_pending_map) { // check pending chunks if (chunk->data == ptr) { - return chunk->resource; + assert(resource.has_value()); //since resource is optional + return *resource; } } auto it = m_used_map.find(ptr); // check used chunks @@ -458,14 +490,16 @@ void ResourceAwarePool::coalesce() noexcept umpire::event::record([&](auto& event) { event.name("coalesce").category(event::category::operation).tag("allocator_name", getName()).tag("replay", "true"); }); - - if (!m_pending_map.empty()) { - for (auto it = m_pending_map.begin(); it != m_pending_map.end(); it++) { - auto pending_chunk = (*it); - if (pending_chunk->free == false && pending_chunk->event.check()) { // a pending chunk is finished... - do_deallocate(pending_chunk, pending_chunk->data); - break; - } + + auto it = m_pending_map.begin(); + while (it != m_pending_map.end()) { + auto pending_chunk = it->second; + if (pending_chunk->free == false && pending_chunk->event.check()) { // a pending chunk is finished... + auto next_it = std::next(it); + do_deallocate(pending_chunk, pending_chunk->data); + it = next_it; + } else { + ++it; } } diff --git a/src/umpire/strategy/ResourceAwarePool.hpp b/src/umpire/strategy/ResourceAwarePool.hpp index ecd0a9c38..cb937afc9 100644 --- a/src/umpire/strategy/ResourceAwarePool.hpp +++ b/src/umpire/strategy/ResourceAwarePool.hpp @@ -27,22 +27,6 @@ using Resource = camp::resources::Resource; using Event = camp::resources::Event; -namespace std { - // Hash function for variant Resource - template<> - struct hash> { - size_t operator()(const std::variant& key) const { - if (std::holds_alternative(key)) { - return 0; // Hash for no resource - } - // If there is a Resource, get it - const auto& resource = std::get(key); - return std::hash{}(&resource); // use the address of the resource object - } - }; - // End of PendingMap definitions -} - namespace umpire { class Allocator; @@ -201,11 +185,7 @@ class ResourceAwarePool : public AllocationStrategy, private mixins::AlignedAllo }; using PointerMap = std::unordered_map; - - // PendingMap definitions - using ResourceKey = std::variant; - using PendingMap = std::unordered_multimap; - + using PendingMap = std::unordered_multimap, Chunk*>; using SizeMap = std::multimap, pool_allocator>>;