Outline RefCountedThreadSafeBase::AddRef and Release on non-X86

base/memory/ref_counted.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_MEMORY_REF_COUNTED_H_
 #define BASE_MEMORY_REF_COUNTED_H_
 
 #include <stddef.h>
 
 #include <cassert>
@@ skipping 124 matching lines @@
  protected:
   explicit RefCountedThreadSafeBase(StartRefCountFromZeroTag) {}
   explicit RefCountedThreadSafeBase(StartRefCountFromOneTag) : ref_count_(1) {
 #if DCHECK_IS_ON()
     needs_adopt_ref_ = true;
 #endif
   }
 
   ~RefCountedThreadSafeBase();
 
-  void AddRef() const {
+// Release and AddRef are suitable for inlining on X86 because they generate
+// very small code sequences. On other platforms (ARM), it causes a size
+// regression and is probably not worth it.
+#if defined(ARCH_CPU_X86_FAMILY)
+  // Returns true if the object should self-delete.
+  bool Release() const { return ReleaseImpl(); }
+  void AddRef() const { AddRefImpl(); }
+#else
+  // Returns true if the object should self-delete.
+  bool Release() const;
+  void AddRef() const;
+#endif
+
+ private:
+  template <typename U>
+  friend scoped_refptr<U> base::AdoptRef(U*);
+
+  void Adopted() const {
+#if DCHECK_IS_ON()
+    DCHECK(needs_adopt_ref_);
+    needs_adopt_ref_ = false;
+#endif
+  }
+
+  ALWAYS_INLINE void AddRefImpl() const {
 #if DCHECK_IS_ON()
     DCHECK(!in_dtor_);
     DCHECK(!needs_adopt_ref_)
         << "This RefCounted object is created with non-zero reference count."
         << " The first reference to such a object has to be made by AdoptRef or"
         << " MakeRefCounted.";
 #endif
     AtomicRefCountInc(&ref_count_);
   }
 
-  // Returns true if the object should self-delete.
-  bool Release() const {
+  ALWAYS_INLINE bool ReleaseImpl() const {

[(unused - use chromium), 2017/06/30 16:19:06]

Is the ALWAYS_INLINE needed? Seems like the compiler should be able to figure
this out.

[hans, 2017/06/30 16:21:13]

It should.

But I think the macro also expresses our intent here: this really should be
inlined without question, i.e. this is basically the macro approach discussed on
the bug.

 #if DCHECK_IS_ON()
     DCHECK(!in_dtor_);
     DCHECK(!AtomicRefCountIsZero(&ref_count_));
 #endif
     if (!AtomicRefCountDec(&ref_count_)) {
 #if DCHECK_IS_ON()
       in_dtor_ = true;
 #endif
       return true;
     }
     return false;
   }
 
- private:
-  template <typename U>
-  friend scoped_refptr<U> base::AdoptRef(U*);
-
-  void Adopted() const {
-#if DCHECK_IS_ON()
-    DCHECK(needs_adopt_ref_);
-    needs_adopt_ref_ = false;
-#endif
-  }
-
   mutable AtomicRefCount ref_count_{0};
 #if DCHECK_IS_ON()
   mutable bool needs_adopt_ref_ = false;
   mutable bool in_dtor_ = false;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(RefCountedThreadSafeBase);
 };
 
 }  // namespace subtle
@@ skipping 459 matching lines @@
 bool operator!=(std::nullptr_t null, const scoped_refptr<T>& rhs) {
   return !operator==(null, rhs);
 }
 
 template <typename T>
 std::ostream& operator<<(std::ostream& out, const scoped_refptr<T>& p) {
   return out << p.get();
 }
 
 #endif  // BASE_MEMORY_REF_COUNTED_H_