Revert "feature: add optional onChunkFree callback to AbstractBuffersPool"

This reverts commit b7ecf99abb.

Signed-off-by: Compute-Runtime-Validation <compute-runtime-validation@intel.com>
This commit is contained in:
Compute-Runtime-Validation 2023-07-07 04:11:40 +02:00 committed by Compute-Runtime-Automation
parent 8ed2cb2bfe
commit 9c7950cd22
4 changed files with 19 additions and 99 deletions

View File

@ -504,7 +504,7 @@ bool Context::BufferPoolAllocator::isAggregatedSmallBuffersEnabled(Context *cont
(isSupportedForSingleDeviceContexts && context->isSingleDeviceContext());
}
Context::BufferPool::BufferPool(Context *context) : BaseType(context->memoryManager, nullptr) {
Context::BufferPool::BufferPool(Context *context) : BaseType(context->memoryManager) {
static constexpr cl_mem_flags flags{};
[[maybe_unused]] cl_int errcodeRet{};
Buffer::AdditionalBufferCreateArgs bufferCreateArgs{};

View File

@ -7,7 +7,6 @@
#pragma once
#include "shared/source/helpers/constants.h"
#include "shared/source/helpers/non_copyable_or_moveable.h"
#include "shared/source/utilities/stackvec.h"
#include <functional>
@ -32,7 +31,7 @@ struct SmallBuffersParams {
};
template <typename PoolT, typename BufferType, typename BufferParentType = BufferType>
struct AbstractBuffersPool : public SmallBuffersParams<PoolT>, public NonCopyableClass {
struct AbstractBuffersPool : public SmallBuffersParams<PoolT> {
// The prototype of a function allocating the `mainStorage` is not specified.
// That would be an unnecessary limitation here - it is completely up to derived class implementation.
// Perhaps the allocating function needs to leverage `HeapAllocator::allocate()` and also
@ -44,9 +43,8 @@ struct AbstractBuffersPool : public SmallBuffersParams<PoolT>, public NonCopyabl
using Params::smallBufferThreshold;
using Params::startingOffset;
using AllocsVecCRef = const StackVec<NEO::GraphicsAllocation *, 1> &;
using OnChunkFreeCallback = void (PoolT::*)(uint64_t offset, size_t size);
AbstractBuffersPool(MemoryManager *memoryManager, OnChunkFreeCallback onChunkFreeCallback);
AbstractBuffersPool(MemoryManager *memoryManager);
AbstractBuffersPool(AbstractBuffersPool<PoolT, BufferType, BufferParentType> &&bufferPool);
void tryFreeFromPoolBuffer(BufferParentType *possiblePoolBuffer, size_t offset, size_t size);
bool isPoolBuffer(const BufferParentType *buffer) const;
@ -62,7 +60,6 @@ struct AbstractBuffersPool : public SmallBuffersParams<PoolT>, public NonCopyabl
std::unique_ptr<BufferType> mainStorage;
std::unique_ptr<HeapAllocator> chunkAllocator;
std::vector<std::pair<uint64_t, size_t>> chunksToFree;
OnChunkFreeCallback onChunkFreeCallback = nullptr;
};
template <typename BuffersPoolType, typename BufferType, typename BufferParentType = BufferType>
@ -84,11 +81,8 @@ class AbstractBuffersAllocator : public SmallBuffersParams<BuffersPoolType> {
protected:
inline bool isSizeWithinThreshold(size_t size) const { return smallBufferThreshold >= size; }
void tryFreeFromPoolBuffer(BufferParentType *possiblePoolBuffer, size_t offset, size_t size, std::vector<BuffersPoolType> &bufferPoolsVec);
void drain();
void drain(std::vector<BuffersPoolType> &bufferPoolsVec);
void addNewBufferPool(BuffersPoolType &&bufferPool);
void addNewBufferPool(BuffersPoolType &&bufferPool, std::vector<BuffersPoolType> &bufferPoolsVec);
std::mutex mutex;
std::vector<BuffersPoolType> bufferPools;

View File

@ -14,8 +14,7 @@
namespace NEO {
template <typename PoolT, typename BufferType, typename BufferParentType>
AbstractBuffersPool<PoolT, BufferType, BufferParentType>::AbstractBuffersPool(MemoryManager *memoryManager, OnChunkFreeCallback onChunkFreeCb)
: memoryManager{memoryManager}, onChunkFreeCallback{onChunkFreeCb} {
AbstractBuffersPool<PoolT, BufferType, BufferParentType>::AbstractBuffersPool(MemoryManager *mm) : memoryManager{mm} {
static_assert(std::is_base_of_v<BufferParentType, BufferType>);
}
@ -23,13 +22,12 @@ template <typename PoolT, typename BufferType, typename BufferParentType>
AbstractBuffersPool<PoolT, BufferType, BufferParentType>::AbstractBuffersPool(AbstractBuffersPool<PoolT, BufferType, BufferParentType> &&bufferPool)
: memoryManager{bufferPool.memoryManager},
mainStorage{std::move(bufferPool.mainStorage)},
chunkAllocator{std::move(bufferPool.chunkAllocator)},
onChunkFreeCallback{bufferPool.onChunkFreeCallback} {}
chunkAllocator{std::move(bufferPool.chunkAllocator)} {}
template <typename PoolT, typename BufferType, typename BufferParentType>
void AbstractBuffersPool<PoolT, BufferType, BufferParentType>::tryFreeFromPoolBuffer(BufferParentType *possiblePoolBuffer, size_t offset, size_t size) {
if (this->isPoolBuffer(possiblePoolBuffer)) {
this->chunksToFree.push_back({offset, size});
this->chunksToFree.push_back({offset + startingOffset, size});
}
}
@ -50,9 +48,6 @@ void AbstractBuffersPool<PoolT, BufferType, BufferParentType>::drain() {
}
for (auto &chunk : this->chunksToFree) {
this->chunkAllocator->free(chunk.first, chunk.second);
if (static_cast<PoolT *>(this)->onChunkFreeCallback) {
(static_cast<PoolT *>(this)->*onChunkFreeCallback)(chunk.first, chunk.second);
}
}
this->chunksToFree.clear();
}
@ -71,38 +66,23 @@ bool AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::is
template <typename BuffersPoolType, typename BufferType, typename BufferParentType>
void AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::tryFreeFromPoolBuffer(BufferParentType *possiblePoolBuffer, size_t offset, size_t size) {
this->tryFreeFromPoolBuffer(possiblePoolBuffer, offset, size, this->bufferPools);
}
template <typename BuffersPoolType, typename BufferType, typename BufferParentType>
void AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::tryFreeFromPoolBuffer(BufferParentType *possiblePoolBuffer, size_t offset, size_t size, std::vector<BuffersPoolType> &bufferPoolsVec) {
auto lock = std::unique_lock<std::mutex>(this->mutex);
for (auto &bufferPool : bufferPoolsVec) {
for (auto &bufferPool : this->bufferPools) {
bufferPool.tryFreeFromPoolBuffer(possiblePoolBuffer, offset, size); // NOLINT(clang-analyzer-cplusplus.NewDelete)
}
}
template <typename BuffersPoolType, typename BufferType, typename BufferParentType>
void AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::drain() {
this->drain(this->bufferPools);
}
template <typename BuffersPoolType, typename BufferType, typename BufferParentType>
void AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::drain(std::vector<BuffersPoolType> &bufferPoolsVec) {
for (auto &bufferPool : bufferPoolsVec) {
for (auto &bufferPool : this->bufferPools) {
bufferPool.drain();
}
}
template <typename BuffersPoolType, typename BufferType, typename BufferParentType>
void AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::addNewBufferPool(BuffersPoolType &&bufferPool) {
this->addNewBufferPool(std::move(bufferPool), this->bufferPools);
}
template <typename BuffersPoolType, typename BufferType, typename BufferParentType>
void AbstractBuffersAllocator<BuffersPoolType, BufferType, BufferParentType>::addNewBufferPool(BuffersPoolType &&bufferPool, std::vector<BuffersPoolType> &bufferPoolsVec) {
if (bufferPool.mainStorage) {
bufferPoolsVec.push_back(std::move(bufferPool));
this->bufferPools.push_back(std::move(bufferPool));
}
}
} // namespace NEO

View File

@ -34,38 +34,27 @@ struct DummyBuffersPool : public NEO::AbstractBuffersPool<DummyBuffersPool, Dumm
using BaseType = NEO::AbstractBuffersPool<DummyBuffersPool, DummyBuffer>;
static constexpr auto dummyPtr = 0xdeadbeef0000;
DummyBuffersPool(NEO::MemoryManager *memoryManager, uint32_t poolOffset, BaseType::OnChunkFreeCallback onChunkFreeCallback)
: BaseType{memoryManager, onChunkFreeCallback} {
DummyBuffersPool(NEO::MemoryManager *memoryManager, uint32_t poolOffset) : BaseType(memoryManager) {
dummyAllocations.resize(2);
dummyAllocations[0] = reinterpret_cast<NEO::GraphicsAllocation *>(poolOffset + dummyPtr);
dummyAllocations[1] = nullptr; // makes sure nullptrs don't cause SEGFAULTs
}
DummyBuffersPool(NEO::MemoryManager *memoryManager) : DummyBuffersPool(memoryManager, 0x0, &DummyBuffersPool::onChunkFree) {}
DummyBuffersPool(NEO::MemoryManager *memoryManager) : DummyBuffersPool(memoryManager, 0x0) {}
BaseType::AllocsVecCRef getAllocationsVector() {
return dummyAllocations;
}
void onChunkFree(uint64_t offset, size_t size) {
this->freedChunks.push_back({offset, size});
this->onChunkFreeCalled = true;
}
StackVec<NEO::GraphicsAllocation *, 1> dummyAllocations;
std::vector<std::pair<uint64_t, size_t>> freedChunks{};
bool onChunkFreeCalled = false;
};
struct DummyBuffersAllocator : public NEO::AbstractBuffersAllocator<DummyBuffersPool, DummyBuffer> {
using BaseType = NEO::AbstractBuffersAllocator<DummyBuffersPool, DummyBuffer>;
using BaseType::addNewBufferPool;
using BaseType::bufferPools;
using BaseType::drain;
using BaseType::isSizeWithinThreshold;
void drainUnderLock() {
auto lock = std::unique_lock<std::mutex>(this->mutex);
this->BaseType::drain();
}
};
using NEO::MockExecutionEnvironment;
@ -169,13 +158,13 @@ TEST_F(AbstractSmallBuffersTest, givenBuffersAllocatorWhenChunkOfMainStorageTrie
auto &chunksToFree2 = buffersAllocator.bufferPools[1].chunksToFree;
EXPECT_EQ(chunksToFree1.size(), 0u);
EXPECT_EQ(chunksToFree2.size(), 0u);
auto chunkSize = DummyBuffersPool::chunkAlignment * 4;
auto chunkOffset = DummyBuffersPool::chunkAlignment;
auto chunkSize = sizeof(DummyBuffer) / 8;
auto chunkOffset = sizeof(DummyBuffer) / 2;
buffersAllocator.tryFreeFromPoolBuffer(poolStorage2, chunkOffset, chunkSize);
EXPECT_EQ(chunksToFree1.size(), 0u);
EXPECT_EQ(chunksToFree2.size(), 1u);
auto [effectiveChunkOffset, size] = chunksToFree2[0];
EXPECT_EQ(effectiveChunkOffset, chunkOffset);
EXPECT_EQ(effectiveChunkOffset, chunkOffset + DummyBuffersPool::startingOffset);
EXPECT_EQ(size, chunkSize);
buffersAllocator.releaseSmallBufferPool();
@ -204,8 +193,8 @@ TEST_F(AbstractSmallBuffersTest, givenBuffersAllocatorWhenDrainingPoolsThenOnlyA
buffersAllocator.addNewBufferPool(std::move(pool1));
buffersAllocator.addNewBufferPool(std::move(pool2));
auto chunkSize = DummyBuffersPool::chunkAlignment * 4;
auto chunkOffset = DummyBuffersPool::chunkAlignment;
auto chunkSize = sizeof(DummyBuffer) / 16;
auto chunkOffset = sizeof(DummyBuffer) / 2;
for (size_t i = 0; i < 3; i++) {
auto exampleOffset = chunkOffset + i * chunkSize * 2;
buffersAllocator.tryFreeFromPoolBuffer(buffer1, exampleOffset, chunkSize);
@ -214,54 +203,11 @@ TEST_F(AbstractSmallBuffersTest, givenBuffersAllocatorWhenDrainingPoolsThenOnlyA
auto &chunksToFree1 = buffersAllocator.bufferPools[0].chunksToFree;
auto &chunksToFree2 = buffersAllocator.bufferPools[1].chunksToFree;
auto &freedChunks1 = buffersAllocator.bufferPools[0].freedChunks;
auto &freedChunks2 = buffersAllocator.bufferPools[1].freedChunks;
EXPECT_EQ(chunksToFree1.size(), 3u);
EXPECT_EQ(chunksToFree2.size(), 3u);
EXPECT_EQ(freedChunks1.size(), 0u);
EXPECT_EQ(freedChunks2.size(), 0u);
otherMemoryManager->deferAllocInUse = true;
buffersAllocator.drainUnderLock();
buffersAllocator.drain();
EXPECT_EQ(chunksToFree1.size(), 0u);
EXPECT_EQ(chunksToFree2.size(), 3u);
ASSERT_EQ(freedChunks1.size(), 3u);
EXPECT_EQ(freedChunks2.size(), 0u);
EXPECT_TRUE(buffersAllocator.bufferPools[0].onChunkFreeCalled);
EXPECT_FALSE(buffersAllocator.bufferPools[1].onChunkFreeCalled);
for (size_t i = 0; i < 3; i++) {
auto expectedOffset = chunkOffset + i * chunkSize * 2;
auto [freedOffset, freedSize] = freedChunks1[i];
EXPECT_EQ(expectedOffset, freedOffset);
EXPECT_EQ(chunkSize, freedSize);
}
}
TEST_F(AbstractSmallBuffersTest, givenBuffersAllocatorWhenDrainingPoolsThenOnChunkFreeIgnoredIfNotDefined) {
auto pool1 = DummyBuffersPool{this->memoryManager.get(), 0x0, nullptr};
pool1.mainStorage.reset(new DummyBuffer(testVal));
auto buffer1 = pool1.mainStorage.get();
pool1.chunkAllocator.reset(new NEO::HeapAllocator{DummyBuffersPool::startingOffset,
DummyBuffersPool::aggregatedSmallBuffersPoolSize,
DummyBuffersPool::chunkAlignment,
DummyBuffersPool::smallBufferThreshold});
auto buffersAllocator = DummyBuffersAllocator{};
buffersAllocator.addNewBufferPool(std::move(pool1));
auto chunkSize = DummyBuffersPool::chunkAlignment * 4;
auto chunkOffset = DummyBuffersPool::chunkAlignment;
for (size_t i = 0; i < 3; i++) {
auto exampleOffset = chunkOffset + i * chunkSize * 2;
buffersAllocator.tryFreeFromPoolBuffer(buffer1, exampleOffset, chunkSize);
}
auto &chunksToFree1 = buffersAllocator.bufferPools[0].chunksToFree;
auto &freedChunks1 = buffersAllocator.bufferPools[0].freedChunks;
EXPECT_EQ(chunksToFree1.size(), 3u);
EXPECT_EQ(freedChunks1.size(), 0u);
buffersAllocator.drainUnderLock();
EXPECT_EQ(chunksToFree1.size(), 0u);
EXPECT_EQ(freedChunks1.size(), 0u);
EXPECT_FALSE(buffersAllocator.bufferPools[0].onChunkFreeCalled);
}
}