reviewer feedback

3 files changed, 23 insertions, 16 deletions

diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc
index e9c29489a2..6d77e1bbcc 100644
--- a/src/core/lib/channel/channelz.cc
+++ b/src/core/lib/channel/channelz.cc
@@ -57,8 +57,8 @@ char* BaseNode::RenderJsonString() {
 
 CallCountingHelper::CallCountingHelper() {
   num_cores_ = GPR_MAX(1, gpr_cpu_num_cores());
-  per_cpu_counter_data_storage_ =
-      static_cast<CounterData*>(gpr_zalloc(sizeof(CounterData) * num_cores_));
+  per_cpu_counter_data_storage_ = static_cast<AtomicCounterData*>(
+      gpr_zalloc(sizeof(AtomicCounterData) * num_cores_));
 }
 
 CallCountingHelper::~CallCountingHelper() {
@@ -90,28 +90,27 @@ void CallCountingHelper::RecordCallSucceeded() {
       static_cast<gpr_atm>(1));
 }
 
-CallCountingHelper::CounterData CallCountingHelper::Collect() {
-  CounterData out;
-  memset(&out, 0, sizeof(out));
+void CallCountingHelper::CollectData(CounterData* out) {
+  memset(out, 0, sizeof(*out));
   for (size_t core = 0; core < num_cores_; ++core) {
-    out.calls_started_ += gpr_atm_no_barrier_load(
+    out->calls_started_ += gpr_atm_no_barrier_load(
         &per_cpu_counter_data_storage_[core].calls_started_);
-    out.calls_succeeded_ += gpr_atm_no_barrier_load(
+    out->calls_succeeded_ += gpr_atm_no_barrier_load(
         &per_cpu_counter_data_storage_[core].calls_succeeded_);
-    out.calls_failed_ += gpr_atm_no_barrier_load(
+    out->calls_failed_ += gpr_atm_no_barrier_load(
         &per_cpu_counter_data_storage_[core].calls_failed_);
     gpr_atm last_call = gpr_atm_no_barrier_load(
         &per_cpu_counter_data_storage_[core].last_call_started_millis_);
-    if (last_call > out.last_call_started_millis_) {
-      out.last_call_started_millis_ = last_call;
+    if (last_call > out->last_call_started_millis_) {
+      out->last_call_started_millis_ = last_call;
     }
   }
-  return out;
 }
 
 void CallCountingHelper::PopulateCallCounts(grpc_json* json) {
   grpc_json* json_iterator = nullptr;
-  CounterData data = Collect();
+  CounterData data;
+  CollectData(&data);
   if (data.calls_started_ != 0) {
     json_iterator = grpc_json_add_number_string_child(
         json, json_iterator, "callsStarted", data.calls_started_);
diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h
index ee2ca40a20..ae3e47a1f6 100644
--- a/src/core/lib/channel/channelz.h
+++ b/src/core/lib/channel/channelz.h
@@ -101,17 +101,24 @@ class CallCountingHelper {
   // testing peer friend.
   friend class testing::CallCountingHelperPeer;
 
-  struct CounterData {
+  struct AtomicCounterData {
     gpr_atm calls_started_ = 0;
     gpr_atm calls_succeeded_ = 0;
     gpr_atm calls_failed_ = 0;
     gpr_atm last_call_started_millis_ = 0;
   };
 
+  struct CounterData {
+    intptr_t calls_started_ = 0;
+    intptr_t calls_succeeded_ = 0;
+    intptr_t calls_failed_ = 0;
+    intptr_t last_call_started_millis_ = 0;
+  };
+
   // collects the sharded data into one CounterData struct.
-  CounterData Collect();
+  void CollectData(CounterData* out);
 
-  CounterData* per_cpu_counter_data_storage_ = nullptr;
+  AtomicCounterData* per_cpu_counter_data_storage_ = nullptr;
   size_t num_cores_ = 0;
 };
 
diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc
index b7b35aede4..f99893521c 100644
--- a/test/core/channel/channelz_test.cc
+++ b/test/core/channel/channelz_test.cc
@@ -49,7 +49,8 @@ class CallCountingHelperPeer {
  public:
   explicit CallCountingHelperPeer(CallCountingHelper* node) : node_(node) {}
   grpc_millis last_call_started_millis() const {
-    CallCountingHelper::CounterData data = node_->Collect();
+    CallCountingHelper::CounterData data;
+    node_->CollectData(&data);
     return (grpc_millis)gpr_atm_no_barrier_load(
         &data.last_call_started_millis_);
   }