folly::fibers::Baton API consistency with folly::Baton
authorYedidya Feldblum <yfeldblum@fb.com>
Tue, 12 Dec 2017 00:26:17 +0000 (16:26 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Tue, 12 Dec 2017 00:37:31 +0000 (16:37 -0800)
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

folly/fibers/Baton-inl.h
folly/fibers/Baton.cpp
folly/fibers/Baton.h
folly/fibers/TimedMutex-inl.h
folly/fibers/test/FibersTest.cpp

index fe9d45a..6c0ae1e 100644 (file)
@@ -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
index 733147a..baaa15b 100644 (file)
@@ -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);
index 768da60..d2b5885 100644 (file)
@@ -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,
index de5621b..dd363b8 100644 (file)
@@ -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
index e12a78a..c9e555a 100644 (file)
@@ -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));
         }
       }
     });