From 69006e0c30bd27b7f3cdc1553b26f03baee645a5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 1 Jul 2024 13:30:35 -0700 Subject: [PATCH] [benchmarks] Enable callback microbenchmarks (#37077) Since these were disabled they stopped working, and we really need to be tracking overheads here. Closes #37077 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37077 from ctiller:it-must-work aa19a4aa89e8e706b60928abfd2501f5f4a99a07 PiperOrigin-RevId: 648469428 --- CMakeLists.txt | 4 +- build_autogenerated.yaml | 4 +- test/cpp/microbenchmarks/BUILD | 12 +----- .../bm_callback_streaming_ping_pong.cc | 41 ++++++++++--------- .../sanity/check_bad_dependencies.sh | 1 - 5 files changed, 27 insertions(+), 35 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3c6f783dbc9..646d66410b2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4076,8 +4076,8 @@ target_include_directories(benchmark_helpers target_link_libraries(benchmark_helpers ${_gRPC_ALLTARGETS_LIBRARIES} ${_gRPC_BENCHMARK_LIBRARIES} - grpc++_unsecure - grpc_test_util_unsecure + grpc++ + grpc_test_util grpc++_test_config ) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 1e49b675282..9cc8c9f9e95 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -3625,8 +3625,8 @@ libs: - test/cpp/microbenchmarks/helpers.cc deps: - benchmark - - grpc++_unsecure - - grpc_test_util_unsecure + - grpc++ + - grpc_test_util - grpc++_test_config defaults: benchmark - name: grpc++ diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index 11a9b0036b1..548e78185a3 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -98,10 +98,10 @@ grpc_cc_library( "benchmark", ], deps = [ - "//:grpc++_unsecure", + "//:grpc++", "//src/proto/grpc/testing:echo_proto", + "//test/core/test_util:grpc_test_util", "//test/core/test_util:grpc_test_util_base", - "//test/core/test_util:grpc_test_util_unsecure", "//test/cpp/util:test_config", ], ) @@ -350,10 +350,6 @@ grpc_cc_benchmark( srcs = [ "bm_callback_unary_ping_pong.cc", ], - tags = [ - "manual", - "notap", - ], deps = [":callback_unary_ping_pong_h"], ) @@ -377,10 +373,6 @@ grpc_cc_benchmark( srcs = [ "bm_callback_streaming_ping_pong.cc", ], - tags = [ - "manual", - "notap", - ], deps = [":callback_streaming_ping_pong_h"], ) diff --git a/test/cpp/microbenchmarks/bm_callback_streaming_ping_pong.cc b/test/cpp/microbenchmarks/bm_callback_streaming_ping_pong.cc index 78505adacf8..2188ae49333 100644 --- a/test/cpp/microbenchmarks/bm_callback_streaming_ping_pong.cc +++ b/test/cpp/microbenchmarks/bm_callback_streaming_ping_pong.cc @@ -16,6 +16,7 @@ // // +#include "test/core/test_util/build.h" #include "test/core/test_util/test_config.h" #include "test/cpp/microbenchmarks/callback_streaming_ping_pong.h" #include "test/cpp/util/test_config.h" @@ -27,43 +28,43 @@ namespace testing { // CONFIGURATIONS // -// Replace "benchmark::internal::Benchmark" with "::testing::Benchmark" to use -// internal microbenchmarking tooling -static void StreamingPingPongMsgSizeArgs(benchmark::internal::Benchmark* b) { - // base case: 0 byte ping-pong msgs - b->Args({0, 1}); - b->Args({0, 2}); +static const int kMaxMessageSize = [] { + if (BuiltUnderMsan() || BuiltUnderTsan() || BuiltUnderUbsan()) { + // Scale down sizes for intensive benchmarks to avoid timeouts. + return 8 * 1024 * 1024; + } + return 128 * 1024 * 1024; +}(); + +// Generate Args for StreamingPingPong benchmarks. Currently generates args for +// only "small streams" (i.e streams with 0, 1 or 2 messages) +static void StreamingPingPongArgs(benchmark::internal::Benchmark* b) { + int msg_size = 0; + + b->Args({0, 0}); // spl case: 0 ping-pong msgs (msg_size doesn't matter here) - for (int msg_size = 1; msg_size <= 128 * 1024 * 1024; msg_size *= 8) { + for (msg_size = 0; msg_size <= kMaxMessageSize; + msg_size == 0 ? msg_size++ : msg_size *= 8) { b->Args({msg_size, 1}); b->Args({msg_size, 2}); } } -// Replace "benchmark::internal::Benchmark" with "::testing::Benchmark" to use -// internal microbenchmarking tooling -static void StreamingPingPongMsgsNumberArgs(benchmark::internal::Benchmark* b) { - for (int msg_number = 1; msg_number <= 256 * 1024; msg_number *= 8) { - b->Args({0, msg_number}); - b->Args({1024, msg_number}); - } -} - // Streaming with different message size BENCHMARK_TEMPLATE(BM_CallbackBidiStreaming, InProcess, NoOpMutator, NoOpMutator) - ->Apply(StreamingPingPongMsgSizeArgs); + ->Apply(StreamingPingPongArgs); BENCHMARK_TEMPLATE(BM_CallbackBidiStreaming, MinInProcess, NoOpMutator, NoOpMutator) - ->Apply(StreamingPingPongMsgSizeArgs); + ->Apply(StreamingPingPongArgs); // Streaming with different message number BENCHMARK_TEMPLATE(BM_CallbackBidiStreaming, InProcess, NoOpMutator, NoOpMutator) - ->Apply(StreamingPingPongMsgsNumberArgs); + ->Apply(StreamingPingPongArgs); BENCHMARK_TEMPLATE(BM_CallbackBidiStreaming, MinInProcess, NoOpMutator, NoOpMutator) - ->Apply(StreamingPingPongMsgsNumberArgs); + ->Apply(StreamingPingPongArgs); // Client context with different metadata BENCHMARK_TEMPLATE(BM_CallbackBidiStreaming, InProcess, diff --git a/tools/run_tests/sanity/check_bad_dependencies.sh b/tools/run_tests/sanity/check_bad_dependencies.sh index a105ffc056f..68ef2d6cd85 100755 --- a/tools/run_tests/sanity/check_bad_dependencies.sh +++ b/tools/run_tests/sanity/check_bad_dependencies.sh @@ -21,7 +21,6 @@ set -ex test "$(bazel query 'somepath("//:grpc_unsecure", "//third_party:libssl")' 2>/dev/null | wc -l)" -eq 0 || exit 1 test "$(bazel query 'somepath("//:grpc++_unsecure", "//third_party:libssl")' 2>/dev/null | wc -l)" -eq 0 || exit 1 test "$(bazel query 'somepath("//:grpc++_codegen_proto", "//third_party:libssl")' 2>/dev/null | wc -l)" -eq 0 || exit 1 -test "$(bazel query 'somepath("//test/cpp/microbenchmarks:helpers", "//third_party:libssl")' 2>/dev/null | wc -l)" -eq 0 || exit 1 # Make sure that core doesn't depend on anything in C++ library