From 6f196afc9ea527adf8c81cc7cf7e151e20e32116 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Sat, 20 Jan 2018 11:50:08 -0800 Subject: [PATCH] Switch to use manual constructor --- src/core/lib/channel/channel_tracer.cc | 8 +++++++- src/core/lib/surface/channel.cc | 15 +++++++-------- test/core/channel/channel_tracer_test.cc | 2 ++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/core/lib/channel/channel_tracer.cc b/src/core/lib/channel/channel_tracer.cc index bb8ba969eae..df0e56d19a5 100644 --- a/src/core/lib/channel/channel_tracer.cc +++ b/src/core/lib/channel/channel_tracer.cc @@ -56,11 +56,13 @@ struct TraceEvent { }; ChannelTracer::ChannelTracer(size_t max_nodes) - : num_nodes_logged_(0), + : channel_uuid_(-1), + num_nodes_logged_(0), list_size_(0), max_list_size_(max_nodes), head_trace_(0), tail_trace_(0) { + 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( @@ -70,6 +72,7 @@ ChannelTracer::ChannelTracer(size_t max_nodes) } ChannelTracer* ChannelTracer::Ref() { + if (!max_list_size_) return nullptr; // tracing is disabled if max_nodes == 0 gpr_ref(&refs_); return this; } @@ -84,6 +87,7 @@ void ChannelTracer::FreeNode(TraceEvent* node) { } void ChannelTracer::Unref() { + if (!max_list_size_) return; // tracing is disabled if max_nodes == 0 if (gpr_unref(&refs_)) { TraceEvent* it = head_trace_; while (it != nullptr) { @@ -100,6 +104,7 @@ intptr_t ChannelTracer::GetUuid() { return channel_uuid_; } void ChannelTracer::AddTrace(grpc_slice data, grpc_error* error, grpc_connectivity_state connectivity_state, ChannelTracer* referenced_tracer) { + if (!max_list_size_) return; // tracing is disabled if max_nodes == 0 ++num_nodes_logged_; // create and fill up the new node TraceEvent* new_trace_node = @@ -254,6 +259,7 @@ class ChannelTracerRenderer { }; char* ChannelTracer::RenderTrace(bool recursive) { + if (!max_list_size_) return nullptr; // tracing is disabled if max_nodes == 0 ChannelTracerRenderer renderer(this, recursive); return renderer.Run(); } diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 12b920a2c40..5d36e5b313b 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -32,6 +32,7 @@ #include "src/core/lib/debug/stats.h" #include "src/core/lib/iomgr/iomgr.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/support/manual_constructor.h" #include "src/core/lib/support/memory.h" #include "src/core/lib/support/object_registry.h" #include "src/core/lib/support/string.h" @@ -63,7 +64,7 @@ struct grpc_channel { gpr_mu registered_call_mu; registered_call* registered_calls; - grpc_core::ChannelTracer* tracer; + grpc_core::ManualConstructor tracer; char* target; }; @@ -103,7 +104,7 @@ grpc_channel* grpc_channel_create_with_builder( memset(channel, 0, sizeof(*channel)); channel->target = target; channel->is_client = grpc_channel_stack_type_is_client(channel_stack_type); - channel->tracer = nullptr; + bool tracer_initialized = false; gpr_mu_init(&channel->registered_call_mu); channel->registered_calls = nullptr; @@ -195,15 +196,13 @@ grpc_channel* grpc_channel_create_with_builder( 0x1; /* always support no compression */ } else if (0 == strcmp(args->args[i].key, GRPC_ARG_CHANNEL_TRACING_MAX_NODES)) { - GPR_ASSERT(channel->tracer == nullptr); + GPR_ASSERT(!tracer_initialized); + tracer_initialized = true; // max_nodes defaults to 10, clamped between 0 and 100. const grpc_integer_options options = {10, 0, 100}; size_t max_nodes = (size_t)grpc_channel_arg_get_integer(&args->args[i], options); - if (max_nodes > 0) { - channel->tracer = grpc_core::New( - max_nodes); // TODO(ncteisen): leaky yo - } + channel->tracer.Init(max_nodes); } } @@ -214,7 +213,7 @@ grpc_channel* grpc_channel_create_with_builder( } char* grpc_channel_get_trace(grpc_channel* channel, bool recursive) { - return channel->tracer ? channel->tracer->RenderTrace(recursive) : nullptr; + return channel->tracer->RenderTrace(recursive); } grpc_channel* grpc_channel_create(const char* target, diff --git a/test/core/channel/channel_tracer_test.cc b/test/core/channel/channel_tracer_test.cc index 131ea567fc9..c1c9e5b57f5 100644 --- a/test/core/channel/channel_tracer_test.cc +++ b/test/core/channel/channel_tracer_test.cc @@ -41,6 +41,7 @@ static void add_simple_trace(ChannelTracer* tracer) { static void validate_tracer(ChannelTracer* tracer, size_t expected_num_nodes_logged, size_t max_nodes) { + if (!max_nodes) return; char* json_str = tracer->RenderTrace(true); grpc_json* json = grpc_json_parse_string(json_str); validate_channel_data(json, expected_num_nodes_logged, @@ -51,6 +52,7 @@ static void validate_tracer(ChannelTracer* tracer, static void validate_tracer_data_matches_uuid_lookup(ChannelTracer* 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(true); char* uuid_lookup_json_str = ChannelTracer::GetChannelTraceFromUuid(uuid, true);