diff --git a/HeterogeneousCore/AlpakaInterface/interface/CachingAllocator.h b/HeterogeneousCore/AlpakaInterface/interface/CachingAllocator.h index e1543d14a23c8..f72e3e40d9920 100644 --- a/HeterogeneousCore/AlpakaInterface/interface/CachingAllocator.h +++ b/HeterogeneousCore/AlpakaInterface/interface/CachingAllocator.h @@ -376,23 +376,30 @@ namespace cms::alpakatools { for (auto iBlock = begin; iBlock != end; ++iBlock) { if ((reuseSameQueueAllocations_ and (*block.queue == *(iBlock->second.queue))) or alpaka::isComplete(*(iBlock->second.event))) { - // associate the cached buffer to the new queue - auto queue = std::move(*(block.queue)); - // TODO cache (or remove) the debug information and use std::move() - block = iBlock->second; - block.queue = std::move(queue); + // take ownership of the cached block, so that a failure below cannot leave a block that + // has been moved from inside the cache + auto node = cachedBlocks_.extract(iBlock); + BlockDescriptor& cached = node.mapped(); + cachedBytes_.free -= cached.bytes; + + // the queue and the event of the cached block, for the debug message below + auto const* previousQueue = cached.queue->m_spQueueImpl.get(); + auto const* previousEvent = cached.event->m_spEventImpl.get(); + + // take the buffer and the event from the cached block. The queue, the bin and the + // requested size stay those of this allocation. + block.buffer = std::move(cached.buffer); + block.event = std::move(cached.event); + block.bytes = cached.bytes; // if the new queue is on different device than the old event, create a new event if (block.device() != alpaka::getDev(*(block.event))) { block.event = Event{block.device()}; } - // insert the cached block into the live blocks - // TODO cache (or remove) the debug information and use std::move() - liveBlocks_[block.buffer->data()] = block; + liveBlocks_.insert_or_assign(block.buffer->data(), block); // update the accounting information - cachedBytes_.free -= block.bytes; cachedBytes_.live += block.bytes; cachedBytes_.requested += block.requested; @@ -401,13 +408,11 @@ namespace cms::alpakatools { out << "\t" << deviceType_ << " " << alpaka::getName(device_) << " reused cached block at " << block.buffer->data() << " (" << block.bytes << " bytes) for queue " << block.queue->m_spQueueImpl.get() << ", event " << block.event->m_spEventImpl.get() - << " (previously associated with queue " << iBlock->second.queue->m_spQueueImpl.get() << " , event " - << iBlock->second.event->m_spEventImpl.get() << ")." << std::endl; + << " (previously associated with queue " << previousQueue << " , event " << previousEvent << ")." + << std::endl; std::cout << out.str() << std::endl; } - // remove the reused block from the list of cached blocks - cachedBlocks_.erase(iBlock); return true; } } diff --git a/HeterogeneousCore/AlpakaInterface/test/BuildFile.xml b/HeterogeneousCore/AlpakaInterface/test/BuildFile.xml index 4c0d1ffff0b27..f34487cdf5232 100644 --- a/HeterogeneousCore/AlpakaInterface/test/BuildFile.xml +++ b/HeterogeneousCore/AlpakaInterface/test/BuildFile.xml @@ -33,6 +33,12 @@ + + + + + + diff --git a/HeterogeneousCore/AlpakaInterface/test/testCachingAllocator.cc b/HeterogeneousCore/AlpakaInterface/test/testCachingAllocator.cc new file mode 100644 index 0000000000000..bcca7373ab3ef --- /dev/null +++ b/HeterogeneousCore/AlpakaInterface/test/testCachingAllocator.cc @@ -0,0 +1,57 @@ +#include + +#define CATCH_CONFIG_MAIN +#include + +#include + +#include "HeterogeneousCore/AlpakaInterface/interface/AllocatorConfig.h" +#include "HeterogeneousCore/AlpakaInterface/interface/CachingAllocator.h" +#include "HeterogeneousCore/AlpakaInterface/interface/host.h" + +// The allocator is exercised on the host device, so the test needs no accelerator. +using Device = alpaka::DevCpu; +using Queue = alpaka::QueueCpuBlocking; +using Allocator = cms::alpakatools::CachingAllocator; + +TEST_CASE("A reused block reports the size of the new allocation", "[CachingAllocator]") { + auto const& device = cms::alpakatools::host(); + cms::alpakatools::AllocatorConfig config; + // reuse blocks that are associated to the same queue, as the device allocators do + Allocator allocator{device, config, true, false}; + Queue queue{device}; + + // 300 and 500 bytes fall in the same bin, so the second allocation reuses the first block + void* first = allocator.allocate(300, queue); + REQUIRE(allocator.cacheStatus().requested == 300); + REQUIRE(allocator.cacheStatus().live == 512); + + allocator.free(first); + REQUIRE(allocator.cacheStatus().requested == 0); + REQUIRE(allocator.cacheStatus().live == 0); + REQUIRE(allocator.cacheStatus().free == 512); + + void* second = allocator.allocate(500, queue); + REQUIRE(second == first); + REQUIRE(allocator.cacheStatus().live == 512); + // the block is the same, but the reported size is the one of this allocation + REQUIRE(allocator.cacheStatus().requested == 500); + + allocator.free(second); + REQUIRE(allocator.cacheStatus().requested == 0); +} + +TEST_CASE("The accounting returns to zero after many reuses", "[CachingAllocator]") { + auto const& device = cms::alpakatools::host(); + cms::alpakatools::AllocatorConfig config; + Allocator allocator{device, config, true, false}; + Queue queue{device}; + + for (size_t bytes : {300ul, 500ul, 260ul, 511ul, 300ul}) { + void* p = allocator.allocate(bytes, queue); + REQUIRE(allocator.cacheStatus().requested == bytes); + allocator.free(p); + REQUIRE(allocator.cacheStatus().requested == 0); + REQUIRE(allocator.cacheStatus().live == 0); + } +}