From 3be52f9bccf48177944b47228a176233d482c67d Mon Sep 17 00:00:00 2001 From: ncteisen Date: Fri, 11 May 2018 12:34:02 -0400 Subject: [PATCH] C++-ize the registry --- src/core/lib/channel/channel_trace.cc | 4 +- src/core/lib/channel/channelz_registry.cc | 83 ++++++++++++--------- src/core/lib/channel/channelz_registry.h | 51 ++++++++++--- src/core/lib/surface/init.cc | 4 +- test/core/channel/channel_trace_test.cc | 2 +- test/core/channel/channelz_registry_test.cc | 4 +- 6 files changed, 93 insertions(+), 55 deletions(-) diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc index 8665e017763..8b459c10c56 100644 --- a/src/core/lib/channel/channel_trace.cc +++ b/src/core/lib/channel/channel_trace.cc @@ -70,7 +70,7 @@ ChannelTrace::ChannelTrace(size_t max_events) tail_trace_(nullptr) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 gpr_mu_init(&tracer_mu_); - channel_uuid_ = grpc_channelz_registry_register_channel_trace(this); + channel_uuid_ = ChannelzRegistry::Default()->Register(this); time_created_ = grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME); } @@ -83,7 +83,7 @@ ChannelTrace::~ChannelTrace() { it = it->next(); Delete(to_free); } - grpc_channelz_registry_unregister_channel_trace(channel_uuid_); + ChannelzRegistry::Default()->Unregister(channel_uuid_); gpr_mu_destroy(&tracer_mu_); } diff --git a/src/core/lib/channel/channelz_registry.cc b/src/core/lib/channel/channelz_registry.cc index 77e23bf69bc..1680f03522a 100644 --- a/src/core/lib/channel/channelz_registry.cc +++ b/src/core/lib/channel/channelz_registry.cc @@ -18,63 +18,76 @@ #include -#include "src/core/lib/avl/avl.h" #include "src/core/lib/channel/channel_trace.h" #include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/gpr/useful.h" +#include "src/core/lib/gprpp/memory.h" #include #include -// file global lock and avl. -static gpr_mu g_mu; -static grpc_avl g_avl; -static gpr_atm g_uuid = 0; +namespace grpc_core { +namespace { -// avl vtable for uuid (intptr_t) -> ChannelTrace +// singleton instance of the registry. +ChannelzRegistry* g_channelz_registry = nullptr; + +// avl vtable for uuid (intptr_t) -> channelz_obj (void*) // this table is only looking, it does not own anything. -static void destroy_intptr(void* not_used, void* user_data) {} -static void* copy_intptr(void* key, void* user_data) { return key; } -static long compare_intptr(void* key1, void* key2, void* user_data) { +void destroy_intptr(void* not_used, void* user_data) {} +void* copy_intptr(void* key, void* user_data) { return key; } +long compare_intptr(void* key1, void* key2, void* user_data) { return GPR_ICMP(key1, key2); } -static void destroy_channel_trace(void* trace, void* user_data) {} -static void* copy_channel_trace(void* trace, void* user_data) { return trace; } -static const grpc_avl_vtable avl_vtable = { - destroy_intptr, copy_intptr, compare_intptr, destroy_channel_trace, - copy_channel_trace}; +void destroy_channelz_obj(void* channelz_obj, void* user_data) {} +void* copy_channelz_obj(void* channelz_obj, void* user_data) { + return channelz_obj; +} +const grpc_avl_vtable avl_vtable = {destroy_intptr, copy_intptr, compare_intptr, + destroy_channelz_obj, copy_channelz_obj}; + +} // anonymous namespace + +void ChannelzRegistry::Init() { g_channelz_registry = New(); } -void grpc_channelz_registry_init() { - gpr_mu_init(&g_mu); - g_avl = grpc_avl_create(&avl_vtable); +void ChannelzRegistry::Shutdown() { Delete(g_channelz_registry); } + +ChannelzRegistry* ChannelzRegistry::Default() { + GPR_DEBUG_ASSERT(g_channelz_registry != nullptr); + return g_channelz_registry; +} + +ChannelzRegistry::ChannelzRegistry() : uuid_(1) { + gpr_mu_init(&mu_); + avl_ = grpc_avl_create(&avl_vtable); } -void grpc_channelz_registry_shutdown() { - grpc_avl_unref(g_avl, nullptr); - gpr_mu_destroy(&g_mu); +ChannelzRegistry::~ChannelzRegistry() { + grpc_avl_unref(avl_, nullptr); + gpr_mu_destroy(&mu_); } -intptr_t grpc_channelz_registry_register_channel_trace( - grpc_core::ChannelTrace* channel_trace) { - intptr_t prior = gpr_atm_no_barrier_fetch_add(&g_uuid, 1); - gpr_mu_lock(&g_mu); - g_avl = grpc_avl_add(g_avl, (void*)prior, channel_trace, nullptr); - gpr_mu_unlock(&g_mu); +intptr_t ChannelzRegistry::Register(grpc_core::ChannelTrace* channel_trace) { + intptr_t prior = gpr_atm_no_barrier_fetch_add(&uuid_, 1); + gpr_mu_lock(&mu_); + avl_ = grpc_avl_add(avl_, (void*)prior, channel_trace, nullptr); + gpr_mu_unlock(&mu_); return prior; } -void grpc_channelz_registry_unregister_channel_trace(intptr_t uuid) { - gpr_mu_lock(&g_mu); - g_avl = grpc_avl_remove(g_avl, (void*)uuid, nullptr); - gpr_mu_unlock(&g_mu); +void ChannelzRegistry::Unregister(intptr_t uuid) { + gpr_mu_lock(&mu_); + avl_ = grpc_avl_remove(avl_, (void*)uuid, nullptr); + gpr_mu_unlock(&mu_); } -grpc_core::ChannelTrace* grpc_channelz_registry_get_channel_trace( - intptr_t uuid) { - gpr_mu_lock(&g_mu); +grpc_core::ChannelTrace* ChannelzRegistry::Get(intptr_t uuid) { + gpr_mu_lock(&mu_); grpc_core::ChannelTrace* ret = static_cast( - grpc_avl_get(g_avl, (void*)uuid, nullptr)); - gpr_mu_unlock(&g_mu); + grpc_avl_get(avl_, (void*)uuid, nullptr)); + gpr_mu_unlock(&mu_); return ret; } + +} // namespace grpc_core diff --git a/src/core/lib/channel/channelz_registry.h b/src/core/lib/channel/channelz_registry.h index 1f7501a31e6..1e4ee6edd9c 100644 --- a/src/core/lib/channel/channelz_registry.h +++ b/src/core/lib/channel/channelz_registry.h @@ -21,23 +21,50 @@ #include +#include "src/core/lib/avl/avl.h" #include "src/core/lib/channel/channel_trace.h" #include -// TODO(ncteisen): convert this file to C++ +namespace grpc_core { -void grpc_channelz_registry_init(); -void grpc_channelz_registry_shutdown(); +// singleton registry object to track all objects that are needed to support +// channelz bookkeeping. All objects share globally distributed uuids. +class ChannelzRegistry { + public: + // These functions ensure singleton like behavior. We cannot use the normal + // pattern of a get functions with a static function variable due to build + // complications. -// globally registers a ChannelTrace. Returns its unique uuid -intptr_t grpc_channelz_registry_register_channel_trace( - grpc_core::ChannelTrace* channel_trace); -// globally unregisters the ChannelTrace that is associated to uuid. -void grpc_channelz_registry_unregister_channel_trace(intptr_t uuid); -// if object with uuid has previously been registered, returns the ChannelTrace -// associated with that uuid. Else returns nullptr. -grpc_core::ChannelTrace* grpc_channelz_registry_get_channel_trace( - intptr_t uuid); + // To be called in grpc_init() + static void Init(); + + // To be callen in grpc_shutdown(); + static void Shutdown(); + + // Returned the singleton instance of ChannelzRegistry; + static ChannelzRegistry* Default(); + + // globally registers a ChannelTrace. Returns its unique uuid + intptr_t Register(grpc_core::ChannelTrace* channel_trace); + // globally unregisters the ChannelTrace that is associated to uuid. + void Unregister(intptr_t uuid); + // if object with uuid has previously been registered, returns the + // ChannelTrace associated with that uuid. Else returns nullptr. + grpc_core::ChannelTrace* Get(intptr_t uuid); + + private: + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE + + gpr_mu mu_; + grpc_avl avl_; + gpr_atm uuid_; + + ChannelzRegistry(); + ~ChannelzRegistry(); +}; + +} // namespace grpc_core #endif /* GRPC_CORE_LIB_CHANNEL_CHANNELz_REGISTRY_H */ diff --git a/src/core/lib/surface/init.cc b/src/core/lib/surface/init.cc index cbc7c2dc966..16be81e9c2d 100644 --- a/src/core/lib/surface/init.cc +++ b/src/core/lib/surface/init.cc @@ -127,7 +127,7 @@ void grpc_init(void) { grpc_slice_intern_init(); grpc_mdctx_global_init(); grpc_channel_init_init(); - grpc_channelz_registry_init(); + grpc_core::ChannelzRegistry::Init(); grpc_security_pre_init(); grpc_core::ExecCtx::GlobalInit(); grpc_iomgr_init(); @@ -176,7 +176,7 @@ void grpc_shutdown(void) { grpc_mdctx_global_shutdown(); grpc_handshaker_factory_registry_shutdown(); grpc_slice_intern_shutdown(); - grpc_channelz_registry_shutdown(); + grpc_core::ChannelzRegistry::Shutdown(); grpc_stats_shutdown(); grpc_core::Fork::GlobalShutdown(); } diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index da644d00d57..b91956a6635 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -99,7 +99,7 @@ void ValidateTraceDataMatchedUuidLookup(RefCountedPtr tracer) { intptr_t uuid = tracer->GetUuid(); if (uuid == -1) return; // Doesn't make sense to lookup if tracing disabled char* tracer_json_str = tracer->RenderTrace(); - ChannelTrace* uuid_lookup = grpc_channelz_registry_get_channel_trace(uuid); + ChannelTrace* uuid_lookup = ChannelzRegistry::Default()->Get(uuid); char* uuid_lookup_json_str = uuid_lookup->RenderTrace(); EXPECT_EQ(strcmp(tracer_json_str, uuid_lookup_json_str), 0); gpr_free(tracer_json_str); diff --git a/test/core/channel/channelz_registry_test.cc b/test/core/channel/channelz_registry_test.cc index 9372f765164..8bc651d1886 100644 --- a/test/core/channel/channelz_registry_test.cc +++ b/test/core/channel/channelz_registry_test.cc @@ -41,9 +41,7 @@ namespace testing { // Tests basic ChannelTrace functionality like construction, adding trace, and // lookups by uuid. -TEST(ChannelzRegistryTest, BasicTest) { - -} +TEST(ChannelzRegistryTest, BasicTest) {} } // namespace testing } // namespace grpc_core