[clang] [llvm] [Support] Introduce new `AdvisoryLock` interface (PR #130989)
Jan Svoboda via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 12 16:48:59 PDT 2025
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/130989
>From cd0b9a8b878c4ecd929552c836f3d4665e926ef4 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 12 Mar 2025 09:37:27 -0700
Subject: [PATCH 1/2] [Support] Introduce new `AdvisoryLock` interface
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.
---
clang/lib/Frontend/CompilerInstance.cpp | 8 +--
llvm/include/llvm/Support/AdvisoryLock.h | 54 ++++++++++++++++++++
llvm/include/llvm/Support/LockFileManager.h | 33 ++++--------
llvm/lib/Support/LockFileManager.cpp | 11 ++--
llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp | 8 +--
5 files changed, 76 insertions(+), 38 deletions(-)
create mode 100644 llvm/include/llvm/Support/AdvisoryLock.h
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
}
}
>From 0e4a63fe8b6897a6af0811e475e032c973ff2bd1 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 12 Mar 2025 16:17:03 -0700
Subject: [PATCH 2/2] Improve documentation, remove default arg, rename unsafe
unlock
---
clang/lib/Frontend/CompilerInstance.cpp | 4 +--
llvm/include/llvm/Support/AdvisoryLock.h | 32 +++++++++++---------
llvm/include/llvm/Support/LockFileManager.h | 7 +++--
llvm/lib/Support/LockFileManager.cpp | 7 +++--
llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp | 4 +--
5 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index ef09cd9cc43d7..fc350f2ba42e0 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1502,7 +1502,7 @@ static bool compileModuleAndReadASTBehindLock(
// Someone else is responsible for building the module. Wait for them to
// finish.
- switch (Lock.waitForUnlock()) {
+ switch (Lock.waitForUnlockFor(std::chrono::seconds(90))) {
case llvm::WaitForUnlockResult::Success:
break; // The interesting case.
case llvm::WaitForUnlockResult::OwnerDied:
@@ -1514,7 +1514,7 @@ static bool compileModuleAndReadASTBehindLock(
Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout)
<< Module->Name;
// Clear the lock file so that future invocations can make progress.
- Lock.unsafeUnlockShared();
+ Lock.unsafeMaybeUnlock();
continue;
}
diff --git a/llvm/include/llvm/Support/AdvisoryLock.h b/llvm/include/llvm/Support/AdvisoryLock.h
index 57c993f48f240..d1c3ccc187e64 100644
--- a/llvm/include/llvm/Support/AdvisoryLock.h
+++ b/llvm/include/llvm/Support/AdvisoryLock.h
@@ -11,6 +11,8 @@
#include "llvm/Support/Error.h"
+#include <chrono>
+
namespace llvm {
/// Describes the result of waiting for the owner to release the lock.
enum class WaitForUnlockResult {
@@ -24,29 +26,31 @@ enum class WaitForUnlockResult {
/// 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.
+/// 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 the lock without blocking.
+ /// Tries to acquire ownership of 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.
+ /// \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 shared lock, wait until the owner releases the lock.
+ /// For a lock owned by someone else, wait until it is unlocked.
///
/// \param MaxSeconds the maximum total wait time in seconds.
- virtual WaitForUnlockResult waitForUnlockFor(unsigned MaxSeconds) = 0;
- WaitForUnlockResult waitForUnlock() { return waitForUnlockFor(90); }
+ virtual WaitForUnlockResult
+ waitForUnlockFor(std::chrono::seconds MaxSeconds) = 0;
- /// 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;
+ /// 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 previously acquired by \c tryLock().
+ /// Unlocks the lock if its ownership was previously acquired by \c tryLock().
virtual ~AdvisoryLock() = default;
};
} // end namespace llvm
diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index 1653a7416f667..a126fa3d6b529 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -53,13 +53,14 @@ class LockFileManager : public AdvisoryLock {
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 waitForUnlockFor(unsigned MaxSeconds) override;
+ WaitForUnlockResult
+ waitForUnlockFor(std::chrono::seconds 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 unsafeUnlockShared() override;
+ std::error_code unsafeMaybeUnlock() override;
/// Unlocks the lock if previously acquired by \c tryLock().
~LockFileManager() override;
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 1d507d8e3118a..39d50fd9bbb7e 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -264,7 +264,8 @@ LockFileManager::~LockFileManager() {
sys::DontRemoveFileOnSignal(UniqueLockFileName);
}
-WaitForUnlockResult LockFileManager::waitForUnlockFor(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");
@@ -274,7 +275,7 @@ WaitForUnlockResult LockFileManager::waitForUnlockFor(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()) {
@@ -293,6 +294,6 @@ WaitForUnlockResult LockFileManager::waitForUnlockFor(unsigned MaxSeconds) {
return WaitForUnlockResult::Timeout;
}
-std::error_code LockFileManager::unsafeUnlockShared() {
+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 1e381c9987ced..2fcb485a822d2 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
@@ -1553,7 +1553,7 @@ 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()) {
+ switch (Lock.waitForUnlockFor(std::chrono::seconds(90))) {
case WaitForUnlockResult::Success:
break;
case WaitForUnlockResult::OwnerDied:
@@ -1563,7 +1563,7 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
dbgs()
<< "[amdgpu-split-module] unable to acquire lockfile, debug "
"output may be mangled by other processes\n");
- Lock.unsafeUnlockShared();
+ Lock.unsafeMaybeUnlock();
break; // give up
}
}
More information about the llvm-commits
mailing list