From dce6e23d30d9330889bb5fb83c2c4b7383fe21fb Mon Sep 17 00:00:00 2001 From: James Sedgwick Date: Wed, 10 Jun 2015 16:29:08 -0700 Subject: [PATCH] optimize makeFuture and Future::Future() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Summary: No reason to go through the whole Promise rigamarole. Add an appropriate Core ctor and use that to make a completed Future with just the core alloc Note the big win in the `constantFuture` benchmark. ``` Before: ============================================================================ folly/futures/test/Benchmark.cpp relative time/iter iters/s ============================================================================ constantFuture 120.50ns 8.30M promiseAndFuture 91.99% 130.98ns 7.63M withThen 28.17% 427.77ns 2.34M ---------------------------------------------------------------------------- oneThen 430.48ns 2.32M twoThens 58.03% 741.86ns 1.35M fourThens 31.85% 1.35us 739.97K hundredThens 1.61% 26.80us 37.32K ---------------------------------------------------------------------------- no_contention 4.58ms 218.48 contention 83.70% 5.47ms 182.86 ---------------------------------------------------------------------------- throwAndCatch 8.09us 123.55K throwAndCatchWrapped 94.43% 8.57us 116.67K throwWrappedAndCatch 154.69% 5.23us 191.12K throwWrappedAndCatchWrapped 614.06% 1.32us 758.70K ---------------------------------------------------------------------------- throwAndCatchContended 967.54ms 1.03 throwAndCatchWrappedContended 103.48% 935.04ms 1.07 throwWrappedAndCatchContended 148.24% 652.70ms 1.53 throwWrappedAndCatchWrappedContended 14313.28% 6.76ms 147.94 ============================================================================ After: ============================================================================ folly/futures/test/Benchmark.cpp relative time/iter iters/s ============================================================================ constantFuture 69.11ns 14.47M promiseAndFuture 55.12% 125.37ns 7.98M withThen 16.49% 419.18ns 2.39M ---------------------------------------------------------------------------- oneThen 370.39ns 2.70M twoThens 55.11% 672.05ns 1.49M fourThens 29.00% 1.28us 782.89K hundredThens 1.23% 30.22us 33.09K ---------------------------------------------------------------------------- no_contention 4.56ms 219.46 contention 82.82% 5.50ms 181.77 ---------------------------------------------------------------------------- throwAndCatch 8.30us 120.42K throwAndCatchWrapped 96.40% 8.61us 116.08K throwWrappedAndCatch 162.66% 5.11us 195.89K throwWrappedAndCatchWrapped 680.39% 1.22us 819.36K ---------------------------------------------------------------------------- throwAndCatchContended 979.17ms 1.02 throwAndCatchWrappedContended 103.09% 949.84ms 1.05 throwWrappedAndCatchContended 153.55% 637.71ms 1.57 throwWrappedAndCatchWrappedContended 10468.47% 9.35ms 106.91 ============================================================================ ``` Reviewed By: @fugalh, @​hannesr Differential Revision: D2144664 --- folly/futures/Future-inl.h | 58 ++++++++------------------------ folly/futures/Future.h | 3 ++ folly/futures/detail/Core.h | 6 ++++ folly/futures/test/Benchmark.cpp | 2 -- 4 files changed, 23 insertions(+), 46 deletions(-) diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h index 4b2c7477..0b2a301c 100644 --- a/folly/futures/Future-inl.h +++ b/folly/futures/Future-inl.h @@ -45,22 +45,16 @@ Future& Future::operator=(Future&& other) noexcept { template template -Future::Future(T2&& val) : core_(nullptr) { - Promise p; - p.setValue(std::forward(val)); - *this = p.getFuture(); -} +Future::Future(T2&& val) + : core_(new detail::Core(Try(std::forward(val)))) {} template template ::value, int>::type> -Future::Future() : core_(nullptr) { - Promise p; - p.setValue(); - *this = p.getFuture(); -} +Future::Future() + : core_(new detail::Core(Try())) {} template @@ -456,16 +450,12 @@ void Future::raise(exception_wrapper exception) { template Future::type> makeFuture(T&& t) { - Promise::type> p; - p.setValue(std::forward(t)); - return p.getFuture(); + return makeFuture(Try::type>(std::forward(t))); } inline // for multiple translation units Future makeFuture() { - Promise p; - p.setValue(); - return p.getFuture(); + return makeFuture(Try()); } template @@ -473,57 +463,37 @@ auto makeFutureWith( F&& func, typename std::enable_if::value, bool>::type sdf) -> Future { - Promise p; - p.setWith( - [&func]() { - return (func)(); - }); - return p.getFuture(); + return makeFuture(makeTryWith([&func]() { + return (func)(); + })); } template auto makeFutureWith(F const& func) -> Future { F copy = func; - return makeFutureWith(std::move(copy)); + return makeFuture(makeTryWith(std::move(copy))); } template Future makeFuture(std::exception_ptr const& e) { - Promise p; - p.setException(e); - return p.getFuture(); + return makeFuture(Try(e)); } template Future makeFuture(exception_wrapper ew) { - Promise p; - p.setException(std::move(ew)); - return p.getFuture(); + return makeFuture(Try(std::move(ew))); } template typename std::enable_if::value, Future>::type makeFuture(E const& e) { - Promise p; - p.setException(make_exception_wrapper(e)); - return p.getFuture(); + return makeFuture(Try(make_exception_wrapper(e))); } template Future makeFuture(Try&& t) { - Promise::type> p; - p.setTry(std::move(t)); - return p.getFuture(); -} - -template <> -inline Future makeFuture(Try&& t) { - if (t.hasException()) { - return makeFuture(std::move(t.exception())); - } else { - return makeFuture(); - } + return Future(new detail::Core(std::move(t))); } // via diff --git a/folly/futures/Future.h b/folly/futures/Future.h index 4f5db68e..40687b42 100644 --- a/folly/futures/Future.h +++ b/folly/futures/Future.h @@ -432,6 +432,9 @@ class Future { friend class Promise; template friend class Future; + template + friend Future makeFuture(Try&&); + // Variant: returns a value // e.g. f.then([](Try t){ return t.value(); }); template diff --git a/folly/futures/detail/Core.h b/folly/futures/detail/Core.h index de0173e7..27842b55 100644 --- a/folly/futures/detail/Core.h +++ b/folly/futures/detail/Core.h @@ -79,6 +79,12 @@ class Core { /// code but since this is just internal detail code and I don't know how /// off-hand, I'm punting. Core() {} + + explicit Core(Try&& t) + : fsm_(State::OnlyResult), + attached_(1), + result_(std::move(t)) {} + ~Core() { assert(attached_ == 0); } diff --git a/folly/futures/test/Benchmark.cpp b/folly/futures/test/Benchmark.cpp index 54771b1e..386e8b9d 100644 --- a/folly/futures/test/Benchmark.cpp +++ b/folly/futures/test/Benchmark.cpp @@ -46,7 +46,6 @@ BENCHMARK(constantFuture) { makeFuture(42); } -// This shouldn't get too far below 100% BENCHMARK_RELATIVE(promiseAndFuture) { Promise p; Future f = p.getFuture(); @@ -54,7 +53,6 @@ BENCHMARK_RELATIVE(promiseAndFuture) { f.value(); } -// The higher the better. At the time of writing, it's only about 40% :( BENCHMARK_RELATIVE(withThen) { Promise p; Future f = p.getFuture().then(incr); -- 2.34.1