kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [1/2] kudu git commit: arena: fix alignment of NewObject<> results, use variadic template
Date Tue, 28 Mar 2017 04:23:34 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 58e1d26d8 -> 473221e20


arena: fix alignment of NewObject<> results, use variadic template

This makes two fixes to Arena<>::NewObject<>:

* Ensures that the returned object has the proper alignment required by
  the type being allocated. This fixes spurious leak warnings if the
  leak checker is run while there is a live object allocated from an
  arena which has pointers pointing out it. I hit this issue with a
  later patch in this same patch series which changes RowOp to be
  arena-allocated.

* Use C++11 variadic templates instead of multiple definitions for
  different parameter list lengths.

Change-Id: Idad12bee02eebf58bf8d545812a7d161bb829b71
Reviewed-on: http://gerrit.cloudera.org:8080/6492
Reviewed-by: Adar Dembo <adar@cloudera.com>
Tested-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a51c9b02
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a51c9b02
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a51c9b02

Branch: refs/heads/master
Commit: a51c9b0237dc7719e8103e33389ada5072d9631a
Parents: 58e1d26
Author: Todd Lipcon <todd@apache.org>
Authored: Sun Mar 26 18:03:46 2017 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Tue Mar 28 04:11:32 2017 +0000

----------------------------------------------------------------------
 src/kudu/util/memory/arena-test.cc | 12 ++++++++
 src/kudu/util/memory/arena.h       | 54 ++++++---------------------------
 2 files changed, 22 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/a51c9b02/src/kudu/util/memory/arena-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/memory/arena-test.cc b/src/kudu/util/memory/arena-test.cc
index c39b73a..fc3a64d 100644
--- a/src/kudu/util/memory/arena-test.cc
+++ b/src/kudu/util/memory/arena-test.cc
@@ -101,6 +101,18 @@ TEST(TestArena, TestAlignment) {
   }
 }
 
+TEST(TestArena, TestObjectAlignment) {
+  struct MyStruct {
+    int64_t v;
+  };
+  Arena a(256, 256 * 1024);
+  // Allocate a junk byte to ensure that the next allocation isn't "accidentally" aligned.
+  a.AllocateBytes(1);
+  void* v = a.NewObject<MyStruct>();
+  ASSERT_EQ(reinterpret_cast<uintptr_t>(v) % alignof(MyStruct), 0);
+}
+
+
 // MemTrackers update their ancestors when consuming and releasing memory to compute
 // usage totals. However, the lifetimes of parent and child trackers can be different.
 // Validate that child trackers can still correctly update their parent stats even when

http://git-wip-us.apache.org/repos/asf/kudu/blob/a51c9b02/src/kudu/util/memory/arena.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/memory/arena.h b/src/kudu/util/memory/arena.h
index d192a56..18c304d 100644
--- a/src/kudu/util/memory/arena.h
+++ b/src/kudu/util/memory/arena.h
@@ -107,21 +107,12 @@ class ArenaBase {
   // Same as above.
   void * AddBytes(const void *data, size_t len);
 
-  // Handy wrapper for placement-new
-  template<class T>
-  T *NewObject();
-
-  // Handy wrapper for placement-new
-  template<class T, typename A1>
-  T *NewObject(A1 arg1);
-
-  // Handy wrapper for placement-new
-  template<class T, typename A1, typename A2>
-  T *NewObject(A1 arg1, A2 arg2);
-
-  // Handy wrapper for placement-new
-  template<class T, typename A1, typename A2, typename A3>
-  T *NewObject(A1 arg1, A2 arg2, A3 arg3);
+  // Handy wrapper for placement-new.
+  //
+  // This ensures that the returned object is properly aligned based on
+  // alignof(T).
+  template<class T, typename ... Args>
+  T *NewObject(Args&&... args);
 
   // Relocate the given Slice into the arena, setting 'dst' and
   // returning true if successful.
@@ -474,36 +465,11 @@ inline bool ArenaBase<THREADSAFE>::RelocateStringPiece(const StringPiece&
src, S
 }
 
 template<bool THREADSAFE>
-template<class T>
-inline T *ArenaBase<THREADSAFE>::NewObject() {
-  void *mem = AllocateBytes(sizeof(T));
-  if (mem == NULL) throw std::bad_alloc();
-  return new (mem) T();
-}
-
-template<bool THREADSAFE>
-template<class T, typename A1>
-inline T *ArenaBase<THREADSAFE>::NewObject(A1 arg1) {
-  void *mem = AllocateBytes(sizeof(T));
-  if (mem == NULL) throw std::bad_alloc();
-  return new (mem) T(arg1);
-}
-
-
-template<bool THREADSAFE>
-template<class T, typename A1, typename A2>
-inline T *ArenaBase<THREADSAFE>::NewObject(A1 arg1, A2 arg2) {
-  void *mem = AllocateBytes(sizeof(T));
-  if (mem == NULL) throw std::bad_alloc();
-  return new (mem) T(arg1, arg2);
-}
-
-template<bool THREADSAFE>
-template<class T, typename A1, typename A2, typename A3>
-inline T *ArenaBase<THREADSAFE>::NewObject(A1 arg1, A2 arg2, A3 arg3) {
-  void *mem = AllocateBytes(sizeof(T));
+template<class T, class ... Args>
+inline T *ArenaBase<THREADSAFE>::NewObject(Args&&... args) {
+  void *mem = AllocateBytesAligned(sizeof(T), alignof(T));
   if (mem == NULL) throw std::bad_alloc();
-  return new (mem) T(arg1, arg2, arg3);
+  return new (mem) T(std::forward<Args>(args)...);
 }
 
 }  // namespace kudu


Mime
View raw message