Commit 75d20258 authored by Lee Howes's avatar Lee Howes Committed by Facebook Github Bot

Change return type of futures::sleep to SemiFuture

Summary:
futures::sleep returning a Future leads to continuations easily being run on
the Timekeeper's callback. The goal is to change the return type so that
futures::sleep returns a folly::SemiFuture.

Reviewed By: yfeldblum

Differential Revision: D14069549

fbshipit-source-id: b98e2c76ccbf0a80aed28f90a1f3c63f15eb2e5c
parent 268ab2a1
...@@ -77,7 +77,7 @@ class TimedWaitAwaitable { ...@@ -77,7 +77,7 @@ class TimedWaitAwaitable {
bool await_suspend(std::experimental::coroutine_handle<> ch) { bool await_suspend(std::experimental::coroutine_handle<> ch) {
auto sharedState = std::make_shared<SharedState>(ch, storage_); auto sharedState = std::make_shared<SharedState>(ch, storage_);
waitAndNotify(std::move(awaitable_), sharedState).detach(); waitAndNotify(std::move(awaitable_), sharedState).detach();
futures::sleep(duration_).thenValue( futures::sleepUnsafe(duration_).thenValue(
[sharedState = std::move(sharedState)](Unit) { [sharedState = std::move(sharedState)](Unit) {
sharedState->setTimeout(); sharedState->setTimeout();
}); });
......
...@@ -253,8 +253,9 @@ TEST(Coro, CurrentExecutor) { ...@@ -253,8 +253,9 @@ TEST(Coro, CurrentExecutor) {
} }
coro::Task<void> taskTimedWait() { coro::Task<void> taskTimedWait() {
auto ex = co_await coro::co_current_executor;
auto fastFuture = auto fastFuture =
futures::sleep(std::chrono::milliseconds{50}).thenValue([](Unit) { futures::sleep(std::chrono::milliseconds{50}).via(ex).thenValue([](Unit) {
return 42; return 42;
}); });
auto fastResult = co_await coro::timed_wait( auto fastResult = co_await coro::timed_wait(
...@@ -267,7 +268,7 @@ coro::Task<void> taskTimedWait() { ...@@ -267,7 +268,7 @@ coro::Task<void> taskTimedWait() {
}; };
auto throwingFuture = auto throwingFuture =
futures::sleep(std::chrono::milliseconds{50}).thenValue([](Unit) { futures::sleep(std::chrono::milliseconds{50}).via(ex).thenValue([](Unit) {
throw ExpectedException(); throw ExpectedException();
}); });
EXPECT_THROW( EXPECT_THROW(
...@@ -275,14 +276,23 @@ coro::Task<void> taskTimedWait() { ...@@ -275,14 +276,23 @@ coro::Task<void> taskTimedWait() {
std::move(throwingFuture), std::chrono::milliseconds{100}), std::move(throwingFuture), std::chrono::milliseconds{100}),
ExpectedException); ExpectedException);
auto promiseFuturePair = folly::makePromiseContract<folly::Unit>(ex);
auto lifetimeFuture = std::move(promiseFuturePair.second);
auto slowFuture = auto slowFuture =
futures::sleep(std::chrono::milliseconds{200}).thenValue([](Unit) { futures::sleep(std::chrono::milliseconds{200})
return 42; .via(ex)
}); .thenValue([lifetimePromise =
std::move(promiseFuturePair.first)](Unit) mutable {
lifetimePromise.setValue();
return 42;
});
auto slowResult = co_await coro::timed_wait( auto slowResult = co_await coro::timed_wait(
std::move(slowFuture), std::chrono::milliseconds{100}); std::move(slowFuture), std::chrono::milliseconds{100});
EXPECT_FALSE(slowResult); EXPECT_FALSE(slowResult);
// Ensure that task completes for safe executor lifetimes
(void)co_await std::move(lifetimeFuture);
co_return; co_return;
} }
......
...@@ -39,7 +39,7 @@ template class Future<double>; ...@@ -39,7 +39,7 @@ template class Future<double>;
namespace folly { namespace folly {
namespace futures { namespace futures {
Future<Unit> sleep(Duration dur, Timekeeper* tk) { SemiFuture<Unit> sleep(Duration dur, Timekeeper* tk) {
std::shared_ptr<Timekeeper> tks; std::shared_ptr<Timekeeper> tks;
if (LIKELY(!tk)) { if (LIKELY(!tk)) {
tks = folly::detail::getTimekeeperSingleton(); tks = folly::detail::getTimekeeperSingleton();
...@@ -47,14 +47,14 @@ Future<Unit> sleep(Duration dur, Timekeeper* tk) { ...@@ -47,14 +47,14 @@ Future<Unit> sleep(Duration dur, Timekeeper* tk) {
} }
if (UNLIKELY(!tk)) { if (UNLIKELY(!tk)) {
return makeFuture<Unit>(FutureNoTimekeeper()); return makeSemiFuture<Unit>(FutureNoTimekeeper());
} }
return tk->after(dur); return tk->after(dur);
} }
Future<Unit> sleepUnsafe(Duration dur, Timekeeper* tk) { Future<Unit> sleepUnsafe(Duration dur, Timekeeper* tk) {
return sleep(dur, tk); return sleep(dur, tk).toUnsafeFuture();
} }
} // namespace futures } // namespace futures
......
...@@ -181,7 +181,8 @@ retryingPolicyCappedJitteredExponentialBackoff( ...@@ -181,7 +181,8 @@ retryingPolicyCappedJitteredExponentialBackoff(
} }
auto backoff = detail::retryingJitteredExponentialBackoffDur( auto backoff = detail::retryingJitteredExponentialBackoffDur(
n, backoff_min, backoff_max, jitter_param, rngp); n, backoff_min, backoff_max, jitter_param, rngp);
return futures::sleep(backoff).thenValue([](auto&&) { return true; }); return futures::sleep(backoff).toUnsafeFuture().thenValue(
[](auto&&) { return true; });
}); });
}; };
} }
......
...@@ -45,7 +45,7 @@ namespace futures { ...@@ -45,7 +45,7 @@ namespace futures {
/// The Timekeeper thread will be lazily created the first time it is /// The Timekeeper thread will be lazily created the first time it is
/// needed. If your program never uses any timeouts or other time-based /// needed. If your program never uses any timeouts or other time-based
/// Futures you will pay no Timekeeper thread overhead. /// Futures you will pay no Timekeeper thread overhead.
Future<Unit> sleep(Duration, Timekeeper* = nullptr); SemiFuture<Unit> sleep(Duration, Timekeeper* = nullptr);
Future<Unit> sleepUnsafe(Duration, Timekeeper* = nullptr); Future<Unit> sleepUnsafe(Duration, Timekeeper* = nullptr);
/** /**
......
...@@ -1061,7 +1061,6 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) { ...@@ -1061,7 +1061,6 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) {
TEST(SemiFuture, DeferWithNestedSemiFuture) { TEST(SemiFuture, DeferWithNestedSemiFuture) {
auto start = std::chrono::steady_clock::now(); auto start = std::chrono::steady_clock::now();
auto future = futures::sleep(std::chrono::milliseconds{100}) auto future = futures::sleep(std::chrono::milliseconds{100})
.semi()
.deferValue([](auto&&) { .deferValue([](auto&&) {
return futures::sleep(std::chrono::milliseconds{200}); return futures::sleep(std::chrono::milliseconds{200});
}); });
......
...@@ -320,8 +320,9 @@ TEST(Timekeeper, interruptDoesntCrash) { ...@@ -320,8 +320,9 @@ TEST(Timekeeper, interruptDoesntCrash) {
TEST(Timekeeper, chainedInterruptTest) { TEST(Timekeeper, chainedInterruptTest) {
bool test = false; bool test = false;
auto f = auto f = futures::sleep(milliseconds(100)).deferValue([&](auto&&) {
futures::sleep(milliseconds(100)).thenValue([&](auto&&) { test = true; }); test = true;
});
f.cancel(); f.cancel();
f.wait(); f.wait();
EXPECT_FALSE(test); EXPECT_FALSE(test);
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include <folly/executors/InlineExecutor.h> #include <folly/executors/InlineExecutor.h>
#include <folly/futures/Future.h> #include <folly/futures/Future.h>
#include <folly/futures/test/TestExecutor.h>
#include <folly/io/async/EventBase.h> #include <folly/io/async/EventBase.h>
#include <folly/portability/GTest.h> #include <folly/portability/GTest.h>
#include <folly/synchronization/Baton.h> #include <folly/synchronization/Baton.h>
...@@ -272,7 +273,8 @@ TEST(Wait, waitWithDuration) { ...@@ -272,7 +273,8 @@ TEST(Wait, waitWithDuration) {
} }
TEST(Wait, multipleWait) { TEST(Wait, multipleWait) {
auto f = futures::sleep(milliseconds(100)); folly::TestExecutor executor(1);
auto f = futures::sleep(milliseconds(100)).via(&executor);
for (size_t i = 0; i < 5; ++i) { for (size_t i = 0; i < 5; ++i) {
EXPECT_FALSE(f.isReady()); EXPECT_FALSE(f.isReady());
f.wait(milliseconds(3)); f.wait(milliseconds(3));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment