Merge master

8 files changed, 93 insertions, 21 deletions

diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD
index 0c3b9ef816..097e92f583 100644
--- a/test/cpp/microbenchmarks/BUILD
+++ b/test/cpp/microbenchmarks/BUILD
@@ -24,7 +24,7 @@ grpc_cc_test(
     external_deps = [
         "benchmark",
     ],
-    deps = ["//test/core/util:gpr_test_util",]
+    deps = ["//test/core/util:gpr_test_util"],
 )
 
 grpc_cc_library(
@@ -69,6 +69,13 @@ grpc_cc_binary(
 )
 
 grpc_cc_binary(
+    name = "bm_call_create",
+    testonly = 1,
+    srcs = ["bm_call_create.cc"],
+    deps = [":helpers"],
+)
+
+grpc_cc_binary(
     name = "bm_cq",
     testonly = 1,
     srcs = ["bm_cq.cc"],
diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc
index 9516b2e3e2..446dc93edb 100644
--- a/test/cpp/microbenchmarks/bm_call_create.cc
+++ b/test/cpp/microbenchmarks/bm_call_create.cc
@@ -34,7 +34,6 @@
 #include "src/core/ext/filters/http/client/http_client_filter.h"
 #include "src/core/ext/filters/http/message_compress/message_compress_filter.h"
 #include "src/core/ext/filters/http/server/http_server_filter.h"
-#include "src/core/ext/filters/load_reporting/server_load_reporting_filter.h"
 #include "src/core/ext/filters/message_size/message_size_filter.h"
 #include "src/core/lib/channel/channel_stack.h"
 #include "src/core/lib/channel/connected_channel.h"
@@ -133,8 +132,10 @@ static void BM_LameChannelCallCreateCpp(benchmark::State& state) {
   TrackCounters track_counters;
   auto stub =
       grpc::testing::EchoTestService::NewStub(grpc::CreateChannelInternal(
-          "", grpc_lame_client_channel_create(
-                  "localhost:1234", GRPC_STATUS_UNAUTHENTICATED, "blah")));
+          "",
+          grpc_lame_client_channel_create("localhost:1234",
+                                          GRPC_STATUS_UNAUTHENTICATED, "blah"),
+          nullptr));
   grpc::CompletionQueue cq;
   grpc::testing::EchoRequest send_request;
   grpc::testing::EchoResponse recv_response;
@@ -467,7 +468,7 @@ class NoOp {
 
 class SendEmptyMetadata {
  public:
-  SendEmptyMetadata() {
+  SendEmptyMetadata() : op_payload_(nullptr) {
     memset(&op_, 0, sizeof(op_));
     op_.on_complete = GRPC_CLOSURE_INIT(&closure_, DoNothing, nullptr,
                                         grpc_schedule_on_exec_ctx);
diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc
index 6fcf048bf3..85d233dd26 100644
--- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc
+++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc
@@ -27,6 +27,7 @@
 
 #include "src/core/ext/transport/chttp2/transport/hpack_encoder.h"
 #include "src/core/ext/transport/chttp2/transport/hpack_parser.h"
+#include "src/core/ext/transport/chttp2/transport/incoming_metadata.h"
 #include "src/core/lib/slice/slice_internal.h"
 #include "src/core/lib/slice/slice_string_helpers.h"
 #include "src/core/lib/transport/static_metadata.h"
@@ -456,8 +457,9 @@ static void BM_HpackParserParseHeader(benchmark::State& state) {
   std::vector<grpc_slice> benchmark_slices = Fixture::GetBenchmarkSlices();
   grpc_chttp2_hpack_parser p;
   grpc_chttp2_hpack_parser_init(&p);
+  const int kArenaSize = 4096 * 4096;
+  p.on_header_user_data = gpr_arena_create(kArenaSize);
   p.on_header = OnHeader;
-  p.on_header_user_data = nullptr;
   for (auto slice : init_slices) {
     GPR_ASSERT(GRPC_ERROR_NONE == grpc_chttp2_hpack_parser_parse(&p, slice));
   }
@@ -466,7 +468,14 @@ static void BM_HpackParserParseHeader(benchmark::State& state) {
       GPR_ASSERT(GRPC_ERROR_NONE == grpc_chttp2_hpack_parser_parse(&p, slice));
     }
     grpc_core::ExecCtx::Get()->Flush();
+    // Recreate arena every 4k iterations to avoid oom
+    if (0 == (state.iterations() & 0xfff)) {
+      gpr_arena_destroy((gpr_arena*)p.on_header_user_data);
+      p.on_header_user_data = gpr_arena_create(kArenaSize);
+    }
   }
+  // Clean up
+  gpr_arena_destroy((gpr_arena*)p.on_header_user_data);
   for (auto slice : init_slices) grpc_slice_unref(slice);
   for (auto slice : benchmark_slices) grpc_slice_unref(slice);
   grpc_chttp2_hpack_parser_destroy(&p);
@@ -766,8 +775,56 @@ class RepresentativeServerTrailingMetadata {
 
 static void free_timeout(void* p) { gpr_free(p); }
 
-// New implementation.
-static void OnHeaderNew(void* user_data, grpc_mdelem md) {
+// Benchmark the current on_initial_header implementation
+static void OnInitialHeader(void* user_data, grpc_mdelem md) {
+  // Setup for benchmark. This will bloat the absolute values of this benchmark
+  grpc_chttp2_incoming_metadata_buffer buffer((gpr_arena*)user_data);
+  bool seen_error = false;
+
+  // Below here is the code we actually care about benchmarking
+  if (grpc_slice_eq(GRPC_MDKEY(md), GRPC_MDSTR_GRPC_STATUS) &&
+      !grpc_mdelem_eq(md, GRPC_MDELEM_GRPC_STATUS_0)) {
+    seen_error = true;
+  }
+  if (grpc_slice_eq(GRPC_MDKEY(md), GRPC_MDSTR_GRPC_TIMEOUT)) {
+    grpc_millis* cached_timeout =
+        static_cast<grpc_millis*>(grpc_mdelem_get_user_data(md, free_timeout));
+    grpc_millis timeout;
+    if (cached_timeout != nullptr) {
+      timeout = *cached_timeout;
+    } else {
+      if (GPR_UNLIKELY(
+              !grpc_http2_decode_timeout(GRPC_MDVALUE(md), &timeout))) {
+        char* val = grpc_slice_to_c_string(GRPC_MDVALUE(md));
+        gpr_log(GPR_ERROR, "Ignoring bad timeout value '%s'", val);
+        gpr_free(val);
+        timeout = GRPC_MILLIS_INF_FUTURE;
+      }
+      if (GRPC_MDELEM_IS_INTERNED(md)) {
+        /* not already parsed: parse it now, and store the
+         * result away */
+        cached_timeout =
+            static_cast<grpc_millis*>(gpr_malloc(sizeof(grpc_millis)));
+        *cached_timeout = timeout;
+        grpc_mdelem_set_user_data(md, free_timeout, cached_timeout);
+      }
+    }
+    benchmark::DoNotOptimize(timeout);
+    GRPC_MDELEM_UNREF(md);
+  } else {
+    const size_t new_size = buffer.size + GRPC_MDELEM_LENGTH(md);
+    if (!seen_error) {
+      buffer.size = new_size;
+    }
+    grpc_error* error = grpc_chttp2_incoming_metadata_buffer_add(&buffer, md);
+    if (error != GRPC_ERROR_NONE) {
+      GPR_ASSERT(0);
+    }
+  }
+}
+
+// Benchmark timeout handling
+static void OnHeaderTimeout(void* user_data, grpc_mdelem md) {
   if (grpc_slice_eq(GRPC_MDKEY(md), GRPC_MDSTR_GRPC_TIMEOUT)) {
     grpc_millis* cached_timeout =
         static_cast<grpc_millis*>(grpc_mdelem_get_user_data(md, free_timeout));
@@ -853,8 +910,13 @@ BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
                    RepresentativeServerInitialMetadata, UnrefHeader);
 BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
                    RepresentativeServerTrailingMetadata, UnrefHeader);
-
-BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, SameDeadline, OnHeaderNew);
+BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
+                   RepresentativeClientInitialMetadata, OnInitialHeader);
+BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
+                   MoreRepresentativeClientInitialMetadata, OnInitialHeader);
+BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
+                   RepresentativeServerInitialMetadata, OnInitialHeader);
+BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, SameDeadline, OnHeaderTimeout);
 
 }  // namespace hpack_parser_fixtures
 
diff --git a/test/cpp/microbenchmarks/bm_chttp2_transport.cc b/test/cpp/microbenchmarks/bm_chttp2_transport.cc
index 189923a841..f7ae16e61d 100644
--- a/test/cpp/microbenchmarks/bm_chttp2_transport.cc
+++ b/test/cpp/microbenchmarks/bm_chttp2_transport.cc
@@ -262,7 +262,7 @@ static void BM_StreamCreateDestroy(benchmark::State& state) {
   Fixture f(grpc::ChannelArguments(), true);
   Stream s(&f);
   grpc_transport_stream_op_batch op;
-  grpc_transport_stream_op_batch_payload op_payload;
+  grpc_transport_stream_op_batch_payload op_payload(nullptr);
   memset(&op, 0, sizeof(op));
   op.cancel_stream = true;
   op.payload = &op_payload;
@@ -308,8 +308,7 @@ static void BM_StreamCreateSendInitialMetadataDestroy(benchmark::State& state) {
   Fixture f(grpc::ChannelArguments(), true);
   Stream s(&f);
   grpc_transport_stream_op_batch op;
-  grpc_transport_stream_op_batch_payload op_payload;
-  memset(&op_payload, 0, sizeof(op_payload));
+  grpc_transport_stream_op_batch_payload op_payload(nullptr);
   std::unique_ptr<Closure> start;
   std::unique_ptr<Closure> done;
 
@@ -360,8 +359,7 @@ static void BM_TransportEmptyOp(benchmark::State& state) {
   Stream s(&f);
   s.Init(state);
   grpc_transport_stream_op_batch op;
-  grpc_transport_stream_op_batch_payload op_payload;
-  memset(&op_payload, 0, sizeof(op_payload));
+  grpc_transport_stream_op_batch_payload op_payload(nullptr);
   auto reset_op = [&]() {
     memset(&op, 0, sizeof(op));
     op.payload = &op_payload;
@@ -393,8 +391,7 @@ static void BM_TransportStreamSend(benchmark::State& state) {
   auto s = std::unique_ptr<Stream>(new Stream(&f));
   s->Init(state);
   grpc_transport_stream_op_batch op;
-  grpc_transport_stream_op_batch_payload op_payload;
-  memset(&op_payload, 0, sizeof(op_payload));
+  grpc_transport_stream_op_batch_payload op_payload(nullptr);
   auto reset_op = [&]() {
     memset(&op, 0, sizeof(op));
     op.payload = &op_payload;
@@ -526,8 +523,7 @@ static void BM_TransportStreamRecv(benchmark::State& state) {
   Fixture f(grpc::ChannelArguments(), true);
   Stream s(&f);
   s.Init(state);
-  grpc_transport_stream_op_batch_payload op_payload;
-  memset(&op_payload, 0, sizeof(op_payload));
+  grpc_transport_stream_op_batch_payload op_payload(nullptr);
   grpc_transport_stream_op_batch op;
   grpc_core::OrphanablePtr<grpc_core::ByteStream> recv_stream;
   grpc_slice incoming_data = CreateIncomingDataSlice(state.range(0), 16384);
diff --git a/test/cpp/microbenchmarks/bm_fullstack_unary_ping_pong.cc b/test/cpp/microbenchmarks/bm_fullstack_unary_ping_pong.cc
index 5a7a8d5baf..d4bd58b983 100644
--- a/test/cpp/microbenchmarks/bm_fullstack_unary_ping_pong.cc
+++ b/test/cpp/microbenchmarks/bm_fullstack_unary_ping_pong.cc
@@ -31,6 +31,8 @@ auto& force_library_initialization = Library::get();
  * CONFIGURATIONS
  */
 
+// Replace "benchmark::internal::Benchmark" with "::testing::Benchmark" to use
+// internal microbenchmarking tooling
 static void SweepSizesArgs(benchmark::internal::Benchmark* b) {
   b->Args({0, 0});
   for (int i = 1; i <= 128 * 1024 * 1024; i *= 8) {
diff --git a/test/cpp/microbenchmarks/fullstack_fixtures.h b/test/cpp/microbenchmarks/fullstack_fixtures.h
index 9d70277fbc..e57eb6ddd1 100644
--- a/test/cpp/microbenchmarks/fullstack_fixtures.h
+++ b/test/cpp/microbenchmarks/fullstack_fixtures.h
@@ -48,6 +48,7 @@ namespace testing {
 
 class FixtureConfiguration {
  public:
+  virtual ~FixtureConfiguration() {}
   virtual void ApplyCommonChannelArguments(ChannelArguments* c) const {
     c->SetInt(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, INT_MAX);
     c->SetInt(GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, INT_MAX);
@@ -199,7 +200,7 @@ class EndpointPairFixture : public BaseFixture {
       }
 
       grpc_server_setup_transport(server_->c_server(), server_transport_,
-                                  nullptr, server_args);
+                                  nullptr, server_args, 0);
       grpc_chttp2_transport_start_reading(server_transport_, nullptr, nullptr);
     }
 
@@ -217,7 +218,7 @@ class EndpointPairFixture : public BaseFixture {
           "target", &c_args, GRPC_CLIENT_DIRECT_CHANNEL, client_transport_);
       grpc_chttp2_transport_start_reading(client_transport_, nullptr, nullptr);
 
-      channel_ = CreateChannelInternal("", channel);
+      channel_ = CreateChannelInternal("", channel, nullptr);
     }
   }
 
diff --git a/test/cpp/microbenchmarks/helpers.cc b/test/cpp/microbenchmarks/helpers.cc
index e4ba37e2d6..bce72985dc 100644
--- a/test/cpp/microbenchmarks/helpers.cc
+++ b/test/cpp/microbenchmarks/helpers.cc
@@ -38,6 +38,7 @@ void TrackCounters::AddLabel(const grpc::string& label) {
 }
 
 void TrackCounters::AddToLabel(std::ostream& out, benchmark::State& state) {
+#ifdef GRPC_COLLECT_STATS
   grpc_stats_data stats_end;
   grpc_stats_collect(&stats_end);
   grpc_stats_data stats;
@@ -53,6 +54,7 @@ void TrackCounters::AddToLabel(std::ostream& out, benchmark::State& state) {
         << " " << grpc_stats_histogram_name[i] << "-99p:"
         << grpc_stats_histo_percentile(&stats, (grpc_stats_histograms)i, 99.0);
   }
+#endif
 #ifdef GPR_LOW_LEVEL_COUNTERS
   grpc_memory_counters counters_at_end = grpc_memory_counters_snapshot();
   out << " locks/iter:"
diff --git a/test/cpp/microbenchmarks/helpers.h b/test/cpp/microbenchmarks/helpers.h
index 4aabd4094a..25d34b5f87 100644
--- a/test/cpp/microbenchmarks/helpers.h
+++ b/test/cpp/microbenchmarks/helpers.h
@@ -63,6 +63,7 @@ extern gpr_atm gpr_now_call_count;
 class TrackCounters {
  public:
   TrackCounters() { grpc_stats_collect(&stats_begin_); }
+  virtual ~TrackCounters() {}
   virtual void Finish(benchmark::State& state);
   virtual void AddLabel(const grpc::string& label);
   virtual void AddToLabel(std::ostream& out, benchmark::State& state);