From a279ea6be94882796226128e48da4b431e80ab17 Mon Sep 17 00:00:00 2001 From: Yedidya Feldblum Date: Mon, 11 Dec 2017 16:26:17 -0800 Subject: [PATCH] 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 --- folly/fibers/Baton-inl.h | 25 ++++---- folly/fibers/Baton.cpp | 6 +- folly/fibers/Baton.h | 103 ++++++++++++++++++++++++++----- folly/fibers/TimedMutex-inl.h | 6 +- folly/fibers/test/FibersTest.cpp | 22 +++---- 5 files changed, 116 insertions(+), 46 deletions(-) diff --git a/folly/fibers/Baton-inl.h b/folly/fibers/Baton-inl.h index fe9d45a0..6c0ae1ec 100644 --- 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 -bool Baton::timed_wait( - TimeoutController::Duration timeout, +template +bool Baton::try_wait_for( + const std::chrono::duration& 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(mainContextFunc)); auto posted = waitingFiber_ == POSTED; @@ -98,15 +98,16 @@ bool Baton::timed_wait( return posted; } -template -bool Baton::timed_wait(const std::chrono::time_point& timeout) { - auto now = C::now(); +template +bool Baton::try_wait_until( + const std::chrono::time_point& deadline, + F&& mainContextFunc) { + auto now = Clock::now(); - if (LIKELY(now <= timeout)) { - return timed_wait( - std::chrono::duration_cast(timeout - now)); + if (LIKELY(now <= deadline)) { + return try_wait_for(deadline - now, static_cast(mainContextFunc)); } else { - return timed_wait(TimeoutController::Duration(0)); + return try_wait_for(Duration{}, static_cast(mainContextFunc)); } } } // namespace fibers diff --git a/folly/fibers/Baton.cpp b/folly/fibers/Baton.cpp index 733147af..baaa15b0 100644 --- 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 768da60f..d2b5885f 100644 --- 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 - bool timed_wait(const std::chrono::time_point& 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 + bool try_wait_for(const std::chrono::duration& 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 - bool timed_wait(TimeoutController::Duration timeout, F&& mainContextFunc); + template + bool try_wait_for( + const std::chrono::duration& 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 + bool try_wait_until( + const std::chrono::time_point& 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 + bool try_wait_until( + const std::chrono::time_point& 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 + bool try_wait_for( + const std::chrono::time_point& deadline, + F&& mainContextFunc); + + /// Alias to try_wait_for. Deprecated. + template + bool timed_wait(const std::chrono::duration& timeout) { + return try_wait_for(timeout); + } + + /// Alias to try_wait_for. Deprecated. + template + bool timed_wait( + const std::chrono::duration& timeout, + F&& mainContextFunc) { + return try_wait_for(timeout, static_cast(mainContextFunc)); + } + + /// Alias to try_wait_until. Deprecated. + template + bool timed_wait(const std::chrono::time_point& deadline) { + return try_wait_until(deadline); + } + + /// Alias to try_wait_until. Deprecated. + template + bool timed_wait( + const std::chrono::time_point& deadline, + F&& mainContextFunc) { + return try_wait_until(deadline, static_cast(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 de5621bd..dd363b8d 100644 --- a/folly/fibers/TimedMutex-inl.h +++ b/folly/fibers/TimedMutex-inl.h @@ -95,7 +95,7 @@ template bool TimedMutex::timed_lock( const std::chrono::duration& 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::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::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 e12a78a9..c9e555a7 100644 --- 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)); } } }); -- 2.34.1