reviewer feedback

5 files changed, 23 insertions, 25 deletions

diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.cc b/src/core/ext/filters/client_channel/client_channel_channelz.cc
index 8d02304d19..b66c920b90 100644
--- a/src/core/ext/filters/client_channel/client_channel_channelz.cc
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc
@@ -172,9 +172,8 @@ grpc_json* SubchannelNode::RenderJson() {
   if (socket_uuid != 0) {
     grpc_json* array_parent = grpc_json_create_child(
         nullptr, json, "socketRef", nullptr, GRPC_JSON_ARRAY, false);
-    json_iterator =
-        grpc_json_create_child(json_iterator, array_parent, nullptr, nullptr,
-                               GRPC_JSON_OBJECT, false);
+    json_iterator = grpc_json_create_child(json_iterator, array_parent, nullptr,
+                                           nullptr, GRPC_JSON_OBJECT, false);
     grpc_json_add_number_string_child(json_iterator, nullptr, "socketId",
                                       socket_uuid);
   }
diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.h b/src/core/ext/filters/client_channel/client_channel_channelz.h
index a6dae16522..8ce331e529 100644
--- a/src/core/ext/filters/client_channel/client_channel_channelz.h
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.h
@@ -76,9 +76,6 @@ class SubchannelNode : public BaseNode {
 
   grpc_json* RenderJson() override;
 
-  // helper to populate the socket(s) that this subchannel owns.
-  void PopulateChildSockets(grpc_json* json);
-
   // proxy methods to composed classes.
   void AddTraceEvent(ChannelTrace::Severity severity, grpc_slice data) {
     trace_.AddTraceEvent(severity, data);
diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 4c33636646..4756733f1f 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -97,10 +97,6 @@ struct grpc_subchannel {
   /** set during connection */
   grpc_connect_out_args connecting_result;
 
-  /** uuid of this subchannel's socket. 0 if this subchannel is not
-      connected */
-  intptr_t socket_uuid;
-
   /** callback for connection finishing */
   grpc_closure on_connected;
 
@@ -258,7 +254,6 @@ static void disconnect(grpc_subchannel* c) {
   c->disconnected = true;
   grpc_connector_shutdown(c->connector, GRPC_ERROR_CREATE_FROM_STATIC_STRING(
                                             "Subchannel disconnected"));
-  c->socket_uuid = 0;
   c->connected_subchannel.reset();
   gpr_mu_unlock(&c->mu);
 }
@@ -416,9 +411,12 @@ grpc_core::channelz::SubchannelNode* grpc_subchannel_get_channelz_node(
   return subchannel->channelz_subchannel.get();
 }
 
-intptr_t grpc_subchannel_get_child_socket_uuid(
-    grpc_subchannel* subchannel) {
-  return subchannel->socket_uuid;
+intptr_t grpc_subchannel_get_child_socket_uuid(grpc_subchannel* subchannel) {
+  if (subchannel->connected_subchannel != nullptr) {
+    return subchannel->connected_subchannel->socket_uuid();
+  } else {
+    return 0;
+  }
 }
 
 static void continue_connect_locked(grpc_subchannel* c) {
@@ -579,7 +577,6 @@ static void on_connected_subchannel_connectivity_changed(void* p,
                   grpc_connectivity_state_name(
                       connected_subchannel_watcher->connectivity_state));
         }
-        c->socket_uuid = 0;
         c->connected_subchannel.reset();
         grpc_connectivity_state_set(&c->state_tracker,
                                     GRPC_CHANNEL_TRANSIENT_FAILURE,
@@ -632,7 +629,8 @@ static bool publish_transport_locked(grpc_subchannel* c) {
     GRPC_ERROR_UNREF(error);
     return false;
   }
-  c->socket_uuid = grpc_transport_get_socket_uuid(c->connecting_result.transport);
+  intptr_t socket_uuid =
+      grpc_transport_get_socket_uuid(c->connecting_result.transport);
   memset(&c->connecting_result, 0, sizeof(c->connecting_result));
 
   /* initialize state watcher */
@@ -653,7 +651,7 @@ static bool publish_transport_locked(grpc_subchannel* c) {
 
   /* publish */
   c->connected_subchannel.reset(grpc_core::New<grpc_core::ConnectedSubchannel>(
-      stk, c->channelz_subchannel.get()));
+      stk, c->channelz_subchannel.get(), socket_uuid));
   gpr_log(GPR_INFO, "New connected subchannel at %p for subchannel %p",
           c->connected_subchannel.get(), c);
 
@@ -823,10 +821,11 @@ namespace grpc_core {
 
 ConnectedSubchannel::ConnectedSubchannel(
     grpc_channel_stack* channel_stack,
-    channelz::SubchannelNode* channelz_subchannel)
+    channelz::SubchannelNode* channelz_subchannel, intptr_t socket_uuid)
     : RefCountedWithTracing<ConnectedSubchannel>(&grpc_trace_stream_refcount),
       channel_stack_(channel_stack),
-      channelz_subchannel_(channelz_subchannel) {}
+      channelz_subchannel_(channelz_subchannel),
+      socket_uuid_(socket_uuid) {}
 
 ConnectedSubchannel::~ConnectedSubchannel() {
   GRPC_CHANNEL_STACK_UNREF(channel_stack_, "connected_subchannel_dtor");
diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h
index 67a2d49711..5d95a72085 100644
--- a/src/core/ext/filters/client_channel/subchannel.h
+++ b/src/core/ext/filters/client_channel/subchannel.h
@@ -86,7 +86,8 @@ class ConnectedSubchannel : public RefCountedWithTracing<ConnectedSubchannel> {
   };
 
   explicit ConnectedSubchannel(grpc_channel_stack* channel_stack,
-                               channelz::SubchannelNode* channelz_subchannel);
+                               channelz::SubchannelNode* channelz_subchannel,
+                               intptr_t socket_uuid);
   ~ConnectedSubchannel();
 
   grpc_channel_stack* channel_stack() { return channel_stack_; }
@@ -98,12 +99,15 @@ class ConnectedSubchannel : public RefCountedWithTracing<ConnectedSubchannel> {
   channelz::SubchannelNode* channelz_subchannel() {
     return channelz_subchannel_;
   }
+  intptr_t socket_uuid() { return socket_uuid_; }
 
  private:
   grpc_channel_stack* channel_stack_;
   // backpointer to the channelz node in this connected subchannel's
   // owning subchannel.
   channelz::SubchannelNode* channelz_subchannel_;
+  // uuid of this subchannel's socket. 0 if this subchannel is not connected.
+  intptr_t socket_uuid_;
 };
 
 }  // namespace grpc_core
@@ -126,8 +130,7 @@ void grpc_subchannel_call_unref(
 grpc_core::channelz::SubchannelNode* grpc_subchannel_get_channelz_node(
     grpc_subchannel* subchannel);
 
-intptr_t grpc_subchannel_get_child_socket_uuid(
-    grpc_subchannel* subchannel);
+intptr_t grpc_subchannel_get_child_socket_uuid(grpc_subchannel* subchannel);
 
 /** Returns a pointer to the parent data associated with \a subchannel_call.
     The data will be of the size specified in \a parent_data_size
diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc
index b0cccafcf8..45ac782a0e 100644
--- a/test/cpp/microbenchmarks/bm_call_create.cc
+++ b/test/cpp/microbenchmarks/bm_call_create.cc
@@ -449,9 +449,9 @@ grpc_endpoint* GetEndpoint(grpc_transport* self) { return nullptr; }
 intptr_t GetSocketUuid(grpc_transport* t) { return 0; }
 
 static const grpc_transport_vtable dummy_transport_vtable = {
-    0,           "dummy_http2",  InitStream,
-    SetPollset,  SetPollsetSet,  PerformStreamOp,
-    PerformOp,   DestroyStream,  Destroy,
+    0,           "dummy_http2", InitStream,
+    SetPollset,  SetPollsetSet, PerformStreamOp,
+    PerformOp,   DestroyStream, Destroy,
     GetEndpoint, GetSocketUuid};
 
 static grpc_transport dummy_transport = {&dummy_transport_vtable};