folly::fibers::Baton API consistency with folly::Baton

Summary:
[Folly] `folly::fibers::Baton` API consistency with `folly::Baton`.

Specifically, the suite of `wait`, `try_wait`, `try_wait_for`, and `try_wait_until` members and member templates.

Hold onto the `timed_wait` members for now, but mark them deprecated.

Additionally, for consistency, offer main-context function params consistently for all `try_wait_for`, `try_wait_until`, and both variants of `timed_wait`.

Reviewed By: andriigrynenko

Differential Revision: D6531145

fbshipit-source-id: 960fba48716b12b0ef53262786eacab88d8b2375

diff --git a/folly/fibers/Baton-inl.h b/folly/fibers/Baton-inl.h
index fe9d45a0d852b4d7ffb03c29cd5ec55c3268d190..6c0ae1ec7ca2ec443b12abf644def2f8a67ab7fd 100644 (file)
--- a/folly/fibers/Baton-inl.h
+++ b/folly/fibers/Baton-inl.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -66,9 +66,9 @@ void Baton::waitFiber(FiberManager& fm, F&& mainContextFunc) {
   fm.activeFiber_->preempt(Fiber::AWAITING);
 }
 
-template <typename F>
-bool Baton::timed_wait(
-    TimeoutController::Duration timeout,
+template <typename Rep, typename Period, typename F>
+bool Baton::try_wait_for(
+    const std::chrono::duration<Rep, Period>& timeout,
     F&& mainContextFunc) {
   auto fm = FiberManager::getFiberManagerUnsafe();
 
@@ -87,7 +87,7 @@ bool Baton::timed_wait(
   auto id =
       fm->timeoutManager_->registerTimeout(std::ref(timeoutFunc), timeout);
 
-  waitFiber(*fm, std::move(mainContextFunc));
+  waitFiber(*fm, static_cast<F&&>(mainContextFunc));
 
   auto posted = waitingFiber_ == POSTED;
 
@@ -98,15 +98,16 @@ bool Baton::timed_wait(
   return posted;
 }
 
-template <typename C, typename D>
-bool Baton::timed_wait(const std::chrono::time_point<C, D>& timeout) {
-  auto now = C::now();
+template <typename Clock, typename Duration, typename F>
+bool Baton::try_wait_until(
+    const std::chrono::time_point<Clock, Duration>& deadline,
+    F&& mainContextFunc) {
+  auto now = Clock::now();
 
-  if (LIKELY(now <= timeout)) {
-    return timed_wait(
-        std::chrono::duration_cast<std::chrono::milliseconds>(timeout - now));
+  if (LIKELY(now <= deadline)) {
+    return try_wait_for(deadline - now, static_cast<F&&>(mainContextFunc));
   } else {
-    return timed_wait(TimeoutController::Duration(0));
+    return try_wait_for(Duration{}, static_cast<F&&>(mainContextFunc));
   }
 }
 } // namespace fibers

diff --git a/folly/fibers/Baton.cpp b/folly/fibers/Baton.cpp
index 733147af2cf469c1adaf5bc5e91ed0ed1813a739..baaa15b0ae4cd6695b73758482ea5dceff7cc87e 100644 (file)
--- a/folly/fibers/Baton.cpp
+++ b/folly/fibers/Baton.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -41,10 +41,6 @@ void Baton::wait(TimeoutHandler& timeoutHandler) {
   timeoutHandler.cancelTimeout();
 }
 
-bool Baton::timed_wait(TimeoutController::Duration timeout) {
-  return timed_wait(timeout, []() {});
-}
-
 void Baton::waitThread() {
   if (spinWaitForEarlyPost()) {
     assert(waitingFiber_.load(std::memory_order_acquire) == POSTED);

diff --git a/folly/fibers/Baton.h b/folly/fibers/Baton.h
index 768da60f9a9068f8e550e49e637a6fef586cf0e8..d2b5885fd9dfe61f48a2523d015944214fe492a6 100644 (file)
--- a/folly/fibers/Baton.h
+++ b/folly/fibers/Baton.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -64,38 +64,111 @@ class Baton {
   void wait(F&& mainContextFunc);
 
   /**
-   * This is here only not break tao/locks. Please don't use it, because it is
-   * inefficient when used on Fibers.
+   * Checks if the baton has been posted without blocking.
+   *
+   * @return    true iff the baton has been posted.
    */
-  template <typename C, typename D = typename C::duration>
-  bool timed_wait(const std::chrono::time_point<C, D>& timeout);
+  bool try_wait();
 
   /**
-   * Puts active fiber to sleep. Returns when post is called.
+   * Puts active fiber to sleep. Returns when post is called or the timeout
+   * expires.
    *
-   * @param timeout Baton will be automatically awaken if timeout is hit
+   * @param timeout Baton will be automatically awaken if timeout expires
    *
    * @return true if was posted, false if timeout expired
    */
-  bool timed_wait(TimeoutController::Duration timeout);
+  template <typename Rep, typename Period>
+  bool try_wait_for(const std::chrono::duration<Rep, Period>& timeout) {
+    return try_wait_for(timeout, [] {});
+  }
 
   /**
-   * Puts active fiber to sleep. Returns when post is called.
+   * Puts active fiber to sleep. Returns when post is called or the timeout
+   * expires.
    *
-   * @param timeout Baton will be automatically awaken if timeout is hit
+   * @param timeout Baton will be automatically awaken if timeout expires
    * @param mainContextFunc this function is immediately executed on the main
    *        context.
    *
    * @return true if was posted, false if timeout expired
    */
-  template <typename F>
-  bool timed_wait(TimeoutController::Duration timeout, F&& mainContextFunc);
+  template <typename Rep, typename Period, typename F>
+  bool try_wait_for(
+      const std::chrono::duration<Rep, Period>& timeout,
+      F&& mainContextFunc);
 
   /**
-   * Checks if the baton has been posted without blocking.
-   * @return    true iff the baton has been posted.
+   * Puts active fiber to sleep. Returns when post is called or the deadline
+   * expires.
+   *
+   * @param timeout Baton will be automatically awaken if deadline expires
+   *
+   * @return true if was posted, false if timeout expired
    */
-  bool try_wait();
+  template <typename Clock, typename Duration>
+  bool try_wait_until(
+      const std::chrono::time_point<Clock, Duration>& deadline) {
+    return try_wait_until(deadline, [] {});
+  }
+
+  /**
+   * Puts active fiber to sleep. Returns when post is called or the deadline
+   * expires.
+   *
+   * @param timeout Baton will be automatically awaken if deadline expires
+   * @param mainContextFunc this function is immediately executed on the main
+   *        context.
+   *
+   * @return true if was posted, false if timeout expired
+   */
+  template <typename Clock, typename Duration, typename F>
+  bool try_wait_until(
+      const std::chrono::time_point<Clock, Duration>& deadline,
+      F&& mainContextFunc);
+
+  /**
+   * Puts active fiber to sleep. Returns when post is called or the deadline
+   * expires.
+   *
+   * @param timeout Baton will be automatically awaken if deadline expires
+   * @param mainContextFunc this function is immediately executed on the main
+   *        context.
+   *
+   * @return true if was posted, false if timeout expired
+   */
+  template <typename Clock, typename Duration, typename F>
+  bool try_wait_for(
+      const std::chrono::time_point<Clock, Duration>& deadline,
+      F&& mainContextFunc);
+
+  /// Alias to try_wait_for. Deprecated.
+  template <typename Rep, typename Period>
+  bool timed_wait(const std::chrono::duration<Rep, Period>& timeout) {
+    return try_wait_for(timeout);
+  }
+
+  /// Alias to try_wait_for. Deprecated.
+  template <typename Rep, typename Period, typename F>
+  bool timed_wait(
+      const std::chrono::duration<Rep, Period>& timeout,
+      F&& mainContextFunc) {
+    return try_wait_for(timeout, static_cast<F&&>(mainContextFunc));
+  }
+
+  /// Alias to try_wait_until. Deprecated.
+  template <typename Clock, typename Duration>
+  bool timed_wait(const std::chrono::time_point<Clock, Duration>& deadline) {
+    return try_wait_until(deadline);
+  }
+
+  /// Alias to try_wait_until. Deprecated.
+  template <typename Clock, typename Duration, typename F>
+  bool timed_wait(
+      const std::chrono::time_point<Clock, Duration>& deadline,
+      F&& mainContextFunc) {
+    return try_wait_until(deadline, static_cast<F&&>(mainContextFunc));
+  }
 
   /**
    * Wakes up Fiber which was waiting on this Baton (or if no Fiber is waiting,

diff --git a/folly/fibers/TimedMutex-inl.h b/folly/fibers/TimedMutex-inl.h
index de5621bd598e164ae38c7dae733e4d4fe9cd059f..dd363b8d4607dfcb8730659f361f56b984625f82 100644 (file)
--- a/folly/fibers/TimedMutex-inl.h
+++ b/folly/fibers/TimedMutex-inl.h
@@ -95,7 +95,7 @@ template <typename Rep, typename Period>
 bool TimedMutex::timed_lock(
     const std::chrono::duration<Rep, Period>& duration) {
   auto result = lockHelper([&](MutexWaiter& waiter) {
-    if (!waiter.baton.timed_wait(duration)) {
+    if (!waiter.baton.try_wait_for(duration)) {
       // We timed out. Two cases:
       // 1. We're still in the waiter list and we truly timed out
       // 2. We're not in the waiter list anymore. This could happen if the baton
@@ -182,7 +182,7 @@ bool TimedRWMutex<BatonType>::timed_read_lock(
     read_waiters_.push_back(waiter);
     ulock.unlock();
 
-    if (!waiter.baton.timed_wait(duration)) {
+    if (!waiter.baton.try_wait_for(duration)) {
       // We timed out. Two cases:
       // 1. We're still in the waiter list and we truly timed out
       // 2. We're not in the waiter list anymore. This could happen if the baton
@@ -248,7 +248,7 @@ bool TimedRWMutex<BatonType>::timed_write_lock(
   write_waiters_.push_back(waiter);
   ulock.unlock();
 
-  if (!waiter.baton.timed_wait(duration)) {
+  if (!waiter.baton.try_wait_for(duration)) {
     // We timed out. Two cases:
     // 1. We're still in the waiter list and we truly timed out
     // 2. We're not in the waiter list anymore. This could happen if the baton

diff --git a/folly/fibers/test/FibersTest.cpp b/folly/fibers/test/FibersTest.cpp
index e12a78a96dae2d97b192299bb31f2a69957174ec..c9e555a72498e21ef17e4bb5006652e27b680ca2 100644 (file)
--- a/folly/fibers/test/FibersTest.cpp
+++ b/folly/fibers/test/FibersTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -53,7 +53,7 @@ TEST(FiberManager, batonTimedWaitTimeout) {
       manager.addTask([&]() {
         Baton baton;
 
-        auto res = baton.timed_wait(std::chrono::milliseconds(230));
+        auto res = baton.try_wait_for(std::chrono::milliseconds(230));
 
         EXPECT_FALSE(res);
         EXPECT_EQ(5, iterations);
@@ -63,7 +63,7 @@ TEST(FiberManager, batonTimedWaitTimeout) {
       manager.addTask([&]() {
         Baton baton;
 
-        auto res = baton.timed_wait(std::chrono::milliseconds(130));
+        auto res = baton.try_wait_for(std::chrono::milliseconds(130));
 
         EXPECT_FALSE(res);
         EXPECT_EQ(3, iterations);
@@ -95,7 +95,7 @@ TEST(FiberManager, batonTimedWaitPost) {
         Baton baton;
         baton_ptr = &baton;
 
-        auto res = baton.timed_wait(std::chrono::milliseconds(130));
+        auto res = baton.try_wait_for(std::chrono::milliseconds(130));
 
         EXPECT_TRUE(res);
         EXPECT_EQ(2, iterations);
@@ -128,7 +128,7 @@ TEST(FiberManager, batonTimedWaitTimeoutEvb) {
     Baton baton;
 
     auto start = EventBaseLoopController::Clock::now();
-    auto res = baton.timed_wait(std::chrono::milliseconds(timeout_ms));
+    auto res = baton.try_wait_for(std::chrono::milliseconds(timeout_ms));
     auto finish = EventBaseLoopController::Clock::now();
 
     EXPECT_FALSE(res);
@@ -170,7 +170,7 @@ TEST(FiberManager, batonTimedWaitPostEvb) {
       evb.tryRunAfterDelay([&]() { baton.post(); }, 100);
 
       auto start = EventBaseLoopController::Clock::now();
-      auto res = baton.timed_wait(std::chrono::milliseconds(130));
+      auto res = baton.try_wait_for(std::chrono::milliseconds(130));
       auto finish = EventBaseLoopController::Clock::now();
 
       EXPECT_TRUE(res);
@@ -2057,14 +2057,14 @@ TEST(FiberManager, VirtualEventBase) {
 
     getFiberManager(*evb1).addTaskRemote([&] {
       Baton baton;
-      baton.timed_wait(std::chrono::milliseconds{100});
+      baton.try_wait_for(std::chrono::milliseconds{100});
 
       done1 = true;
     });
 
     getFiberManager(evb2).addTaskRemote([&] {
       Baton baton;
-      baton.timed_wait(std::chrono::milliseconds{200});
+      baton.try_wait_for(std::chrono::milliseconds{200});
 
       done2 = true;
     });
@@ -2089,7 +2089,7 @@ TEST(TimedMutex, ThreadsAndFibersDontDeadlock) {
       mutex.unlock();
       {
         Baton b;
-        b.timed_wait(std::chrono::milliseconds(1));
+        b.try_wait_for(std::chrono::milliseconds(1));
       }
     }
   });
@@ -2100,12 +2100,12 @@ TEST(TimedMutex, ThreadsAndFibersDontDeadlock) {
         mutex.lock();
         {
           Baton b;
-          b.timed_wait(std::chrono::milliseconds(1));
+          b.try_wait_for(std::chrono::milliseconds(1));
         }
         mutex.unlock();
         {
           Baton b;
-          b.timed_wait(std::chrono::milliseconds(1));
+          b.try_wait_for(std::chrono::milliseconds(1));
         }
       }
     });