From 3c2024c6af1a4212f8ed76f6220e3f7731959b6b Mon Sep 17 00:00:00 2001 From: ncteisen Date: Fri, 5 Oct 2018 09:41:22 -0700 Subject: [PATCH] reviewer comments --- src/core/lib/channel/channelz.cc | 43 +++++++++++++++--------------- src/core/lib/channel/channelz.h | 16 +++++------ test/core/channel/channelz_test.cc | 3 +-- 3 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 6d77e1bbcce..573292fba12 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -68,41 +68,40 @@ CallCountingHelper::~CallCountingHelper() { void CallCountingHelper::RecordCallStarted() { gpr_atm_no_barrier_fetch_add( &per_cpu_counter_data_storage_[grpc_core::ExecCtx::Get()->starting_cpu()] - .calls_started_, + .calls_started, static_cast(1)); gpr_atm_no_barrier_store( &per_cpu_counter_data_storage_[grpc_core::ExecCtx::Get()->starting_cpu()] - .last_call_started_millis_, + .last_call_started_millis, (gpr_atm)ExecCtx::Get()->Now()); } void CallCountingHelper::RecordCallFailed() { gpr_atm_no_barrier_fetch_add( &per_cpu_counter_data_storage_[grpc_core::ExecCtx::Get()->starting_cpu()] - .calls_failed_, + .calls_failed, static_cast(1)); } void CallCountingHelper::RecordCallSucceeded() { gpr_atm_no_barrier_fetch_add( &per_cpu_counter_data_storage_[grpc_core::ExecCtx::Get()->starting_cpu()] - .calls_succeeded_, + .calls_succeeded, static_cast(1)); } 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( - &per_cpu_counter_data_storage_[core].calls_started_); - out->calls_succeeded_ += gpr_atm_no_barrier_load( - &per_cpu_counter_data_storage_[core].calls_succeeded_); - out->calls_failed_ += gpr_atm_no_barrier_load( - &per_cpu_counter_data_storage_[core].calls_failed_); + out->calls_started += gpr_atm_no_barrier_load( + &per_cpu_counter_data_storage_[core].calls_started); + out->calls_succeeded += gpr_atm_no_barrier_load( + &per_cpu_counter_data_storage_[core].calls_succeeded); + 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; + &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; } } } @@ -111,20 +110,20 @@ void CallCountingHelper::PopulateCallCounts(grpc_json* json) { grpc_json* json_iterator = nullptr; CounterData data; CollectData(&data); - if (data.calls_started_ != 0) { + if (data.calls_started != 0) { json_iterator = grpc_json_add_number_string_child( - json, json_iterator, "callsStarted", data.calls_started_); + json, json_iterator, "callsStarted", data.calls_started); } - if (data.calls_succeeded_ != 0) { + if (data.calls_succeeded != 0) { json_iterator = grpc_json_add_number_string_child( - json, json_iterator, "callsSucceeded", data.calls_succeeded_); + json, json_iterator, "callsSucceeded", data.calls_succeeded); } - if (data.calls_failed_) { + if (data.calls_failed) { json_iterator = grpc_json_add_number_string_child( - json, json_iterator, "callsFailed", data.calls_failed_); + json, json_iterator, "callsFailed", data.calls_failed); } - if (data.calls_started_ != 0) { - gpr_timespec ts = grpc_millis_to_timespec(data.last_call_started_millis_, + if (data.calls_started != 0) { + gpr_timespec ts = grpc_millis_to_timespec(data.last_call_started_millis, GPR_CLOCK_REALTIME); json_iterator = grpc_json_create_child(json_iterator, json, "lastCallStartedTimestamp", diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index ae3e47a1f60..27f6bfc8825 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -102,17 +102,17 @@ class CallCountingHelper { friend class testing::CallCountingHelperPeer; struct AtomicCounterData { - gpr_atm calls_started_ = 0; - gpr_atm calls_succeeded_ = 0; - gpr_atm calls_failed_ = 0; - gpr_atm last_call_started_millis_ = 0; + 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; + 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. diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index f99893521c1..2454079237c 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -51,8 +51,7 @@ class CallCountingHelperPeer { grpc_millis last_call_started_millis() const { CallCountingHelper::CounterData data; node_->CollectData(&data); - return (grpc_millis)gpr_atm_no_barrier_load( - &data.last_call_started_millis_); + return (grpc_millis)gpr_atm_no_barrier_load(&data.last_call_started_millis); } private: