Author: Jan Svoboda Date: 2025-03-13T10:54:51-07:00 New Revision: d8dfdafc1d75ab17f742d40ab93240a10c216507
URL: https://github.com/llvm/llvm-project/commit/d8dfdafc1d75ab17f742d40ab93240a10c216507 DIFF: https://github.com/llvm/llvm-project/commit/d8dfdafc1d75ab17f742d40ab93240a10c216507.diff LOG: [Support] Introduce new `AdvisoryLock` interface (#130989) 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. Added: llvm/include/llvm/Support/AdvisoryLock.h Modified: clang/lib/Frontend/CompilerInstance.cpp llvm/include/llvm/Support/LockFileManager.h llvm/lib/Support/LockFileManager.cpp llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp Removed: ################################################################################ diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 44f4f48ef94e8..fc350f2ba42e0 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1502,19 +1502,19 @@ static bool compileModuleAndReadASTBehindLock( // Someone else is responsible for building the module. Wait for them to // finish. - switch (Lock.waitForUnlock()) { - case llvm::LockFileManager::Res_Success: + switch (Lock.waitForUnlockFor(std::chrono::seconds(90))) { + 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.unsafeMaybeUnlock(); continue; } diff --git a/llvm/include/llvm/Support/AdvisoryLock.h b/llvm/include/llvm/Support/AdvisoryLock.h new file mode 100644 index 0000000000000..d1c3ccc187e64 --- /dev/null +++ b/llvm/include/llvm/Support/AdvisoryLock.h @@ -0,0 +1,58 @@ +//===----------------------------------------------------------------------===// +// +// 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" + +#include <chrono> + +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 ownership of the lock simultaneously. +/// Typically, threads/processes waiting for the lock to be unlocked will +/// validate that the computation was performed by the expected thread/process +/// and re-run the computation if not. +class AdvisoryLock { +public: + /// Tries to acquire ownership of the lock without blocking. + /// + /// \returns true if ownership of the lock was acquired successfully, false if + /// the lock is already owned by someone else, or \c Error in case of an + /// unexpected failure. + virtual Expected<bool> tryLock() = 0; + + /// For a lock owned by someone else, wait until it is unlocked. + /// + /// \param MaxSeconds the maximum total wait time in seconds. + virtual WaitForUnlockResult + waitForUnlockFor(std::chrono::seconds MaxSeconds) = 0; + + /// For a lock owned by someone else, unlock it. A permitted side-effect is + /// that another thread/process may acquire ownership of the lock before the + /// existing owner unlocks it. This is an unsafe operation. + virtual std::error_code unsafeMaybeUnlock() = 0; + + /// Unlocks the lock if its ownership was 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..a126fa3d6b529 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 diff erent 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,24 @@ 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(std::chrono::seconds MaxSeconds) override; /// Remove the lock file. This may delete a diff erent lock file than /// the one previously read if there is a race. - std::error_code unsafeRemoveLockFile(); -}; + std::error_code unsafeMaybeUnlock() 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..39d50fd9bbb7e 100644 --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -264,8 +264,8 @@ LockFileManager::~LockFileManager() { sys::DontRemoveFileOnSignal(UniqueLockFileName); } -LockFileManager::WaitForUnlockResult -LockFileManager::waitForUnlock(const unsigned MaxSeconds) { +WaitForUnlockResult +LockFileManager::waitForUnlockFor(std::chrono::seconds MaxSeconds) { auto *LockFileOwner = std::get_if<OwnedByAnother>(&Owner); assert(LockFileOwner && "waiting for lock to be unlocked without knowing the owner"); @@ -275,25 +275,25 @@ LockFileManager::waitForUnlock(const unsigned MaxSeconds) { // algorithm. This improves performance on machines with high core counts // when the file lock is heavily contended by multiple clang processes using namespace std::chrono_literals; - ExponentialBackoff Backoff(std::chrono::seconds(MaxSeconds), 10ms, 500ms); + ExponentialBackoff Backoff(MaxSeconds, 10ms, 500ms); // Wait first as this is only called when the lock is known to be held. while (Backoff.waitForNextAttempt()) { // 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::unsafeMaybeUnlock() { return sys::fs::remove(LockFileName); } diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp index e2e449f1c8a38..2fcb485a822d2 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp @@ -1553,17 +1553,17 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M, dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug " "output may be mangled by other processes\n"); } else if (!Owned) { - switch (Lock.waitForUnlock()) { - case LockFileManager::Res_Success: + switch (Lock.waitForUnlockFor(std::chrono::seconds(90))) { + 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.unsafeMaybeUnlock(); break; // give up } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
