Devirtualize base::BindState to save 1% of Chrome's binary size (1MB)

Every call to base::Bind results in a new class declaration. Since
base::internal::BindState is a virtual class, this also results in a
bespoke vtable being defined. And that's a lot of vtables. Like 1MB
worth of vtables, or 1% of Chrome's binary size.

However, this vtable's sole purpose is for RefCountedThreadSafe::
Release(). Release() _does_ need to know precisely how to delete the
BindState, but it doesn't need a vtable to do it. All it needs is a
function pointer.

This CL de-virtualizes base::BindState and instead moves the vtable
pointer (which is merely an array holding the destructor) to a function
pointer that contains the destructor instead. Since BindState previously
contained a pointer-to-vtable the net memory effect of having the
function pointer instead should be offset.

And of course a 1MB binary size drop also means reduced memory
requirements for the shared text-segment pages.

Locally produced numbers for ChromiumFramework on Mac:
pre-strip:
 - before: 192,031kB
 - after: 185,800kB
post-strip:
 - before: 101,353kB
 - after: 100,353kB

BUG=486594

Review URL: https://codereview.chromium.org/1129353003

Cr-Commit-Position: refs/heads/master@{#329818}

base/bind_internal.h

@@ -364,9 +364,10 @@ template <typename Runnable, typename RunType, typename BoundArgList>
 struct BindState;
 
 template <typename Runnable,
-          typename R, typename... Args,
+          typename R,
+          typename... Args,
           typename... BoundArgs>
-struct BindState<Runnable, R(Args...), TypeList<BoundArgs...>>
+struct BindState<Runnable, R(Args...), TypeList<BoundArgs...>> final
     : public BindStateBase {
  private:
   using StorageType = BindState<Runnable, R(Args...), TypeList<BoundArgs...>>;
@@ -398,14 +399,21 @@ struct BindState<Runnable, R(Args...), TypeList<BoundArgs...>>
   using UnboundRunType = MakeFunctionType<R, UnboundArgs>;
 
   BindState(const Runnable& runnable, const BoundArgs&... bound_args)
-      : runnable_(runnable), ref_(bound_args...), bound_args_(bound_args...) {}
+      : BindStateBase(&Destroy),
+        runnable_(runnable),
+        ref_(bound_args...),
+        bound_args_(bound_args...) {}
 
   RunnableType runnable_;
   MaybeScopedRefPtr<HasIsMethodTag<Runnable>::value, BoundArgs...> ref_;
   Tuple<BoundArgs...> bound_args_;
 
  private:
-  ~BindState() override {}
+  ~BindState() {}
+
+  static void Destroy(BindStateBase* self) {
+    delete static_cast<BindState*>(self);
+  }
 };
 
 }  // namespace internal

base/callback_internal.cc

@@ -9,6 +9,15 @@
 namespace base {
 namespace internal {
 
+void BindStateBase::AddRef() {
+  AtomicRefCountInc(&ref_count_);
+}
+
+void BindStateBase::Release() {
+  if (!AtomicRefCountDec(&ref_count_))
+    destructor_(this);
+}
+
 CallbackBase::CallbackBase(const CallbackBase& c) = default;
 CallbackBase& CallbackBase::operator=(const CallbackBase& c) = default;
 
@@ -27,7 +36,7 @@ bool CallbackBase::Equals(const CallbackBase& other) const {
 CallbackBase::CallbackBase(BindStateBase* bind_state)
     : bind_state_(bind_state),
       polymorphic_invoke_(NULL) {
-  DCHECK(!bind_state_.get() || bind_state_->HasOneRef());
+  DCHECK(!bind_state_.get() || bind_state_->ref_count_ == 1);
 }
 
 CallbackBase::~CallbackBase() {

base/callback_internal.h

@@ -10,15 +10,19 @@
 
 #include <stddef.h>
 
+#include "base/atomic_ref_count.h"
 #include "base/base_export.h"
+#include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/template_util.h"
 
 template <typename T>
 class ScopedVector;
 
 namespace base {
 namespace internal {
+class CallbackBase;
 
 // BindStateBase is used to provide an opaque handle that the Callback
 // class can use to represent a function object with bound arguments.  It
@@ -26,10 +30,30 @@ namespace internal {
 // DoInvoke function to perform the function execution.  This allows
 // us to shield the Callback class from the types of the bound argument via
 // "type erasure."
-class BindStateBase : public RefCountedThreadSafe<BindStateBase> {
+// At the base level, the only task is to add reference counting data. Don't use
+// RefCountedThreadSafe since it requires the destructor to be a virtual method.
+// Creating a vtable for every BindState template instantiation results in a lot
+// of bloat. Its only task is to call the destructor which can be done with a
+// function pointer.
+class BindStateBase {
  protected:
-  friend class RefCountedThreadSafe<BindStateBase>;
-  virtual ~BindStateBase() {}
+  explicit BindStateBase(void (*destructor)(BindStateBase*))
+      : ref_count_(0), destructor_(destructor) {}
+  ~BindStateBase() = default;
+
+ private:
+  friend class scoped_refptr<BindStateBase>;
+  friend class CallbackBase;
+
+  void AddRef();
+  void Release();
+
+  AtomicRefCount ref_count_;
+
+  // Pointer to a function that will properly destroy |this|.
+  void (*destructor_)(BindStateBase*);
+
+  DISALLOW_COPY_AND_ASSIGN(BindStateBase);
 };
 
 // Holds the Callback methods that don't require specialization to reduce

base/callback_unittest.cc

@@ -35,9 +35,13 @@ template <>
 struct BindState<void(void), void(void), void(FakeInvoker)>
     : public BindStateBase {
  public:
+  BindState() : BindStateBase(&Destroy) {}
   typedef FakeInvoker InvokerType;
  private:
-  ~BindState() override {}
+  ~BindState() {}
+  static void Destroy(BindStateBase* self) {
+    delete static_cast<BindState*>(self);
+  }
 };
 
 template <>
@@ -45,9 +49,13 @@ struct BindState<void(void), void(void),
                            void(FakeInvoker, FakeInvoker)>
     : public BindStateBase {
  public:
+  BindState() : BindStateBase(&Destroy) {}
   typedef FakeInvoker InvokerType;
  private:
-  ~BindState() override {}
+  ~BindState() {}
+  static void Destroy(BindStateBase* self) {
+    delete static_cast<BindState*>(self);
+  }
 };
 }  // namespace internal