From af3dfd8c359b5aeac839e987a81b5a472e99234a Mon Sep 17 00:00:00 2001 From: Tommi Date: Wed, 24 Apr 2024 10:54:19 +0200 Subject: [PATCH] Make WeakPtr slightly cheaper to allocate and use This changes Flag to not inherit from a virtual interface. Also fixing iwyu and build dependencies. Bug: none Change-Id: Iba6e095ec771d8975a32059041185270d32e51be Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/348940 Commit-Queue: Tomas Gunnarsson Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#42162} --- rtc_base/BUILD.gn | 2 ++ rtc_base/weak_ptr.cc | 14 ++++---------- rtc_base/weak_ptr.h | 21 +++++++++++++-------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 69b434394f..b3cb17c274 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -788,6 +788,8 @@ rtc_library("weak_ptr") { "weak_ptr.h", ] deps = [ + ":checks", + ":macromagic", ":refcount", "../api:scoped_refptr", "../api:sequence_checker", diff --git a/rtc_base/weak_ptr.cc b/rtc_base/weak_ptr.cc index 3bfa71b0b4..cec0808093 100644 --- a/rtc_base/weak_ptr.cc +++ b/rtc_base/weak_ptr.cc @@ -16,25 +16,19 @@ namespace rtc { namespace internal { -WeakReference::Flag::Flag() : is_valid_(true) {} - void WeakReference::Flag::Invalidate() { - RTC_DCHECK(checker_.IsCurrent()) - << "WeakPtrs must be invalidated on the same sequence."; + RTC_DCHECK_RUN_ON(&checker_); is_valid_ = false; } bool WeakReference::Flag::IsValid() const { - RTC_DCHECK(checker_.IsCurrent()) - << "WeakPtrs must be checked on the same sequence."; + RTC_DCHECK_RUN_ON(&checker_); return is_valid_; } -WeakReference::Flag::~Flag() {} - WeakReference::WeakReference() {} -WeakReference::WeakReference(const Flag* flag) : flag_(flag) {} +WeakReference::WeakReference(const RefCountedFlag* flag) : flag_(flag) {} WeakReference::~WeakReference() {} @@ -55,7 +49,7 @@ WeakReferenceOwner::~WeakReferenceOwner() { WeakReference WeakReferenceOwner::GetRef() const { // If we hold the last reference to the Flag then create a new one. if (!HasRefs()) - flag_ = new RefCountedObject(); + flag_ = new WeakReference::RefCountedFlag(); return WeakReference(flag_.get()); } diff --git a/rtc_base/weak_ptr.h b/rtc_base/weak_ptr.h index 7e75b5b9be..67b6ec1542 100644 --- a/rtc_base/weak_ptr.h +++ b/rtc_base/weak_ptr.h @@ -16,9 +16,11 @@ #include "api/scoped_refptr.h" #include "api/sequence_checker.h" +#include "rtc_base/checks.h" #include "rtc_base/ref_count.h" #include "rtc_base/ref_counted_object.h" #include "rtc_base/system/no_unique_address.h" +#include "rtc_base/thread_annotations.h" // The implementation is borrowed from chromium except that it does not // implement SupportsWeakPtr. @@ -92,25 +94,28 @@ class WeakReference { public: // Although Flag is bound to a specific sequence, it may be // deleted from another via base::WeakPtr::~WeakPtr(). - class Flag : public RefCountInterface { + class Flag { public: - Flag(); + Flag() = default; void Invalidate(); bool IsValid() const; private: - friend class RefCountedObject; + friend class FinalRefCountedObject; - ~Flag() override; + ~Flag() = default; RTC_NO_UNIQUE_ADDRESS ::webrtc::SequenceChecker checker_{ webrtc::SequenceChecker::kDetached}; - bool is_valid_; + bool is_valid_ RTC_GUARDED_BY(checker_) = true; }; + // `RefCountedFlag` is the reference counted (shared), non-virtual, flag type. + using RefCountedFlag = FinalRefCountedObject; + WeakReference(); - explicit WeakReference(const Flag* flag); + explicit WeakReference(const RefCountedFlag* flag); ~WeakReference(); WeakReference(WeakReference&& other); @@ -121,7 +126,7 @@ class WeakReference { bool is_valid() const; private: - scoped_refptr flag_; + scoped_refptr flag_; }; class WeakReferenceOwner { @@ -136,7 +141,7 @@ class WeakReferenceOwner { void Invalidate(); private: - mutable scoped_refptr> flag_; + mutable scoped_refptr flag_; }; // This class simplifies the implementation of WeakPtr's type conversion