https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/129751
>From 5f119a3a4ae8642d6ab0498c1cf6b39d5099c835 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Thu, 27 Feb 2025 15:23:18 -0800 Subject: [PATCH 1/5] [clang][modules][deps] Add mutex as an alternative to file lock --- .../include/clang/Frontend/CompilerInstance.h | 13 ++- .../clang/Serialization/ModuleCacheLock.h | 31 +++++++ .../DependencyScanningService.h | 7 ++ .../DependencyScanning/ModuleCacheMutexLock.h | 31 +++++++ clang/lib/Frontend/CompilerInstance.cpp | 47 ++++++----- clang/lib/Serialization/CMakeLists.txt | 1 + clang/lib/Serialization/ModuleCacheLock.cpp | 60 ++++++++++++++ .../Tooling/DependencyScanning/CMakeLists.txt | 1 + .../DependencyScanningWorker.cpp | 7 +- .../ModuleCacheMutexLock.cpp | 81 +++++++++++++++++++ 10 files changed, 254 insertions(+), 25 deletions(-) create mode 100644 clang/include/clang/Serialization/ModuleCacheLock.h create mode 100644 clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h create mode 100644 clang/lib/Serialization/ModuleCacheLock.cpp create mode 100644 clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 8b539dfc92960..bede160991443 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -53,6 +53,7 @@ class FileManager; class FrontendAction; class InMemoryModuleCache; class Module; +class ModuleCacheLock; class Preprocessor; class Sema; class SourceManager; @@ -96,9 +97,12 @@ class CompilerInstance : public ModuleLoader { /// The source manager. IntrusiveRefCntPtr<SourceManager> SourceMgr; - /// The cache of PCM files. + /// The local in-memory cache of PCM files. IntrusiveRefCntPtr<InMemoryModuleCache> ModuleCache; + /// The lock managing the global cache of PCM files. + std::shared_ptr<ModuleCacheLock> ModuleCacheLck; + /// The preprocessor. std::shared_ptr<Preprocessor> PP; @@ -209,7 +213,8 @@ class CompilerInstance : public ModuleLoader { explicit CompilerInstance( std::shared_ptr<PCHContainerOperations> PCHContainerOps = std::make_shared<PCHContainerOperations>(), - InMemoryModuleCache *SharedModuleCache = nullptr); + InMemoryModuleCache *SharedModuleCache = nullptr, + std::shared_ptr<ModuleCacheLock> ModuleCacheLck = nullptr); ~CompilerInstance() override; /// @name High-Level Operations @@ -897,6 +902,10 @@ class CompilerInstance : public ModuleLoader { void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS); InMemoryModuleCache &getModuleCache() const { return *ModuleCache; } + + std::shared_ptr<ModuleCacheLock> getModuleCacheLockPtr() const { + return ModuleCacheLck; + } }; } // end namespace clang diff --git a/clang/include/clang/Serialization/ModuleCacheLock.h b/clang/include/clang/Serialization/ModuleCacheLock.h new file mode 100644 index 0000000000000..9435735b1a1bd --- /dev/null +++ b/clang/include/clang/Serialization/ModuleCacheLock.h @@ -0,0 +1,31 @@ +#ifndef LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H +#define LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H + +#include "clang/Basic/LLVM.h" +#include "llvm/Support/LockFileManager.h" + +namespace clang { +enum class LockResult { Owned, Shared, Error }; +enum class WaitForUnlockResult { Success, OwnerDied, Timeout }; + +class ModuleCacheLockManager { +public: + virtual operator LockResult() const = 0; + virtual WaitForUnlockResult waitForUnlock() = 0; + virtual void unsafeRemoveLock() = 0; + virtual std::string getErrorMessage() const = 0; + virtual ~ModuleCacheLockManager() = default; +}; + +class ModuleCacheLock { +public: + virtual void prepareLock(StringRef ModuleFilename) = 0; + virtual std::unique_ptr<ModuleCacheLockManager> + tryLock(StringRef ModuleFilename) = 0; + virtual ~ModuleCacheLock() = default; +}; + +std::shared_ptr<ModuleCacheLock> getModuleCacheFileLock(); +} // namespace clang + +#endif diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h index f002f8645d3f6..f6914b90ac67e 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H +#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h" #include "llvm/ADT/BitmaskEnum.h" @@ -95,6 +96,10 @@ class DependencyScanningService { return SharedCache; } + ModuleCacheMutexes &getSharedModuleCacheMutexes() { + return ModuleCacheMutexes; + } + private: const ScanningMode Mode; const ScanningOutputFormat Format; @@ -106,6 +111,8 @@ class DependencyScanningService { const bool TraceVFS; /// The global file system cache. DependencyScanningFilesystemSharedCache SharedCache; + /// The global module cache mutexes. + ModuleCacheMutexes ModuleCacheMutexes; }; } // end namespace dependencies diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h new file mode 100644 index 0000000000000..312972b26702c --- /dev/null +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h @@ -0,0 +1,31 @@ +#ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H +#define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H + +#include "clang/Serialization/ModuleCacheLock.h" +#include "llvm/ADT/StringMap.h" + +#include <condition_variable> + +namespace clang { +namespace tooling { +namespace dependencies { +struct ModuleCacheMutexWrapper { + std::mutex Mutex; + std::condition_variable CondVar; + bool Done = false; + + ModuleCacheMutexWrapper() = default; +}; + +struct ModuleCacheMutexes { + std::mutex Mutex; + llvm::StringMap<std::shared_ptr<ModuleCacheMutexWrapper>> Map; +}; + +std::shared_ptr<ModuleCacheLock> +getModuleCacheMutexLock(ModuleCacheMutexes &Mutexes); +} // namespace dependencies +} // namespace tooling +} // namespace clang + +#endif diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index c11c857ea0606..3d02d42032e21 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -39,6 +39,7 @@ #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/GlobalModuleIndex.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "clang/Serialization/ModuleCacheLock.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" @@ -66,11 +67,14 @@ using namespace clang; CompilerInstance::CompilerInstance( std::shared_ptr<PCHContainerOperations> PCHContainerOps, - InMemoryModuleCache *SharedModuleCache) + InMemoryModuleCache *SharedModuleCache, + std::shared_ptr<ModuleCacheLock> ModuleCacheLck) : ModuleLoader(/* BuildingModule = */ SharedModuleCache), Invocation(new CompilerInvocation()), ModuleCache(SharedModuleCache ? SharedModuleCache : new InMemoryModuleCache), + ModuleCacheLck(ModuleCacheLck ? ModuleCacheLck + : getModuleCacheFileLock()), ThePCHContainerOperations(std::move(PCHContainerOps)) {} CompilerInstance::~CompilerInstance() { @@ -1227,7 +1231,8 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, // CompilerInstance::CompilerInstance is responsible for finalizing the // buffers to prevent use-after-frees. CompilerInstance Instance(ImportingInstance.getPCHContainerOperations(), - &ImportingInstance.getModuleCache()); + &ImportingInstance.getModuleCache(), + ImportingInstance.getModuleCacheLockPtr()); auto &Inv = *Invocation; Instance.setInvocation(std::move(Invocation)); @@ -1471,47 +1476,45 @@ static bool compileModuleAndReadASTBehindLock( Diags.Report(ModuleNameLoc, diag::remark_module_lock) << ModuleFileName << Module->Name; - // FIXME: have LockFileManager return an error_code so that we can - // avoid the mkdir when the directory already exists. - StringRef Dir = llvm::sys::path::parent_path(ModuleFileName); - llvm::sys::fs::create_directories(Dir); + auto Lock = ImportingInstance.getModuleCacheLockPtr(); + Lock->prepareLock(ModuleFileName); while (true) { - llvm::LockFileManager Locked(ModuleFileName); - switch (Locked) { - case llvm::LockFileManager::LFS_Error: + auto Locked = Lock->tryLock(ModuleFileName); + switch (*Locked) { + case LockResult::Error: // ModuleCache takes care of correctness and locks are only necessary for // performance. Fallback to building the module in case of any lock // related errors. Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure) - << Module->Name << Locked.getErrorMessage(); + << Module->Name << Locked->getErrorMessage(); // Clear out any potential leftover. - Locked.unsafeRemoveLockFile(); + Locked->unsafeRemoveLock(); [[fallthrough]]; - case llvm::LockFileManager::LFS_Owned: + case LockResult::Owned: // We're responsible for building the module ourselves. return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc, ModuleNameLoc, Module, ModuleFileName); - case llvm::LockFileManager::LFS_Shared: + case LockResult::Shared: break; // The interesting case. } // Someone else is responsible for building the module. Wait for them to // finish. - switch (Locked.waitForUnlock()) { - case llvm::LockFileManager::Res_Success: + switch (Locked->waitForUnlock()) { + case WaitForUnlockResult::Success: break; // The interesting case. - case llvm::LockFileManager::Res_OwnerDied: + case WaitForUnlockResult::OwnerDied: continue; // try again to get the lock. - case llvm::LockFileManager::Res_Timeout: - // Since ModuleCache takes care of correctness, we try waiting for - // another process to complete the build so clang does not do it done - // twice. If case of timeout, build it ourselves. + case WaitForUnlockResult::Timeout: + // Since the InMemoryModuleCache takes care of correctness, we try waiting + // for someone else to complete the build so that it does not happen + // twice. In case of timeout, build it ourselves. Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout) << Module->Name; - // Clear the lock file so that future invocations can make progress. - Locked.unsafeRemoveLockFile(); + // Remove the lock so that future invocations can make progress. + Locked->unsafeRemoveLock(); continue; } diff --git a/clang/lib/Serialization/CMakeLists.txt b/clang/lib/Serialization/CMakeLists.txt index b1fc0345047f2..acbd8669caaed 100644 --- a/clang/lib/Serialization/CMakeLists.txt +++ b/clang/lib/Serialization/CMakeLists.txt @@ -18,6 +18,7 @@ add_clang_library(clangSerialization GeneratePCH.cpp GlobalModuleIndex.cpp InMemoryModuleCache.cpp + ModuleCacheLock.cpp ModuleFile.cpp ModuleFileExtension.cpp ModuleManager.cpp diff --git a/clang/lib/Serialization/ModuleCacheLock.cpp b/clang/lib/Serialization/ModuleCacheLock.cpp new file mode 100644 index 0000000000000..0c59ed7f28b98 --- /dev/null +++ b/clang/lib/Serialization/ModuleCacheLock.cpp @@ -0,0 +1,60 @@ +#include "clang/Serialization/ModuleCacheLock.h" + +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" + +using namespace clang; + +namespace { +struct ModuleCacheFileLockManager : ModuleCacheLockManager { + llvm::LockFileManager Lock; + + ModuleCacheFileLockManager(StringRef ModuleFilename) : Lock(ModuleFilename) {} + + operator LockResult() const override { + switch (Lock) { + case llvm::LockFileManager::LFS_Owned: + return LockResult::Owned; + case llvm::LockFileManager::LFS_Shared: + return LockResult::Shared; + case llvm::LockFileManager::LFS_Error: + return LockResult::Error; + } + } + + WaitForUnlockResult waitForUnlock() override { + switch (Lock.waitForUnlock()) { + case llvm::LockFileManager::Res_Success: + return WaitForUnlockResult::Success; + case llvm::LockFileManager::Res_OwnerDied: + return WaitForUnlockResult::OwnerDied; + case llvm::LockFileManager::Res_Timeout: + return WaitForUnlockResult::Timeout; + } + } + + void unsafeRemoveLock() override { Lock.unsafeRemoveLockFile(); } + + std::string getErrorMessage() const override { + return Lock.getErrorMessage(); + } +}; + +struct ModuleCacheFileLock : ModuleCacheLock { + void prepareLock(StringRef ModuleFilename) override { + // FIXME: have LockFileManager return an error_code so that we can + // avoid the mkdir when the directory already exists. + StringRef Dir = llvm::sys::path::parent_path(ModuleFilename); + llvm::sys::fs::create_directories(Dir); + } + + std::unique_ptr<ModuleCacheLockManager> + tryLock(StringRef ModuleFilename) override { + return std::make_unique<ModuleCacheFileLockManager>(ModuleFilename); + } +}; +} // namespace + +std::shared_ptr<ModuleCacheLock> clang::getModuleCacheFileLock() { + return std::make_unique<ModuleCacheFileLock>(); +} diff --git a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt index 66795b0be0baa..7ae35e69d7000 100644 --- a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt +++ b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt @@ -11,6 +11,7 @@ add_clang_library(clangDependencyScanning DependencyScanningService.cpp DependencyScanningWorker.cpp DependencyScanningTool.cpp + ModuleCacheMutexLock.cpp ModuleDepCollector.cpp DEPENDS diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 697f26ee5d12f..fdc86d1764eea 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -22,6 +22,7 @@ #include "clang/Lex/PreprocessorOptions.h" #include "clang/Serialization/ObjectFilePCHContainerReader.h" #include "clang/Tooling/DependencyScanning/DependencyScanningService.h" +#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" @@ -315,7 +316,10 @@ class DependencyScanningAction : public tooling::ToolAction { Scanned = true; // Create a compiler instance to handle the actual work. - ScanInstanceStorage.emplace(std::move(PCHContainerOps)); + ModuleCacheLck = + getModuleCacheMutexLock(Service.getSharedModuleCacheMutexes()); + ScanInstanceStorage.emplace(std::move(PCHContainerOps), nullptr, + ModuleCacheLck); CompilerInstance &ScanInstance = *ScanInstanceStorage; ScanInstance.setInvocation(std::move(Invocation)); @@ -479,6 +483,7 @@ class DependencyScanningAction : public tooling::ToolAction { llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS; bool DisableFree; std::optional<StringRef> ModuleName; + std::shared_ptr<ModuleCacheLock> ModuleCacheLck; std::optional<CompilerInstance> ScanInstanceStorage; std::shared_ptr<ModuleDepCollector> MDC; std::vector<std::string> LastCC1Arguments; diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp new file mode 100644 index 0000000000000..162b5a4b213f5 --- /dev/null +++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp @@ -0,0 +1,81 @@ +#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" + +using namespace clang; +using namespace tooling; +using namespace dependencies; + +namespace { +struct ModuleCacheMutexLockManager : ModuleCacheLockManager { + ModuleCacheMutexes &Mutexes; + StringRef Filename; + + std::shared_ptr<ModuleCacheMutexWrapper> MutexWrapper; + bool Owning; + + ModuleCacheMutexLockManager(ModuleCacheMutexes &Mutexes, StringRef Filename) + : Mutexes(Mutexes), Filename(Filename) { + Owning = false; + { + std::lock_guard Lock(Mutexes.Mutex); + auto &MutexWrapperInMap = Mutexes.Map[Filename]; + if (!MutexWrapperInMap) { + MutexWrapperInMap = std::make_shared<ModuleCacheMutexWrapper>(); + Owning = true; + } + // Increment the reference count of the mutex here in the critical section + // to guarantee that it's kept alive even when another thread removes it + // via \c unsafeRemoveLock(). + MutexWrapper = MutexWrapperInMap; + } + + if (Owning) + MutexWrapper->Mutex.lock(); + } + + ~ModuleCacheMutexLockManager() override { + if (Owning) { + MutexWrapper->Done = true; + MutexWrapper->CondVar.notify_all(); + MutexWrapper->Mutex.unlock(); + } + } + + operator LockResult() const override { + return Owning ? LockResult::Owned : LockResult::Shared; + } + + WaitForUnlockResult waitForUnlock() override { + assert(!Owning); + std::unique_lock Lock(MutexWrapper->Mutex); + bool Done = MutexWrapper->CondVar.wait_for( + Lock, std::chrono::seconds(90), [&] { return MutexWrapper->Done; }); + return Done ? WaitForUnlockResult::Success : WaitForUnlockResult::Timeout; + } + + void unsafeRemoveLock() override { + std::lock_guard Lock(Mutexes.Mutex); + Mutexes.Map[Filename].reset(); + } + + std::string getErrorMessage() const override { + llvm_unreachable("ModuleCacheMutexLockManager cannot fail"); + } +}; + +struct ModuleCacheMutexLock : ModuleCacheLock { + ModuleCacheMutexes &Mutexes; + + ModuleCacheMutexLock(ModuleCacheMutexes &Mutexes) : Mutexes(Mutexes) {} + + void prepareLock(StringRef Filename) override {} + + std::unique_ptr<ModuleCacheLockManager> tryLock(StringRef Filename) override { + return std::make_unique<ModuleCacheMutexLockManager>(Mutexes, Filename); + } +}; +} // namespace + +std::shared_ptr<ModuleCacheLock> +dependencies::getModuleCacheMutexLock(ModuleCacheMutexes &Mutexes) { + return std::make_shared<ModuleCacheMutexLock>(Mutexes); +} >From 1ac1f238e144ca4677af80c30073ffc56daebca7 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Tue, 4 Mar 2025 09:47:46 -0800 Subject: [PATCH 2/5] clang-format --- .../Tooling/DependencyScanning/DependencyScanningService.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h index f6914b90ac67e..d4e4dba8058f1 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h @@ -9,8 +9,8 @@ #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H -#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h" +#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" #include "llvm/ADT/BitmaskEnum.h" namespace clang { >From 3e46954d2984083273f8c694fa86c37d7b289711 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Wed, 5 Mar 2025 14:27:50 -0800 Subject: [PATCH 3/5] Use `std::shared_mutex`, remove timeouts, remove support for `unsafeRemoveLock()` --- .../DependencyScanning/ModuleCacheMutexLock.h | 12 +--- .../ModuleCacheMutexLock.cpp | 57 ++++++------------- 2 files changed, 18 insertions(+), 51 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h index 312972b26702c..0e7f9031f153b 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h @@ -4,22 +4,14 @@ #include "clang/Serialization/ModuleCacheLock.h" #include "llvm/ADT/StringMap.h" -#include <condition_variable> +#include <shared_mutex> namespace clang { namespace tooling { namespace dependencies { -struct ModuleCacheMutexWrapper { - std::mutex Mutex; - std::condition_variable CondVar; - bool Done = false; - - ModuleCacheMutexWrapper() = default; -}; - struct ModuleCacheMutexes { std::mutex Mutex; - llvm::StringMap<std::shared_ptr<ModuleCacheMutexWrapper>> Map; + llvm::StringMap<std::unique_ptr<std::shared_mutex>> Map; }; std::shared_ptr<ModuleCacheLock> diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp index 162b5a4b213f5..a6d9526bc13e7 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp @@ -6,55 +6,23 @@ using namespace dependencies; namespace { struct ModuleCacheMutexLockManager : ModuleCacheLockManager { - ModuleCacheMutexes &Mutexes; - StringRef Filename; - - std::shared_ptr<ModuleCacheMutexWrapper> MutexWrapper; - bool Owning; - - ModuleCacheMutexLockManager(ModuleCacheMutexes &Mutexes, StringRef Filename) - : Mutexes(Mutexes), Filename(Filename) { - Owning = false; - { - std::lock_guard Lock(Mutexes.Mutex); - auto &MutexWrapperInMap = Mutexes.Map[Filename]; - if (!MutexWrapperInMap) { - MutexWrapperInMap = std::make_shared<ModuleCacheMutexWrapper>(); - Owning = true; - } - // Increment the reference count of the mutex here in the critical section - // to guarantee that it's kept alive even when another thread removes it - // via \c unsafeRemoveLock(). - MutexWrapper = MutexWrapperInMap; - } + std::unique_lock<std::shared_mutex> OwningLock; - if (Owning) - MutexWrapper->Mutex.lock(); - } - - ~ModuleCacheMutexLockManager() override { - if (Owning) { - MutexWrapper->Done = true; - MutexWrapper->CondVar.notify_all(); - MutexWrapper->Mutex.unlock(); - } - } + ModuleCacheMutexLockManager(std::shared_mutex &Mutex) + : OwningLock(Mutex, std::try_to_lock_t{}) {} operator LockResult() const override { - return Owning ? LockResult::Owned : LockResult::Shared; + return OwningLock ? LockResult::Owned : LockResult::Shared; } WaitForUnlockResult waitForUnlock() override { - assert(!Owning); - std::unique_lock Lock(MutexWrapper->Mutex); - bool Done = MutexWrapper->CondVar.wait_for( - Lock, std::chrono::seconds(90), [&] { return MutexWrapper->Done; }); - return Done ? WaitForUnlockResult::Success : WaitForUnlockResult::Timeout; + assert(!OwningLock); + std::shared_lock Lock(*OwningLock.mutex()); + return WaitForUnlockResult::Success; } void unsafeRemoveLock() override { - std::lock_guard Lock(Mutexes.Mutex); - Mutexes.Map[Filename].reset(); + llvm_unreachable("ModuleCacheMutexLockManager cannot remove locks"); } std::string getErrorMessage() const override { @@ -70,7 +38,14 @@ struct ModuleCacheMutexLock : ModuleCacheLock { void prepareLock(StringRef Filename) override {} std::unique_ptr<ModuleCacheLockManager> tryLock(StringRef Filename) override { - return std::make_unique<ModuleCacheMutexLockManager>(Mutexes, Filename); + auto &Mutex = [&]() -> std::shared_mutex & { + std::lock_guard Lock(Mutexes.Mutex); + auto &Mutex = Mutexes.Map[Filename]; + if (!Mutex) + Mutex = std::make_unique<std::shared_mutex>(); + return *Mutex; + }(); + return std::make_unique<ModuleCacheMutexLockManager>(Mutex); } }; } // namespace >From 60ca622aa6906cbe8f8ebe28d06ae928f3d8f2f6 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Wed, 5 Mar 2025 16:54:55 -0800 Subject: [PATCH 4/5] Add missing include --- clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp index a6d9526bc13e7..b9a0b8d4d9472 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp @@ -1,5 +1,7 @@ #include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" +#include <mutex> + using namespace clang; using namespace tooling; using namespace dependencies; >From b55b649f5e88b0bc8bd9a452ab5662481ac59963 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Wed, 5 Mar 2025 16:56:42 -0800 Subject: [PATCH 5/5] Add license header to new files --- clang/include/clang/Serialization/ModuleCacheLock.h | 8 ++++++++ .../Tooling/DependencyScanning/ModuleCacheMutexLock.h | 8 ++++++++ .../Tooling/DependencyScanning/ModuleCacheMutexLock.cpp | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/clang/include/clang/Serialization/ModuleCacheLock.h b/clang/include/clang/Serialization/ModuleCacheLock.h index 9435735b1a1bd..414063bcf5f67 100644 --- a/clang/include/clang/Serialization/ModuleCacheLock.h +++ b/clang/include/clang/Serialization/ModuleCacheLock.h @@ -1,3 +1,11 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + #ifndef LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H #define LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h index 0e7f9031f153b..719921b259caa 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h @@ -1,3 +1,11 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp index b9a0b8d4d9472..95667edd0dcd6 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp @@ -1,3 +1,11 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + #include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h" #include <mutex> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits