From 2146cd07ee483cd2d5a6e424fc8a6d30268fc03a Mon Sep 17 00:00:00 2001 From: Dominik Dabek Date: Wed, 13 Dec 2023 11:34:50 +0000 Subject: [PATCH] refactor: SortedVectorBasedAllocationTracker Move code out to base class. This will allow to use the sorted vector class with different values than only SvmData. Signed-off-by: Dominik Dabek --- .../memory_manager/unified_memory_manager.cpp | 66 +++------------- .../memory_manager/unified_memory_manager.h | 18 ++--- shared/source/utilities/CMakeLists.txt | 3 +- shared/source/utilities/sorted_vector.h | 75 +++++++++++++++++++ .../unified_memory_manager_cache_tests.cpp | 8 +- .../unified_memory_manager_tests.cpp | 4 +- .../test/unit_test/utilities/CMakeLists.txt | 1 + .../utilities/sorted_vector_tests.cpp | 22 ++++++ 8 files changed, 123 insertions(+), 74 deletions(-) create mode 100644 shared/source/utilities/sorted_vector.h create mode 100644 shared/test/unit_test/utilities/sorted_vector_tests.cpp diff --git a/shared/source/memory_manager/unified_memory_manager.cpp b/shared/source/memory_manager/unified_memory_manager.cpp index 7ce57ea93b..d82fa587b2 100644 --- a/shared/source/memory_manager/unified_memory_manager.cpp +++ b/shared/source/memory_manager/unified_memory_manager.cpp @@ -42,25 +42,6 @@ void SVMAllocsManager::MapBasedAllocationTracker::remove(const SvmAllocationData allocations.erase(iter); } -void SVMAllocsManager::SortedVectorBasedAllocationTracker::insert(const SvmAllocationData &allocationsPair) { - allocations.push_back(std::make_pair(reinterpret_cast(allocationsPair.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress()), std::make_unique(allocationsPair))); - for (size_t i = allocations.size() - 1; i > 0; --i) { - if (allocations[i].first < allocations[i - 1].first) { - std::iter_swap(allocations.begin() + i, allocations.begin() + i - 1); - } else { - break; - } - } -} - -void SVMAllocsManager::SortedVectorBasedAllocationTracker::remove(const SvmAllocationData &allocationsPair) { - auto gpuAddress = reinterpret_cast(allocationsPair.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress()); - auto removeIt = std::remove_if(allocations.begin(), allocations.end(), [&gpuAddress](const auto &other) { - return gpuAddress == other.first; - }); - allocations.erase(removeIt); -} - void SVMAllocsManager::SvmAllocationCache::insert(size_t size, void *ptr) { std::lock_guard lock(this->mtx); allocations.emplace(std::lower_bound(allocations.begin(), allocations.end(), size), size, ptr); @@ -132,34 +113,6 @@ SvmAllocationData *SVMAllocsManager::MapBasedAllocationTracker::get(const void * return nullptr; } -SvmAllocationData *SVMAllocsManager::SortedVectorBasedAllocationTracker::get(const void *ptr) { - if (allocations.size() == 0) { - return nullptr; - } - if (!ptr) { - return nullptr; - } - - int begin = 0; - int end = static_cast(allocations.size() - 1); - while (end >= begin) { - int currentPos = (begin + end) / 2; - const auto &allocation = allocations[currentPos]; - if (allocation.first == ptr || (allocation.first < ptr && - (reinterpret_cast(ptr) < (reinterpret_cast(allocation.first) + allocation.second->size)))) { - return allocation.second.get(); - } else if (ptr < allocation.first) { - end = currentPos - 1; - continue; - } else { - begin = currentPos + 1; - continue; - } - } - - return nullptr; -} - void SVMAllocsManager::MapOperationsTracker::insert(SvmMapOperation mapOperation) { operations.insert(std::make_pair(mapOperation.regionSvmPtr, mapOperation)); } @@ -286,7 +239,7 @@ void *SVMAllocsManager::createHostUnifiedMemoryAllocation(size_t size, allocData.setAllocId(this->allocationsCounter++); std::unique_lock lock(mtx); - this->svmAllocs.insert(allocData); + this->svmAllocs.insert(usmPtr, allocData); return usmPtr; } @@ -368,9 +321,9 @@ void *SVMAllocsManager::createUnifiedMemoryAllocation(size_t size, allocData.setAllocId(this->allocationsCounter++); std::unique_lock lock(mtx); - this->svmAllocs.insert(allocData); auto retPtr = reinterpret_cast(unifiedMemoryAllocation->getGpuAddress()); + this->svmAllocs.insert(retPtr, allocData); UNRECOVERABLE_IF(useExternalHostPtrForCpu && (externalPtr != retPtr)); return retPtr; @@ -455,8 +408,9 @@ void *SVMAllocsManager::createUnifiedKmdMigratedAllocation(size_t size, const Sv allocData.setAllocId(this->allocationsCounter++); std::unique_lock lock(mtx); - this->svmAllocs.insert(allocData); - return allocationGpu->getUnderlyingBuffer(); + auto retPtr = allocationGpu->getUnderlyingBuffer(); + this->svmAllocs.insert(retPtr, allocData); + return retPtr; } void SVMAllocsManager::setUnifiedAllocationProperties(GraphicsAllocation *allocation, const SvmAllocationProperties &svmProperties) { @@ -466,12 +420,12 @@ void SVMAllocsManager::setUnifiedAllocationProperties(GraphicsAllocation *alloca void SVMAllocsManager::insertSVMAlloc(const SvmAllocationData &svmAllocData) { std::unique_lock lock(mtx); - svmAllocs.insert(svmAllocData); + svmAllocs.insert(reinterpret_cast(svmAllocData.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress()), svmAllocData); } void SVMAllocsManager::removeSVMAlloc(const SvmAllocationData &svmAllocData) { std::unique_lock lock(mtx); - svmAllocs.remove(svmAllocData); + svmAllocs.remove(reinterpret_cast(svmAllocData.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress())); } bool SVMAllocsManager::freeSVMAlloc(void *ptr, bool blocking) { @@ -608,7 +562,7 @@ void *SVMAllocsManager::createZeroCopySvmAllocation(size_t size, const SvmAlloca allocData.size = size; std::unique_lock lock(mtx); - this->svmAllocs.insert(allocData); + this->svmAllocs.insert(usmPtr, allocData); return usmPtr; } @@ -676,14 +630,14 @@ void *SVMAllocsManager::createUnifiedAllocationWithDeviceStorage(size_t size, co allocData.setAllocId(this->allocationsCounter++); std::unique_lock lock(mtx); - this->svmAllocs.insert(allocData); + this->svmAllocs.insert(svmPtr, allocData); return svmPtr; } void SVMAllocsManager::freeSVMData(SvmAllocationData *svmData) { std::unique_lock lockForIndirect(mtxForIndirectAccess); std::unique_lock lock(mtx); - svmAllocs.remove(*svmData); + svmAllocs.remove(reinterpret_cast(svmData->gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress())); } void SVMAllocsManager::freeZeroCopySvmAllocation(SvmAllocationData *svmData) { diff --git a/shared/source/memory_manager/unified_memory_manager.h b/shared/source/memory_manager/unified_memory_manager.h index 7cf83bcd09..81e44298b3 100644 --- a/shared/source/memory_manager/unified_memory_manager.h +++ b/shared/source/memory_manager/unified_memory_manager.h @@ -11,6 +11,7 @@ #include "shared/source/memory_manager/multi_graphics_allocation.h" #include "shared/source/memory_manager/residency_container.h" #include "shared/source/unified_memory/unified_memory.h" +#include "shared/source/utilities/sorted_vector.h" #include "memory_properties_flags.h" @@ -84,18 +85,13 @@ struct SvmMapOperation { class SVMAllocsManager { public: - class SortedVectorBasedAllocationTracker { - friend class SVMAllocsManager; - - public: - using SvmAllocationContainer = std::vector>>; - void insert(const SvmAllocationData &); - void remove(const SvmAllocationData &); - SvmAllocationData *get(const void *); - size_t getNumAllocs() const { return allocations.size(); }; - - SvmAllocationContainer allocations; + struct CompareAcceptOffsetSvmPointers { + bool operator()(const std::unique_ptr &svmData, const void *ptr, const void *otherPtr) { + return ptr == otherPtr || (otherPtr < ptr && + (reinterpret_cast(ptr) < (reinterpret_cast(otherPtr) + svmData->size))); + } }; + using SortedVectorBasedAllocationTracker = BaseSortedPointerWithValueVector; class MapBasedAllocationTracker { friend class SVMAllocsManager; diff --git a/shared/source/utilities/CMakeLists.txt b/shared/source/utilities/CMakeLists.txt index b3a9434675..4926658f92 100644 --- a/shared/source/utilities/CMakeLists.txt +++ b/shared/source/utilities/CMakeLists.txt @@ -1,5 +1,5 @@ # -# Copyright (C) 2019-2022 Intel Corporation +# Copyright (C) 2019-2023 Intel Corporation # # SPDX-License-Identifier: MIT # @@ -37,6 +37,7 @@ set(NEO_CORE_UTILITIES ${CMAKE_CURRENT_SOURCE_DIR}/software_tags.h ${CMAKE_CURRENT_SOURCE_DIR}/software_tags_manager.cpp ${CMAKE_CURRENT_SOURCE_DIR}/software_tags_manager.h + ${CMAKE_CURRENT_SOURCE_DIR}/sorted_vector.h ${CMAKE_CURRENT_SOURCE_DIR}/spinlock.h ${CMAKE_CURRENT_SOURCE_DIR}/stackvec.h ${CMAKE_CURRENT_SOURCE_DIR}/tag_allocator.cpp diff --git a/shared/source/utilities/sorted_vector.h b/shared/source/utilities/sorted_vector.h new file mode 100644 index 0000000000..17e09b3954 --- /dev/null +++ b/shared/source/utilities/sorted_vector.h @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2018-2023 Intel Corporation + * + * SPDX-License-Identifier: MIT + * + */ + +#pragma once +#include +#include +#include + +namespace NEO { + +template +class BaseSortedPointerWithValueVector { + public: + using PointerPair = std::pair>; + using Container = std::vector; + + BaseSortedPointerWithValueVector() : compareFunctor(Compare()){}; + + void insert(const void *ptr, const ValueType &value) { + allocations.push_back(std::make_pair(ptr, std::make_unique(value))); + for (size_t i = allocations.size() - 1; i > 0; --i) { + if (allocations[i].first < allocations[i - 1].first) { + std::iter_swap(allocations.begin() + i, allocations.begin() + i - 1); + } else { + break; + } + } + } + + void remove(const void *ptr) { + auto removeIt = std::remove_if(allocations.begin(), allocations.end(), [&ptr](const PointerPair &other) { + return ptr == other.first; + }); + allocations.erase(removeIt); + } + + ValueType *get(const void *ptr) { + if (allocations.size() == 0) { + return nullptr; + } + + if (nullptr == ptr) { + return nullptr; + } + + int begin = 0; + int end = static_cast(allocations.size() - 1); + while (end >= begin) { + int currentPos = (begin + end) / 2; + const auto &allocation = allocations[currentPos]; + + if (compareFunctor(allocation.second, ptr, allocation.first)) { + return allocation.second.get(); + } else if (ptr < allocation.first) { + end = currentPos - 1; + continue; + } else { + begin = currentPos + 1; + continue; + } + } + + return nullptr; + } + + size_t getNumAllocs() const { return allocations.size(); } + + Container allocations; + Compare compareFunctor; +}; +} // namespace NEO diff --git a/shared/test/unit_test/memory_manager/unified_memory_manager_cache_tests.cpp b/shared/test/unit_test/memory_manager/unified_memory_manager_cache_tests.cpp index 23b77c0cf1..b5b3658e84 100644 --- a/shared/test/unit_test/memory_manager/unified_memory_manager_cache_tests.cpp +++ b/shared/test/unit_test/memory_manager/unified_memory_manager_cache_tests.cpp @@ -35,12 +35,12 @@ TEST(SortedVectorBasedAllocationTrackerTests, givenSortedVectorBasedAllocationTr for (uint32_t i = graphicsAllocationsSize - 1; i >= graphicsAllocationsSize / 2; --i) { data.gpuAllocations.addAllocation(&graphicsAllocations[i]); data.device = reinterpret_cast(graphicsAllocations[i].getGpuAddress()); - tracker.insert(data); + tracker.insert(reinterpret_cast(data.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress()), data); } for (uint32_t i = 0; i < graphicsAllocationsSize / 2; ++i) { data.gpuAllocations.addAllocation(&graphicsAllocations[i]); data.device = reinterpret_cast(graphicsAllocations[i].getGpuAddress()); - tracker.insert(data); + tracker.insert(reinterpret_cast(data.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress()), data); } EXPECT_EQ(tracker.getNumAllocs(), graphicsAllocationsSize); @@ -56,7 +56,7 @@ TEST(SortedVectorBasedAllocationTrackerTests, givenSortedVectorBasedAllocationTr MockGraphicsAllocation graphicsAlloc{reinterpret_cast(0x0), MemoryConstants::pageSize64k}; data.gpuAllocations.addAllocation(&graphicsAlloc); data.device = reinterpret_cast(graphicsAlloc.getGpuAddress()); - tracker.insert(data); + tracker.insert(reinterpret_cast(data.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress()), data); EXPECT_EQ(tracker.getNumAllocs(), graphicsAllocationsSize + 1); for (uint64_t i = 0; i < graphicsAllocationsSize + 1; ++i) { @@ -69,7 +69,7 @@ TEST(SortedVectorBasedAllocationTrackerTests, givenSortedVectorBasedAllocationTr auto data2 = tracker.get(addr2); EXPECT_EQ(data1->device, addr1); EXPECT_EQ(data2->device, addr2); - tracker.remove(*data2); + tracker.remove(addr2); EXPECT_EQ(tracker.getNumAllocs(), graphicsAllocationsSize); for (uint64_t i = 0; i < graphicsAllocationsSize; ++i) { if (i < 2) { diff --git a/shared/test/unit_test/memory_manager/unified_memory_manager_tests.cpp b/shared/test/unit_test/memory_manager/unified_memory_manager_tests.cpp index 03dfc3cd9e..1f722176e1 100644 --- a/shared/test/unit_test/memory_manager/unified_memory_manager_tests.cpp +++ b/shared/test/unit_test/memory_manager/unified_memory_manager_tests.cpp @@ -223,7 +223,7 @@ TEST_F(SVMLocalMemoryAllocatorTest, whenMultiplePointerWithOffsetPassedThenPrope allocationData.memoryType = InternalMemoryType::HOST_UNIFIED_MEMORY; allocationData.size = mockAllocation.getUnderlyingBufferSize(); allocationData.gpuAllocations.addAllocation(&mockAllocation); - svmManager->svmAllocs.insert(allocationData); + svmManager->svmAllocs.insert(unalignedPointer, allocationData); auto offsetedPointer = ptrOffset(ptr, 2048); auto usmAllocationData = svmManager->getSVMAlloc(offsetedPointer); @@ -234,7 +234,7 @@ TEST_F(SVMLocalMemoryAllocatorTest, whenMultiplePointerWithOffsetPassedThenPrope usmAllocationData = svmManager->getSVMAlloc(unalignedPointer); EXPECT_NE(nullptr, usmAllocationData); - svmManager->svmAllocs.remove(allocationData); + svmManager->svmAllocs.remove(reinterpret_cast(allocationData.gpuAllocations.getDefaultGraphicsAllocation()->getGpuAddress())); svmManager->freeSVMAlloc(ptr, true); svmManager->freeSVMAlloc(ptr2, true); } diff --git a/shared/test/unit_test/utilities/CMakeLists.txt b/shared/test/unit_test/utilities/CMakeLists.txt index 5d226f7eb1..216468c2c4 100644 --- a/shared/test/unit_test/utilities/CMakeLists.txt +++ b/shared/test/unit_test/utilities/CMakeLists.txt @@ -21,6 +21,7 @@ target_sources(neo_shared_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/perf_profiler_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/reference_tracked_object_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/software_tags_manager_tests.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/sorted_vector_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/spinlock_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tag_allocator_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/timer_util_tests.cpp diff --git a/shared/test/unit_test/utilities/sorted_vector_tests.cpp b/shared/test/unit_test/utilities/sorted_vector_tests.cpp new file mode 100644 index 0000000000..32059e63a6 --- /dev/null +++ b/shared/test/unit_test/utilities/sorted_vector_tests.cpp @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2023 Intel Corporation + * + * SPDX-License-Identifier: MIT + * + */ + +#include "shared/source/utilities/sorted_vector.h" + +#include "gtest/gtest.h" + +struct Comparator { + bool operator()(const std::unique_ptr &svmData, const void *ptr, const void *otherPtr) { + return false; + } +}; +using TestedSortedVector = NEO::BaseSortedPointerWithValueVector; + +TEST(SortedVectorTest, givenBaseSortedVectorWhenGettingNullptrThenNullptrIsReturned) { + TestedSortedVector testedVector; + EXPECT_EQ(nullptr, testedVector.get(nullptr)); +}