From: James Sedgwick Date: Thu, 8 Jan 2015 15:48:23 +0000 (-0800) Subject: exception_ptr -> exception_wrapper migration X-Git-Tag: v0.22.0~22 X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=352bbea652ce674fca3e5d8696215bfc88458753 exception_ptr -> exception_wrapper migration Summary: integrate exception_wrapper everywhere, and deprecate public methods that take ptrs directly to discourage their use note that this will break for throwing non-exceptions, which is probably fine this change opens the door to interesting optimizations for those interested, e.g. Future::then(/* func throwing Exn1 and Exn2 */) that autowraps the given types into the resultant future new benchmark: ``` throwAndCatch 23.69us 42.21K throwAndCatchWrapped 119.53% 19.82us 50.45K throwWrappedAndCatchWrapped 350.16% 6.77us 147.80K ``` Test Plan: existing unit tests, suspected potential perf wins confirmed by benchmark, will wait for windtunnel to see other wins/regressions Reviewed By: hans@fb.com Subscribers: search-fbcode-diffs@, apodsiadlo, alikhtarov, andrii, trunkagent, fugalh, njormrod, folly-diffs@, bmatheny FB internal diff: D1644912 Signature: t1:1644912:1420731849:3dc658dc03bfd6e75d61158808c7dad96092ecfb --- diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h index 2fca5c68..14df3299 100644 --- a/folly/ExceptionWrapper.h +++ b/folly/ExceptionWrapper.h @@ -102,15 +102,50 @@ namespace folly { * */ class exception_wrapper { + protected: + template + struct optimize; + public: - exception_wrapper() : throwfn_(nullptr) { } - - // Implicitly construct an exception_wrapper from any std::exception - template ::value>::type> - /* implicit */ exception_wrapper(T&& exn) { - item_ = std::make_shared(std::forward(exn)); - throwfn_ = folly::detail::Thrower::doThrow; + exception_wrapper() = default; + + // Implicitly construct an exception_wrapper from a qualifying exception. + // See the optimize struct for details. + template ::value>::type> + /* implicit */ exception_wrapper(Ex&& exn) { + item_ = std::make_shared(std::forward(exn)); + throwfn_ = folly::detail::Thrower::doThrow; + } + + // The following two constructors are meant to emulate the behavior of + // try_and_catch in performance sensitive code as well as to be flexible + // enough to wrap exceptions of unknown type. There is an overload that + // takes an exception reference so that the wrapper can extract and store + // the exception's type and what() when possible. + // + // The canonical use case is to construct an all-catching exception wrapper + // with minimal overhead like so: + // + // try { + // // some throwing code + // } catch (const std::exception& e) { + // // won't lose e's type and what() + // exception_wrapper ew{std::current_exception(), e}; + // } catch (...) { + // // everything else + // exception_wrapper ew{std::current_exception()}; + // } + // + // try_and_catch is cleaner and preferable. Use it unless you're sure you need + // something like this instead. + template + explicit exception_wrapper(std::exception_ptr eptr, Ex& exn) { + assign_eptr(eptr, exn); + } + + explicit exception_wrapper(std::exception_ptr eptr) { + assign_eptr(eptr); } void throwException() const { @@ -185,14 +220,41 @@ class exception_wrapper { return false; } + // If this exception wrapper wraps an exception of type Ex, with_exception + // will call f with the wrapped exception as an argument and return true, and + // will otherwise return false. + template + typename std::enable_if< + std::is_base_of::type>::value, + bool>::type + with_exception(F f) { + return with_exception1::type>(f, this); + } + + // Const overload template - bool with_exception(F f) { - return with_exception1(f, this); + typename std::enable_if< + std::is_base_of::type>::value, + bool>::type + with_exception(F f) const { + return with_exception1::type>(f, this); } + // Overload for non-exceptions. Always rethrows. template - bool with_exception(F f) const { - return with_exception1(f, this); + typename std::enable_if< + !std::is_base_of::type>::value, + bool>::type + with_exception(F f) const { + try { + throwException(); + } catch (typename std::decay::type& e) { + f(e); + return true; + } catch (...) { + // fall through + } + return false; } std::exception_ptr getExceptionPtr() const { @@ -209,11 +271,30 @@ class exception_wrapper { } protected: + template + struct optimize { + static const bool value = + std::is_base_of::value && + std::is_copy_assignable::value && + !std::is_abstract::value; + }; + + template + void assign_eptr(std::exception_ptr eptr, Ex& e) { + this->eptr_ = eptr; + this->estr_ = exceptionStr(e).toStdString(); + this->ename_ = demangle(typeid(e)).toStdString(); + } + + void assign_eptr(std::exception_ptr eptr) { + this->eptr_ = eptr; + } + // Optimized case: if we know what type the exception is, we can // store a copy of the concrete type, and a helper function so we // can rethrow it. std::shared_ptr item_; - void (*throwfn_)(std::exception*); + void (*throwfn_)(std::exception*){nullptr}; // Fallback case: store the library wrapper, which is less efficient // but gets the job done. Also store exceptionPtr() the name of the // exception type, so we can at least get those back out without @@ -318,29 +399,14 @@ class try_and_catch : try_and_catch() : Base() {} template - void assign_eptr(Ex& e) { - this->eptr_ = std::current_exception(); - this->estr_ = exceptionStr(e).toStdString(); - this->ename_ = demangle(typeid(e)).toStdString(); - } - - template - struct optimize { - static const bool value = - std::is_base_of::value && - std::is_copy_assignable::value && - !std::is_abstract::value; - }; - - template - typename std::enable_if::value>::type - assign_exception(Ex& e) { - assign_eptr(e); + typename std::enable_if::value>::type + assign_exception(Ex& e, std::exception_ptr eptr) { + exception_wrapper::assign_eptr(eptr, e); } template - typename std::enable_if::value>::type - assign_exception(Ex& e) { + typename std::enable_if::value>::type + assign_exception(Ex& e, std::exception_ptr eptr) { this->item_ = std::make_shared(e); this->throwfn_ = folly::detail::Thrower::doThrow; } @@ -351,9 +417,9 @@ class try_and_catch : Base::call_fn(std::move(fn)); } catch (LastException& e) { if (typeid(e) == typeid(LastException&)) { - assign_exception(e); + assign_exception(e, std::current_exception()); } else { - assign_eptr(e); + exception_wrapper::assign_eptr(std::current_exception(), e); } } } diff --git a/folly/wangle/futures/Future-inl.h b/folly/wangle/futures/Future-inl.h index 2c2f0aa6..a980ff03 100644 --- a/folly/wangle/futures/Future-inl.h +++ b/folly/wangle/futures/Future-inl.h @@ -131,8 +131,8 @@ Future::then(F&& func) { setCallback_( [p, funcm](Try&& t) mutable { p->fulfil([&]() { - return (*funcm)(std::move(t)); - }); + return (*funcm)(std::move(t)); + }); }); return std::move(f); @@ -159,7 +159,7 @@ Future::then(F&& func) { setCallback_( [p, funcm](Try&& t) mutable { if (t.hasException()) { - p->setException(t.getException()); + p->setException(std::move(t.exception())); } else { p->fulfil([&]() { return (*funcm)(std::move(t.value())); @@ -189,7 +189,7 @@ Future::then(F&& func) { setCallback_( [p, funcm](Try&& t) mutable { if (t.hasException()) { - p->setException(t.getException()); + p->setException(std::move(t.exception())); } else { p->fulfil([&]() { return (*funcm)(); @@ -224,10 +224,12 @@ Future::then(F&& func) { auto f2 = (*funcm)(std::move(t)); // that didn't throw, now we can steal p f2.setCallback_([p](Try&& b) mutable { - p->fulfilTry(std::move(b)); - }); + p->fulfilTry(std::move(b)); + }); + } catch (const std::exception& e) { + p->setException(exception_wrapper(std::current_exception(), e)); } catch (...) { - p->setException(std::current_exception()); + p->setException(exception_wrapper(std::current_exception())); } }); @@ -255,15 +257,17 @@ Future::then(F&& func) { setCallback_( [p, funcm](Try&& t) mutable { if (t.hasException()) { - p->setException(t.getException()); + p->setException(std::move(t.exception())); } else { try { auto f2 = (*funcm)(std::move(t.value())); f2.setCallback_([p](Try&& b) mutable { - p->fulfilTry(std::move(b)); - }); + p->fulfilTry(std::move(b)); + }); + } catch (const std::exception& e) { + p->setException(exception_wrapper(std::current_exception(), e)); } catch (...) { - p->setException(std::current_exception()); + p->setException(exception_wrapper(std::current_exception())); } } }); @@ -291,15 +295,17 @@ Future::then(F&& func) { setCallback_( [p, funcm](Try&& t) mutable { if (t.hasException()) { - p->setException(t.getException()); + p->setException(t.exception()); } else { try { auto f2 = (*funcm)(); f2.setCallback_([p](Try&& b) mutable { - p->fulfilTry(std::move(b)); - }); + p->fulfilTry(std::move(b)); + }); + } catch (const std::exception& e) { + p->setException(exception_wrapper(std::current_exception(), e)); } catch (...) { - p->setException(std::current_exception()); + p->setException(exception_wrapper(std::current_exception())); } } }); @@ -329,17 +335,13 @@ Future::onError(F&& func) { auto pm = folly::makeMoveWrapper(std::move(p)); auto funcm = folly::makeMoveWrapper(std::move(func)); setCallback_([pm, funcm](Try&& t) mutable { - try { - t.throwIfFailed(); - } catch (Exn& e) { - pm->fulfil([&]{ - return (*funcm)(e); - }); - return; - } catch (...) { - // fall through + if (!t.template withException([&] (Exn& e) { + pm->fulfil([&]{ + return (*funcm)(e); + }); + })) { + pm->fulfilTry(std::move(t)); } - pm->fulfilTry(std::move(t)); }); return f; @@ -362,22 +364,20 @@ Future::onError(F&& func) { auto pm = folly::makeMoveWrapper(std::move(p)); auto funcm = folly::makeMoveWrapper(std::move(func)); setCallback_([pm, funcm](Try&& t) mutable { - try { - t.throwIfFailed(); - } catch (Exn& e) { - try { - auto f2 = (*funcm)(e); - f2.setCallback_([pm](Try&& t2) mutable { - pm->fulfilTry(std::move(t2)); - }); - } catch (...) { - pm->setException(std::current_exception()); - } - return; - } catch (...) { - // fall through + if (!t.template withException([&] (Exn& e) { + try { + auto f2 = (*funcm)(e); + f2.setCallback_([pm](Try&& t2) mutable { + pm->fulfilTry(std::move(t2)); + }); + } catch (const std::exception& e) { + pm->setException(exception_wrapper(std::current_exception(), e)); + } catch (...) { + pm->setException(exception_wrapper(std::current_exception())); + } + })) { + pm->fulfilTry(std::move(t)); } - pm->fulfilTry(std::move(t)); }); return f; @@ -433,8 +433,8 @@ bool Future::isReady() const { } template -void Future::raise(std::exception_ptr exception) { - core_->raise(exception); +void Future::raise(exception_wrapper exception) { + core_->raise(std::move(exception)); } // makeFuture @@ -483,32 +483,38 @@ Future makeFuture(std::exception_ptr const& e) { return std::move(f); } +template +Future makeFuture(exception_wrapper ew) { + Promise p; + p.setException(std::move(ew)); + return p.getFuture(); +} + template typename std::enable_if::value, Future>::type makeFuture(E const& e) { Promise p; auto f = p.getFuture(); - p.fulfil([&]() -> T { throw e; }); + p.setException(make_exception_wrapper(e)); return std::move(f); } template Future makeFuture(Try&& t) { - try { + if (t.hasException()) { + return makeFuture(std::move(t.exception())); + } else { return makeFuture(std::move(t.value())); - } catch (...) { - return makeFuture(std::current_exception()); } } template <> inline Future makeFuture(Try&& t) { - try { - t.throwIfFailed(); + if (t.hasException()) { + return makeFuture(std::move(t.exception())); + } else { return makeFuture(); - } catch (...) { - return makeFuture(std::current_exception()); } } @@ -730,7 +736,9 @@ namespace { t.value(); p.setException(TimedOut()); } catch (std::exception const& e) { - p.setException(std::current_exception()); + p.setException(exception_wrapper(std::current_exception(), e)); + } catch (...) { + p.setException(exception_wrapper(std::current_exception())); } baton.post(); } @@ -803,8 +811,12 @@ Future Future::within(Duration dur, E e, Timekeeper* tk) { try { t.throwIfFailed(); ctx->promise.setException(std::move(ctx->exception)); - } catch (std::exception const&) { - ctx->promise.setException(std::current_exception()); + } catch (std::exception const& e) { + ctx->promise.setException( + exception_wrapper(std::current_exception(), e)); + } catch (...) { + ctx->promise.setException( + exception_wrapper(std::current_exception())); } } }); diff --git a/folly/wangle/futures/Future.h b/folly/wangle/futures/Future.h index 066b7465..f94a6b7d 100644 --- a/folly/wangle/futures/Future.h +++ b/folly/wangle/futures/Future.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -445,7 +446,8 @@ class Future { template void raise(E&& exception) { - raise(std::make_exception_ptr(std::forward(exception))); + raise(make_exception_wrapper::type>( + std::move(exception))); } /// Raise an interrupt. If the promise holder has an interrupt @@ -456,7 +458,7 @@ class Future { /// preventing the asynchronous operation (if in time), and the promise /// holder setting an exception on the future. (That may happen /// asynchronously, of course.) - void raise(std::exception_ptr interrupt); + void raise(exception_wrapper interrupt); void cancel() { raise(FutureCancellation()); @@ -527,7 +529,11 @@ auto makeFutureTry( /// /// auto f = makeFuture(std::current_exception()); template -Future makeFuture(std::exception_ptr const& e); +Future makeFuture(std::exception_ptr const& e) DEPRECATED; + +/// Make a failed Future from an exception_wrapper. +template +Future makeFuture(exception_wrapper ew); /** Make a Future from an exception type E that can be passed to std::make_exception_ptr(). */ diff --git a/folly/wangle/futures/Promise-inl.h b/folly/wangle/futures/Promise-inl.h index ec1afe17..d7ad250c 100644 --- a/folly/wangle/futures/Promise-inl.h +++ b/folly/wangle/futures/Promise-inl.h @@ -78,19 +78,31 @@ Future Promise::getFuture() { template template -void Promise::setException(E const& e) { - setException(std::make_exception_ptr(e)); +typename std::enable_if::value>::type +Promise::setException(E const& e) { + setException(make_exception_wrapper(e)); } template void Promise::setException(std::exception_ptr const& e) { + try { + std::rethrow_exception(e); + } catch (const std::exception& e) { + setException(exception_wrapper(std::current_exception(), e)); + } catch (...) { + setException(exception_wrapper(std::current_exception())); + } +} + +template +void Promise::setException(exception_wrapper ew) { throwIfFulfilled(); - core_->setResult(Try(e)); + core_->setResult(Try(std::move(ew))); } template void Promise::setInterruptHandler( - std::function fn) { + std::function fn) { core_->setInterruptHandler(std::move(fn)); } diff --git a/folly/wangle/futures/Promise.h b/folly/wangle/futures/Promise.h index 7d8609bc..af324a9d 100644 --- a/folly/wangle/futures/Promise.h +++ b/folly/wangle/futures/Promise.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include @@ -42,6 +43,9 @@ public: once, thereafter Future already retrieved exception will be raised. */ Future getFuture(); + /** Fulfil the Promise with an exception_wrapper */ + void setException(exception_wrapper ew); + /** Fulfil the Promise with an exception_ptr, e.g. try { ... @@ -49,20 +53,22 @@ public: p.setException(std::current_exception()); } */ - void setException(std::exception_ptr const&); + void setException(std::exception_ptr const&) DEPRECATED; /** Fulfil the Promise with an exception type E, which can be passed to std::make_exception_ptr(). Useful for originating exceptions. If you - caught an exception the exception_ptr form is more appropriate. + caught an exception the exception_wrapper form is more appropriate. */ - template void setException(E const&); + template + typename std::enable_if::value>::type + setException(E const&); /// Set an interrupt handler to handle interrupts. See the documentation for /// Future::raise(). Your handler can do whatever it wants, but if you /// bother to set one then you probably will want to fulfil the promise with /// an exception (or special value) indicating how the interrupt was /// handled. - void setInterruptHandler(std::function); + void setInterruptHandler(std::function); /** Fulfil this Promise (only for Promise) */ void setValue(); diff --git a/folly/wangle/futures/Try-inl.h b/folly/wangle/futures/Try-inl.h index 44f2cea4..63a005bd 100644 --- a/folly/wangle/futures/Try-inl.h +++ b/folly/wangle/futures/Try-inl.h @@ -27,7 +27,7 @@ Try::Try(Try&& t) : contains_(t.contains_) { if (contains_ == Contains::VALUE) { new (&value_)T(std::move(t.value_)); } else if (contains_ == Contains::EXCEPTION) { - new (&e_)std::exception_ptr(t.e_); + new (&e_)std::unique_ptr(std::move(t.e_)); } } @@ -38,7 +38,7 @@ Try& Try::operator=(Try&& t) { if (contains_ == Contains::VALUE) { new (&value_)T(std::move(t.value_)); } else if (contains_ == Contains::EXCEPTION) { - new (&e_)std::exception_ptr(t.e_); + new (&e_)std::unique_ptr(std::move(t.e_)); } return *this; } @@ -48,7 +48,7 @@ Try::~Try() { if (contains_ == Contains::VALUE) { value_.~T(); } else if (contains_ == Contains::EXCEPTION) { - e_.~exception_ptr(); + e_.~unique_ptr(); } } @@ -68,7 +68,7 @@ template void Try::throwIfFailed() const { if (contains_ != Contains::VALUE) { if (contains_ == Contains::EXCEPTION) { - std::rethrow_exception(e_); + e_->throwException(); } else { throw UsingUninitializedTry(); } @@ -77,7 +77,7 @@ void Try::throwIfFailed() const { void Try::throwIfFailed() const { if (!hasValue_) { - std::rethrow_exception(e_); + e_->throwException(); } } @@ -97,10 +97,11 @@ typename std::enable_if< makeTryFunction(F&& f) { typedef typename std::result_of::type ResultType; try { - auto value = f(); - return Try(std::move(value)); + return Try(f()); + } catch (std::exception& e) { + return Try(exception_wrapper(std::current_exception(), e)); } catch (...) { - return Try(std::current_exception()); + return Try(exception_wrapper(std::current_exception())); } } @@ -112,8 +113,10 @@ makeTryFunction(F&& f) { try { f(); return Try(); + } catch (std::exception& e) { + return Try(exception_wrapper(std::current_exception(), e)); } catch (...) { - return Try(std::current_exception()); + return Try(exception_wrapper(std::current_exception())); } } diff --git a/folly/wangle/futures/Try.h b/folly/wangle/futures/Try.h index a90aa4d5..a1101d37 100644 --- a/folly/wangle/futures/Try.h +++ b/folly/wangle/futures/Try.h @@ -19,7 +19,10 @@ #include #include #include +#include #include +#include +#include #include namespace folly { namespace wangle { @@ -41,7 +44,19 @@ class Try { Try() : contains_(Contains::NOTHING) {} explicit Try(const T& v) : contains_(Contains::VALUE), value_(v) {} explicit Try(T&& v) : contains_(Contains::VALUE), value_(std::move(v)) {} - explicit Try(std::exception_ptr e) : contains_(Contains::EXCEPTION), e_(e) {} + explicit Try(exception_wrapper e) + : contains_(Contains::EXCEPTION), + e_(folly::make_unique(std::move(e))) {} + explicit Try(std::exception_ptr ep) DEPRECATED + : contains_(Contains::EXCEPTION) { + try { + std::rethrow_exception(ep); + } catch (const std::exception& e) { + e_ = folly::make_unique(std::current_exception(), e); + } catch (...) { + e_ = folly::make_unique(std::current_exception()); + } + } // move Try(Try&& t); @@ -67,19 +82,31 @@ class Try { bool hasValue() const { return contains_ == Contains::VALUE; } bool hasException() const { return contains_ == Contains::EXCEPTION; } - std::exception_ptr getException() const { + template + bool hasException() const { + return hasException() && e_->is_compatible_with(); + } + + exception_wrapper& exception() { if (UNLIKELY(!hasException())) { - throw WangleException( - "getException(): Try does not contain an exception"); + throw WangleException("exception(): Try does not contain an exception"); } - return e_; + return *e_; + } + + template + bool withException(F func) const { + if (!hasException()) { + return false; + } + return e_->with_exception(std::move(func)); } private: Contains contains_; union { T value_; - std::exception_ptr e_; + std::unique_ptr e_; }; }; @@ -87,7 +114,29 @@ template <> class Try { public: Try() : hasValue_(true) {} - explicit Try(std::exception_ptr e) : hasValue_(false), e_(e) {} + explicit Try(exception_wrapper e) + : hasValue_(false), + e_(folly::make_unique(std::move(e))) {} + explicit Try(std::exception_ptr ep) DEPRECATED : hasValue_(false) { + try { + std::rethrow_exception(ep); + } catch (const std::exception& e) { + e_ = folly::make_unique(std::current_exception(), e); + } catch (...) { + e_ = folly::make_unique(std::current_exception()); + } + } + + Try& operator=(const Try& t) { + hasValue_ = t.hasValue_; + if (t.e_) { + e_ = folly::make_unique(*t.e_); + } + return *this; + } + Try(const Try& t) { + *this = t; + } void value() const { throwIfFailed(); } void operator*() const { return value(); } @@ -97,17 +146,29 @@ class Try { bool hasValue() const { return hasValue_; } bool hasException() const { return !hasValue_; } - std::exception_ptr getException() const { + template + bool hasException() const { + return hasException() && e_->is_compatible_with(); + } + + exception_wrapper& exception() { if (UNLIKELY(!hasException())) { - throw WangleException( - "getException(): Try does not contain an exception"); + throw WangleException("exception(): Try does not contain an exception"); + } + return *e_; + } + + template + bool withException(F func) const { + if (!hasException()) { + return false; } - return e_; + return e_->with_exception(std::move(func)); } private: bool hasValue_; - std::exception_ptr e_; + std::unique_ptr e_{nullptr}; }; /** diff --git a/folly/wangle/futures/detail/Core.h b/folly/wangle/futures/detail/Core.h index cd9e5af5..ae081027 100644 --- a/folly/wangle/futures/detail/Core.h +++ b/folly/wangle/futures/detail/Core.h @@ -132,7 +132,7 @@ class Core : protected FSM { // Called by a destructing Promise void detachPromise() { if (!ready()) { - setResult(Try(std::make_exception_ptr(BrokenPromise()))); + setResult(Try(exception_wrapper(BrokenPromise()))); } detachOne(); } @@ -154,18 +154,18 @@ class Core : protected FSM { executor_ = x; } - void raise(std::exception_ptr const& e) { + void raise(exception_wrapper const& e) { FSM_START case State::Interruptible: FSM_UPDATE2(State::Interrupted, - [&]{ interrupt_ = e; }, - [&]{ interruptHandler_(interrupt_); }); + [&]{ interrupt_ = folly::make_unique(e); }, + [&]{ interruptHandler_(*interrupt_); }); break; case State::Waiting: case State::Interrupted: FSM_UPDATE(State::Interrupted, - [&]{ interrupt_ = e; }); + [&]{ interrupt_ = folly::make_unique(e); }); break; case State::Done: @@ -173,7 +173,7 @@ class Core : protected FSM { FSM_END } - void setInterruptHandler(std::function fn) { + void setInterruptHandler(std::function fn) { FSM_START case State::Waiting: case State::Interruptible: @@ -182,7 +182,7 @@ class Core : protected FSM { break; case State::Interrupted: - fn(interrupt_); + fn(*interrupt_); FSM_BREAK case State::Done: @@ -228,8 +228,8 @@ class Core : protected FSM { std::atomic detached_ {0}; std::atomic active_ {true}; std::atomic executor_ {nullptr}; - std::exception_ptr interrupt_; - std::function interruptHandler_; + std::unique_ptr interrupt_; + std::function interruptHandler_; }; template diff --git a/folly/wangle/futures/test/Benchmark.cpp b/folly/wangle/futures/test/Benchmark.cpp index 863fa95e..dc811ce3 100644 --- a/folly/wangle/futures/test/Benchmark.cpp +++ b/folly/wangle/futures/test/Benchmark.cpp @@ -165,6 +165,132 @@ BENCHMARK_RELATIVE(contention) { producer.join(); } +BENCHMARK_DRAW_LINE(); + +// The old way. Throw an exception, and rethrow to access it upstream. +void throwAndCatchImpl() { + makeFuture() + .then([](Try&&){ throw std::runtime_error("oh no"); }) + .then([](Try&& t) { + try { + t.value(); + } catch(const std::runtime_error& e) { + // ... + return; + } + CHECK(false); + }); +} + +// Not much better. Throw an exception, and access it via the wrapper upstream. +// Actually a little worse due to wrapper overhead. then() won't know that the +// exception is a runtime_error, so will have to store it as an exception_ptr +// anyways. withException will therefore have to rethrow. Note that if we threw +// std::exception instead, we would see some wins, as that's the type then() +// will try to wrap, so no exception_ptrs/rethrows are necessary. +void throwAndCatchWrappedImpl() { + makeFuture() + .then([](Try&&){ throw std::runtime_error("oh no"); }) + .then([](Try&& t) { + auto caught = t.withException( + [](const std::runtime_error& e){ + // ... + }); + CHECK(caught); + }); +} + +// Better. Wrap an exception, and rethrow to access it upstream. +void throwWrappedAndCatchImpl() { + makeFuture() + .then([](Try&&){ + return makeFuture(std::runtime_error("oh no")); + }) + .then([](Try&& t) { + try { + t.value(); + } catch(const std::runtime_error& e) { + // ... + return; + } + CHECK(false); + }); +} + +// The new way. Wrap an exception, and access it via the wrapper upstream +void throwWrappedAndCatchWrappedImpl() { + makeFuture() + .then([](Try&&){ + return makeFuture(std::runtime_error("oh no")); + }) + .then([](Try&& t){ + auto caught = t.withException( + [](const std::runtime_error& e){ + // ... + }); + CHECK(caught); + }); +} + +// Simulate heavy contention on func +void contend(void(*func)()) { + folly::BenchmarkSuspender s; + const int N = 100; + const int iters = 1000; + pthread_barrier_t barrier; + pthread_barrier_init(&barrier, nullptr, N+1); + std::vector threads; + for (int i = 0; i < N; i++) { + threads.push_back(std::thread([&](){ + pthread_barrier_wait(&barrier); + for (int j = 0; j < iters; j++) { + func(); + } + })); + } + pthread_barrier_wait(&barrier); + s.dismiss(); + for (auto& t : threads) { + t.join(); + } + s.rehire(); + pthread_barrier_destroy(&barrier); +} + +BENCHMARK(throwAndCatch) { + throwAndCatchImpl(); +} + +BENCHMARK_RELATIVE(throwAndCatchWrapped) { + throwAndCatchWrappedImpl(); +} + +BENCHMARK_RELATIVE(throwWrappedAndCatch) { + throwWrappedAndCatchImpl(); +} + +BENCHMARK_RELATIVE(throwWrappedAndCatchWrapped) { + throwWrappedAndCatchWrappedImpl(); +} + +BENCHMARK_DRAW_LINE(); + +BENCHMARK(throwAndCatchContended) { + contend(throwAndCatchImpl); +} + +BENCHMARK_RELATIVE(throwAndCatchWrappedContended) { + contend(throwAndCatchWrappedImpl); +} + +BENCHMARK_RELATIVE(throwWrappedAndCatchContended) { + contend(throwWrappedAndCatchImpl); +} + +BENCHMARK_RELATIVE(throwWrappedAndCatchWrappedContended) { + contend(throwWrappedAndCatchWrappedImpl); +} + int main(int argc, char** argv) { gflags::ParseCommandLineFlags(&argc, &argv, true); folly::runBenchmarks(); diff --git a/folly/wangle/futures/test/FutureTest.cpp b/folly/wangle/futures/test/FutureTest.cpp index 07f24621..939ff2eb 100644 --- a/folly/wangle/futures/test/FutureTest.cpp +++ b/folly/wangle/futures/test/FutureTest.cpp @@ -500,7 +500,7 @@ TEST(Promise, setException) { try { throw eggs; } catch (...) { - p.setException(std::current_exception()); + p.setException(exception_wrapper(std::current_exception())); } EXPECT_THROW(f.value(), eggs_t); } diff --git a/folly/wangle/futures/test/Interrupts.cpp b/folly/wangle/futures/test/Interrupts.cpp index 3ecf050d..9204efaf 100644 --- a/folly/wangle/futures/test/Interrupts.cpp +++ b/folly/wangle/futures/test/Interrupts.cpp @@ -20,20 +20,21 @@ #include using namespace folly::wangle; +using folly::exception_wrapper; TEST(Interrupts, raise) { std::runtime_error eggs("eggs"); Promise p; - p.setInterruptHandler([&](std::exception_ptr e) { - EXPECT_THROW(std::rethrow_exception(e), decltype(eggs)); + p.setInterruptHandler([&](const exception_wrapper& e) { + EXPECT_THROW(e.throwException(), decltype(eggs)); }); p.getFuture().raise(eggs); } TEST(Interrupts, cancel) { Promise p; - p.setInterruptHandler([&](std::exception_ptr e) { - EXPECT_THROW(std::rethrow_exception(e), FutureCancellation); + p.setInterruptHandler([&](const exception_wrapper& e) { + EXPECT_THROW(e.throwException(), FutureCancellation); }); p.getFuture().cancel(); } @@ -41,7 +42,7 @@ TEST(Interrupts, cancel) { TEST(Interrupts, handleThenInterrupt) { Promise p; bool flag = false; - p.setInterruptHandler([&](std::exception_ptr e) { flag = true; }); + p.setInterruptHandler([&](const exception_wrapper& e) { flag = true; }); p.getFuture().cancel(); EXPECT_TRUE(flag); } @@ -50,14 +51,14 @@ TEST(Interrupts, interruptThenHandle) { Promise p; bool flag = false; p.getFuture().cancel(); - p.setInterruptHandler([&](std::exception_ptr e) { flag = true; }); + p.setInterruptHandler([&](const exception_wrapper& e) { flag = true; }); EXPECT_TRUE(flag); } TEST(Interrupts, interruptAfterFulfilNoop) { Promise p; bool flag = false; - p.setInterruptHandler([&](std::exception_ptr e) { flag = true; }); + p.setInterruptHandler([&](const exception_wrapper& e) { flag = true; }); p.setValue(); p.getFuture().cancel(); EXPECT_FALSE(flag); @@ -66,7 +67,7 @@ TEST(Interrupts, interruptAfterFulfilNoop) { TEST(Interrupts, secondInterruptNoop) { Promise p; int count = 0; - p.setInterruptHandler([&](std::exception_ptr e) { count++; }); + p.setInterruptHandler([&](const exception_wrapper& e) { count++; }); auto f = p.getFuture(); f.cancel(); f.cancel(); diff --git a/folly/wangle/futures/test/Try.cpp b/folly/wangle/futures/test/Try.cpp index 2c8a6068..9b2f4f74 100644 --- a/folly/wangle/futures/test/Try.cpp +++ b/folly/wangle/futures/test/Try.cpp @@ -38,7 +38,7 @@ TEST(Try, makeTryFunctionThrow) { }; auto result = makeTryFunction(func); - EXPECT_TRUE(result.hasException()); + EXPECT_TRUE(result.hasException()); } TEST(Try, makeTryFunctionVoid) { @@ -57,5 +57,5 @@ TEST(Try, makeTryFunctionVoidThrow) { }; auto result = makeTryFunction(func); - EXPECT_TRUE(result.hasException()); + EXPECT_TRUE(result.hasException()); }