[Support] Introduce new AdvisoryLock interface

[jansvoboda11]

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.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

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 --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
         }
       }

[Bigcheese]

I have a few comments on naming, but overall this looks good.

[Bigcheese]

lgtm.