From 0c0f3f0556f09e2c0dbed1d42ddec057742226cd Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 21 Jan 2020 10:16:46 -0800 Subject: [PATCH 1/3] Declare microbenchmarks as tests rather than binaries, fix issues --- test/cpp/microbenchmarks/BUILD | 121 +++++++++++------- test/cpp/microbenchmarks/bm_call_create.cc | 3 +- .../microbenchmarks/bm_opencensus_plugin.cc | 10 +- test/cpp/microbenchmarks/bm_timer.cc | 2 +- tools/remote_build/rbe_common.bazelrc | 4 +- 5 files changed, 89 insertions(+), 51 deletions(-) diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index db7c9db05ae..bf6b205a19b 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -14,7 +14,7 @@ licenses(["notice"]) # Apache v2 -load("//bazel:grpc_build_system.bzl", "grpc_cc_binary", "grpc_cc_library", "grpc_cc_test", "grpc_package") +load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_package") grpc_package(name = "test/cpp/microbenchmarks") @@ -48,75 +48,98 @@ grpc_cc_library( ], ) -grpc_cc_binary( - name = "bm_closure", +# Need a secure version of helpers to benchmark opencensus +grpc_cc_library( + name = "helpers_secure", testonly = 1, + srcs = ["helpers.cc"], + hdrs = [ + "fullstack_context_mutators.h", + "fullstack_fixtures.h", + "helpers.h", + ], + external_deps = [ + "benchmark", + ], + tags = ["no_windows"], + deps = [ + "//:grpc++", + "//src/proto/grpc/testing:echo_proto", + "//test/core/util:grpc_test_util", + "//test/cpp/util:test_config", + ], +) + +grpc_cc_test( + name = "bm_closure", srcs = ["bm_closure.cc"], tags = ["no_windows"], deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_alarm", - testonly = 1, srcs = ["bm_alarm.cc"], tags = ["no_windows"], deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_arena", - testonly = 1, + size = "enormous", srcs = ["bm_arena.cc"], - tags = ["no_windows"], + tags = [ + "no_windows", + "notsan", + ], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_byte_buffer", - testonly = 1, srcs = ["bm_byte_buffer.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_channel", - testonly = 1, srcs = ["bm_channel.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_call_create", - testonly = 1, srcs = ["bm_call_create.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_cq", - testonly = 1, srcs = ["bm_cq.cc"], tags = ["no_windows"], deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_cq_multiple_threads", - testonly = 1, srcs = ["bm_cq_multiple_threads.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_error", - testonly = 1, srcs = ["bm_error.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) @@ -130,9 +153,9 @@ grpc_cc_library( deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_fullstack_streaming_ping_pong", - testonly = 1, + size = "large", srcs = [ "bm_fullstack_streaming_ping_pong.cc", ], @@ -152,9 +175,8 @@ grpc_cc_library( deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_fullstack_streaming_pump", - testonly = 1, srcs = [ "bm_fullstack_streaming_pump.cc", ], @@ -165,13 +187,16 @@ grpc_cc_binary( deps = [":fullstack_streaming_pump_h"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_fullstack_trickle", - testonly = 1, + size = "enormous", srcs = ["bm_fullstack_trickle.cc"], tags = [ "no_mac", # to emulate "excluded_poll_engines: poll" "no_windows", + "noasan", + "nomsan", + "notsan", ], deps = [":helpers"], ) @@ -185,9 +210,8 @@ grpc_cc_library( deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_fullstack_unary_ping_pong", - testonly = 1, srcs = [ "bm_fullstack_unary_ping_pong.cc", ], @@ -198,47 +222,50 @@ grpc_cc_binary( deps = [":fullstack_unary_ping_pong_h"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_metadata", - testonly = 1, srcs = ["bm_metadata.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_chttp2_hpack", - testonly = 1, srcs = ["bm_chttp2_hpack.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_opencensus_plugin", - testonly = 1, srcs = ["bm_opencensus_plugin.cc"], language = "C++", deps = [ - ":helpers", + ":helpers_secure", "//:grpc_opencensus_plugin", "//src/proto/grpc/testing:echo_proto", ], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_timer", - testonly = 1, srcs = ["bm_timer.cc"], tags = ["no_windows"], + uses_polling = False, deps = [":helpers"], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_threadpool", - testonly = 1, + size = "enormous", srcs = ["bm_threadpool.cc"], - tags = ["no_windows"], + tags = [ + "no_windows", + "notsan", + ], + uses_polling = False, deps = [":helpers"], ) @@ -253,7 +280,7 @@ grpc_cc_library( deps = [ ":helpers", "//src/proto/grpc/testing:echo_proto", - "//test/cpp/util:test_util", + "//test/cpp/util:test_util_unsecure", ], ) @@ -269,9 +296,8 @@ grpc_cc_library( ], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_callback_unary_ping_pong", - testonly = 1, srcs = [ "bm_callback_unary_ping_pong.cc", ], @@ -291,12 +317,15 @@ grpc_cc_library( ], ) -grpc_cc_binary( +grpc_cc_test( name = "bm_callback_streaming_ping_pong", - testonly = 1, + size = "large", srcs = [ "bm_callback_streaming_ping_pong.cc", ], - tags = ["no_windows"], + tags = [ + "no_mac", # to emulate "excluded_poll_engines: poll" + "no_windows", + ], deps = [":callback_streaming_ping_pong_h"], ) diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index f09f9e4baa3..803ad38f1c5 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -513,7 +513,8 @@ static void BM_IsolatedFilter(benchmark::State& state) { static_cast(gpr_zalloc(channel_size)); GPR_ASSERT(GRPC_LOG_IF_ERROR( "channel_stack_init", - grpc_channel_stack_init(1, FilterDestroy, channel_stack, &filters[0], + grpc_channel_stack_init(1, FilterDestroy, channel_stack, + filters.size() == 0 ? nullptr : &filters[0], filters.size(), &channel_args, fixture.flags & REQUIRES_TRANSPORT ? &dummy_transport::dummy_transport diff --git a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc index 4f4884bef9b..908d34aa19f 100644 --- a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc +++ b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc @@ -22,7 +22,8 @@ #include "absl/base/call_once.h" #include "absl/strings/str_cat.h" -#include "include/grpc++/grpc++.h" +#include "include/grpc/grpc.h" +#include "include/grpcpp/grpcpp.h" #include "include/grpcpp/opencensus.h" #include "opencensus/stats/stats.h" #include "src/cpp/ext/filters/census/grpc_plugin.h" @@ -100,6 +101,13 @@ static void BM_E2eLatencyCensusDisabled(benchmark::State& state) { BENCHMARK(BM_E2eLatencyCensusDisabled); static void BM_E2eLatencyCensusEnabled(benchmark::State& state) { + // Avoid a data race between registering plugin and shutdown of previous + // test (order-dependent) by doing an init/shutdown so that any previous + // shutdowns are fully complete first. + grpc_init(); + grpc_shutdown_blocking(); + + // Now start the test by registering the plugin (once in the execution) RegisterOnce(); // This we can safely repeat, and doing so clears accumulated data to avoid // initialization costs varying between runs. diff --git a/test/cpp/microbenchmarks/bm_timer.cc b/test/cpp/microbenchmarks/bm_timer.cc index 7a493df5826..db6ab0a073e 100644 --- a/test/cpp/microbenchmarks/bm_timer.cc +++ b/test/cpp/microbenchmarks/bm_timer.cc @@ -81,7 +81,7 @@ static void BM_TimerBatch(benchmark::State& state) { grpc_timer_init(&timer_closure->timer, deadline, &timer_closure->closure); } if (check) { - grpc_millis next; + grpc_millis next = GRPC_MILLIS_INF_FUTURE; grpc_timer_check(&next); } for (grpc_millis deadline = start; deadline != end; deadline += increment) { diff --git a/tools/remote_build/rbe_common.bazelrc b/tools/remote_build/rbe_common.bazelrc index af024760cd4..5b5dcdf9d93 100644 --- a/tools/remote_build/rbe_common.bazelrc +++ b/tools/remote_build/rbe_common.bazelrc @@ -55,7 +55,7 @@ build:asan --copt=-gmlt # use double the default value for "moderate" and "long" timeout as sanitizer # tests tend to be slower build:asan --test_timeout=60,600,1800,3600 -build:asan --test_tag_filters=-no_linux,-qps_json_driver +build:asan --test_tag_filters=-no_linux,-noasan,-qps_json_driver # memory sanitizer: most settings are already in %workspace%/.bazelrc # we only need a few additional ones that are Foundry specific @@ -78,7 +78,7 @@ build:tsan --copt=-gmlt # use double the default value for "moderate" and "long" timeout as sanitizer # tests tend to be slower build:tsan --test_timeout=60,600,1800,3600 -build:tsan --test_tag_filters=-no_linux,-qps_json_driver +build:tsan --test_tag_filters=-no_linux,-notsan,-qps_json_driver build:tsan --extra_execution_platforms=@rbe_default//config:platform # undefined behavior sanitizer: most settings are already in %workspace%/.bazelrc From 1513f3a7b45e8b070cf6d03ee17f37a80189f378 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 5 Feb 2020 11:38:50 -0800 Subject: [PATCH 2/3] Reduce unneeded sizes and disable tests on mac for resource limits --- test/cpp/microbenchmarks/BUILD | 73 +++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index bf6b205a19b..abd5cb32e21 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -39,7 +39,6 @@ grpc_cc_library( external_deps = [ "benchmark", ], - tags = ["no_windows"], deps = [ "//:grpc++_unsecure", "//src/proto/grpc/testing:echo_proto", @@ -61,7 +60,6 @@ grpc_cc_library( external_deps = [ "benchmark", ], - tags = ["no_windows"], deps = [ "//:grpc++", "//src/proto/grpc/testing:echo_proto", @@ -73,22 +71,28 @@ grpc_cc_library( grpc_cc_test( name = "bm_closure", srcs = ["bm_closure.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], deps = [":helpers"], ) grpc_cc_test( name = "bm_alarm", srcs = ["bm_alarm.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], deps = [":helpers"], ) grpc_cc_test( name = "bm_arena", - size = "enormous", srcs = ["bm_arena.cc"], tags = [ + "no_mac", "no_windows", "notsan", ], @@ -99,7 +103,10 @@ grpc_cc_test( grpc_cc_test( name = "bm_byte_buffer", srcs = ["bm_byte_buffer.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -107,7 +114,10 @@ grpc_cc_test( grpc_cc_test( name = "bm_channel", srcs = ["bm_channel.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -115,7 +125,10 @@ grpc_cc_test( grpc_cc_test( name = "bm_call_create", srcs = ["bm_call_create.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -123,14 +136,20 @@ grpc_cc_test( grpc_cc_test( name = "bm_cq", srcs = ["bm_cq.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], deps = [":helpers"], ) grpc_cc_test( name = "bm_cq_multiple_threads", srcs = ["bm_cq_multiple_threads.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -138,7 +157,10 @@ grpc_cc_test( grpc_cc_test( name = "bm_error", srcs = ["bm_error.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -149,7 +171,10 @@ grpc_cc_library( hdrs = [ "fullstack_streaming_ping_pong.h", ], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], deps = [":helpers"], ) @@ -189,7 +214,7 @@ grpc_cc_test( grpc_cc_test( name = "bm_fullstack_trickle", - size = "enormous", + size = "large", srcs = ["bm_fullstack_trickle.cc"], tags = [ "no_mac", # to emulate "excluded_poll_engines: poll" @@ -225,7 +250,10 @@ grpc_cc_test( grpc_cc_test( name = "bm_metadata", srcs = ["bm_metadata.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -233,7 +261,10 @@ grpc_cc_test( grpc_cc_test( name = "bm_chttp2_hpack", srcs = ["bm_chttp2_hpack.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) @@ -252,16 +283,19 @@ grpc_cc_test( grpc_cc_test( name = "bm_timer", srcs = ["bm_timer.cc"], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], uses_polling = False, deps = [":helpers"], ) grpc_cc_test( name = "bm_threadpool", - size = "enormous", srcs = ["bm_threadpool.cc"], tags = [ + "no_mac", "no_windows", "notsan", ], @@ -301,7 +335,10 @@ grpc_cc_test( srcs = [ "bm_callback_unary_ping_pong.cc", ], - tags = ["no_windows"], + tags = [ + "no_mac", + "no_windows", + ], deps = [":callback_unary_ping_pong_h"], ) From f7d857a8e4543f7d1b8181612a3b46a4a33fe79f Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 13 Feb 2020 10:12:12 -0800 Subject: [PATCH 3/3] Make threadpool and trickle tests manual only (excessive time/resources) --- test/cpp/microbenchmarks/BUILD | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index abd5cb32e21..df2cedfeed3 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -216,13 +216,7 @@ grpc_cc_test( name = "bm_fullstack_trickle", size = "large", srcs = ["bm_fullstack_trickle.cc"], - tags = [ - "no_mac", # to emulate "excluded_poll_engines: poll" - "no_windows", - "noasan", - "nomsan", - "notsan", - ], + tags = ["manual"], deps = [":helpers"], ) @@ -293,12 +287,9 @@ grpc_cc_test( grpc_cc_test( name = "bm_threadpool", + size = "large", srcs = ["bm_threadpool.cc"], - tags = [ - "no_mac", - "no_windows", - "notsan", - ], + tags = ["manual"], uses_polling = False, deps = [":helpers"], )