From 967bbcd5d38e37f3640101a630e660c1276d7ee3 Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Thu, 13 Sep 2018 15:49:52 -0700 Subject: Fixing benchmark name and adding a new one --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 65 +++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) (limited to 'test/cpp/microbenchmarks') diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 6fcf048bf3..8d1aa3f10c 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" @@ -766,8 +767,59 @@ 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* arena = gpr_arena_create(1024); + grpc_chttp2_incoming_metadata_buffer_init(&buffer, arena); + 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_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(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); + } + } + gpr_arena_destroy(arena); +} + +// 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_mdelem_get_user_data(md, free_timeout)); @@ -853,8 +905,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 -- cgit v1.2.3 From 91727bd015c9fd0ac9076b191520504bc227f685 Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Thu, 13 Sep 2018 18:47:40 -0700 Subject: Move arena create outside of benchmark, format, and typo fix --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'test/cpp/microbenchmarks') diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 8d1aa3f10c..f160e77634 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -457,8 +457,10 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { std::vector benchmark_slices = Fixture::GetBenchmarkSlices(); grpc_chttp2_hpack_parser p; grpc_chttp2_hpack_parser_init(&p); + const int kArenaSize = 4096; + gpr_arena* arena = gpr_arena_create(kArenaSize); p.on_header = OnHeader; - p.on_header_user_data = nullptr; + p.on_header_user_data = arena; for (auto slice : init_slices) { GPR_ASSERT(GRPC_ERROR_NONE == grpc_chttp2_hpack_parser_parse(&p, slice)); } @@ -467,6 +469,11 @@ 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 64k iterations to avoid oom + if (0 == (state.iterations() & 0xffff)) { + gpr_arena_destroy(arena); + arena = gpr_arena_create(kArenaSize); + } } for (auto slice : init_slices) grpc_slice_unref(slice); for (auto slice : benchmark_slices) grpc_slice_unref(slice); @@ -769,10 +776,9 @@ static void free_timeout(void* p) { gpr_free(p); } // 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 + // Setup for benchmark. This will bloat the absolute values of this benchmark grpc_chttp2_incoming_metadata_buffer buffer; - gpr_arena* arena = gpr_arena_create(1024); - grpc_chttp2_incoming_metadata_buffer_init(&buffer, arena); + grpc_chttp2_incoming_metadata_buffer_init(&buffer, (gpr_arena*)user_data); bool seen_error = false; // Below here is the code we actually care about benchmarking @@ -815,7 +821,6 @@ static void OnInitialHeader(void* user_data, grpc_mdelem md) { GPR_ASSERT(0); } } - gpr_arena_destroy(arena); } // Benchmark timeout handling -- cgit v1.2.3 From 29d9489ea9fa4b6249c3243bc122a65746c928ac Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Wed, 19 Sep 2018 20:46:17 -0700 Subject: Increase initial arena size to be more representative of real workload scenario and increase frequency of recreating the arena to avoid oom --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'test/cpp/microbenchmarks') diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index f160e77634..741825cada 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -457,7 +457,7 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { std::vector benchmark_slices = Fixture::GetBenchmarkSlices(); grpc_chttp2_hpack_parser p; grpc_chttp2_hpack_parser_init(&p); - const int kArenaSize = 4096; + const int kArenaSize = 4096 * 4096; gpr_arena* arena = gpr_arena_create(kArenaSize); p.on_header = OnHeader; p.on_header_user_data = arena; @@ -469,8 +469,8 @@ 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 64k iterations to avoid oom - if (0 == (state.iterations() & 0xffff)) { + // Recreate arena every 4k iterations to avoid oom + if (0 == (state.iterations() & 0xfff)) { gpr_arena_destroy(arena); arena = gpr_arena_create(kArenaSize); } -- cgit v1.2.3 From 4b721fbde0b6a623968987b52a9500471242cfcb Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Wed, 19 Sep 2018 20:51:45 -0700 Subject: Destroy arena at end of benchmark to not leak memory --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'test/cpp/microbenchmarks') diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 741825cada..528cabd0e3 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -475,6 +475,8 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { arena = gpr_arena_create(kArenaSize); } } + // Clean up + gpr_arena_destroy(arena); for (auto slice : init_slices) grpc_slice_unref(slice); for (auto slice : benchmark_slices) grpc_slice_unref(slice); grpc_chttp2_hpack_parser_destroy(&p); -- cgit v1.2.3 From d44feec92ffd1edff7836409f38069edc0836d63 Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Wed, 19 Sep 2018 22:40:59 -0700 Subject: Reassign arena pointer instead of stomping on memory --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'test/cpp/microbenchmarks') diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 528cabd0e3..ba712548e6 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -458,9 +458,8 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { grpc_chttp2_hpack_parser p; grpc_chttp2_hpack_parser_init(&p); const int kArenaSize = 4096 * 4096; - gpr_arena* arena = gpr_arena_create(kArenaSize); + p.on_header_user_data = gpr_arena_create(kArenaSize); p.on_header = OnHeader; - p.on_header_user_data = arena; for (auto slice : init_slices) { GPR_ASSERT(GRPC_ERROR_NONE == grpc_chttp2_hpack_parser_parse(&p, slice)); } @@ -471,12 +470,12 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { grpc_core::ExecCtx::Get()->Flush(); // Recreate arena every 4k iterations to avoid oom if (0 == (state.iterations() & 0xfff)) { - gpr_arena_destroy(arena); - arena = gpr_arena_create(kArenaSize); + gpr_arena_destroy((gpr_arena*)p.on_header_user_data); + p.on_header_user_data = gpr_arena_create(kArenaSize); } } // Clean up - gpr_arena_destroy(arena); + 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); -- cgit v1.2.3 From 4c6e7ce15dea5cdb51ef2021aa539d16cbd30991 Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Thu, 20 Sep 2018 12:56:56 -0700 Subject: Destroy metadata buffer at end of benchmark loop --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'test/cpp/microbenchmarks') diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index ba712548e6..ba4b57ad22 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -822,6 +822,7 @@ static void OnInitialHeader(void* user_data, grpc_mdelem md) { GPR_ASSERT(0); } } + grpc_chttp2_incoming_metadata_buffer_destroy(&buffer); } // Benchmark timeout handling -- cgit v1.2.3