From 0593c2242fcbf86010f9399e5655186e9534e71a Mon Sep 17 00:00:00 2001 From: Hannes Roth Date: Thu, 14 May 2015 11:55:27 -0700 Subject: [PATCH] (Wangle) chain -> thenMulti + thenMultiWithExecutor Summary: If we make `chain` a member function we can avoid the type issues and infer everything. I also added thenMulti for symmetry. Sadly the compiler doesn't like having a thenMulti with an optional `Executor*` as the first argument, it fails after some deductions. Hence `thenMultiWithExecutor`. itssobeautiful Test Plan: Run all the tests. Reviewed By: hans@fb.com Subscribers: trunkagent, folly-diffs@, jsedgwick, yfeldblum, chalfant FB internal diff: D2021000 Signature: t1:2021000:1431557618:169447dc9d747b23a8a1ba830e78c43713d09a96 --- folly/futures/Future-inl.h | 59 ++++++++++++++++++++++------------ folly/futures/Future.h | 36 +++++++++++++++++++++ folly/futures/helpers.h | 13 -------- folly/futures/test/ViaTest.cpp | 32 ++++++++++++++++-- 4 files changed, 103 insertions(+), 37 deletions(-) diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h index f17eabfa..aecfa578 100644 --- a/folly/futures/Future-inl.h +++ b/folly/futures/Future-inl.h @@ -1018,30 +1018,47 @@ Future Future::filter(F predicate) { }); } -namespace futures { - namespace { - template - Future chainHelper(Future f) { - return f; - } +template +template +auto Future::thenMulti(Callback&& fn) + -> decltype(this->then(std::forward(fn))) { + // thenMulti with one callback is just a then + return then(std::forward(fn)); +} - template - Future chainHelper(F f, Fn fn, Callbacks... fns) { - return chainHelper(f.then(fn), fns...); - } - } +template +template +auto Future::thenMulti(Callback&& fn, Callbacks&&... fns) + -> decltype(this->then(std::forward(fn)). + thenMulti(std::forward(fns)...)) { + // thenMulti with two callbacks is just then(a).thenMulti(b, ...) + return then(std::forward(fn)). + thenMulti(std::forward(fns)...); +} - template - std::function(Try)> - chain(Callbacks... fns) { - MoveWrapper> pw; - MoveWrapper> fw(chainHelper(pw->getFuture(), fns...)); - return [=](Try t) mutable { - pw->setTry(std::move(t)); - return std::move(*fw); - }; - } +template +template +auto Future::thenMultiWithExecutor(Executor* x, Callback&& fn, + Callbacks&&... fns) + -> decltype(this->then(std::forward(fn)). + thenMulti(std::forward(fns)...)) { + // thenMultiExecutor with two callbacks is + // via(x).then(a).thenMulti(b, ...).via(oldX) + auto oldX = getExecutor(); + setExecutor(x); + return then(std::forward(fn)). + thenMulti(std::forward(fns)...).via(oldX); +} +template +template +auto Future::thenMultiWithExecutor(Executor* x, Callback&& fn) + -> decltype(this->then(std::forward(fn))) { + // thenMulti with one callback is just a then with an executor + return then(x, std::forward(fn)); +} + +namespace futures { template std::vector> map(It first, It last, F func) { std::vector> results; diff --git a/folly/futures/Future.h b/folly/futures/Future.h index ccea88b3..4f5db68e 100644 --- a/folly/futures/Future.h +++ b/folly/futures/Future.h @@ -380,6 +380,42 @@ class Future { template Future reduce(I&& initial, F&& func); + /// Create a Future chain from a sequence of callbacks. i.e. + /// + /// f.then(a).then(b).then(c) + /// + /// where f is a Future and the result of the chain is a Future + /// becomes + /// + /// f.thenMulti(a, b, c); + template + auto thenMulti(Callback&& fn, Callbacks&&... fns) + -> decltype(this->then(std::forward(fn)). + thenMulti(std::forward(fns)...)); + + // Nothing to see here, just thenMulti's base case + template + auto thenMulti(Callback&& fn) + -> decltype(this->then(std::forward(fn))); + + /// Create a Future chain from a sequence of callbacks. i.e. + /// + /// f.via(executor).then(a).then(b).then(c).via(oldExecutor) + /// + /// where f is a Future and the result of the chain is a Future + /// becomes + /// + /// f.thenMultiWithExecutor(executor, a, b, c); + template + auto thenMultiWithExecutor(Executor* x, Callback&& fn, Callbacks&&... fns) + -> decltype(this->then(std::forward(fn)). + thenMulti(std::forward(fns)...)); + + // Nothing to see here, just thenMultiWithExecutor's base case + template + auto thenMultiWithExecutor(Executor* x, Callback&& fn) + -> decltype(this->then(std::forward(fn))); + protected: typedef detail::Core* corePtr; diff --git a/folly/futures/helpers.h b/folly/futures/helpers.h index ded42702..775989e8 100644 --- a/folly/futures/helpers.h +++ b/folly/futures/helpers.h @@ -39,19 +39,6 @@ namespace futures { /// Futures you will pay no Timekeeper thread overhead. Future sleep(Duration, Timekeeper* = nullptr); - /// Create a Future chain from a sequence of callbacks. i.e. - /// - /// f.then(a).then(b).then(c); - /// - /// where f is a Future and the result of the chain is a Future - /// becomes - /// - /// f.then(chain(a, b, c)); - // If anyone figures how to get chain to deduce A and Z, I'll buy you a drink. - template - std::function(Try)> - chain(Callbacks... fns); - /** * Set func as the callback for each input Future and return a vector of * Futures containing the results in the input order. diff --git a/folly/futures/test/ViaTest.cpp b/folly/futures/test/ViaTest.cpp index 061dc4c4..edaed774 100644 --- a/folly/futures/test/ViaTest.cpp +++ b/folly/futures/test/ViaTest.cpp @@ -171,16 +171,16 @@ TEST_F(ViaFixture, viaAssignment) { TEST(Via, chain1) { EXPECT_EQ(42, makeFuture() - .then(futures::chain([] { return 42; })) + .thenMulti([] { return 42; }) .get()); } TEST(Via, chain3) { int count = 0; - auto f = makeFuture().then(futures::chain( + auto f = makeFuture().thenMulti( [&]{ count++; return 3.14159; }, [&](double) { count++; return std::string("hello"); }, - [&]{ count++; return makeFuture(42); })); + [&]{ count++; return makeFuture(42); }); EXPECT_EQ(42, f.get()); EXPECT_EQ(3, count); } @@ -228,6 +228,32 @@ TEST(Via, priority) { EXPECT_EQ(3, exe.count2); } +TEST_F(ViaFixture, chainX1) { + EXPECT_EQ(42, + makeFuture() + .thenMultiWithExecutor(eastExecutor.get(),[] { return 42; }) + .get()); +} + +TEST_F(ViaFixture, chainX3) { + auto westThreadId = std::this_thread::get_id(); + int count = 0; + auto f = via(westExecutor.get()).thenMultiWithExecutor( + eastExecutor.get(), + [&]{ + EXPECT_NE(std::this_thread::get_id(), westThreadId); + count++; return 3.14159; + }, + [&](double) { count++; return std::string("hello"); }, + [&]{ count++; }) + .then([&](){ + EXPECT_EQ(std::this_thread::get_id(), westThreadId); + return makeFuture(42); + }); + EXPECT_EQ(42, f.getVia(waiter.get())); + EXPECT_EQ(3, count); +} + TEST(Via, then2) { ManualExecutor x1, x2; bool a = false, b = false, c = false; -- 2.34.1