Merge pull request #17360 from soheilhy/refcounted-no-trace

Add debug-only tracing to grpc_core::RefCount

src/core/ext/filters/client_channel/health/health_check_client.cc

@@ -51,8 +51,7 @@ HealthCheckClient::HealthCheckClient(
     RefCountedPtr<ConnectedSubchannel> connected_subchannel,
     grpc_pollset_set* interested_parties,
     grpc_core::RefCountedPtr<grpc_core::channelz::SubchannelNode> channelz_node)
-    : InternallyRefCountedWithTracing<HealthCheckClient>(
+    : InternallyRefCounted<HealthCheckClient>(&grpc_health_check_client_trace),
-          &grpc_health_check_client_trace),
       service_name_(service_name),
       connected_subchannel_(std::move(connected_subchannel)),
       interested_parties_(interested_parties),
@@ -281,8 +280,7 @@ bool DecodeResponse(grpc_slice_buffer* slice_buffer, grpc_error** error) {
 HealthCheckClient::CallState::CallState(
     RefCountedPtr<HealthCheckClient> health_check_client,
     grpc_pollset_set* interested_parties)
-    : InternallyRefCountedWithTracing<CallState>(
+    : InternallyRefCounted<CallState>(&grpc_health_check_client_trace),
-          &grpc_health_check_client_trace),
       health_check_client_(std::move(health_check_client)),
       pollent_(grpc_polling_entity_create_from_pollset_set(interested_parties)),
       arena_(gpr_arena_create(health_check_client_->connected_subchannel_

src/core/ext/filters/client_channel/health/health_check_client.h

@@ -41,8 +41,7 @@
 
 namespace grpc_core {
 
-class HealthCheckClient
+class HealthCheckClient : public InternallyRefCounted<HealthCheckClient> {
-    : public InternallyRefCountedWithTracing<HealthCheckClient> {
  public:
   HealthCheckClient(const char* service_name,
                     RefCountedPtr<ConnectedSubchannel> connected_subchannel,
@@ -61,7 +60,7 @@ class HealthCheckClient
 
  private:
   // Contains a call to the backend and all the data related to the call.
-  class CallState : public InternallyRefCountedWithTracing<CallState> {
+  class CallState : public InternallyRefCounted<CallState> {
    public:
     CallState(RefCountedPtr<HealthCheckClient> health_check_client,
               grpc_pollset_set* interested_parties_);

src/core/ext/filters/client_channel/lb_policy.cc

@@ -27,7 +27,7 @@ grpc_core::DebugOnlyTraceFlag grpc_trace_lb_policy_refcount(
 namespace grpc_core {
 
 LoadBalancingPolicy::LoadBalancingPolicy(const Args& args)
-    : InternallyRefCountedWithTracing(&grpc_trace_lb_policy_refcount),
+    : InternallyRefCounted(&grpc_trace_lb_policy_refcount),
       combiner_(GRPC_COMBINER_REF(args.combiner, "lb_policy")),
       client_channel_factory_(args.client_channel_factory),
       interested_parties_(grpc_pollset_set_create()),

src/core/ext/filters/client_channel/lb_policy.h

@@ -42,8 +42,7 @@ namespace grpc_core {
 ///
 /// Any I/O done by the LB policy should be done under the pollset_set
 /// returned by \a interested_parties().
-class LoadBalancingPolicy
+class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
-    : public InternallyRefCountedWithTracing<LoadBalancingPolicy> {
  public:
   struct Args {
     /// The combiner under which all LB policy calls will be run.

src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc

@@ -171,8 +171,7 @@ class GrpcLb : public LoadBalancingPolicy {
   };
 
   /// Contains a call to the LB server and all the data related to the call.
-  class BalancerCallState
+  class BalancerCallState : public InternallyRefCounted<BalancerCallState> {
-      : public InternallyRefCountedWithTracing<BalancerCallState> {
    public:
     explicit BalancerCallState(
         RefCountedPtr<LoadBalancingPolicy> parent_grpclb_policy);
@@ -499,7 +498,7 @@ grpc_lb_addresses* ProcessServerlist(const grpc_grpclb_serverlist* serverlist) {
 
 GrpcLb::BalancerCallState::BalancerCallState(
     RefCountedPtr<LoadBalancingPolicy> parent_grpclb_policy)
-    : InternallyRefCountedWithTracing<BalancerCallState>(&grpc_lb_glb_trace),
+    : InternallyRefCounted<BalancerCallState>(&grpc_lb_glb_trace),
       grpclb_policy_(std::move(parent_grpclb_policy)) {
   GPR_ASSERT(grpclb_policy_ != nullptr);
   GPR_ASSERT(!grpclb_policy()->shutting_down_);

src/core/ext/filters/client_channel/lb_policy/subchannel_list.h

@@ -186,8 +186,7 @@ class SubchannelData {
 
 // A list of subchannels.
 template <typename SubchannelListType, typename SubchannelDataType>
-class SubchannelList
+class SubchannelList : public InternallyRefCounted<SubchannelListType> {
-    : public InternallyRefCountedWithTracing<SubchannelListType> {
  public:
   typedef InlinedVector<SubchannelDataType, 10> SubchannelVector;
 
@@ -226,8 +225,7 @@ class SubchannelList
   // Note: Caller must ensure that this is invoked inside of the combiner.
   void Orphan() override {
     ShutdownLocked();
-    InternallyRefCountedWithTracing<SubchannelListType>::Unref(DEBUG_LOCATION,
+    InternallyRefCounted<SubchannelListType>::Unref(DEBUG_LOCATION, "shutdown");
-                                                               "shutdown");
   }
 
   GRPC_ABSTRACT_BASE_CLASS
@@ -493,7 +491,7 @@ SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
     const grpc_lb_addresses* addresses, grpc_combiner* combiner,
     grpc_client_channel_factory* client_channel_factory,
     const grpc_channel_args& args)
-    : InternallyRefCountedWithTracing<SubchannelListType>(tracer),
+    : InternallyRefCounted<SubchannelListType>(tracer),
       policy_(policy),
       tracer_(tracer),
       combiner_(GRPC_COMBINER_REF(combiner, "subchannel_list")) {

src/core/ext/filters/client_channel/lb_policy/xds/xds.cc

@@ -166,8 +166,7 @@ class XdsLb : public LoadBalancingPolicy {
   };
 
   /// Contains a call to the LB server and all the data related to the call.
-  class BalancerCallState
+  class BalancerCallState : public InternallyRefCounted<BalancerCallState> {
-      : public InternallyRefCountedWithTracing<BalancerCallState> {
    public:
     explicit BalancerCallState(
         RefCountedPtr<LoadBalancingPolicy> parent_xdslb_policy);
@@ -488,7 +487,7 @@ grpc_lb_addresses* ProcessServerlist(const xds_grpclb_serverlist* serverlist) {
 
 XdsLb::BalancerCallState::BalancerCallState(
     RefCountedPtr<LoadBalancingPolicy> parent_xdslb_policy)
-    : InternallyRefCountedWithTracing<BalancerCallState>(&grpc_lb_xds_trace),
+    : InternallyRefCounted<BalancerCallState>(&grpc_lb_xds_trace),
       xdslb_policy_(std::move(parent_xdslb_policy)) {
   GPR_ASSERT(xdslb_policy_ != nullptr);
   GPR_ASSERT(!xdslb_policy()->shutting_down_);

src/core/ext/filters/client_channel/resolver.cc

@@ -27,7 +27,7 @@ grpc_core::DebugOnlyTraceFlag grpc_trace_resolver_refcount(false,
 namespace grpc_core {
 
 Resolver::Resolver(grpc_combiner* combiner)
-    : InternallyRefCountedWithTracing(&grpc_trace_resolver_refcount),
+    : InternallyRefCounted(&grpc_trace_resolver_refcount),
       combiner_(GRPC_COMBINER_REF(combiner, "resolver")) {}
 
 Resolver::~Resolver() { GRPC_COMBINER_UNREF(combiner_, "resolver"); }

src/core/ext/filters/client_channel/resolver.h

@@ -44,7 +44,7 @@ namespace grpc_core {
 ///
 /// Note: All methods with a "Locked" suffix must be called from the
 /// combiner passed to the constructor.
-class Resolver : public InternallyRefCountedWithTracing<Resolver> {
+class Resolver : public InternallyRefCounted<Resolver> {
  public:
   // Not copyable nor movable.
   Resolver(const Resolver&) = delete;

src/core/ext/filters/client_channel/subchannel.cc

@@ -1072,7 +1072,7 @@ ConnectedSubchannel::ConnectedSubchannel(
     grpc_core::RefCountedPtr<grpc_core::channelz::SubchannelNode>
         channelz_subchannel,
     intptr_t socket_uuid)
-    : RefCountedWithTracing<ConnectedSubchannel>(&grpc_trace_stream_refcount),
+    : RefCounted<ConnectedSubchannel>(&grpc_trace_stream_refcount),
       channel_stack_(channel_stack),
       channelz_subchannel_(std::move(channelz_subchannel)),
       socket_uuid_(socket_uuid) {}

src/core/ext/filters/client_channel/subchannel.h

@@ -72,7 +72,7 @@ typedef struct grpc_subchannel_key grpc_subchannel_key;
 
 namespace grpc_core {
 
-class ConnectedSubchannel : public RefCountedWithTracing<ConnectedSubchannel> {
+class ConnectedSubchannel : public RefCounted<ConnectedSubchannel> {
  public:
   struct CallArgs {
     grpc_polling_entity* pollent;

src/core/ext/transport/chttp2/transport/chttp2_transport.cc

@@ -207,29 +207,6 @@ grpc_chttp2_transport::~grpc_chttp2_transport() {
   gpr_free(peer_string);
 }
 
-#ifndef NDEBUG
-void grpc_chttp2_unref_transport(grpc_chttp2_transport* t, const char* reason,
-                                 const char* file, int line) {
-  if (grpc_trace_chttp2_refcount.enabled()) {
-    const grpc_core::RefCount::Value val = t->refs.get();
-    gpr_log(GPR_DEBUG, "chttp2:unref:%p %" PRIdPTR "->%" PRIdPTR " %s [%s:%d]",
-            t, val, val - 1, reason, file, line);
-  }
-  if (!t->refs.Unref()) return;
-  grpc_core::Delete(t);
-}
-
-void grpc_chttp2_ref_transport(grpc_chttp2_transport* t, const char* reason,
-                               const char* file, int line) {
-  if (grpc_trace_chttp2_refcount.enabled()) {
-    const grpc_core::RefCount::Value val = t->refs.get();
-    gpr_log(GPR_DEBUG, "chttp2:  ref:%p %" PRIdPTR "->%" PRIdPTR " %s [%s:%d]",
-            t, val, val + 1, reason, file, line);
-  }
-  t->refs.Ref();
-}
-#endif
-
 static const grpc_transport_vtable* get_vtable(void);
 
 /* Returns whether bdp is enabled */
@@ -481,7 +458,8 @@ static void init_keepalive_pings_if_enabled(grpc_chttp2_transport* t) {
 grpc_chttp2_transport::grpc_chttp2_transport(
     const grpc_channel_args* channel_args, grpc_endpoint* ep, bool is_client,
     grpc_resource_user* resource_user)
-    : ep(ep),
+    : refs(1, &grpc_trace_chttp2_refcount),
+      ep(ep),
       peer_string(grpc_endpoint_get_peer(ep)),
       resource_user(resource_user),
       combiner(grpc_combiner_create()),

src/core/ext/transport/chttp2/transport/internal.h

@@ -792,10 +792,18 @@ void grpc_chttp2_stream_unref(grpc_chttp2_stream* s);
   grpc_chttp2_ref_transport(t, r, __FILE__, __LINE__)
 #define GRPC_CHTTP2_UNREF_TRANSPORT(t, r) \
   grpc_chttp2_unref_transport(t, r, __FILE__, __LINE__)
-void grpc_chttp2_unref_transport(grpc_chttp2_transport* t, const char* reason,
+inline void grpc_chttp2_unref_transport(grpc_chttp2_transport* t,
-                                 const char* file, int line);
+                                        const char* reason, const char* file,
-void grpc_chttp2_ref_transport(grpc_chttp2_transport* t, const char* reason,
+                                        int line) {
-                               const char* file, int line);
+  if (t->refs.Unref(grpc_core::DebugLocation(file, line), reason)) {
+    grpc_core::Delete(t);
+  }
+}
+inline void grpc_chttp2_ref_transport(grpc_chttp2_transport* t,
+                                      const char* reason, const char* file,
+                                      int line) {
+  t->refs.Ref(grpc_core::DebugLocation(file, line), reason);
+}
 #else
 #define GRPC_CHTTP2_REF_TRANSPORT(t, r) grpc_chttp2_ref_transport(t)
 #define GRPC_CHTTP2_UNREF_TRANSPORT(t, r) grpc_chttp2_unref_transport(t)

src/core/lib/debug/trace.h

@@ -102,8 +102,9 @@ typedef TraceFlag DebugOnlyTraceFlag;
 #else
 class DebugOnlyTraceFlag {
  public:
-  DebugOnlyTraceFlag(bool default_enabled, const char* name) {}
+  constexpr DebugOnlyTraceFlag(bool default_enabled, const char* name) {}
-  bool enabled() { return false; }
+  constexpr bool enabled() const { return false; }
+  constexpr const char* name() const { return "DebugOnlyTraceFlag"; }
 
  private:
   void set_enabled(bool enabled) {}

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()) {
+    IncrementRefCount(location, reason);
-      const grpc_core::RefCount::Value old_refs = refs_.get();
+    return RefCountedPtr<Child>(static_cast<Child*>(this));
-      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();
   }
 
-  // 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()) {
+    if (refs_.Unref(location, reason)) {
-      const grpc_core::RefCount::Value old_refs = refs_.get();
+      Delete(static_cast<Child*>(this));
-      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);
     }
-    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_;
 };
 

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_;
 };
 
@@ -134,54 +181,6 @@ class RefCount {
 //
 template <typename Child, typename Impl = PolymorphicRefCount>
 class RefCounted : public Impl {
- public:
-  RefCountedPtr<Child> Ref() GRPC_MUST_USE_RESULT {
-    IncrementRefCount();
-    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();
@@ -190,58 +189,43 @@ class RefCountedWithTracing : public Impl {
 
   RefCountedPtr<Child> Ref(const DebugLocation& location,
                            const char* reason) GRPC_MUST_USE_RESULT {
-    if (location.Log() && trace_flag_ != nullptr && trace_flag_->enabled()) {
+    IncrementRefCount(location, reason);
-      const RefCount::Value old_refs = refs_.get();
+    return RefCountedPtr<Child>(static_cast<Child*>(this));
-      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();
   }
 
   // TODO(roth): Once all of our code is converted to C++ and can use
-  // RefCountedPtr<> instead of manual ref-counting, make the Unref() methods
+  // RefCountedPtr<> instead of manual ref-counting, make this method
-  // private, since they will only be used by RefCountedPtr<>, which is a
+  // 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()) {
+    if (refs_.Unref(location, reason)) {
-      const RefCount::Value old_refs = refs_.get();
+      Delete(static_cast<Child*>(this));
-      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);
     }
-    Unref();
   }
 
   // Not copyable nor movable.
-  RefCountedWithTracing(const RefCountedWithTracing&) = delete;
+  RefCounted(const RefCounted&) = delete;
-  RefCountedWithTracing& operator=(const RefCountedWithTracing&) = delete;
+  RefCounted& operator=(const RefCounted&) = delete;
 
   GRPC_ABSTRACT_BASE_CLASS
 
  protected:
   GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
 
-  RefCountedWithTracing()
+  // TraceFlagT is defined to accept both DebugOnlyTraceFlag and TraceFlag.
-      : RefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {}
+  // Note: RefCount tracing is only enabled on debug builds, even when a
-
+  //       TraceFlag is used.
-  explicit RefCountedWithTracing(TraceFlag* trace_flag)
+  template <typename TraceFlagT = TraceFlag>
-      : trace_flag_(trace_flag) {}
+  explicit RefCounted(TraceFlagT* trace_flag = nullptr)
-
+      : refs_(1, trace_flag) {}
-#ifdef NDEBUG
-  explicit RefCountedWithTracing(DebugOnlyTraceFlag* trace_flag)
-      : RefCountedWithTracing() {}
-#endif
 
   // 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_;
 };
 

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();
+    reset(other.value_);
-    value_ = other.value_;
     other.value_ = nullptr;
     return *this;
   }
   template <typename Y>
   RefCountedPtr& operator=(RefCountedPtr<Y>&& other) {
-    if (value_ != nullptr) value_->Unref();
+    reset(other.value_);
-    value_ = 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();
+    reset(other.value_);
-    value_ = 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();
+    reset(other.value_);
-    value_ = 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;
   }
-
+  template <typename Y>
-  void reset() {
+  void reset(const DebugLocation& location, const char* reason,
-    if (value_ != nullptr) value_->Unref();
+             Y* value = nullptr) {
-    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

test/core/gprpp/orphanable_test.cc

@@ -83,11 +83,11 @@ TEST(OrphanablePtr, InternallyRefCounted) {
 // things build properly in both debug and non-debug cases.
 DebugOnlyTraceFlag baz_tracer(true, "baz");
 
-class Baz : public InternallyRefCountedWithTracing<Baz> {
+class Baz : public InternallyRefCounted<Baz> {
  public:
   Baz() : Baz(0) {}
   explicit Baz(int value)
-      : InternallyRefCountedWithTracing<Baz>(&baz_tracer), value_(value) {}
+      : InternallyRefCounted<Baz>(&baz_tracer), value_(value) {}
   void Orphan() override { Unref(); }
   int value() const { return value_; }
 

test/core/gprpp/ref_counted_ptr_test.cc

@@ -163,9 +163,9 @@ TEST(MakeRefCounted, Args) {
 
 TraceFlag foo_tracer(true, "foo");
 
-class FooWithTracing : public RefCountedWithTracing<FooWithTracing> {
+class FooWithTracing : public RefCounted<FooWithTracing> {
  public:
-  FooWithTracing() : RefCountedWithTracing(&foo_tracer) {}
+  FooWithTracing() : RefCounted(&foo_tracer) {}
 };
 
 TEST(RefCountedPtr, RefCountedWithTracing) {

test/core/gprpp/ref_counted_test.cc

@@ -74,9 +74,9 @@ TEST(RefCountedNonPolymorphic, ExtraRef) {
 // things build properly in both debug and non-debug cases.
 DebugOnlyTraceFlag foo_tracer(true, "foo");
 
-class FooWithTracing : public RefCountedWithTracing<FooWithTracing> {
+class FooWithTracing : public RefCounted<FooWithTracing> {
  public:
-  FooWithTracing() : RefCountedWithTracing(&foo_tracer) {}
+  FooWithTracing() : RefCounted(&foo_tracer) {}
 };
 
 TEST(RefCountedWithTracing, Basic) {
@@ -92,10 +92,9 @@ TEST(RefCountedWithTracing, Basic) {
 }
 
 class FooNonPolymorphicWithTracing
-    : public RefCountedWithTracing<FooNonPolymorphicWithTracing,
+    : public RefCounted<FooNonPolymorphicWithTracing, NonPolymorphicRefCount> {
-                                   NonPolymorphicRefCount> {
  public:
-  FooNonPolymorphicWithTracing() : RefCountedWithTracing(&foo_tracer) {}
+  FooNonPolymorphicWithTracing() : RefCounted(&foo_tracer) {}
 };
 
 TEST(RefCountedNonPolymorphicWithTracing, Basic) {