From 2aa0ae0ecab34e0f1b41d06bf49268c50db0f9fc Mon Sep 17 00:00:00 2001 From: Fabian Zwolinski Date: Thu, 16 Nov 2023 14:24:52 +0000 Subject: [PATCH] refactor: linux cl_cache eviction mechanism Refactored eviction mechanism works as follows: - eviction is needed only if total size of cache binaries + size of the new binary exceed cache limit - single evition call removes files with a summed size of 1/3 of the cache limit - if new binary can not fit in the cache size limit even after eviction, it will not be saved - cache limit applies only to files in cache directory with .cl_cache/.l0_cache extension. Only these files are counted and only these files are removed Minor: - rename variables for better readability - add `const` where possible Related-To: NEO-4262 Signed-off-by: Fabian Zwolinski --- .../linux/compiler_cache_linux.cpp | 96 +++++---- .../linux/compiler_cache_tests_linux.cpp | 186 +++++++++++++++++- 2 files changed, 239 insertions(+), 43 deletions(-) diff --git a/shared/source/compiler_interface/linux/compiler_cache_linux.cpp b/shared/source/compiler_interface/linux/compiler_cache_linux.cpp index 4d197f8697..f800ba25a1 100644 --- a/shared/source/compiler_interface/linux/compiler_cache_linux.cpp +++ b/shared/source/compiler_interface/linux/compiler_cache_linux.cpp @@ -31,7 +31,8 @@ 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; } @@ -50,38 +51,39 @@ bool compareByLastAccessTime(const ElementsStruct &a, ElementsStruct &b) { bool CompilerCache::evictCache(uint64_t &bytesEvicted) { struct dirent **files = 0; - int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); + const 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 vec; - vec.reserve(static_cast(filesCount)); + std::vector cacheFiles; + cacheFiles.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) { - vec.push_back(std::move(fileElement)); + cacheFiles.push_back(std::move(fileElement)); } - } - - for (int i = 0; i < filesCount; ++i) { free(files[i]); } + free(files); - std::sort(vec.begin(), vec.end(), compareByLastAccessTime); + std::sort(cacheFiles.begin(), cacheFiles.end(), compareByLastAccessTime); - size_t evictionLimit = config.cacheSize / 3; + bytesEvicted = 0; + const auto evictionLimit = config.cacheSize / 3; - 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; + for (const auto &file : cacheFiles) { + auto res = NEO::SysCalls::unlink(file.path); + if (res == -1) { + continue; + } - if (evictionSizeCount > evictionLimit) { + bytesEvicted += file.statEl.st_size; + if (bytesEvicted > evictionLimit) { return true; } } @@ -146,7 +148,7 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, } } - int lockErr = NEO::SysCalls::flock(std::get(fd), LOCK_EX); + const 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); @@ -159,7 +161,7 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, if (countDirectorySize) { struct dirent **files = {}; - int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); + const int filesCount = NEO::SysCalls::scandir(config.cacheDir.c_str(), &files, filterFunction, NULL); if (filesCount == -1) { unlockFileAndClose(std::get(fd)); @@ -168,30 +170,28 @@ void CompilerCache::lockConfigFileAndReadSize(const std::string &configFilePath, return; } - std::vector vec; - vec.reserve(static_cast(filesCount)); + std::vector cacheFiles; + cacheFiles.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) { - vec.push_back(std::move(fileElement)); + cacheFiles.push_back(std::move(fileElement)); } } - } - - for (int i = 0; i < filesCount; ++i) { free(files[i]); } + free(files); - for (auto &element : vec) { + for (const auto &element : cacheFiles) { directorySize += element.statEl.st_size; } } else { - ssize_t readErr = NEO::SysCalls::pread(std::get(fd), &directorySize, sizeof(directorySize), 0); + const ssize_t readErr = NEO::SysCalls::pread(std::get(fd), &directorySize, sizeof(directorySize), 0); if (readErr < 0) { directorySize = 0; @@ -202,8 +202,22 @@ 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) { + if (pBinary == nullptr || binarySize == 0 || binarySize > config.cacheSize) { return false; } @@ -211,7 +225,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 filePath = joinPath(config.cacheDir, kernelFileHash + config.cacheFileExtension); + std::string cacheFilePath = joinPath(config.cacheDir, kernelFileHash + config.cacheFileExtension); UnifiedHandle fd{-1}; size_t directorySize = 0u; @@ -222,33 +236,37 @@ bool CompilerCache::cacheBinary(const std::string &kernelFileHash, const char *p return false; } + HandleGuard configGuard(std::get(fd)); + struct stat statbuf = {}; - if (NEO::SysCalls::stat(filePath, &statbuf) == 0) { - unlockFileAndClose(std::get(fd)); + if (NEO::SysCalls::stat(cacheFilePath, &statbuf) == 0) { return true; } - size_t maxSize = config.cacheSize; - - if (maxSize < directorySize + binarySize) { + const size_t maxSize = config.cacheSize; + if (maxSize < (directorySize + binarySize)) { uint64_t bytesEvicted{0u}; - if (!evictCache(bytesEvicted)) { - unlockFileAndClose(std::get(fd)); + 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); + } 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, filePath)) { - unlockFileAndClose(std::get(fd)); + if (!renameTempFileBinaryToProperName(tmpFilePath, cacheFilePath)) { return false; } @@ -256,8 +274,6 @@ 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 a522c4d4c7..39b350b119 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,9 +428,189 @@ TEST(CompilerCacheTests, GivenCompilerCacheWhenLockConfigFileThenFdIsSetProperSi EXPECT_EQ(directory, LockConfigFileAndConfigFileIsCreatedInMeantime::configSize); } -class CompilerCacheFailingLokcConfigFileAndReadSizeLinux : public CompilerCache { +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 { public: - CompilerCacheFailingLokcConfigFileAndReadSizeLinux(const CompilerCacheConfig &config) : CompilerCache(config) {} + 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) {} using CompilerCache::createUniqueTempFileAndWriteData; using CompilerCache::evictCache; using CompilerCache::lockConfigFileAndReadSize; @@ -443,7 +623,7 @@ class CompilerCacheFailingLokcConfigFileAndReadSizeLinux : public CompilerCache }; TEST(CompilerCacheTests, GivenCompilerCacheWhenLockConfigFileFailThenCacheBinaryReturnsFalse) { - CompilerCacheFailingLokcConfigFileAndReadSizeLinux cache({true, ".cl_cache", "/home/cl_cache/", MemoryConstants::megaByte}); + CompilerCacheFailingLockConfigFileAndReadSizeLinux cache({true, ".cl_cache", "/home/cl_cache/", MemoryConstants::megaByte}); EXPECT_FALSE(cache.cacheBinary("config.file", "1", 1)); }