Reviewer feedback: singleton TraceFlagList

2 files changed, 46 insertions, 26 deletions

diff --git a/src/core/lib/debug/trace.cc b/src/core/lib/debug/trace.cc
index f7cf49530f..04558fd91e 100644
--- a/src/core/lib/debug/trace.cc
+++ b/src/core/lib/debug/trace.cc
@@ -29,40 +29,31 @@ int grpc_tracer_set_enabled(const char* name, int enabled);
 
 namespace grpc_core {
 
-// there is only ever one global list of tracers for the entire program.
-static TraceFlag* g_root_tracer = nullptr;
+TraceFlagList* TraceFlagList::instance_ = nullptr;
 
-// Flags register themselves on the list during construction
-TraceFlag::TraceFlag(bool default_enabled, const char* name)
-    : next_tracer_(g_root_tracer), name_(name), value_(default_enabled) {
-  g_root_tracer = this;
-}
-
-void TraceFlag::LogAllTracers() {
-  gpr_log(GPR_DEBUG, "available tracers:");
-  TraceFlag* t;
-  for (t = g_root_tracer; t != nullptr; t = t->next_tracer_) {
-    gpr_log(GPR_DEBUG, "\t%s", t->name_);
-  }
+TraceFlagList* TraceFlagList::Get() {
+  if (instance_ == nullptr)
+    instance_ = static_cast<TraceFlagList*>(gpr_malloc(sizeof(TraceFlagList)));
+  return instance_;
 }
 
-bool TraceFlag::Set(const char* name, bool enabled) {
+bool TraceFlagList::Set(const char* name, bool enabled) {
   TraceFlag* t;
   if (0 == strcmp(name, "all")) {
-    for (t = g_root_tracer; t; t = t->next_tracer_) {
+    for (t = root_tracer_; t; t = t->next_tracer_) {
       t->set_enabled(enabled);
     }
   } else if (0 == strcmp(name, "list_tracers")) {
     LogAllTracers();
   } else if (0 == strcmp(name, "refcount")) {
-    for (t = g_root_tracer; t; t = t->next_tracer_) {
+    for (t = root_tracer_; t; t = t->next_tracer_) {
       if (strstr(t->name_, "refcount") != nullptr) {
         t->set_enabled(enabled);
       }
     }
   } else {
     bool found = false;
-    for (t = g_root_tracer; t; t = t->next_tracer_) {
+    for (t = root_tracer_; t; t = t->next_tracer_) {
       if (0 == strcmp(name, t->name_)) {
         t->set_enabled(enabled);
         found = true;
@@ -76,6 +67,25 @@ bool TraceFlag::Set(const char* name, bool enabled) {
   return true;
 }
 
+void TraceFlagList::Add(TraceFlag* flag) {
+  flag->next_tracer_ = root_tracer_;
+  root_tracer_ = flag;
+}
+
+void TraceFlagList::LogAllTracers() {
+  gpr_log(GPR_DEBUG, "available tracers:");
+  TraceFlag* t;
+  for (t = root_tracer_; t != nullptr; t = t->next_tracer_) {
+    gpr_log(GPR_DEBUG, "\t%s", t->name_);
+  }
+}
+
+// Flags register themselves on the list during construction
+TraceFlag::TraceFlag(bool default_enabled, const char* name)
+    : name_(name), value_(default_enabled) {
+  TraceFlagList::Get()->Add(this);
+}
+
 }  // namespace grpc_core
 
 static void add(const char* beg, const char* end, char*** ss, size_t* ns) {
@@ -111,9 +121,9 @@ static void parse(const char* s) {
 
   for (i = 0; i < nstrings; i++) {
     if (strings[i][0] == '-') {
-      grpc_core::TraceFlag::Set(strings[i] + 1, false);
+      grpc_core::TraceFlagList::Get()->Set(strings[i] + 1, false);
     } else {
-      grpc_core::TraceFlag::Set(strings[i], true);
+      grpc_core::TraceFlagList::Get()->Set(strings[i], true);
     }
   }
 
@@ -134,5 +144,5 @@ void grpc_tracer_init(const char* env_var) {
 void grpc_tracer_shutdown(void) {}
 
 int grpc_tracer_set_enabled(const char* name, int enabled) {
-  return grpc_core::TraceFlag::Set(name, enabled != 0);
+  return grpc_core::TraceFlagList::Get()->Set(name, enabled != 0);
 }
diff --git a/src/core/lib/debug/trace.h b/src/core/lib/debug/trace.h
index 9bda744f38..3371a7c20d 100644
--- a/src/core/lib/debug/trace.h
+++ b/src/core/lib/debug/trace.h
@@ -44,9 +44,23 @@ void grpc_tracer_shutdown(void);
 
 namespace grpc_core {
 
+class TraceFlag;
+class TraceFlagList {
+ public:
+  static TraceFlagList* Get();
+  bool Set(const char* name, bool enabled);
+  void Add(TraceFlag* flag);
+
+ private:
+  static TraceFlagList* instance_;
+  void LogAllTracers();
+  TraceFlagList() : root_tracer_(nullptr) {}
+  TraceFlag* root_tracer_;
+};
+
 class TraceFlag {
   friend class TraceFlagPeer;
-  friend class TracerList;
+  friend class TraceFlagList;
 
  public:
   TraceFlag(bool default_enabled, const char* name);
@@ -54,8 +68,6 @@ class TraceFlag {
 
   const char* name() const { return name_; }
 
-  static bool Set(const char* name, bool enabled);
-
   bool enabled() {
 #ifdef GRPC_THREADSAFE_TRACER
     return gpr_atm_no_barrier_load(&value_) != 0;
@@ -65,8 +77,6 @@ class TraceFlag {
   }
 
  private:
-  static void LogAllTracers();
-
   void set_enabled(bool enabled) {
 #ifdef GRPC_THREADSAFE_TRACER
     gpr_atm_no_barrier_store(&value_, enabled);