Split Baton wait methods into fast and slow paths

Summary:
[Folly] Split `Baton` wait methods into fast and slow paths.

Force-inline the fast paths, and force-outline the slow paths.

Reviewed By: magedm

Differential Revision: D6501992

fbshipit-source-id: 611e26b3cfeef01eb8d3a3500ae3ebc26bee6e86

diff --git a/folly/synchronization/Baton.h b/folly/synchronization/Baton.h
index 8a2c0c2cacfe54b60630c618ff06e2f1256b6968..ef87125286723f6aad8fd4daf7ed6d50b0178681 100644 (file)
--- a/folly/synchronization/Baton.h
+++ b/folly/synchronization/Baton.h
@@ -22,6 +22,7 @@
 #include <atomic>
 #include <thread>
 
 #include <atomic>
 #include <thread>
 

+#include <folly/Likely.h>

 #include <folly/detail/Futex.h>
 #include <folly/detail/MemoryIdler.h>
 #include <folly/portability/Asm.h>
 #include <folly/detail/Futex.h>
 #include <folly/detail/MemoryIdler.h>
 #include <folly/portability/Asm.h>


@@ -199,55 +200,12 @@ struct Baton {
   /// could be relaxed somewhat without any perf or size regressions,
   /// but by making this condition very restrictive we can provide better
   /// checking in debug builds.
   /// could be relaxed somewhat without any perf or size regressions,
   /// but by making this condition very restrictive we can provide better
   /// checking in debug builds.

-  void wait() {
-    if (spinWaitForEarlyDelivery()) {
-      assert(state_.load(std::memory_order_acquire) == EARLY_DELIVERY);
-      return;
-    }
-
-    if (!Blocking) {
-      while (!try_wait()) {
-        std::this_thread::yield();
-      }
-      return;
-    }
-
-    // guess we have to block :(
-    uint32_t expected = INIT;
-    if (!state_.compare_exchange_strong(expected, WAITING)) {
-      // CAS failed, last minute reprieve
-      assert(expected == EARLY_DELIVERY);
+  FOLLY_ALWAYS_INLINE void wait() {
+    if (try_wait()) {

       return;
     }
 
       return;
     }
 

-    while (true) {
-      detail::MemoryIdler::futexWait(state_, WAITING);
-
-      // state_ is the truth even if FUTEX_WAIT reported a matching
-      // FUTEX_WAKE, since we aren't using type-stable storage and we
-      // don't guarantee reuse.  The scenario goes like this: thread
-      // A's last touch of a Baton is a call to wake(), which stores
-      // LATE_DELIVERY and gets an unlucky context switch before delivering
-      // the corresponding futexWake.  Thread B sees LATE_DELIVERY
-      // without consuming a futex event, because it calls futexWait
-      // with an expected value of WAITING and hence doesn't go to sleep.
-      // B returns, so the Baton's memory is reused and becomes another
-      // Baton (or a reuse of this one).  B calls futexWait on the new
-      // Baton lifetime, then A wakes up and delivers a spurious futexWake
-      // to the same memory location.  B's futexWait will then report a
-      // consumed wake event even though state_ is still WAITING.
-      //
-      // It would be possible to add an extra state_ dance to communicate
-      // that the futexWake has been sent so that we can be sure to consume
-      // it before returning, but that would be a perf and complexity hit.
-      uint32_t s = state_.load(std::memory_order_acquire);
-      assert(s == WAITING || s == LATE_DELIVERY);
-
-      if (s == LATE_DELIVERY) {
-        return;
-      }
-      // retry
-    }
+    waitSlow();

   }
 
   /// Similar to wait, but doesn't block the thread if it hasn't been posted.
   }
 
   /// Similar to wait, but doesn't block the thread if it hasn't been posted.


@@ -261,10 +219,10 @@ struct Baton {
   ///   call wait, try_wait or timed_wait on the same baton without resetting
   ///
   /// @return       true if baton has been posted, false othewise
   ///   call wait, try_wait or timed_wait on the same baton without resetting
   ///
   /// @return       true if baton has been posted, false othewise

-  bool try_wait() const {
+  FOLLY_ALWAYS_INLINE bool try_wait() const {

     auto s = state_.load(std::memory_order_acquire);
     assert(s == INIT || s == EARLY_DELIVERY);
     auto s = state_.load(std::memory_order_acquire);
     assert(s == INIT || s == EARLY_DELIVERY);

-    return s == EARLY_DELIVERY;
+    return LIKELY(s == EARLY_DELIVERY);

   }
 
   /// Similar to wait, but with a timeout. The thread is unblocked if the
   }
 
   /// Similar to wait, but with a timeout. The thread is unblocked if the


@@ -279,12 +237,17 @@ struct Baton {
   /// @return               true if the baton was posted to before timeout,
   ///                       false otherwise
   template <typename Rep, typename Period>
   /// @return               true if the baton was posted to before timeout,
   ///                       false otherwise
   template <typename Rep, typename Period>

-  bool try_wait_for(const std::chrono::duration<Rep, Period>& timeout) {
+  FOLLY_ALWAYS_INLINE bool try_wait_for(
+      const std::chrono::duration<Rep, Period>& timeout) {

     static_assert(
         Blocking, "Non-blocking Baton does not support try_wait_for.");
 
     static_assert(
         Blocking, "Non-blocking Baton does not support try_wait_for.");
 

+    if (try_wait()) {
+      return true;
+    }
+

     auto deadline = std::chrono::steady_clock::now() + timeout;
     auto deadline = std::chrono::steady_clock::now() + timeout;

-    return try_wait_until(deadline);
+    return tryWaitUntilSlow(deadline);

   }
 
   /// Similar to wait, but with a deadline. The thread is unblocked if the
   }
 
   /// Similar to wait, but with a deadline. The thread is unblocked if the


@@ -299,48 +262,29 @@ struct Baton {
   /// @return               true if the baton was posted to before deadline,
   ///                       false otherwise
   template <typename Clock, typename Duration>
   /// @return               true if the baton was posted to before deadline,
   ///                       false otherwise
   template <typename Clock, typename Duration>

-  bool try_wait_until(
+  FOLLY_ALWAYS_INLINE bool try_wait_until(

       const std::chrono::time_point<Clock, Duration>& deadline) {
     static_assert(
         Blocking, "Non-blocking Baton does not support try_wait_until.");
 
       const std::chrono::time_point<Clock, Duration>& deadline) {
     static_assert(
         Blocking, "Non-blocking Baton does not support try_wait_until.");
 

-    if (spinWaitForEarlyDelivery()) {
-      assert(state_.load(std::memory_order_acquire) == EARLY_DELIVERY);
-      return true;
-    }
-
-    // guess we have to block :(
-    uint32_t expected = INIT;
-    if (!state_.compare_exchange_strong(expected, WAITING)) {
-      // CAS failed, last minute reprieve
-      assert(expected == EARLY_DELIVERY);
+    if (try_wait()) {

       return true;
     }
 
       return true;
     }
 

-    while (true) {
-      auto rv = state_.futexWaitUntil(WAITING, deadline);
-      if (rv == folly::detail::FutexResult::TIMEDOUT) {
-        state_.store(TIMED_OUT, std::memory_order_release);
-        return false;
-      }
-
-      uint32_t s = state_.load(std::memory_order_acquire);
-      assert(s == WAITING || s == LATE_DELIVERY);
-      if (s == LATE_DELIVERY) {
-        return true;
-      }
-    }
+    return tryWaitUntilSlow(deadline);

   }
 
   /// Alias to try_wait_for. Deprecated.
   template <typename Rep, typename Period>
   }
 
   /// Alias to try_wait_for. Deprecated.
   template <typename Rep, typename Period>

-  bool timed_wait(const std::chrono::duration<Rep, Period>& timeout) {
+  FOLLY_ALWAYS_INLINE bool timed_wait(
+      const std::chrono::duration<Rep, Period>& timeout) {

     return try_wait_for(timeout);
   }
 
   /// Alias to try_wait_until. Deprecated.
   template <typename Clock, typename Duration>
     return try_wait_for(timeout);
   }
 
   /// Alias to try_wait_until. Deprecated.
   template <typename Clock, typename Duration>

-  bool timed_wait(const std::chrono::time_point<Clock, Duration>& deadline) {
+  FOLLY_ALWAYS_INLINE bool timed_wait(
+      const std::chrono::time_point<Clock, Duration>& deadline) {

     return try_wait_until(deadline);
   }
 
     return try_wait_until(deadline);
   }
 


@@ -350,7 +294,7 @@ struct Baton {
     EARLY_DELIVERY = 1,
     WAITING = 2,
     LATE_DELIVERY = 3,
     EARLY_DELIVERY = 1,
     WAITING = 2,
     LATE_DELIVERY = 3,

-    TIMED_OUT = 4
+    TIMED_OUT = 4,

   };
 
   enum {
   };
 
   enum {


@@ -377,14 +321,14 @@ struct Baton {
   //               false otherwise. If the function returns true then
   //               state_ is guaranteed to be EARLY_DELIVERY
   bool spinWaitForEarlyDelivery() {
   //               false otherwise. If the function returns true then
   //               state_ is guaranteed to be EARLY_DELIVERY
   bool spinWaitForEarlyDelivery() {

-
-    static_assert(PreBlockAttempts > 0,
+    static_assert(
+        PreBlockAttempts > 0,

         "isn't this assert clearer than an uninitialized variable warning?");
     for (int i = 0; i < PreBlockAttempts; ++i) {
       if (try_wait()) {
         "isn't this assert clearer than an uninitialized variable warning?");
     for (int i = 0; i < PreBlockAttempts; ++i) {
       if (try_wait()) {

-        // hooray!

         return true;
       }
         return true;
       }

+

       // The pause instruction is the polite way to spin, but it doesn't
       // actually affect correctness to omit it if we don't have it.
       // Pausing donates the full capabilities of the current core to
       // The pause instruction is the polite way to spin, but it doesn't
       // actually affect correctness to omit it if we don't have it.
       // Pausing donates the full capabilities of the current core to


@@ -395,6 +339,88 @@ struct Baton {
     return false;
   }
 
     return false;
   }
 

+  FOLLY_NOINLINE void waitSlow() {
+    if (spinWaitForEarlyDelivery()) {
+      assert(state_.load(std::memory_order_acquire) == EARLY_DELIVERY);
+      return;
+    }
+
+    if (!Blocking) {
+      while (!try_wait()) {
+        std::this_thread::yield();
+      }
+      return;
+    }
+
+    // guess we have to block :(
+    uint32_t expected = INIT;
+    if (!state_.compare_exchange_strong(expected, WAITING)) {
+      // CAS failed, last minute reprieve
+      assert(expected == EARLY_DELIVERY);
+      return;
+    }
+
+    while (true) {
+      detail::MemoryIdler::futexWait(state_, WAITING);
+
+      // state_ is the truth even if FUTEX_WAIT reported a matching
+      // FUTEX_WAKE, since we aren't using type-stable storage and we
+      // don't guarantee reuse.  The scenario goes like this: thread
+      // A's last touch of a Baton is a call to wake(), which stores
+      // LATE_DELIVERY and gets an unlucky context switch before delivering
+      // the corresponding futexWake.  Thread B sees LATE_DELIVERY
+      // without consuming a futex event, because it calls futexWait
+      // with an expected value of WAITING and hence doesn't go to sleep.
+      // B returns, so the Baton's memory is reused and becomes another
+      // Baton (or a reuse of this one).  B calls futexWait on the new
+      // Baton lifetime, then A wakes up and delivers a spurious futexWake
+      // to the same memory location.  B's futexWait will then report a
+      // consumed wake event even though state_ is still WAITING.
+      //
+      // It would be possible to add an extra state_ dance to communicate
+      // that the futexWake has been sent so that we can be sure to consume
+      // it before returning, but that would be a perf and complexity hit.
+      uint32_t s = state_.load(std::memory_order_acquire);
+      assert(s == WAITING || s == LATE_DELIVERY);
+
+      if (s == LATE_DELIVERY) {
+        return;
+      }
+      // retry
+    }
+  }
+
+  template <typename Clock, typename Duration>
+  FOLLY_NOINLINE bool tryWaitUntilSlow(
+      const std::chrono::time_point<Clock, Duration>& deadline) {
+    if (spinWaitForEarlyDelivery()) {
+      assert(state_.load(std::memory_order_acquire) == EARLY_DELIVERY);
+      return true;
+    }
+
+    // guess we have to block :(
+    uint32_t expected = INIT;
+    if (!state_.compare_exchange_strong(expected, WAITING)) {
+      // CAS failed, last minute reprieve
+      assert(expected == EARLY_DELIVERY);
+      return true;
+    }
+
+    while (true) {
+      auto rv = state_.futexWaitUntil(WAITING, deadline);
+      if (rv == folly::detail::FutexResult::TIMEDOUT) {
+        state_.store(TIMED_OUT, std::memory_order_release);
+        return false;
+      }
+
+      uint32_t s = state_.load(std::memory_order_acquire);
+      assert(s == WAITING || s == LATE_DELIVERY);
+      if (s == LATE_DELIVERY) {
+        return true;
+      }
+    }
+  }
+

   detail::Futex<Atom> state_;
 };
 
   detail::Futex<Atom> state_;
 };