Repository: incubator-impala Updated Branches: refs/heads/master b8b64e110 -> 1495b2007
Avoid std::function when possible. std::function does some tricky stuff under the hood, and can in fact throw in its constructor when it allocates heap memory. This patch changes ScopeExitTrigger to use templates instead, avoiding the complexity of std::function. It uses a std::make_pair-like constructor helper so clients don't have to try to name the types of lambdas. Change-Id: If9dc034c2e094ea7f87f78d8d9101a71d8d2e295 Reviewed-on: http://gerrit.cloudera.org:8080/5230 Reviewed-by: Jim Apple <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/730ac1ba Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/730ac1ba Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/730ac1ba Branch: refs/heads/master Commit: 730ac1bae1ecee03837fb7298e8bdc23ddfd8638 Parents: b8b64e1 Author: Jim Apple <[email protected]> Authored: Fri Nov 25 23:49:26 2016 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Mon Dec 5 22:37:36 2016 +0000 ---------------------------------------------------------------------- be/src/service/fe-support.cc | 3 ++- be/src/util/scope-exit-trigger.h | 25 ++++++++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/730ac1ba/be/src/service/fe-support.cc ---------------------------------------------------------------------- diff --git a/be/src/service/fe-support.cc b/be/src/service/fe-support.cc index b3e3d0c..9483ccc 100644 --- a/be/src/service/fe-support.cc +++ b/be/src/service/fe-support.cc @@ -98,7 +98,8 @@ Java_org_apache_impala_service_FeSupport_NativeEvalConstExprs( query_ctx.request.query_options.max_errors = 1; RuntimeState state(query_ctx); // Make sure to close the runtime state no matter how this scope is exited. - ScopeExitTrigger close_runtime_state([&state]() { state.ReleaseResources(); }); + const auto close_runtime_state = + MakeScopeExitTrigger([&state]() { state.ReleaseResources(); }); THROW_IF_ERROR_RET(jni_frame.push(env), env, JniUtil::internal_exc_class(), result_bytes); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/730ac1ba/be/src/util/scope-exit-trigger.h ---------------------------------------------------------------------- diff --git a/be/src/util/scope-exit-trigger.h b/be/src/util/scope-exit-trigger.h index bc2b702..79e482e 100644 --- a/be/src/util/scope-exit-trigger.h +++ b/be/src/util/scope-exit-trigger.h @@ -18,23 +18,38 @@ #ifndef IMPALA_UTIL_SCOPE_EXIT_TRIGGER_H #define IMPALA_UTIL_SCOPE_EXIT_TRIGGER_H -#include <functional> - namespace impala { /// Utility class that calls a client-supplied function when it is destroyed. /// /// Use judiciously - scope exits can be hard to reason about, and this class should not /// act as proxy for work-performing d'tors, which we try to avoid. +template <typename T> +class ScopeExitTrigger; + +// The only way to construct ScopeExitTriggers is with MakeScopeExitTrigger, to enable +// lambdas, which have indescribable types, to be the contained trigger, like +// +// const auto foo = MakeScopeExitTrigger([](){}); +template <typename T> +ScopeExitTrigger<T> MakeScopeExitTrigger(const T& trigger); + +template <typename T> class ScopeExitTrigger { public: - ScopeExitTrigger(const std::function<void()>& trigger) : trigger_(trigger) {} - ~ScopeExitTrigger() { trigger_(); } + friend ScopeExitTrigger<T> MakeScopeExitTrigger<>(const T& trigger); + private: - std::function<void()> trigger_; + explicit ScopeExitTrigger(const T& trigger) : trigger_(trigger) {} + T trigger_; }; + +template <typename T> +ScopeExitTrigger<T> MakeScopeExitTrigger(const T& trigger) { + return ScopeExitTrigger<T>(trigger); +} } #endif
