Switch to using RefCounted

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

@@ -41,6 +41,7 @@
 #include "src/core/lib/debug/stats.h"
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/manual_constructor.h"
+#include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/sockaddr_utils.h"
 #include "src/core/lib/iomgr/timer.h"
 #include "src/core/lib/profiling/timers.h"
@@ -134,7 +135,7 @@ struct grpc_subchannel {
   /** our alarm */
   grpc_timer alarm;
 
-  grpc_core::ManualConstructor<grpc_core::ChannelTracer> tracer;
+  grpc_core::RefCountedPtr<grpc_core::ChannelTracer> tracer;
 };
 
 struct grpc_subchannel_call {

src/core/lib/channel/channel_tracer.cc

@@ -40,12 +40,12 @@ class TraceEvent {
  public:
   TraceEvent(grpc_slice data, grpc_error* error,
              grpc_connectivity_state connectivity_state,
-             ChannelTracer* referenced_tracer)
+             RefCountedPtr<ChannelTracer> referenced_tracer)
       : data_(data),
         error_(error),
         connectivity_state_(connectivity_state),
         next_(nullptr) {
-    referenced_tracer_ = referenced_tracer ? referenced_tracer->Ref() : nullptr;
+    referenced_tracer_ = referenced_tracer;
     time_created_ = gpr_now(GPR_CLOCK_REALTIME);
   }
 
@@ -58,7 +58,7 @@ class TraceEvent {
   grpc_connectivity_state connectivity_state_;
   TraceEvent* next_;
   // the tracer object for the (sub)channel that this trace node refers to.
-  ChannelTracer* referenced_tracer_;
+  RefCountedPtr<ChannelTracer> referenced_tracer_;
 };
 
 ChannelTracer::ChannelTracer(size_t max_nodes)
@@ -71,46 +71,34 @@ ChannelTracer::ChannelTracer(size_t max_nodes)
       tail_trace_(nullptr) {
   if (!max_list_size_) return;  // tracing is disabled if max_nodes == 0
   gpr_mu_init(&tracer_mu_);
-  gpr_ref_init(&refs_, 1);
   channel_uuid_ = grpc_object_registry_register_object(
       this, GRPC_OBJECT_REGISTRY_CHANNEL_TRACER);
   max_list_size_ = max_nodes;
   time_created_ = gpr_now(GPR_CLOCK_REALTIME);
 }
 
-ChannelTracer* ChannelTracer::Ref() {
-  if (!max_list_size_) return nullptr;  // tracing is disabled if max_nodes == 0
-  gpr_ref(&refs_);
-  return this;
-}
-
 void ChannelTracer::FreeNode(TraceEvent* node) {
   GRPC_ERROR_UNREF(node->error_);
-  if (node->referenced_tracer_) {
-    node->referenced_tracer_->Unref();
-  }
   grpc_slice_unref_internal(node->data_);
   gpr_free(node);
 }
 
-void ChannelTracer::Unref() {
+ChannelTracer::~ChannelTracer() {
   if (!max_list_size_) return;  // tracing is disabled if max_nodes == 0
-  if (gpr_unref(&refs_)) {
-    TraceEvent* it = head_trace_;
-    while (it != nullptr) {
-      TraceEvent* to_free = it;
-      it = it->next_;
-      FreeNode(to_free);
-    }
-    gpr_mu_destroy(&tracer_mu_);
+  TraceEvent* it = head_trace_;
+  while (it != nullptr) {
+    TraceEvent* to_free = it;
+    it = it->next_;
+    FreeNode(to_free);
   }
+  gpr_mu_destroy(&tracer_mu_);
 }
 
 intptr_t ChannelTracer::GetUuid() { return channel_uuid_; }
 
 void ChannelTracer::AddTrace(grpc_slice data, grpc_error* error,
                              grpc_connectivity_state connectivity_state,
-                             ChannelTracer* referenced_tracer) {
+                             RefCountedPtr<ChannelTracer> referenced_tracer) {
   if (!max_list_size_) return;  // tracing is disabled if max_nodes == 0
   ++num_nodes_logged_;
   if (referenced_tracer != nullptr) ++num_children_seen_;
@@ -136,6 +124,11 @@ void ChannelTracer::AddTrace(grpc_slice data, grpc_error* error,
   }
 }
 
+void ChannelTracer::AddTrace(grpc_slice data, grpc_error* error,
+                             grpc_connectivity_state connectivity_state) {
+  AddTrace(data, error, connectivity_state, RefCountedPtr<ChannelTracer>());
+}
+
 // returns an allocated string that represents tm according to RFC-3339.
 static char* fmt_time(gpr_timespec tm) {
   char buffer[35];
@@ -297,12 +290,12 @@ class ChannelTracerRenderer {
       // If we are recursively populating everything, and this node
       // references a tracer we haven't seen yet, we render that tracer
       // in full, adding it to the parent JSON's "children" field.
-      if (children && !TracerAlreadySeen(node->referenced_tracer_)) {
+      if (children && !TracerAlreadySeen(node->referenced_tracer_.get())) {
         grpc_json* referenced_tracer = grpc_json_create_child(
             nullptr, children, nullptr, nullptr, GRPC_JSON_OBJECT, false);
-        AddSeenTracer(node->referenced_tracer_);
+        AddSeenTracer(node->referenced_tracer_.get());
         ChannelTracer* saved = current_tracer_;
-        current_tracer_ = node->referenced_tracer_;
+        current_tracer_ = node->referenced_tracer_.get();
         RecursivelyPopulateJson(referenced_tracer);
         current_tracer_ = saved;
       }

src/core/lib/channel/channel_tracer.h

@@ -20,6 +20,8 @@
 #define GRPC_CORE_LIB_CHANNEL_CHANNEL_TRACER_H
 
 #include <grpc/grpc.h>
+#include "src/core/lib/gprpp/ref_counted.h"
+#include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/json/json.h"
 
@@ -27,22 +29,24 @@ namespace grpc_core {
 
 class TraceEvent;
 
-class ChannelTracer {
+class ChannelTracer : public RefCounted {
  public:
   ChannelTracer(size_t max_nodes);
-  ~ChannelTracer() {}
-
-  // TODO(ncteisen): incorporate RefCounted class
-  ChannelTracer* Ref();
-  void Unref();
+  ~ChannelTracer();
 
   /* returns the tracers uuid */
   intptr_t GetUuid();
 
   /* Adds a new trace node to the tracing object */
+  void AddTrace(grpc_slice data, grpc_error* error,
+                grpc_connectivity_state connectivity_state);
+
+  /* Adds a new trace node to the tracing object. This trace node refers to a
+     an event on a child of the channel. For example this could log when a
+     particular subchannel becomes connected */
   void AddTrace(grpc_slice data, grpc_error* error,
                 grpc_connectivity_state connectivity_state,
-                ChannelTracer* subchannel);
+                RefCountedPtr<ChannelTracer> referenced_tracer);
 
   /* Returns the tracing data rendered as a grpc json string.
      The string is owned by the caller and must be freed. If recursive
@@ -59,7 +63,6 @@ class ChannelTracer {
   void FreeNode(TraceEvent* node);
 
   friend class ChannelTracerRenderer;
-  gpr_refcount refs_;
   gpr_mu tracer_mu_;
   intptr_t channel_uuid_;
   uint64_t num_nodes_logged_;

src/core/lib/surface/channel.cc

@@ -34,6 +34,7 @@
 #include "src/core/lib/gpr/string.h"
 #include "src/core/lib/gprpp/manual_constructor.h"
 #include "src/core/lib/gprpp/memory.h"
+#include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/iomgr.h"
 #include "src/core/lib/slice/slice_internal.h"
 #include "src/core/lib/surface/api_trace.h"
@@ -64,7 +65,7 @@ struct grpc_channel {
   gpr_mu registered_call_mu;
   registered_call* registered_calls;
 
-  grpc_core::ManualConstructor<grpc_core::ChannelTracer> tracer;
+  grpc_core::RefCountedPtr<grpc_core::ChannelTracer> tracer;
 
   char* target;
 };
@@ -202,13 +203,14 @@ grpc_channel* grpc_channel_create_with_builder(
       const grpc_integer_options options = {10, 0, 100};
       size_t max_nodes =
           (size_t)grpc_channel_arg_get_integer(&args->args[i], options);
-      channel->tracer.Init(max_nodes);
+      channel->tracer =
+          grpc_core::MakeRefCounted<grpc_core::ChannelTracer>(max_nodes);
     }
   }
 
   grpc_channel_args_destroy(args);
   channel->tracer->AddTrace(grpc_slice_from_static_string("Channel created"),
-                            GRPC_ERROR_NONE, GRPC_CHANNEL_IDLE, nullptr);
+                            GRPC_ERROR_NONE, GRPC_CHANNEL_IDLE);
   return channel;
 }