llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> This PR abstracts the `LockFileManager` API into new `AdvisoryLock` interface. This is so that we can create an alternative implementation for Clang implicitly-built modules that is optimized for single-process environment. --- Full diff: https://github.com/llvm/llvm-project/pull/130989.diff 5 Files Affected: - (modified) clang/lib/Frontend/CompilerInstance.cpp (+4-4) - (added) llvm/include/llvm/Support/AdvisoryLock.h (+54) - (modified) llvm/include/llvm/Support/LockFileManager.h (+9-24) - (modified) llvm/lib/Support/LockFileManager.cpp (+5-6) - (modified) llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp (+4-4) ``````````diff diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 44f4f48ef94e8..ef09cd9cc43d7 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1503,18 +1503,18 @@ static bool compileModuleAndReadASTBehindLock( // Someone else is responsible for building the module. Wait for them to // finish. switch (Lock.waitForUnlock()) { - case llvm::LockFileManager::Res_Success: + case llvm::WaitForUnlockResult::Success: break; // The interesting case. - case llvm::LockFileManager::Res_OwnerDied: + case llvm::WaitForUnlockResult::OwnerDied: continue; // try again to get the lock. - case llvm::LockFileManager::Res_Timeout: + case llvm::WaitForUnlockResult::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. Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout) << Module->Name; // Clear the lock file so that future invocations can make progress. - Lock.unsafeRemoveLockFile(); + Lock.unsafeUnlockShared(); continue; } diff --git a/llvm/include/llvm/Support/AdvisoryLock.h b/llvm/include/llvm/Support/AdvisoryLock.h new file mode 100644 index 0000000000000..57c993f48f240 --- /dev/null +++ b/llvm/include/llvm/Support/AdvisoryLock.h @@ -0,0 +1,54 @@ +//===----------------------------------------------------------------------===// +// +// 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_SUPPORT_ADVISORYLOCK_H +#define LLVM_SUPPORT_ADVISORYLOCK_H + +#include "llvm/Support/Error.h" + +namespace llvm { +/// Describes the result of waiting for the owner to release the lock. +enum class WaitForUnlockResult { + /// The lock was released successfully. + Success, + /// Owner died while holding the lock. + OwnerDied, + /// Reached timeout while waiting for the owner to release the lock. + Timeout, +}; + +/// A synchronization primitive with weak mutual exclusion guarantees. +/// Implementations of this interface may allow multiple threads/processes to +/// acquire the lock simultaneously. Typically, threads/processes waiting for +/// the lock to be unlocked will validate the computation produced valid result. +class AdvisoryLock { +public: + /// Tries to acquire the lock without blocking. + /// + /// \returns true if the lock was successfully acquired (owned lock), false if + /// the lock is already held by someone else (shared lock), or \c Error in + /// case of unexpected failure. + virtual Expected<bool> tryLock() = 0; + + /// For a shared lock, wait until the owner releases the lock. + /// + /// \param MaxSeconds the maximum total wait time in seconds. + virtual WaitForUnlockResult waitForUnlockFor(unsigned MaxSeconds) = 0; + WaitForUnlockResult waitForUnlock() { return waitForUnlockFor(90); } + + /// Unlocks a shared lock. This may allow another thread/process to acquire + /// the lock before the existing owner released it and notify waiting + /// threads/processes. This is an unsafe operation. + virtual std::error_code unsafeUnlockShared() = 0; + + /// Unlocks the lock if previously acquired by \c tryLock(). + virtual ~AdvisoryLock() = default; +}; +} // end namespace llvm + +#endif diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h index cf02b41a6f729..1653a7416f667 100644 --- a/llvm/include/llvm/Support/LockFileManager.h +++ b/llvm/include/llvm/Support/LockFileManager.h @@ -9,15 +9,13 @@ #define LLVM_SUPPORT_LOCKFILEMANAGER_H #include "llvm/ADT/SmallString.h" -#include "llvm/Support/Error.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/AdvisoryLock.h" #include <optional> #include <string> -#include <system_error> #include <variant> namespace llvm { -class StringRef; - /// Class that manages the creation of a lock file to aid implicit coordination /// between different processes. /// @@ -25,19 +23,7 @@ class StringRef; /// atomicity of the file system to ensure that only a single process can create /// that ".lock" file. When the lock file is removed, the owning process has /// finished the operation. -class LockFileManager { -public: - /// Describes the result of waiting for the owner to release the lock. - enum WaitForUnlockResult { - /// The lock was released successfully. - Res_Success, - /// Owner died while holding the lock. - Res_OwnerDied, - /// Reached timeout while waiting for the owner to release the lock. - Res_Timeout - }; - -private: +class LockFileManager : public AdvisoryLock { SmallString<128> FileName; SmallString<128> LockFileName; SmallString<128> UniqueLockFileName; @@ -61,24 +47,23 @@ class LockFileManager { /// Does not try to acquire the lock. LockFileManager(StringRef FileName); - /// Unlocks the lock if previously acquired by \c tryLock(). - ~LockFileManager(); - /// Tries to acquire the lock without blocking. /// \returns true if the lock was successfully acquired, false if the lock is /// already held by someone else, or \c Error in case of unexpected failure. - Expected<bool> tryLock(); + Expected<bool> tryLock() override; /// For a shared lock, wait until the owner releases the lock. /// Total timeout for the file to appear is ~1.5 minutes. /// \param MaxSeconds the maximum total wait time in seconds. - WaitForUnlockResult waitForUnlock(const unsigned MaxSeconds = 90); + WaitForUnlockResult waitForUnlockFor(unsigned MaxSeconds) override; /// Remove the lock file. This may delete a different lock file than /// the one previously read if there is a race. - std::error_code unsafeRemoveLockFile(); -}; + std::error_code unsafeUnlockShared() override; + /// Unlocks the lock if previously acquired by \c tryLock(). + ~LockFileManager() override; +}; } // end namespace llvm #endif // LLVM_SUPPORT_LOCKFILEMANAGER_H diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp index 7cf9db379974f..1d507d8e3118a 100644 --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -264,8 +264,7 @@ LockFileManager::~LockFileManager() { sys::DontRemoveFileOnSignal(UniqueLockFileName); } -LockFileManager::WaitForUnlockResult -LockFileManager::waitForUnlock(const unsigned MaxSeconds) { +WaitForUnlockResult LockFileManager::waitForUnlockFor(unsigned MaxSeconds) { auto *LockFileOwner = std::get_if<OwnedByAnother>(&Owner); assert(LockFileOwner && "waiting for lock to be unlocked without knowing the owner"); @@ -282,18 +281,18 @@ LockFileManager::waitForUnlock(const unsigned MaxSeconds) { // FIXME: implement event-based waiting if (sys::fs::access(LockFileName.c_str(), sys::fs::AccessMode::Exist) == errc::no_such_file_or_directory) - return Res_Success; + return WaitForUnlockResult::Success; // If the process owning the lock died without cleaning up, just bail out. if (!processStillExecuting(LockFileOwner->OwnerHostName, LockFileOwner->OwnerPID)) - return Res_OwnerDied; + return WaitForUnlockResult::OwnerDied; } // Give up. - return Res_Timeout; + return WaitForUnlockResult::Timeout; } -std::error_code LockFileManager::unsafeRemoveLockFile() { +std::error_code LockFileManager::unsafeUnlockShared() { return sys::fs::remove(LockFileName); } diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp index e2e449f1c8a38..1e381c9987ced 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp @@ -1554,16 +1554,16 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M, "output may be mangled by other processes\n"); } else if (!Owned) { switch (Lock.waitForUnlock()) { - case LockFileManager::Res_Success: + case WaitForUnlockResult::Success: break; - case LockFileManager::Res_OwnerDied: + case WaitForUnlockResult::OwnerDied: continue; // try again to get the lock. - case LockFileManager::Res_Timeout: + case WaitForUnlockResult::Timeout: LLVM_DEBUG( dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug " "output may be mangled by other processes\n"); - Lock.unsafeRemoveLockFile(); + Lock.unsafeUnlockShared(); break; // give up } } `````````` </details> https://github.com/llvm/llvm-project/pull/130989 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
