Add debug-only tracing to grpc_core::RefCount

Also, this patch removes the *WithTracing variants in favor of the new
API.

4 files changed, 102 insertions, 173 deletions

diff --git a/src/core/lib/debug/trace.h b/src/core/lib/debug/trace.h
index fe6301a3fc..5ed52454bd 100644
--- a/src/core/lib/debug/trace.h
+++ b/src/core/lib/debug/trace.h
@@ -102,8 +102,9 @@ typedef TraceFlag DebugOnlyTraceFlag;
 #else
 class DebugOnlyTraceFlag {
  public:
-  DebugOnlyTraceFlag(bool default_enabled, const char* name) {}
-  bool enabled() { return false; }
+  constexpr DebugOnlyTraceFlag(bool default_enabled, const char* name) {}
+  constexpr bool enabled() const { return false; }
+  constexpr const char* name() const { return "DebugOnlyTraceFlag"; }
 
  private:
   void set_enabled(bool enabled) {}
diff --git a/src/core/lib/gprpp/orphanable.h b/src/core/lib/gprpp/orphanable.h
index 3eb510165e..9053c60111 100644
--- a/src/core/lib/gprpp/orphanable.h
+++ b/src/core/lib/gprpp/orphanable.h
@@ -90,104 +90,41 @@ class InternallyRefCounted : public Orphanable {
   template <typename T>
   friend class RefCountedPtr;
 
-  InternallyRefCounted() = default;
+  // TraceFlagT is defined to accept both DebugOnlyTraceFlag and TraceFlag.
+  // Note: RefCount tracing is only enabled on debug builds, even when a
+  //       TraceFlag is used.
+  template <typename TraceFlagT = TraceFlag>
+  explicit InternallyRefCounted(TraceFlagT* trace_flag = nullptr)
+      : refs_(1, trace_flag) {}
   virtual ~InternallyRefCounted() = default;
 
   RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
     IncrementRefCount();
     return RefCountedPtr<Child>(static_cast<Child*>(this));
   }
-
-  void Unref() {
-    if (refs_.Unref()) {
-      Delete(static_cast<Child*>(this));
-    }
-  }
-
- private:
-  void IncrementRefCount() { refs_.Ref(); }
-
-  grpc_core::RefCount refs_;
-};
-
-// An alternative version of the InternallyRefCounted base class that
-// supports tracing.  This is intended to be used in cases where the
-// object will be handled both by idiomatic C++ code using smart
-// pointers and legacy code that is manually calling Ref() and Unref().
-// Once all of our code is converted to idiomatic C++, we may be able to
-// eliminate this class.
-template <typename Child>
-class InternallyRefCountedWithTracing : public Orphanable {
- public:
-  // Not copyable nor movable.
-  InternallyRefCountedWithTracing(const InternallyRefCountedWithTracing&) =
-      delete;
-  InternallyRefCountedWithTracing& operator=(
-      const InternallyRefCountedWithTracing&) = delete;
-
-  GRPC_ABSTRACT_BASE_CLASS
-
- protected:
-  GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
-
-  // Allow RefCountedPtr<> to access Unref() and IncrementRefCount().
-  template <typename T>
-  friend class RefCountedPtr;
-
-  InternallyRefCountedWithTracing()
-      : InternallyRefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {}
-
-  explicit InternallyRefCountedWithTracing(TraceFlag* trace_flag)
-      : trace_flag_(trace_flag) {}
-
-#ifdef NDEBUG
-  explicit InternallyRefCountedWithTracing(DebugOnlyTraceFlag* trace_flag)
-      : InternallyRefCountedWithTracing() {}
-#endif
-
-  virtual ~InternallyRefCountedWithTracing() = default;
-
-  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
-    IncrementRefCount();
-    return RefCountedPtr<Child>(static_cast<Child*>(this));
-  }
-
   RefCountedPtr<Child> Ref(const DebugLocation& location,
                            const char* reason) GRPC_MUST_USE_RESULT {
-    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
-      const grpc_core::RefCount::Value old_refs = refs_.get();
-      gpr_log(GPR_INFO, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s",
-              trace_flag_->name(), this, location.file(), location.line(),
-              old_refs, old_refs + 1, reason);
-    }
-    return Ref();
+    IncrementRefCount(location, reason);
+    return RefCountedPtr<Child>(static_cast<Child*>(this));
   }
 
-  // TODO(roth): Once all of our code is converted to C++ and can use
-  // RefCountedPtr<> instead of manual ref-counting, make the Unref() methods
-  // private, since they will only be used by RefCountedPtr<>, which is a
-  // friend of this class.
-
   void Unref() {
     if (refs_.Unref()) {
       Delete(static_cast<Child*>(this));
     }
   }
-
   void Unref(const DebugLocation& location, const char* reason) {
-    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
-      const grpc_core::RefCount::Value old_refs = refs_.get();
-      gpr_log(GPR_INFO, "%s:%p %s:%d unref %" PRIdPTR " -> %" PRIdPTR " %s",
-              trace_flag_->name(), this, location.file(), location.line(),
-              old_refs, old_refs - 1, reason);
+    if (refs_.Unref(location, reason)) {
+      Delete(static_cast<Child*>(this));
     }
-    Unref();
   }
 
  private:
   void IncrementRefCount() { refs_.Ref(); }
+  void IncrementRefCount(const DebugLocation& location, const char* reason) {
+    refs_.Ref(location, reason);
+  }
 
-  TraceFlag* trace_flag_ = nullptr;
   grpc_core::RefCount refs_;
 };
 
diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h
index 98de1a3653..fa97ffcfed 100644
--- a/src/core/lib/gprpp/ref_counted.h
+++ b/src/core/lib/gprpp/ref_counted.h
@@ -74,12 +74,34 @@ class RefCount {
   using Value = intptr_t;
 
   // `init` is the initial refcount stored in this object.
-  constexpr explicit RefCount(Value init = 1) : value_(init) {}
+  //
+  // TraceFlagT is defined to accept both DebugOnlyTraceFlag and TraceFlag.
+  // Note: RefCount tracing is only enabled on debug builds, even when a
+  //       TraceFlag is used.
+  template <typename TraceFlagT = TraceFlag>
+  constexpr explicit RefCount(Value init = 1, TraceFlagT* trace_flag = nullptr)
+      :
+#ifndef NDEBUG
+        trace_flag_(trace_flag),
+#endif
+        value_(init) {
+  }
 
   // Increases the ref-count by `n`.
   void Ref(Value n = 1) {
     GPR_ATM_INC_ADD_THEN(value_.fetch_add(n, std::memory_order_relaxed));
   }
+  void Ref(const DebugLocation& location, const char* reason, Value n = 1) {
+#ifndef NDEBUG
+    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
+      const RefCount::Value old_refs = get();
+      gpr_log(GPR_INFO, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s",
+              trace_flag_->name(), this, location.file(), location.line(),
+              old_refs, old_refs + n, reason);
+    }
+#endif
+    Ref(n);
+  }
 
   // Similar to Ref() with an assert on the ref-count being non-zero.
   void RefNonZero() {
@@ -91,6 +113,17 @@ class RefCount {
     Ref();
 #endif
   }
+  void RefNonZero(const DebugLocation& location, const char* reason) {
+#ifndef NDEBUG
+    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
+      const RefCount::Value old_refs = get();
+      gpr_log(GPR_INFO, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s",
+              trace_flag_->name(), this, location.file(), location.line(),
+              old_refs, old_refs + 1, reason);
+    }
+#endif
+    RefNonZero();
+  }
 
   // Decrements the ref-count and returns true if the ref-count reaches 0.
   bool Unref() {
@@ -99,10 +132,24 @@ class RefCount {
     GPR_DEBUG_ASSERT(prior > 0);
     return prior == 1;
   }
+  bool Unref(const DebugLocation& location, const char* reason) {
+#ifndef NDEBUG
+    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
+      const RefCount::Value old_refs = get();
+      gpr_log(GPR_INFO, "%s:%p %s:%d unref %" PRIdPTR " -> %" PRIdPTR " %s",
+              trace_flag_->name(), this, location.file(), location.line(),
+              old_refs, old_refs - 1, reason);
+    }
+#endif
+    return Unref();
+  }
 
+ private:
   Value get() const { return value_.load(std::memory_order_relaxed); }
 
- private:
+#ifndef NDEBUG
+  TraceFlag* trace_flag_;
+#endif
   std::atomic<Value> value_;
 };
 
@@ -140,108 +187,45 @@ class RefCounted : public Impl {
     return RefCountedPtr<Child>(static_cast<Child*>(this));
   }
 
-  // TODO(roth): Once all of our code is converted to C++ and can use
-  // RefCountedPtr<> instead of manual ref-counting, make this method
-  // private, since it will only be used by RefCountedPtr<>, which is a
-  // friend of this class.
-  void Unref() {
-    if (refs_.Unref()) {
-      Delete(static_cast<Child*>(this));
-    }
-  }
-
-  // Not copyable nor movable.
-  RefCounted(const RefCounted&) = delete;
-  RefCounted& operator=(const RefCounted&) = delete;
-
-  GRPC_ABSTRACT_BASE_CLASS
-
- protected:
-  GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
-
-  RefCounted() = default;
-
-  // Note: Depending on the Impl used, this dtor can be implicitly virtual.
-  ~RefCounted() = default;
-
- private:
-  // Allow RefCountedPtr<> to access IncrementRefCount().
-  template <typename T>
-  friend class RefCountedPtr;
-
-  void IncrementRefCount() { refs_.Ref(); }
-
-  RefCount refs_;
-};
-
-// An alternative version of the RefCounted base class that
-// supports tracing.  This is intended to be used in cases where the
-// object will be handled both by idiomatic C++ code using smart
-// pointers and legacy code that is manually calling Ref() and Unref().
-// Once all of our code is converted to idiomatic C++, we may be able to
-// eliminate this class.
-template <typename Child, typename Impl = PolymorphicRefCount>
-class RefCountedWithTracing : public Impl {
- public:
-  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
-    IncrementRefCount();
-    return RefCountedPtr<Child>(static_cast<Child*>(this));
-  }
-
   RefCountedPtr<Child> Ref(const DebugLocation& location,
                            const char* reason) GRPC_MUST_USE_RESULT {
-    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
-      const RefCount::Value old_refs = refs_.get();
-      gpr_log(GPR_INFO, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s",
-              trace_flag_->name(), this, location.file(), location.line(),
-              old_refs, old_refs + 1, reason);
-    }
-    return Ref();
+    IncrementRefCount(location, reason);
+    return RefCountedPtr<Child>(static_cast<Child*>(this));
   }
 
   // TODO(roth): Once all of our code is converted to C++ and can use
-  // RefCountedPtr<> instead of manual ref-counting, make the Unref() methods
-  // private, since they will only be used by RefCountedPtr<>, which is a
+  // RefCountedPtr<> instead of manual ref-counting, make this method
+  // private, since it will only be used by RefCountedPtr<>, which is a
   // friend of this class.
-
   void Unref() {
     if (refs_.Unref()) {
       Delete(static_cast<Child*>(this));
     }
   }
-
   void Unref(const DebugLocation& location, const char* reason) {
-    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
-      const RefCount::Value old_refs = refs_.get();
-      gpr_log(GPR_INFO, "%s:%p %s:%d unref %" PRIdPTR " -> %" PRIdPTR " %s",
-              trace_flag_->name(), this, location.file(), location.line(),
-              old_refs, old_refs - 1, reason);
+    if (refs_.Unref(location, reason)) {
+      Delete(static_cast<Child*>(this));
     }
-    Unref();
   }
 
   // Not copyable nor movable.
-  RefCountedWithTracing(const RefCountedWithTracing&) = delete;
-  RefCountedWithTracing& operator=(const RefCountedWithTracing&) = delete;
+  RefCounted(const RefCounted&) = delete;
+  RefCounted& operator=(const RefCounted&) = delete;
 
   GRPC_ABSTRACT_BASE_CLASS
 
  protected:
   GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
 
-  RefCountedWithTracing()
-      : RefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {}
-
-  explicit RefCountedWithTracing(TraceFlag* trace_flag)
-      : trace_flag_(trace_flag) {}
-
-#ifdef NDEBUG
-  explicit RefCountedWithTracing(DebugOnlyTraceFlag* trace_flag)
-      : RefCountedWithTracing() {}
-#endif
+  // TraceFlagT is defined to accept both DebugOnlyTraceFlag and TraceFlag.
+  // Note: RefCount tracing is only enabled on debug builds, even when a
+  //       TraceFlag is used.
+  template <typename TraceFlagT = TraceFlag>
+  explicit RefCounted(TraceFlagT* trace_flag = nullptr)
+      : refs_(1, trace_flag) {}
 
   // Note: Depending on the Impl used, this dtor can be implicitly virtual.
-  ~RefCountedWithTracing() = default;
+  ~RefCounted() = default;
 
  private:
   // Allow RefCountedPtr<> to access IncrementRefCount().
@@ -249,8 +233,10 @@ class RefCountedWithTracing : public Impl {
   friend class RefCountedPtr;
 
   void IncrementRefCount() { refs_.Ref(); }
+  void IncrementRefCount(const DebugLocation& location, const char* reason) {
+    refs_.Ref(location, reason);
+  }
 
-  TraceFlag* trace_flag_ = nullptr;
   RefCount refs_;
 };
 
diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h
index facd7c6dce..1ed5d584c7 100644
--- a/src/core/lib/gprpp/ref_counted_ptr.h
+++ b/src/core/lib/gprpp/ref_counted_ptr.h
@@ -24,6 +24,7 @@
 #include <type_traits>
 #include <utility>
 
+#include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/memory.h"
 
 namespace grpc_core {
@@ -55,15 +56,13 @@ class RefCountedPtr {
 
   // Move assignment.
   RefCountedPtr& operator=(RefCountedPtr&& other) {
-    if (value_ != nullptr) value_->Unref();
-    value_ = other.value_;
+    reset(other.value_);
     other.value_ = nullptr;
     return *this;
   }
   template <typename Y>
   RefCountedPtr& operator=(RefCountedPtr<Y>&& other) {
-    if (value_ != nullptr) value_->Unref();
-    value_ = other.value_;
+    reset(other.value_);
     other.value_ = nullptr;
     return *this;
   }
@@ -86,8 +85,7 @@ class RefCountedPtr {
     // Note: Order of reffing and unreffing is important here in case value_
     // and other.value_ are the same object.
     if (other.value_ != nullptr) other.value_->IncrementRefCount();
-    if (value_ != nullptr) value_->Unref();
-    value_ = other.value_;
+    reset(other.value_);
     return *this;
   }
   template <typename Y>
@@ -97,8 +95,7 @@ class RefCountedPtr {
     // Note: Order of reffing and unreffing is important here in case value_
     // and other.value_ are the same object.
     if (other.value_ != nullptr) other.value_->IncrementRefCount();
-    if (value_ != nullptr) value_->Unref();
-    value_ = other.value_;
+    reset(other.value_);
     return *this;
   }
 
@@ -107,21 +104,29 @@ class RefCountedPtr {
   }
 
   // If value is non-null, we take ownership of a ref to it.
-  void reset(T* value) {
+  void reset(T* value = nullptr) {
     if (value_ != nullptr) value_->Unref();
     value_ = value;
   }
+  void reset(const DebugLocation& location, const char* reason,
+             T* value = nullptr) {
+    if (value_ != nullptr) value_->Unref(location, reason);
+    value_ = value;
+  }
   template <typename Y>
-  void reset(Y* value) {
+  void reset(Y* value = nullptr) {
     static_assert(std::has_virtual_destructor<T>::value,
                   "T does not have a virtual dtor");
     if (value_ != nullptr) value_->Unref();
     value_ = value;
   }
-
-  void reset() {
-    if (value_ != nullptr) value_->Unref();
-    value_ = nullptr;
+  template <typename Y>
+  void reset(const DebugLocation& location, const char* reason,
+             Y* value = nullptr) {
+    static_assert(std::has_virtual_destructor<T>::value,
+                  "T does not have a virtual dtor");
+    if (value_ != nullptr) value_->Unref(location, reason);
+    value_ = value;
   }
 
   // TODO(roth): This method exists solely as a transition mechanism to allow