From c9726dbb1049d5c63cf106c54ac642dcffa1aeab Mon Sep 17 00:00:00 2001 From: Maciej Bielski Date: Thu, 5 Dec 2024 14:31:28 +0000 Subject: [PATCH] refactor: simplify tracking CacheRegion reservations Leverage features of the mechanism to simplify implementation: - The maximum number of possible cache-region reservations is a small value known at compile-time - Each reservation is unique (described by `CacheRegion`) so can have a dedicated entry with either zero (free) or non-zero (reserved) value So, there is no need for a dynamic collection (unordered_map here) to keep track of reservations. A simple array is enough for that purpose. Also, add some helper-code to enable array-indexing with the values of `CacheRegion` enum. Related-To: NEO-12837 Signed-off-by: Maciej Bielski --- shared/source/helpers/common_types.h | 18 +++++++++++ .../source/os_interface/linux/cache_info.cpp | 32 +++++++++++-------- shared/source/os_interface/linux/cache_info.h | 11 ++++--- .../os_interface/linux/drm_mock_cache_info.h | 2 +- .../linux/drm_cache_info_tests.cpp | 7 ++-- 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/shared/source/helpers/common_types.h b/shared/source/helpers/common_types.h index 1ecf922c31..04674344eb 100644 --- a/shared/source/helpers/common_types.h +++ b/shared/source/helpers/common_types.h @@ -22,6 +22,23 @@ class Device; using DeviceVector = std::vector>; using PrivateAllocsToReuseContainer = StackVec, 8>; +// std::to_underlying is C++23 feature +template +constexpr auto toUnderlying(EnumT scopedEnumValue) { + static_assert(std::is_enum_v); + static_assert(!std::is_convertible_v>); + + return static_cast>(scopedEnumValue); +} + +template +constexpr auto toEnum(std::underlying_type_t region) { + static_assert(std::is_enum_v); + static_assert(!std::is_convertible_v>); + + return static_cast(region); +} + enum class DebugPauseState : uint32_t { disabled, waitingForFirstSemaphore, @@ -55,6 +72,7 @@ enum class CacheRegion : uint16_t { count, none = 0xFFFF }; +constexpr auto toCacheRegion(std::underlying_type_t region) { return toEnum(region); } enum class CacheLevel : uint16_t { defaultLevel = 0, diff --git a/shared/source/os_interface/linux/cache_info.cpp b/shared/source/os_interface/linux/cache_info.cpp index 871365146e..6081a6cdfd 100644 --- a/shared/source/os_interface/linux/cache_info.cpp +++ b/shared/source/os_interface/linux/cache_info.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2022-2023 Intel Corporation + * Copyright (C) 2022-2024 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -14,10 +14,16 @@ namespace NEO { CacheInfo::~CacheInfo() { - for (auto const &cacheRegion : cacheRegionsReserved) { - cacheReserve.freeCache(CacheLevel::level3, cacheRegion.first); + // skip the defaultRegion + constexpr auto regionStart{toUnderlying(CacheRegion::region1)}; + constexpr auto regionEnd{toUnderlying(CacheRegion::count)}; + + for (auto regionIndex{regionStart}; regionIndex < regionEnd; ++regionIndex) { + if (reservedCacheRegionsSize[regionIndex]) { + cacheReserve.freeCache(CacheLevel::level3, toCacheRegion(regionIndex)); + reservedCacheRegionsSize[regionIndex] = 0u; + } } - cacheRegionsReserved.clear(); } CacheRegion CacheInfo::reserveRegion(size_t cacheReservationSize) { @@ -30,28 +36,28 @@ CacheRegion CacheInfo::reserveRegion(size_t cacheReservationSize) { if (regionIndex == CacheRegion::none) { return CacheRegion::none; } - cacheRegionsReserved.insert({regionIndex, cacheReservationSize}); + DEBUG_BREAK_IF(regionIndex == CacheRegion::defaultRegion); + DEBUG_BREAK_IF(regionIndex >= CacheRegion::count); + reservedCacheRegionsSize[toUnderlying(regionIndex)] = cacheReservationSize; return regionIndex; } CacheRegion CacheInfo::freeRegion(CacheRegion regionIndex) { - auto search = cacheRegionsReserved.find(regionIndex); - if (search != cacheRegionsReserved.end()) { - cacheRegionsReserved.erase(search); + if (regionIndex < CacheRegion::count && reservedCacheRegionsSize[toUnderlying(regionIndex)] > 0u) { + reservedCacheRegionsSize[toUnderlying(regionIndex)] = 0u; return cacheReserve.freeCache(CacheLevel::level3, regionIndex); } return CacheRegion::none; } -bool CacheInfo::isRegionReserved(CacheRegion regionIndex, [[maybe_unused]] size_t regionSize) const { - auto search = cacheRegionsReserved.find(regionIndex); - if (search != cacheRegionsReserved.end()) { +bool CacheInfo::isRegionReserved(CacheRegion regionIndex, [[maybe_unused]] size_t expectedRegionSize) const { + if (regionIndex < CacheRegion::count && reservedCacheRegionsSize[toUnderlying(regionIndex)]) { if (debugManager.flags.ClosNumCacheWays.get() != -1) { auto numWays = debugManager.flags.ClosNumCacheWays.get(); - regionSize = (numWays * maxReservationCacheSize) / maxReservationNumWays; + expectedRegionSize = (numWays * maxReservationCacheSize) / maxReservationNumWays; } - DEBUG_BREAK_IF(search->second != regionSize); + DEBUG_BREAK_IF(expectedRegionSize != reservedCacheRegionsSize[toUnderlying(regionIndex)]); return true; } return false; diff --git a/shared/source/os_interface/linux/cache_info.h b/shared/source/os_interface/linux/cache_info.h index d9f7d0adc1..a69f3016a6 100644 --- a/shared/source/os_interface/linux/cache_info.h +++ b/shared/source/os_interface/linux/cache_info.h @@ -7,12 +7,13 @@ #pragma once +#include "shared/source/helpers/common_types.h" #include "shared/source/os_interface/linux/clos_cache.h" #include "shared/source/utilities/spinlock.h" -#include -#include -#include +#include +#include +#include namespace NEO { @@ -24,6 +25,8 @@ struct CacheInfo { maxReservationNumCacheRegions(maxReservationNumCacheRegions), maxReservationNumWays(maxReservationNumWays), cacheReserve{ioctlHelper} { + + reservedCacheRegionsSize.fill(0UL); } MOCKABLE_VIRTUAL ~CacheInfo(); @@ -69,7 +72,7 @@ struct CacheInfo { uint32_t maxReservationNumCacheRegions; uint16_t maxReservationNumWays; ClosCacheReservation cacheReserve; - std::unordered_map cacheRegionsReserved; + std::array reservedCacheRegionsSize; SpinLock mtx; }; diff --git a/shared/test/common/os_interface/linux/drm_mock_cache_info.h b/shared/test/common/os_interface/linux/drm_mock_cache_info.h index fc719914b1..cfa2956df3 100644 --- a/shared/test/common/os_interface/linux/drm_mock_cache_info.h +++ b/shared/test/common/os_interface/linux/drm_mock_cache_info.h @@ -13,10 +13,10 @@ namespace NEO { struct MockCacheInfo : public CacheInfo { - using CacheInfo::cacheRegionsReserved; using CacheInfo::freeRegion; using CacheInfo::getRegion; using CacheInfo::isRegionReserved; + using CacheInfo::reservedCacheRegionsSize; using CacheInfo::reserveRegion; MockCacheInfo(IoctlHelper &ioctlHelper, size_t maxReservationCacheSize, uint32_t maxReservationNumCacheRegions, uint16_t maxReservationNumWays) diff --git a/shared/test/unit_test/os_interface/linux/drm_cache_info_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_cache_info_tests.cpp index 5f6f416fda..8281d6ad9e 100644 --- a/shared/test/unit_test/os_interface/linux/drm_cache_info_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/drm_cache_info_tests.cpp @@ -132,9 +132,8 @@ TEST(DrmCacheInfoTest, givenCacheInfoWhenSpecificNumCacheWaysIsRequestedThenRese EXPECT_EQ(CacheRegion::region1, cacheInfo.reserveCacheRegion(maxReservationCacheSize)); EXPECT_TRUE(cacheInfo.isRegionReserved(CacheRegion::region1, maxReservationCacheSize)); - auto cacheRegion = cacheInfo.cacheRegionsReserved.begin(); - EXPECT_EQ(CacheRegion::region1, cacheRegion->first); - EXPECT_EQ(maxReservationCacheSize / 2, cacheRegion->second); + auto expectedRegionSize = cacheInfo.reservedCacheRegionsSize[toUnderlying(CacheRegion::region1)]; + EXPECT_EQ(maxReservationCacheSize / 2, expectedRegionSize); EXPECT_EQ(CacheRegion::region1, cacheInfo.freeCacheRegion(CacheRegion::region1)); } @@ -161,6 +160,6 @@ TEST(DrmCacheInfoTest, givenCacheInfoCreatedWhenFreeCacheRegionIsCalledForNonRes DrmQueryMock drm(*executionEnvironment->rootDeviceEnvironments[0]); MockCacheInfo cacheInfo(*drm.getIoctlHelper(), 32 * MemoryConstants::kiloByte, 2, 32); - cacheInfo.cacheRegionsReserved.insert({CacheRegion::region1, MemoryConstants::kiloByte}); + cacheInfo.reservedCacheRegionsSize[toUnderlying(CacheRegion::region1)] = MemoryConstants::kiloByte; EXPECT_EQ(CacheRegion::none, cacheInfo.freeCacheRegion(CacheRegion::region1)); }