reviewer feedback

12 files changed, 26 insertions, 36 deletions

diff --git a/src/core/ext/filters/client_channel/connector.h b/src/core/ext/filters/client_channel/connector.h
index 556594929c..ea34dcdab5 100644
--- a/src/core/ext/filters/client_channel/connector.h
+++ b/src/core/ext/filters/client_channel/connector.h
@@ -47,6 +47,9 @@ typedef struct {
 
   /** channel arguments (to be passed to the filters) */
   grpc_channel_args* channel_args;
+
+  /** socket uuid of the connected transport. 0 if not available */
+  intptr_t socket_uuid;
 } grpc_connect_out_args;
 
 struct grpc_connector_vtable {
diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 4756733f1f..2847f4bdc1 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -629,8 +629,7 @@ static bool publish_transport_locked(grpc_subchannel* c) {
     GRPC_ERROR_UNREF(error);
     return false;
   }
-  intptr_t socket_uuid =
-      grpc_transport_get_socket_uuid(c->connecting_result.transport);
+  intptr_t socket_uuid = c->connecting_result.socket_uuid;
   memset(&c->connecting_result, 0, sizeof(c->connecting_result));
 
   /* initialize state watcher */
diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h
index 5d95a72085..699f93a8e7 100644
--- a/src/core/ext/filters/client_channel/subchannel.h
+++ b/src/core/ext/filters/client_channel/subchannel.h
@@ -107,7 +107,7 @@ class ConnectedSubchannel : public RefCountedWithTracing<ConnectedSubchannel> {
   // owning subchannel.
   channelz::SubchannelNode* channelz_subchannel_;
   // uuid of this subchannel's socket. 0 if this subchannel is not connected.
-  intptr_t socket_uuid_;
+  const intptr_t socket_uuid_;
 };
 
 }  // namespace grpc_core
diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc
index e7522ffba8..0ac84032fd 100644
--- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc
+++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc
@@ -117,6 +117,8 @@ static void on_handshake_done(void* arg, grpc_error* error) {
                                           c->args.interested_parties);
     c->result->transport =
         grpc_create_chttp2_transport(args->args, args->endpoint, true);
+    c->result->socket_uuid =
+        grpc_chttp2_transport_get_socket_uuid(c->result->transport);
     GPR_ASSERT(c->result->transport);
     // TODO(roth): We ideally want to wait until we receive HTTP/2
     // settings from the server before we consider the connection
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
index 45ef4161f7..776c15138b 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
@@ -3157,16 +3157,6 @@ static grpc_endpoint* chttp2_get_endpoint(grpc_transport* t) {
   return (reinterpret_cast<grpc_chttp2_transport*>(t))->ep;
 }
 
-static intptr_t get_socket_uuid(grpc_transport* transport) {
-  grpc_chttp2_transport* t =
-      reinterpret_cast<grpc_chttp2_transport*>(transport);
-  if (t->channelz_socket != nullptr) {
-    return t->channelz_socket->uuid();
-  } else {
-    return 0;
-  }
-}
-
 static const grpc_transport_vtable vtable = {sizeof(grpc_chttp2_stream),
                                              "chttp2",
                                              init_stream,
@@ -3176,11 +3166,20 @@ static const grpc_transport_vtable vtable = {sizeof(grpc_chttp2_stream),
                                              perform_transport_op,
                                              destroy_stream,
                                              destroy_transport,
-                                             chttp2_get_endpoint,
-                                             get_socket_uuid};
+                                             chttp2_get_endpoint};
 
 static const grpc_transport_vtable* get_vtable(void) { return &vtable; }
 
+intptr_t grpc_chttp2_transport_get_socket_uuid(grpc_transport* transport) {
+  grpc_chttp2_transport* t =
+      reinterpret_cast<grpc_chttp2_transport*>(transport);
+  if (t->channelz_socket != nullptr) {
+    return t->channelz_socket->uuid();
+  } else {
+    return 0;
+  }
+}
+
 grpc_transport* grpc_create_chttp2_transport(
     const grpc_channel_args* channel_args, grpc_endpoint* ep, bool is_client) {
   grpc_chttp2_transport* t = static_cast<grpc_chttp2_transport*>(
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.h b/src/core/ext/transport/chttp2/transport/chttp2_transport.h
index 9d55b3f4b0..e5872fee43 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.h
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.h
@@ -34,6 +34,8 @@ extern bool g_flow_control_enabled;
 grpc_transport* grpc_create_chttp2_transport(
     const grpc_channel_args* channel_args, grpc_endpoint* ep, bool is_client);
 
+intptr_t grpc_chttp2_transport_get_socket_uuid(grpc_transport* transport);
+
 /// Takes ownership of \a read_buffer, which (if non-NULL) contains
 /// leftover bytes previously read from the endpoint (e.g., by handshakers).
 /// If non-null, \a notify_on_receive_settings will be scheduled when
diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc
index bb20d9db08..81e2634e3a 100644
--- a/src/core/ext/transport/cronet/transport/cronet_transport.cc
+++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc
@@ -1439,8 +1439,6 @@ static grpc_endpoint* get_endpoint(grpc_transport* gt) { return nullptr; }
 
 static void perform_op(grpc_transport* gt, grpc_transport_op* op) {}
 
-static intptr_t get_socket_uuid(grpc_transport* t) { return 0; }
-
 static const grpc_transport_vtable grpc_cronet_vtable = {
     sizeof(stream_obj),
     "cronet_http",
@@ -1451,8 +1449,7 @@ static const grpc_transport_vtable grpc_cronet_vtable = {
     perform_op,
     destroy_stream,
     destroy_transport,
-    get_endpoint,
-    get_socket_uuid};
+    get_endpoint};
 
 grpc_transport* grpc_create_cronet_transport(void* engine, const char* target,
                                              const grpc_channel_args* args,
diff --git a/src/core/ext/transport/inproc/inproc_transport.cc b/src/core/ext/transport/inproc/inproc_transport.cc
index 56951f8338..b0ca7f8207 100644
--- a/src/core/ext/transport/inproc/inproc_transport.cc
+++ b/src/core/ext/transport/inproc/inproc_transport.cc
@@ -1170,8 +1170,6 @@ static void set_pollset_set(grpc_transport* gt, grpc_stream* gs,
 
 static grpc_endpoint* get_endpoint(grpc_transport* t) { return nullptr; }
 
-static intptr_t get_socket_uuid(grpc_transport* t) { return 0; }
-
 /*******************************************************************************
  * GLOBAL INIT AND DESTROY
  */
@@ -1196,7 +1194,7 @@ static const grpc_transport_vtable inproc_vtable = {
     sizeof(inproc_stream), "inproc",        init_stream,
     set_pollset,           set_pollset_set, perform_stream_op,
     perform_transport_op,  destroy_stream,  destroy_transport,
-    get_endpoint,          get_socket_uuid};
+    get_endpoint};
 
 /*******************************************************************************
  * Main inproc transport functions
diff --git a/src/core/lib/transport/transport.cc b/src/core/lib/transport/transport.cc
index c8fdbb4de2..cbdb77c844 100644
--- a/src/core/lib/transport/transport.cc
+++ b/src/core/lib/transport/transport.cc
@@ -199,10 +199,6 @@ grpc_endpoint* grpc_transport_get_endpoint(grpc_transport* transport) {
   return transport->vtable->get_endpoint(transport);
 }
 
-intptr_t grpc_transport_get_socket_uuid(grpc_transport* transport) {
-  return transport->vtable->get_socket_uuid(transport);
-}
-
 // This comment should be sung to the tune of
 // "Supercalifragilisticexpialidocious":
 //
diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h
index aad133f6c3..9e784635c6 100644
--- a/src/core/lib/transport/transport.h
+++ b/src/core/lib/transport/transport.h
@@ -366,8 +366,6 @@ void grpc_transport_destroy(grpc_transport* transport);
 /* Get the endpoint used by \a transport */
 grpc_endpoint* grpc_transport_get_endpoint(grpc_transport* transport);
 
-intptr_t grpc_transport_get_socket_uuid(grpc_transport* transport);
-
 /* Allocate a grpc_transport_op, and preconfigure the on_consumed closure to
    \a on_consumed and then delete the returned transport op */
 grpc_transport_op* grpc_make_transport_op(grpc_closure* on_consumed);
diff --git a/src/core/lib/transport/transport_impl.h b/src/core/lib/transport/transport_impl.h
index d609470ef0..ba5e05df0a 100644
--- a/src/core/lib/transport/transport_impl.h
+++ b/src/core/lib/transport/transport_impl.h
@@ -60,8 +60,6 @@ typedef struct grpc_transport_vtable {
 
   /* implementation of grpc_transport_get_endpoint */
   grpc_endpoint* (*get_endpoint)(grpc_transport* self);
-
-  intptr_t (*get_socket_uuid)(grpc_transport* self);
 } grpc_transport_vtable;
 
 /* an instance of a grpc transport */
diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc
index 45ac782a0e..9516b2e3e2 100644
--- a/test/cpp/microbenchmarks/bm_call_create.cc
+++ b/test/cpp/microbenchmarks/bm_call_create.cc
@@ -446,13 +446,11 @@ void Destroy(grpc_transport* self) {}
 /* implementation of grpc_transport_get_endpoint */
 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,
-    GetEndpoint, GetSocketUuid};
+    0,          "dummy_http2", InitStream,
+    SetPollset, SetPollsetSet, PerformStreamOp,
+    PerformOp,  DestroyStream, Destroy,
+    GetEndpoint};
 
 static grpc_transport dummy_transport = {&dummy_transport_vtable};