Make folly::allocator_delete take advantage of EBO

Summary: It wasn't.

Test Plan:
I have a case using this that now passes a static_assert
that it's the same size as the default_deleter version.  I didn't add
a static_assert in folly's tests because right now
is_simple_allocator<int,std::allocator<int>>::value is true (I think
because the tested members come from a base class), which breaks easy
test cases.

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D833142

diff --git a/folly/Memory.h b/folly/Memory.h
index 6ce239f450fc3af43132ca18472228aad09356d2..3be3d78f8c86bca3844056ccbbf08282e786691f 100644 (file)
--- a/folly/Memory.h
+++ b/folly/Memory.h
@@ -42,15 +42,21 @@ std::unique_ptr<T, Dp> make_unique(Args&&... args) {
   return std::unique_ptr<T, Dp>(new T(std::forward<Args>(args)...));
 }
 
-/**
- * Wrap a SimpleAllocator into a STL-compliant allocator.
+/*
+ * StlAllocator wraps a SimpleAllocator into a STL-compliant
+ * allocator, maintaining an instance pointer to the simple allocator
+ * object.  The underlying SimpleAllocator object must outlive all
+ * instances of StlAllocator using it.
+ *
+ * A SimpleAllocator must provide two methods:
  *
- * The SimpleAllocator must provide two methods:
  *    void* allocate(size_t size);
  *    void deallocate(void* ptr);
- * which, respectively, allocate a block of size bytes (aligned to the maximum
- * alignment required on your system), throwing std::bad_alloc if the
- * allocation can't be satisfied, and free a previously allocated block.
+ *
+ * which, respectively, allocate a block of size bytes (aligned to the
+ * maximum alignment required on your system), throwing std::bad_alloc
+ * if the allocation can't be satisfied, and free a previously
+ * allocated block.
  *
  * Note that the following allocator resembles the standard allocator
  * quite well:
@@ -67,11 +73,15 @@ std::unique_ptr<T, Dp> make_unique(Args&&... args) {
  *   }
  * };
  *
+ * But note that if you pass StlAllocator<MallocAllocator,...> to a
+ * standard container it will be larger due to the contained state
+ * pointer.
+ *
  * author: Tudor Bosman <tudorb@fb.com>
  */
 
 // This would be so much simpler with std::allocator_traits, but gcc 4.6.2
-// doesn't support it
+// doesn't support it.
 template <class Alloc, class T> class StlAllocator;
 
 template <class Alloc> class StlAllocator<Alloc, void> {
@@ -177,54 +187,49 @@ class StlAllocator {
  */
 template <typename T, typename Allocator>
 typename Allocator::template rebind<T>::other rebind_allocator(
-  Allocator const &allocator
+  Allocator const& allocator
 ) {
   return typename Allocator::template rebind<T>::other(allocator);
 }
 
 /*
- * Helper classes/functions for creating a unique_ptr using a custom allocator
+ * Helper classes/functions for creating a unique_ptr using a custom
+ * allocator.
  *
  * @author: Marcelo Juchem <marcelo@fb.com>
  */
 
-// A deleter implementation based on std::default_delete,
-// which uses a custom allocator to free memory
+// Derives from the allocator to take advantage of the empty base
+// optimization when possible.
 template <typename Allocator>
-class allocator_delete {
+class allocator_delete
+  : private std::remove_reference<Allocator>::type
+{
   typedef typename std::remove_reference<Allocator>::type allocator_type;
 
 public:
   allocator_delete() = default;
 
-  explicit allocator_delete(const allocator_type& allocator):
-    allocator_(allocator)
+  explicit allocator_delete(const allocator_type& allocator)
+    : allocator_type(allocator)
   {}
 
-  explicit allocator_delete(allocator_type&& allocator):
-    allocator_(std::move(allocator))
+  explicit allocator_delete(allocator_type&& allocator)
+    : allocator_type(std::move(allocator))
   {}
 
   template <typename U>
-  allocator_delete(const allocator_delete<U>& other):
-    allocator_(other.get_allocator())
+  allocator_delete(const allocator_delete<U>& other)
+    : allocator_type(other.get_allocator())
   {}
 
-  allocator_type& get_allocator() const {
-    return allocator_;
-  }
+  allocator_type& get_allocator() const { return *this; }
 
   void operator()(typename allocator_type::pointer p) const {
-    if (!p) {
-      return;
-    }
-
-    allocator_.destroy(p);
-    allocator_.deallocate(p, 1);
+    if (!p) return;
+    const_cast<allocator_delete*>(this)->destroy(p);
+    const_cast<allocator_delete*>(this)->deallocate(p, 1);
   }
-
-private:
-  mutable allocator_type allocator_;
 };
 
 template <typename T, typename Allocator>

diff --git a/folly/test/ArenaSmartPtrTest.cpp b/folly/test/ArenaSmartPtrTest.cpp
index 54b8262a10634990c3bd8e1b1b89faca395b7bd4..0881d706e0788503adef8fb6176846234b09829d 100644 (file)
--- a/folly/test/ArenaSmartPtrTest.cpp
+++ b/folly/test/ArenaSmartPtrTest.cpp
@@ -25,6 +25,11 @@
 
 using namespace folly;
 
+static_assert(
+  is_simple_allocator<int,SysArena>::value,
+  "SysArena should be a simple allocator"
+);
+
 struct global_counter {
   global_counter(): count_(0) {}