Make base::WeakPtr::Get() fast

base/memory/weak_ptr.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.
 
 // Weak pointers are pointers to an object that do not affect its lifetime,
 // and which may be invalidated (i.e. reset to nullptr) by the object, or its
 // owner, at any time, most commonly when the object is about to be deleted.
 
 // Weak pointers are useful when an object needs to be accessed safely by one
 // or more objects other than its owner, and those callers can cope with the
@@ skipping 72 matching lines @@
 
 template <typename T> class SupportsWeakPtr;
 template <typename T> class WeakPtr;
 
 namespace internal {
 // These classes are part of the WeakPtr implementation.
 // DO NOT USE THESE CLASSES DIRECTLY YOURSELF.
 
 class BASE_EXPORT WeakReference {
  public:
-
-  class BASE_EXPORT NewFlag {
+  // Although Flag is bound to a specific SequencedTaskRunner, it may be
+  // deleted from another via base::WeakPtr::~WeakPtr().
+  class BASE_EXPORT Flag : public RefCountedThreadSafe<Flag> {
    public:
-    constexpr static uintptr_t kTrueMask = ~static_cast<uintptr_t>(0);
-
-    static NewFlag *NullFlag() { return &g_null_flag; }
-    NewFlag() : is_valid_(kTrueMask), ref_count_(0) {
-#if DCHECK_IS_ON()
-      // Flags only become bound when checked for validity, or invalidated,
-      // so that we can check that later validity/invalidation operations on
-      // the same Flag take place on the same sequenced thread.
-      sequence_checker_.DetachFromSequence();
-#endif
-    }
+    Flag();
+    static Flag* nullFlag();
 
     void Invalidate() {
 #if DCHECK_IS_ON()
-      if (this == NullFlag()) {
-        DCHECK(!is_valid_);
+      if (this == nullFlag())
         return;
-      }
       // The flag being invalidated with a single ref implies that there are no
       // weak pointers in existence. Allow deletion on other thread in this case.
       DCHECK(sequence_checker_.CalledOnValidSequence() || HasOneRef())
           << "WeakPtrs must be invalidated on the same sequenced thread.";
 #endif
       is_valid_ = 0;
     }
-
     uintptr_t IsValid() const {
 #if DCHECK_IS_ON()
-      if (this == NullFlag()) {
+      if (this == nullFlag()) {
         DCHECK(!is_valid_);
-        return false;
+        return 0;
       }
       DCHECK(sequence_checker_.CalledOnValidSequence())
           << "WeakPtrs must be checked on the same sequenced thread.";
 #endif
       return is_valid_;
     }
 
-    uintptr_t NumRefs() const { return ref_count_; }
+   private:
+    friend class base::RefCountedThreadSafe<Flag>;
 
-    void AddRef() const { AtomicRefCountInc(&ref_count_); }
-    void Release() const {
-      if (!AtomicRefCountDec(&ref_count_))
-        delete this;
-    }
+    enum NullFlagTag { kNullFlagTag };
+    Flag(NullFlagTag);
 
-   private:
-    ~NewFlag() = default;
-    enum NullFlagTag { kNullFlagTag };
-#if DCHECK_IS_ON()
-    NewFlag(NullFlagTag);
-#else
-    constexpr NewFlag(NullFlagTag);
-#endif
-
-    static NewFlag g_null_flag;
+    ~Flag();
 
     uintptr_t is_valid_;
-    mutable AtomicRefCount ref_count_;
 #if DCHECK_IS_ON()
+    // Even if SequenceChecker is an empty class in non-dcheck builds, it still
+    // takes up space in the class.
     SequenceChecker sequence_checker_;
 #endif
   };
 
-  // Although Flag is bound to a specific SequencedTaskRunner, it may be
-  // deleted from another via base::WeakPtr::~WeakPtr().
-  class BASE_EXPORT Flag : public RefCountedThreadSafe<Flag> {
-   public:
-    Flag();
-
-    void Invalidate();
-    bool IsValid() const;
-
-   private:
-    friend class base::RefCountedThreadSafe<Flag>;
-
-    ~Flag();
-
-    SequenceChecker sequence_checker_;
-    bool is_valid_;
-  };
-
   WeakReference();
-  explicit WeakReference(/*const Flag* flag,*/ const NewFlag* new_flag);
+  explicit WeakReference(const Flag* flag);
   ~WeakReference();
 
   WeakReference(WeakReference&& other);
   WeakReference(const WeakReference& other);
   WeakReference& operator=(WeakReference&& other) = default;
   WeakReference& operator=(const WeakReference& other) = default;
 
-  uintptr_t is_valid() const { return new_flag_->IsValid(); }
+  uintptr_t is_valid() const { return flag_->IsValid(); }
 
  private:
-  //scoped_refptr<const Flag> flag_;
-  scoped_refptr<const NewFlag> new_flag_;
+  // Note: To avoid null-checks, flag_ always points to either Flag::nullFlag()
+  // or some other object.
+  scoped_refptr<const Flag> flag_;
 };
 
 class BASE_EXPORT WeakReferenceOwner {
  public:
   WeakReferenceOwner();
   ~WeakReferenceOwner();
 
   WeakReference GetRef() const;
 
-  bool HasRefs() const {
-    return (new_flag_->IsValid() & new_flag_->NumRefs()) > 1;
-
-#if 0

[Nico, 2017/06/27 22:05:41]

This looks like your diffbase might be off?

[hans, 2017/06/27 22:09:50]

Yeah, something is not right here :-(

-    return new_flag_ != WeakReference::NewFlag::NullFlag() &&
-           !new_flag_->HasOneRef();
-#endif
-    //return flag_.get() && !flag_->HasOneRef();
-  }
+  bool HasRefs() const { return flag_->IsValid() && !flag_->HasOneRef(); }
 
   void Invalidate();
 
  private:
-  //mutable scoped_refptr<WeakReference::Flag> flag_;
-  mutable scoped_refptr<WeakReference::NewFlag> new_flag_;
+  mutable scoped_refptr<WeakReference::Flag> flag_;
 };
 
 // This class simplifies the implementation of WeakPtr's type conversion
 // constructor by avoiding the need for a public accessor for ref_.  A
 // WeakPtr<T> cannot access the private members of WeakPtr<U>, so this
 // base class gives us a way to access ref_ in a protected fashion.
 class BASE_EXPORT WeakPtrBase {
  public:
   WeakPtrBase();
   ~WeakPtrBase();
@@ skipping 119 matching lines @@
 }
 template <class T>
 bool operator==(const WeakPtr<T>& weak_ptr, std::nullptr_t) {
   return weak_ptr.get() == nullptr;
 }
 template <class T>
 bool operator==(std::nullptr_t, const WeakPtr<T>& weak_ptr) {
   return weak_ptr == nullptr;
 }
 
-// A class may be composed of a WeakPtrFactory and thereby
-// control how it exposes weak pointers to itself.  This is helpful if you only
-// need weak pointers within the implementation of a class.  This class is also
-// useful when working with primitive types.  For example, you could have a
-// WeakPtrFactory<bool> that is used to pass around a weak reference to a bool.
+namespace internal {
 class BASE_EXPORT WeakPtrFactoryBase {
  protected:
   WeakPtrFactoryBase(uintptr_t ptr);
   ~WeakPtrFactoryBase();
   internal::WeakReferenceOwner weak_reference_owner_;
   uintptr_t ptr_;
 };
+}  // namespace internal
 
+// A class may be composed of a WeakPtrFactory and thereby
+// control how it exposes weak pointers to itself.  This is helpful if you only
+// need weak pointers within the implementation of a class.  This class is also
+// useful when working with primitive types.  For example, you could have a
+// WeakPtrFactory<bool> that is used to pass around a weak reference to a bool.
 template <class T>
-class WeakPtrFactory : public WeakPtrFactoryBase {
+class WeakPtrFactory : public internal::WeakPtrFactoryBase {
  public:
-  explicit WeakPtrFactory(T* ptr) : WeakPtrFactoryBase(reinterpret_cast<uintptr_t>(ptr)) { }
+  explicit WeakPtrFactory(T* ptr)
+      : WeakPtrFactoryBase(reinterpret_cast<uintptr_t>(ptr)) {}
 
   ~WeakPtrFactory() {}
 
   WeakPtr<T> GetWeakPtr() {
     DCHECK(ptr_);
-    return WeakPtr<T>(weak_reference_owner_.GetRef(), reinterpret_cast<T*>(ptr_));
+    return WeakPtr<T>(weak_reference_owner_.GetRef(),
+                      reinterpret_cast<T*>(ptr_));
   }
 
   // Call this method to invalidate all existing weak pointers.
   void InvalidateWeakPtrs() {
     DCHECK(ptr_);
     weak_reference_owner_.Invalidate();
   }
 
   // Call this method to determine if any weak pointers exist.
   bool HasWeakPtrs() const {
@@ skipping 46 matching lines @@
 //   base::WeakPtr<Derived> ptr = derived.AsWeakPtr();  // Fails.
 
 template <typename Derived>
 WeakPtr<Derived> AsWeakPtr(Derived* t) {
   return internal::SupportsWeakPtrBase::StaticAsWeakPtr<Derived>(t);
 }
 
 }  // namespace base
 
 #endif  // BASE_MEMORY_WEAK_PTR_H_