IMPALA-4752: make ObjectPool more efficient Previously it was implemented as a vector of pointers to dynamically allocated wrapper objects, where each wrapper object stored a pointer to the object and a pointer to a vtable, which had a pointer to a destructor, which then calls the actual object's destructor.
Instead of doing this, this patch changes ObjectPool to stores the object pointer and function pointer inline in the vector. This avoids an unnecessary malloc() and free() pair per object. Testing: Ran core tests, which should exercise this heavily during query execution. Change-Id: I1e6a40ac798fa64da4767f530c48710cba623ea5 Reviewed-on: http://gerrit.cloudera.org:8080/5666 Reviewed-by: Tim Armstrong <[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/c4f2458e Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c4f2458e Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c4f2458e Branch: refs/heads/master Commit: c4f2458e394cbba793e65ffeebac09e948b6198c Parents: b989efb Author: Tim Armstrong <[email protected]> Authored: Tue Jan 10 09:26:15 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Fri Jan 13 03:14:49 2017 +0000 ---------------------------------------------------------------------- be/src/common/object-pool.h | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c4f2458e/be/src/common/object-pool.h ---------------------------------------------------------------------- diff --git a/be/src/common/object-pool.h b/be/src/common/object-pool.h index 4463fd8..6375d34 100644 --- a/be/src/common/object-pool.h +++ b/be/src/common/object-pool.h @@ -23,6 +23,7 @@ #include <boost/thread/locks.hpp> #include <boost/thread/mutex.hpp> +#include "gutil/macros.h" #include "util/spinlock.h" namespace impala { @@ -32,47 +33,36 @@ namespace impala { /// Thread-safe. class ObjectPool { public: - ObjectPool(): objects_() {} - + ObjectPool() {} ~ObjectPool() { Clear(); } template <class T> T* Add(T* t) { - // Create the object to be pushed to the shared vector outside the critical section. // TODO: Consider using a lock-free structure. - SpecificElement<T>* obj = new SpecificElement<T>(t); - DCHECK(obj != NULL); boost::lock_guard<SpinLock> l(lock_); - objects_.push_back(obj); + objects_.emplace_back( + Element{t, [](void* obj) { delete reinterpret_cast<T*>(obj); }}); return t; } void Clear() { boost::lock_guard<SpinLock> l(lock_); - for (ElementVector::iterator i = objects_.begin(); - i != objects_.end(); ++i) { - delete *i; - } + for (Element& elem : objects_) elem.delete_fn(elem.obj); objects_.clear(); } private: - struct GenericElement { - virtual ~GenericElement() {} - }; + DISALLOW_COPY_AND_ASSIGN(ObjectPool); - template <class T> - struct SpecificElement : GenericElement { - SpecificElement(T* t): t(t) {} - ~SpecificElement() { - delete t; - } + /// A generic deletion function pointer. Deletes its first argument. + typedef void (*DeleteFn)(void*); - T* t; + /// For each object, a pointer to the object and a function that deletes it. + struct Element { + void* obj; + DeleteFn delete_fn; }; - - typedef std::vector<GenericElement*> ElementVector; - ElementVector objects_; + std::vector<Element> objects_; SpinLock lock_; };
