From 09756ca36e5532949813466bf023180b3e3fa746 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 15 Jul 2020 23:59:05 +0000 Subject: [PATCH 1/7] Add some benchmark test docs Signed-off-by: Phil Genera --- bazel/envoy_test.bzl | 7 +++++-- test/README.md | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/bazel/envoy_test.bzl b/bazel/envoy_test.bzl index ace2fe600cb4e..1002fe9ed572e 100644 --- a/bazel/envoy_test.bzl +++ b/bazel/envoy_test.bzl @@ -241,7 +241,8 @@ def envoy_cc_test_binary( **kargs ) -# Envoy benchmark binaries should be specified with this function. +# Envoy benchmark binaries should be specified with this function. bazel run +# these targets to measure performance. def envoy_cc_benchmark_binary( name, deps = [], @@ -252,7 +253,9 @@ def envoy_cc_benchmark_binary( **kargs ) -# Tests to validate that Envoy benchmarks run successfully should be specified with this function. +# Tests to validate that Envoy benchmarks run successfully should be specified +# with this function. Not for actual performance measurements: iteratons will be +# skipped in the interest of execution time. def envoy_benchmark_test( name, benchmark_binary, diff --git a/test/README.md b/test/README.md index 3617c73719c1d..6959045727e05 100644 --- a/test/README.md +++ b/test/README.md @@ -119,3 +119,19 @@ test infrastructure that wants to be agnostic to which `TimeSystem` is used in a test. When no `TimeSystem` is instantiated in a test, the `Event::GlobalTimeSystem` will lazy-initialize itself into a concrete `TimeSystem`. Currently this is `TestRealTimeSystem` but will be changed in the future to `SimulatedTimeSystem`. + + +## Benchmark tests + +Envoy uses [Google Benchmark](https://github.com/google/benchmark/) for +microbenchmarks. There are custom bazel rules, `envoy_cc_benchmark_binary` and +`envoy_benchmark_test`, to execute them locally and in CI environments +respectively. `envoy_benchmark_test` rules call the bechmark binary from a +[script](https://github.com/envoyproxy/envoy/blob/master/bazel/test_for_benchmark_wrapper.sh) +which runs the test quickly instead of rigourously. In order to collect +meaningful bechmarks, `bazel run -c opt` the benchmark binary target on a +quiescent machine. + +If you would like to detect when your benchmark test is running under the +wrapper, call +[`Envoy::benchmark::skipExpensiveBechmarks()`](https://github.com/envoyproxy/envoy/blob/master/test/benchmark/main.h). From 7cb3ec8f4d82a98b96d01bbf66e3cd7b1524c38c Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Thu, 16 Jul 2020 00:14:43 +0000 Subject: [PATCH 2/7] typo Signed-off-by: Phil Genera --- test/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/README.md b/test/README.md index 6959045727e05..c69d29170f4a2 100644 --- a/test/README.md +++ b/test/README.md @@ -126,7 +126,7 @@ will lazy-initialize itself into a concrete `TimeSystem`. Currently this is Envoy uses [Google Benchmark](https://github.com/google/benchmark/) for microbenchmarks. There are custom bazel rules, `envoy_cc_benchmark_binary` and `envoy_benchmark_test`, to execute them locally and in CI environments -respectively. `envoy_benchmark_test` rules call the bechmark binary from a +respectively. `envoy_benchmark_test` rules call the benchmark binary from a [script](https://github.com/envoyproxy/envoy/blob/master/bazel/test_for_benchmark_wrapper.sh) which runs the test quickly instead of rigourously. In order to collect meaningful bechmarks, `bazel run -c opt` the benchmark binary target on a From c14f22887ef275ba5662a6edff0a1e8cbaea235c Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 21 Jul 2020 18:29:17 +0000 Subject: [PATCH 3/7] response to review comments Signed-off-by: Phil Genera --- bazel/envoy_test.bzl | 4 ++-- test/README.md | 7 ++++--- test/benchmark/BUILD | 1 + test/benchmark/main.cc | 11 ++++++++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/bazel/envoy_test.bzl b/bazel/envoy_test.bzl index 1002fe9ed572e..3a914d5bc2225 100644 --- a/bazel/envoy_test.bzl +++ b/bazel/envoy_test.bzl @@ -254,8 +254,8 @@ def envoy_cc_benchmark_binary( ) # Tests to validate that Envoy benchmarks run successfully should be specified -# with this function. Not for actual performance measurements: iteratons will be -# skipped in the interest of execution time. +# with this function. Not for actual performance measurements: iteratons and +# expensive benchmarks will be skipped in the interest of execution time. def envoy_benchmark_test( name, benchmark_binary, diff --git a/test/README.md b/test/README.md index c69d29170f4a2..2746efe98c8db 100644 --- a/test/README.md +++ b/test/README.md @@ -128,9 +128,10 @@ microbenchmarks. There are custom bazel rules, `envoy_cc_benchmark_binary` and `envoy_benchmark_test`, to execute them locally and in CI environments respectively. `envoy_benchmark_test` rules call the benchmark binary from a [script](https://github.com/envoyproxy/envoy/blob/master/bazel/test_for_benchmark_wrapper.sh) -which runs the test quickly instead of rigourously. In order to collect -meaningful bechmarks, `bazel run -c opt` the benchmark binary target on a -quiescent machine. +which runs the benchmark with a minimal number of iterations and skipping +expensive benchmarks to quickly verify that the binary is able to run to +completion. In order to collect meaningful bechmarks, `bazel run -c opt` the +benchmark binary target on a quiescent machine. If you would like to detect when your benchmark test is running under the wrapper, call diff --git a/test/benchmark/BUILD b/test/benchmark/BUILD index fa01e3b1ce638..afcb2602898de 100644 --- a/test/benchmark/BUILD +++ b/test/benchmark/BUILD @@ -17,6 +17,7 @@ envoy_cc_test_library( "tclap", ], deps = [ + "//source/common/common:minimal_logger_lib", "//test/test_common:environment_lib", ], ) diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index 6c23c1031a6c4..6af2c1d72bbb1 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -4,6 +4,8 @@ #include "test/test_common/environment.h" +#include "common/common/logger.h" + #include "benchmark/benchmark.h" #include "tclap/CmdLine.h" @@ -37,4 +39,11 @@ int main(int argc, char** argv) { benchmark::RunSpecifiedBenchmarks(); } -bool Envoy::benchmark::skipExpensiveBenchmarks() { return skip_expensive_benchmarks; } +bool Envoy::benchmark::skipExpensiveBenchmarks() { + if (skip_expensive_benchmarks) { + auto logger = Logger::Registry::getLog(Logger::Id::testing); + logger.warn("Expensive benchmarks are being skipped; see test/README.md for more information"); + } + + return skip_expensive_benchmarks; +} From 4007e211326b49223abe397a0f2887ffd4cf1c84 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 21 Jul 2020 18:29:54 +0000 Subject: [PATCH 4/7] formatting Signed-off-by: Phil Genera --- test/benchmark/main.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index 6af2c1d72bbb1..ecf53f941a91b 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -2,10 +2,10 @@ // This is an Envoy driver for benchmarks. #include "test/benchmark/main.h" -#include "test/test_common/environment.h" - #include "common/common/logger.h" +#include "test/test_common/environment.h" + #include "benchmark/benchmark.h" #include "tclap/CmdLine.h" From 4e487a82ebc27d14d8b8c84df0d440215ccf4226 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 21 Jul 2020 20:20:12 +0000 Subject: [PATCH 5/7] respond to review comments Signed-off-by: Phil Genera --- test/benchmark/main.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index ecf53f941a91b..f6f03b5040a1e 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -41,8 +41,7 @@ int main(int argc, char** argv) { bool Envoy::benchmark::skipExpensiveBenchmarks() { if (skip_expensive_benchmarks) { - auto logger = Logger::Registry::getLog(Logger::Id::testing); - logger.warn("Expensive benchmarks are being skipped; see test/README.md for more information"); + ENVOY_LOG_MISC(warn, "Expensive benchmarks are being skipped; see test/README.md for more information"); } return skip_expensive_benchmarks; From 92d56d2dd67dfa649a3d1e2bc64890ae8a9d51bd Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 21 Jul 2020 20:20:40 +0000 Subject: [PATCH 6/7] formatting Signed-off-by: Phil Genera --- test/benchmark/main.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index f6f03b5040a1e..c18113c86ada3 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -41,7 +41,8 @@ int main(int argc, char** argv) { bool Envoy::benchmark::skipExpensiveBenchmarks() { if (skip_expensive_benchmarks) { - ENVOY_LOG_MISC(warn, "Expensive benchmarks are being skipped; see test/README.md for more information"); + ENVOY_LOG_MISC( + warn, "Expensive benchmarks are being skipped; see test/README.md for more information"); } return skip_expensive_benchmarks; From 811a31548da564bb0a186395b8d8081af4406a4e Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 22 Jul 2020 14:04:36 +0000 Subject: [PATCH 7/7] response to review comments Signed-off-by: Phil Genera --- test/benchmark/main.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index c18113c86ada3..3c79ff36b2e0f 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -9,6 +9,8 @@ #include "benchmark/benchmark.h" #include "tclap/CmdLine.h" +using namespace Envoy; + static bool skip_expensive_benchmarks = false; // Boilerplate main(), which discovers benchmarks and runs them. This uses two @@ -17,7 +19,7 @@ static bool skip_expensive_benchmarks = false; // separated by --. // TODO(pgenera): convert this to abseil/flags/ when benchmark also adopts abseil. int main(int argc, char** argv) { - Envoy::TestEnvironment::initializeTestMain(argv[0]); + TestEnvironment::initializeTestMain(argv[0]); // NOLINTNEXTLINE(clang-analyzer-optin.cplusplus.VirtualCall) TCLAP::CmdLine cmd("envoy-benchmark-test", ' ', "0.1"); @@ -35,15 +37,14 @@ int main(int argc, char** argv) { skip_expensive_benchmarks = skip_switch.getValue(); - benchmark::Initialize(&argc, argv); - benchmark::RunSpecifiedBenchmarks(); -} + ::benchmark::Initialize(&argc, argv); -bool Envoy::benchmark::skipExpensiveBenchmarks() { if (skip_expensive_benchmarks) { ENVOY_LOG_MISC( - warn, "Expensive benchmarks are being skipped; see test/README.md for more information"); + critical, + "Expensive benchmarks are being skipped; see test/README.md for more information"); } - - return skip_expensive_benchmarks; + ::benchmark::RunSpecifiedBenchmarks(); } + +bool Envoy::benchmark::skipExpensiveBenchmarks() { return skip_expensive_benchmarks; }