From 1395a2cb386e3980d63f7180e7646d73dedffd9a Mon Sep 17 00:00:00 2001 From: Compute-Runtime-Validation Date: Tue, 28 Nov 2023 03:53:39 +0100 Subject: [PATCH] Revert "refactor: linux cl_cache eviction mechanism" This reverts commit a02ac1c14043e22429a9f24a29c7dd9516c46dc9. Signed-off-by: Compute-Runtime-Validation --- .../linux/compiler_cache_linux.cpp | 96 ++++----- .../linux/compiler_cache_tests_linux.cpp | 186 +----------------- 2 files changed, 43 insertions(+), 239 deletions(-) diff --git a/shared/source/compiler_interface/linux/compiler_cache_linux.cpp b/shared/source/compiler_interface/linux/compiler_cache_linux.cpp index f800ba25a1..4d197f8697 100644 --- a/shared/source/compiler_interface/linux/compiler_cache_linux.cpp +++ b/shared/source/compiler_interface/linux/compiler_cache_linux.cpp @@ -31,8 +31,7 @@ namespace NEO { int filterFunction(const struct dirent *file) { std::string_view fileName = file->d_name; - if (fileName.find(".cl_cache") != fileName.npos || - fileName.find(".l0_cache") != fileName.npos) { + if (fileName.find(".cl_cache") != fileName.npos || fileName.find(".l0_cache") != fileName.npos) { return 1; } @@ -51,39 +50,38 @@ bool compareByLastAccessTime(const ElementsStruct &a, ElementsStruct &b) { bool CompilerCache::evictCache(uint64_t &bytesEvicted) { struct dirent **files = 0; - const int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); + int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); if (filesCount == -1) { NEO::printDebugString(NEO::DebugManager.flags.PrintDebugMessages.get(), stderr, "PID %d [Cache failure]: Scandir failed! errno: %d\n", NEO::SysCalls::getProcessId(), errno); return false; } - std::vector cacheFiles; - cacheFiles.reserve(static_cast(filesCount)); + std::vector vec; + vec.reserve(static_cast(filesCount)); for (int i = 0; i < filesCount; ++i) { ElementsStruct fileElement = {}; fileElement.path = joinPath(config.cacheDir, files[i]->d_name); if (NEO::SysCalls::stat(fileElement.path.c_str(), &fileElement.statEl) == 0) { - cacheFiles.push_back(std::move(fileElement)); + vec.push_back(std::move(fileElement)); } - free(files[i]); } + for (int i = 0; i < filesCount; ++i) { + free(files[i]); + } free(files); - std::sort(cacheFiles.begin(), cacheFiles.end(), compareByLastAccessTime); + std::sort(vec.begin(), vec.end(), compareByLastAccessTime); - bytesEvicted = 0; - const auto evictionLimit = config.cacheSize / 3; + size_t evictionLimit = config.cacheSize / 3; - for (const auto &file : cacheFiles) { - auto res = NEO::SysCalls::unlink(file.path); - if (res == -1) { - continue; - } + size_t evictionSizeCount = 0; + for (size_t i = 0; i < vec.size(); ++i) { + NEO::SysCalls::unlink(vec[i].path); + evictionSizeCount += vec[i].statEl.st_size; - bytesEvicted += file.statEl.st_size; - if (bytesEvicted > evictionLimit) { + if (evictionSizeCount > evictionLimit) { return true; } } @@ -148,7 +146,7 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, } } - const int lockErr = NEO::SysCalls::flock(std::get(fd), LOCK_EX); + int lockErr = NEO::SysCalls::flock(std::get(fd), LOCK_EX); if (lockErr < 0) { NEO::printDebugString(NEO::DebugManager.flags.PrintDebugMessages.get(), stderr, "PID %d [Cache failure]: Lock config file failed! errno: %d\n", NEO::SysCalls::getProcessId(), errno); @@ -161,7 +159,7 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, if (countDirectorySize) { struct dirent **files = {}; - const int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); + int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); if (filesCount == -1) { unlockFileAndClose(std::get(fd)); @@ -170,28 +168,30 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, return; } - std::vector cacheFiles; - cacheFiles.reserve(static_cast(filesCount)); + std::vector vec; + vec.reserve(static_cast(filesCount)); for (int i = 0; i < filesCount; ++i) { std::string_view fileName = files[i]->d_name; if (fileName.find(config.cacheFileExtension) != fileName.npos) { ElementsStruct fileElement = {}; fileElement.path = joinPath(config.cacheDir, files[i]->d_name); if (NEO::SysCalls::stat(fileElement.path.c_str(), &fileElement.statEl) == 0) { - cacheFiles.push_back(std::move(fileElement)); + vec.push_back(std::move(fileElement)); } } - free(files[i]); } + for (int i = 0; i < filesCount; ++i) { + free(files[i]); + } free(files); - for (const auto &element : cacheFiles) { + for (auto &element : vec) { directorySize += element.statEl.st_size; } } else { - const ssize_t readErr = NEO::SysCalls::pread(std::get(fd), &directorySize, sizeof(directorySize), 0); + ssize_t readErr = NEO::SysCalls::pread(std::get(fd), &directorySize, sizeof(directorySize), 0); if (readErr < 0) { directorySize = 0; @@ -202,22 +202,8 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, } } -class HandleGuard { - public: - HandleGuard() = delete; - explicit HandleGuard(int &fileDescriptor) : fd(fileDescriptor) {} - ~HandleGuard() { - if (fd != -1) { - unlockFileAndClose(fd); - } - } - - private: - int fd = -1; -}; - bool CompilerCache::cacheBinary(const std::string &kernelFileHash, const char *pBinary, size_t binarySize) { - if (pBinary == nullptr || binarySize == 0 || binarySize > config.cacheSize) { + if (pBinary == nullptr || binarySize == 0) { return false; } @@ -225,7 +211,7 @@ bool CompilerCache::cacheBinary(const std::string &kernelFileHash, const char *p constexpr std::string_view configFileName = "config.file"; std::string configFilePath = joinPath(config.cacheDir, configFileName.data()); - std::string cacheFilePath = joinPath(config.cacheDir, kernelFileHash + config.cacheFileExtension); + std::string filePath = joinPath(config.cacheDir, kernelFileHash + config.cacheFileExtension); UnifiedHandle fd{-1}; size_t directorySize = 0u; @@ -236,37 +222,33 @@ bool CompilerCache::cacheBinary(const std::string &kernelFileHash, const char *p return false; } - HandleGuard configGuard(std::get(fd)); - struct stat statbuf = {}; - if (NEO::SysCalls::stat(cacheFilePath, &statbuf) == 0) { + if (NEO::SysCalls::stat(filePath, &statbuf) == 0) { + unlockFileAndClose(std::get(fd)); return true; } - const size_t maxSize = config.cacheSize; - if (maxSize < (directorySize + binarySize)) { + size_t maxSize = config.cacheSize; + + if (maxSize < directorySize + binarySize) { uint64_t bytesEvicted{0u}; - const auto evictSuccess = evictCache(bytesEvicted); - const auto availableSpace = maxSize - directorySize + bytesEvicted; - - directorySize = std::max(0, directorySize - bytesEvicted); - - if (!evictSuccess || binarySize > availableSpace) { - if (bytesEvicted > 0) { - NEO::SysCalls::pwrite(std::get(fd), &directorySize, sizeof(directorySize), 0); - } + if (!evictCache(bytesEvicted)) { + unlockFileAndClose(std::get(fd)); return false; } } std::string tmpFileName = "cl_cache.XXXXXX"; + std::string tmpFilePath = joinPath(config.cacheDir, tmpFileName); if (!createUniqueTempFileAndWriteData(tmpFilePath.data(), pBinary, binarySize)) { + unlockFileAndClose(std::get(fd)); return false; } - if (!renameTempFileBinaryToProperName(tmpFilePath, cacheFilePath)) { + if (!renameTempFileBinaryToProperName(tmpFilePath, filePath)) { + unlockFileAndClose(std::get(fd)); return false; } @@ -274,6 +256,8 @@ bool CompilerCache::cacheBinary(const std::string &kernelFileHash, const char *p NEO::SysCalls::pwrite(std::get(fd), &directorySize, sizeof(directorySize), 0); + unlockFileAndClose(std::get(fd)); + return true; } diff --git a/shared/test/unit_test/compiler_interface/linux/compiler_cache_tests_linux.cpp b/shared/test/unit_test/compiler_interface/linux/compiler_cache_tests_linux.cpp index 39b350b119..a522c4d4c7 100644 --- a/shared/test/unit_test/compiler_interface/linux/compiler_cache_tests_linux.cpp +++ b/shared/test/unit_test/compiler_interface/linux/compiler_cache_tests_linux.cpp @@ -428,189 +428,9 @@ TEST(CompilerCacheTests, GivenCompilerCacheWhenLockConfigFileThenFdIsSetProperSi EXPECT_EQ(directory, LockConfigFileAndConfigFileIsCreatedInMeantime::configSize); } -TEST(CompilerCacheTests, GivenCacheBinaryWhenBinarySizeIsOverCacheLimitThenEarlyReturnFalse) { - const size_t cacheSize = MemoryConstants::megaByte; - CompilerCacheMockLinux cache({true, ".cl_cache", "/home/cl_cache/", cacheSize}); - - auto result = cache.cacheBinary("fileHash", "123456", cacheSize * 2); - EXPECT_FALSE(result); -} - -namespace PWriteCallsCountedAndDirSizeWritten { -size_t pWriteCalled = 0u; -size_t dirSize = 0u; -decltype(NEO::SysCalls::sysCallsPwrite) mockPwrite = [](int fd, const void *buf, size_t count, off_t offset) -> ssize_t { - pWriteCalled++; - memcpy(&dirSize, buf, sizeof(dirSize)); - return 0; -}; -} // namespace PWriteCallsCountedAndDirSizeWritten - -class CompilerCacheEvictionTestsMockLinux : public CompilerCache { +class CompilerCacheFailingLokcConfigFileAndReadSizeLinux : public CompilerCache { public: - CompilerCacheEvictionTestsMockLinux(const CompilerCacheConfig &config) : CompilerCache(config) {} - using CompilerCache::createUniqueTempFileAndWriteData; - using CompilerCache::evictCache; - using CompilerCache::lockConfigFileAndReadSize; - using CompilerCache::renameTempFileBinaryToProperName; - - bool createUniqueTempFileAndWriteData(char *tmpFilePathTemplate, const char *pBinary, size_t binarySize) override { - createUniqueTempFileAndWriteDataCalled++; - return createUniqueTempFileAndWriteDataResult; - } - size_t createUniqueTempFileAndWriteDataCalled = 0u; - bool createUniqueTempFileAndWriteDataResult = true; - - bool renameTempFileBinaryToProperName(const std::string &oldName, const std::string &kernelFileHash) override { - renameTempFileBinaryToProperNameCalled++; - return renameTempFileBinaryToProperNameResult; - } - size_t renameTempFileBinaryToProperNameCalled = 0u; - bool renameTempFileBinaryToProperNameResult = true; - - bool evictCache(uint64_t &bytesEvicted) override { - bytesEvicted = evictCacheBytesEvicted; - return evictCacheResult; - } - uint64_t evictCacheBytesEvicted = 0u; - bool evictCacheResult = true; - - void lockConfigFileAndReadSize(const std::string &configFilePath, UnifiedHandle &fd, size_t &directorySize) override { - lockConfigFileAndReadSizeCalled++; - std::get(fd) = lockConfigFileAndReadSizeFd; - directorySize = lockConfigFileAndReadSizeDirSize; - return; - } - size_t lockConfigFileAndReadSizeCalled = 0u; - int lockConfigFileAndReadSizeFd = -1; - size_t lockConfigFileAndReadSizeDirSize = 0u; -}; - -TEST(CompilerCacheTests, GivenCacheDirectoryFilledToTheLimitWhenNewBinaryFitsAfterEvictionThenWriteCacheAndUpdateConfigAndReturnTrue) { - const size_t cacheSize = 10; - CompilerCacheEvictionTestsMockLinux cache({true, ".cl_cache", "/home/cl_cache/", cacheSize}); - - cache.lockConfigFileAndReadSizeFd = 1; - cache.lockConfigFileAndReadSizeDirSize = 6; - - VariableBackup statBackup(&NEO::SysCalls::sysCallsStat, [](const std::string &filePath, struct stat *statbuf) -> int { return -1; }); - - cache.evictCacheResult = true; - cache.evictCacheBytesEvicted = cacheSize / 3; - - cache.createUniqueTempFileAndWriteDataResult = true; - cache.renameTempFileBinaryToProperNameResult = true; - - VariableBackup pWriteBackup(&NEO::SysCalls::sysCallsPwrite, PWriteCallsCountedAndDirSizeWritten::mockPwrite); - VariableBackup pWriteCalledBackup(&PWriteCallsCountedAndDirSizeWritten::pWriteCalled, 0); - VariableBackup dirSizeBackup(&PWriteCallsCountedAndDirSizeWritten::dirSize, 0); - - const std::string kernelFileHash = "7e3291364d8df42"; - const char *binary = "123456"; - const size_t binarySize = strlen(binary); - auto result = cache.cacheBinary(kernelFileHash, binary, binarySize); - - const size_t expectedDirectorySize = 6 - (cacheSize / 3) + binarySize; - - EXPECT_TRUE(result); - EXPECT_EQ(1u, cache.lockConfigFileAndReadSizeCalled); - EXPECT_EQ(1u, cache.createUniqueTempFileAndWriteDataCalled); - EXPECT_EQ(1u, cache.renameTempFileBinaryToProperNameCalled); - EXPECT_EQ(1u, PWriteCallsCountedAndDirSizeWritten::pWriteCalled); - - EXPECT_EQ(expectedDirectorySize, PWriteCallsCountedAndDirSizeWritten::dirSize); -} - -TEST(CompilerCacheTests, GivenCacheBinaryWhenBinaryDoesntFitAfterEvictionThenWriteToConfigAndReturnFalse) { - const size_t cacheSize = 10; - CompilerCacheEvictionTestsMockLinux cache({true, ".cl_cache", "/home/cl_cache/", cacheSize}); - cache.lockConfigFileAndReadSizeFd = 1; - cache.lockConfigFileAndReadSizeDirSize = 9; - - VariableBackup statBackup(&NEO::SysCalls::sysCallsStat, [](const std::string &filePath, struct stat *statbuf) -> int { return -1; }); - - cache.evictCacheResult = true; - cache.evictCacheBytesEvicted = cacheSize / 3; - - VariableBackup pWriteBackup(&NEO::SysCalls::sysCallsPwrite, PWriteCallsCountedAndDirSizeWritten::mockPwrite); - VariableBackup pWriteCalledBackup(&PWriteCallsCountedAndDirSizeWritten::pWriteCalled, 0); - VariableBackup dirSizeBackup(&PWriteCallsCountedAndDirSizeWritten::dirSize, 0); - - const std::string kernelFileHash = "7e3291364d8df42"; - const char *binary = "123456"; - const size_t binarySize = strlen(binary); - auto result = cache.cacheBinary(kernelFileHash, binary, binarySize); - - const size_t expectedDirectorySize = 9 - (cacheSize / 3); - - EXPECT_FALSE(result); - EXPECT_EQ(1u, cache.lockConfigFileAndReadSizeCalled); - EXPECT_EQ(1u, PWriteCallsCountedAndDirSizeWritten::pWriteCalled); - - EXPECT_EQ(0u, cache.createUniqueTempFileAndWriteDataCalled); - EXPECT_EQ(0u, cache.renameTempFileBinaryToProperNameCalled); - - EXPECT_EQ(expectedDirectorySize, PWriteCallsCountedAndDirSizeWritten::dirSize); -} - -TEST(CompilerCacheTests, GivenCacheDirectoryFilledToTheLimitWhenNoBytesHaveBeenEvictedAndNewBinaryDoesntFitAfterEvictionThenDontWriteToConfigAndReturnFalse) { - const size_t cacheSize = 10; - CompilerCacheEvictionTestsMockLinux cache({true, ".cl_cache", "/home/cl_cache/", cacheSize}); - cache.lockConfigFileAndReadSizeFd = 1; - cache.lockConfigFileAndReadSizeDirSize = 9; - - VariableBackup statBackup(&NEO::SysCalls::sysCallsStat, [](const std::string &filePath, struct stat *statbuf) -> int { return -1; }); - - cache.evictCacheResult = true; - cache.evictCacheBytesEvicted = 0; - - VariableBackup pWriteBackup(&NEO::SysCalls::sysCallsPwrite, PWriteCallsCountedAndDirSizeWritten::mockPwrite); - VariableBackup pWriteCalledBackup(&PWriteCallsCountedAndDirSizeWritten::pWriteCalled, 0); - VariableBackup dirSizeBackup(&PWriteCallsCountedAndDirSizeWritten::dirSize, 0); - - const std::string kernelFileHash = "7e3291364d8df42"; - const char *binary = "123456"; - const size_t binarySize = strlen(binary); - auto result = cache.cacheBinary(kernelFileHash, binary, binarySize); - - EXPECT_FALSE(result); - EXPECT_EQ(1u, cache.lockConfigFileAndReadSizeCalled); - - EXPECT_EQ(0u, PWriteCallsCountedAndDirSizeWritten::pWriteCalled); - EXPECT_EQ(0u, cache.createUniqueTempFileAndWriteDataCalled); - EXPECT_EQ(0u, cache.renameTempFileBinaryToProperNameCalled); -} - -TEST(CompilerCacheTests, GivenCacheBinaryWhenEvictCacheFailsThenReturnFalse) { - const size_t cacheSize = 10; - CompilerCacheEvictionTestsMockLinux cache({true, ".cl_cache", "/home/cl_cache/", cacheSize}); - cache.lockConfigFileAndReadSizeFd = 1; - cache.lockConfigFileAndReadSizeDirSize = 5; - - VariableBackup statBackup(&NEO::SysCalls::sysCallsStat, [](const std::string &filePath, struct stat *statbuf) -> int { return -1; }); - - cache.evictCacheResult = false; - - VariableBackup pWriteBackup(&NEO::SysCalls::sysCallsPwrite, PWriteCallsCountedAndDirSizeWritten::mockPwrite); - VariableBackup pWriteCalledBackup(&PWriteCallsCountedAndDirSizeWritten::pWriteCalled, 0); - VariableBackup dirSizeBackup(&PWriteCallsCountedAndDirSizeWritten::dirSize, 0); - - const std::string kernelFileHash = "7e3291364d8df42"; - const char *binary = "123456"; - const size_t binarySize = strlen(binary); - auto result = cache.cacheBinary(kernelFileHash, binary, binarySize); - - EXPECT_FALSE(result); - EXPECT_EQ(1u, cache.lockConfigFileAndReadSizeCalled); - - EXPECT_EQ(0u, PWriteCallsCountedAndDirSizeWritten::pWriteCalled); - EXPECT_EQ(0u, cache.createUniqueTempFileAndWriteDataCalled); - EXPECT_EQ(0u, cache.renameTempFileBinaryToProperNameCalled); -} - -class CompilerCacheFailingLockConfigFileAndReadSizeLinux : public CompilerCache { - public: - CompilerCacheFailingLockConfigFileAndReadSizeLinux(const CompilerCacheConfig &config) : CompilerCache(config) {} + CompilerCacheFailingLokcConfigFileAndReadSizeLinux(const CompilerCacheConfig &config) : CompilerCache(config) {} using CompilerCache::createUniqueTempFileAndWriteData; using CompilerCache::evictCache; using CompilerCache::lockConfigFileAndReadSize; @@ -623,7 +443,7 @@ class CompilerCacheFailingLockConfigFileAndReadSizeLinux : public CompilerCache }; TEST(CompilerCacheTests, GivenCompilerCacheWhenLockConfigFileFailThenCacheBinaryReturnsFalse) { - CompilerCacheFailingLockConfigFileAndReadSizeLinux cache({true, ".cl_cache", "/home/cl_cache/", MemoryConstants::megaByte}); + CompilerCacheFailingLokcConfigFileAndReadSizeLinux cache({true, ".cl_cache", "/home/cl_cache/", MemoryConstants::megaByte}); EXPECT_FALSE(cache.cacheBinary("config.file", "1", 1)); }