[llvm][Support] Add ExponentialBackoff helper

[Bigcheese]

This provides a simple way to implement exponential backoff using a do while loop.

Usage example (also see the change to LockFileManager.cpp):

ExponentialBackoff Backoff(10s);
do {
  if (tryToDoSomething())
    return ItWorked;
} while (Backoff.waitForNextAttempt());
return Timeout;

Abstracting this out of LockFileManager as the module build daemon will need it.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Michael Spencer (Bigcheese)

Changes

This provides a simple way to implement exponential backoff using a do while loop.

Usage example (also see the change to LockFileManager.cpp):

ExponentialBackoff Backoff(10s);
do {
  if (tryToDoSomething())
    return ItWorked;
} while (Backoff.waitForNextAttempt());
return Timeout;

Abstracting this out of LockFileManager as the module build daemon will need it.

---

Full diff: https://github.com/llvm/llvm-project/pull/81206.diff

6 Files Affected:

• (added) llvm/include/llvm/Support/ExponentialBackoff.h (+65)
• (modified) llvm/lib/Support/CMakeLists.txt (+1)
• (added) llvm/lib/Support/ExponentialBackoff.cpp (+29)
• (modified) llvm/lib/Support/LockFileManager.cpp (+7-31)
• (modified) llvm/unittests/Support/CMakeLists.txt (+1)
• (added) llvm/unittests/Support/ExponentialBackoffTest.cpp (+31)

diff --git a/llvm/include/llvm/Support/ExponentialBackoff.h b/llvm/include/llvm/Support/ExponentialBackoff.h
new file mode 100644
index 0000000000000..8208a748eac2a
--- /dev/null
+++ b/llvm/include/llvm/Support/ExponentialBackoff.h
@@ -0,0 +1,65 @@
+//===- llvm/Support/ExponentialBackoff.h ------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines a helper class for implementing exponential backoff.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_EXPONENTIALBACKOFF_H
+#define LLVM_EXPONENTIALBACKOFF_H
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include <chrono>
+#include <random>
+
+namespace llvm {
+
+/// A class to help implement exponential backoff.
+///
+/// Example usage:
+/// \code
+///   ExponentialBackoff Backoff(10s);
+///   do {
+///     if (tryToDoSomething())
+///       return ItWorked;
+///   } while (Backoff.waitForNextAttempt());
+///   return Timeout;
+/// \endcode
+class ExponentialBackoff {
+public:
+  using duration = std::chrono::steady_clock::duration;
+  using time_point = std::chrono::steady_clock::time_point;
+
+  /// \param Timeout the maximum wall time this should run for starting when
+  ///        this object is constructed.
+  /// \param MinWait the minimum amount of time `waitForNextAttempt` will sleep
+  ///        for.
+  /// \param MaxWait the maximum amount of time `waitForNextAttempt` will sleep
+  ///        for.
+  ExponentialBackoff(duration Timeout,
+                     duration MinWait = std::chrono::milliseconds(10),
+                     duration MaxWait = std::chrono::milliseconds(500))
+      : MinWait(MinWait), MaxWait(MaxWait),
+        EndTime(std::chrono::steady_clock::now() + Timeout) {}
+
+  /// Blocks while waiting for the next attempt.
+  /// \returns true if you should try again, false if the timeout has been
+  /// reached.
+  bool waitForNextAttempt();
+
+private:
+  duration MinWait;
+  duration MaxWait;
+  time_point EndTime;
+  std::random_device RandDev;
+  int64_t CurrentMultiplier = 1;
+};
+
+} // end namespace llvm
+
+#endif // LLVM_EXPONENTIALBACKOFF_H
diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
index e19223fdef4f1..1f2d82427552f 100644
--- a/llvm/lib/Support/CMakeLists.txt
+++ b/llvm/lib/Support/CMakeLists.txt
@@ -176,6 +176,7 @@ add_llvm_component_library(LLVMSupport
   ELFAttributes.cpp
   Error.cpp
   ErrorHandling.cpp
+  ExponentialBackoff.cpp
   ExtensibleRTTI.cpp
   FileCollector.cpp
   FileUtilities.cpp
diff --git a/llvm/lib/Support/ExponentialBackoff.cpp b/llvm/lib/Support/ExponentialBackoff.cpp
new file mode 100644
index 0000000000000..7e68cf67ad385
--- /dev/null
+++ b/llvm/lib/Support/ExponentialBackoff.cpp
@@ -0,0 +1,29 @@
+//===- llvm/Support/ExponentialBackoff.h ------------------------*- C++ -*-===//
+//
+// 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 "llvm/Support/ExponentialBackoff.h"
+#include <thread>
+
+using namespace llvm;
+
+bool ExponentialBackoff::waitForNextAttempt() {
+  auto Now = std::chrono::steady_clock::now();
+  if (Now >= EndTime)
+    return false;
+
+  duration CurMaxWait = std::min(MinWait * CurrentMultiplier, MaxWait);
+  std::uniform_int_distribution<uint64_t> Dist(MinWait.count(),
+                                               CurMaxWait.count());
+  // Use random_device directly instead of a PRNG as uniform_int_distribution
+  // often only takes a few samples anyway.
+  duration WaitDuration = std::min(duration(Dist(RandDev)), EndTime - Now);
+  if (CurMaxWait < MaxWait)
+    CurrentMultiplier *= 2;
+  std::this_thread::sleep_for(WaitDuration);
+  return true;
+}
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index a2b0fe8ca8f2e..34c7a16b24be4 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/ExponentialBackoff.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Process.h"
@@ -20,7 +21,6 @@
 #include <chrono>
 #include <ctime>
 #include <memory>
-#include <random>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <system_error>
@@ -295,29 +295,15 @@ LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
     return Res_Success;
 
   // Since we don't yet have an event-based method to wait for the lock file,
-  // implement randomized exponential backoff, similar to Ethernet collision
+  // use randomized exponential backoff, similar to Ethernet collision
   // algorithm. This improves performance on machines with high core counts
   // when the file lock is heavily contended by multiple clang processes
-  const unsigned long MinWaitDurationMS = 10;
-  const unsigned long MaxWaitMultiplier = 50; // 500ms max wait
-  unsigned long WaitMultiplier = 1;
-  unsigned long ElapsedTimeSeconds = 0;
+  using namespace std::chrono_literals;
+  ExponentialBackoff Backoff(std::chrono::seconds(MaxSeconds), 10ms, 500ms);
 
-  std::random_device Device;
-  std::default_random_engine Engine(Device());
-
-  auto StartTime = std::chrono::steady_clock::now();
-
-  do {
+  // Wait first as this is only called when the lock is known to be held.
+  while (Backoff.waitForNextAttempt()) {
     // FIXME: implement event-based waiting
-
-    // Sleep for the designated interval, to allow the owning process time to
-    // finish up and remove the lock file.
-    std::uniform_int_distribution<unsigned long> Distribution(1,
-                                                              WaitMultiplier);
-    unsigned long WaitDurationMS = MinWaitDurationMS * Distribution(Engine);
-    std::this_thread::sleep_for(std::chrono::milliseconds(WaitDurationMS));
-
     if (sys::fs::access(LockFileName.c_str(), sys::fs::AccessMode::Exist) ==
         errc::no_such_file_or_directory) {
       // If the original file wasn't created, somone thought the lock was dead.
@@ -329,17 +315,7 @@ LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
     // If the process owning the lock died without cleaning up, just bail out.
     if (!processStillExecuting((*Owner).first, (*Owner).second))
       return Res_OwnerDied;
-
-    WaitMultiplier *= 2;
-    if (WaitMultiplier > MaxWaitMultiplier) {
-      WaitMultiplier = MaxWaitMultiplier;
-    }
-
-    ElapsedTimeSeconds = std::chrono::duration_cast<std::chrono::seconds>(
-                             std::chrono::steady_clock::now() - StartTime)
-                             .count();
-
-  } while (ElapsedTimeSeconds < MaxSeconds);
+  }
 
   // Give up.
   return Res_Timeout;
diff --git a/llvm/unittests/Support/CMakeLists.txt b/llvm/unittests/Support/CMakeLists.txt
index df35a7b7f3626..15a126279125c 100644
--- a/llvm/unittests/Support/CMakeLists.txt
+++ b/llvm/unittests/Support/CMakeLists.txt
@@ -38,6 +38,7 @@ add_llvm_unittest(SupportTests
   ErrnoTest.cpp
   ErrorOrTest.cpp
   ErrorTest.cpp
+  ExponentialBackoffTest.cpp
   ExtensibleRTTITest.cpp
   FileCollectorTest.cpp
   FileOutputBufferTest.cpp
diff --git a/llvm/unittests/Support/ExponentialBackoffTest.cpp b/llvm/unittests/Support/ExponentialBackoffTest.cpp
new file mode 100644
index 0000000000000..327c86c5442f2
--- /dev/null
+++ b/llvm/unittests/Support/ExponentialBackoffTest.cpp
@@ -0,0 +1,31 @@
+//===- unittests/ExponentialBackoffTest.cpp -------------------------------===//
+//
+// 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 "llvm/Support/ExponentialBackoff.h"
+#include "gtest/gtest.h"
+#include <chrono>
+
+using namespace llvm;
+using namespace std::chrono_literals;
+
+namespace {
+
+TEST(ExponentialBackoffTest, Timeout) {
+  auto Start = std::chrono::steady_clock::now();
+  // Use short enough times that this test runs quickly.
+  ExponentialBackoff Backoff(100ms, 1ms, 10ms);
+  do {
+  } while(Backoff.waitForNextAttempt());
+  auto Duration = std::chrono::steady_clock::now() - Start;
+  EXPECT_GE(Duration, 100ms);
+}
+
+// Testing individual wait duration is omitted as those tests would be
+// non-deterministic.
+
+} // end anonymous namespace

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Bigcheese]

Requesting review from @ladd who did the original randomized backoff impl.

[dwblaikie]

Looks OK to me

[dwblaikie]

Could check the bounds of the waits in the loop? (like check that it's at least 1ms, less than 15 or something?)

Though, yeah, any testing here will be either slow or brittle or both - so don't do it if it's just impractical to make it reliable.

[Bigcheese]

Yeah, I was concerned about edge cases with virtual machines or other weirdness. I locally checked and it seemed reasonable. Thanks.