From a6b559a76ae3d96f8df4abe07173c8b56fbfc032 Mon Sep 17 00:00:00 2001
From: David Garcia Quintas <dgq@google.com>
Date: Wed, 20 May 2015 22:08:24 -0700
Subject: [PATCH] Improvements to reporting mechanism based on comments.
Turned the reporter into a composite, much cleaner arch.
---
.../cpp/qps/async_streaming_ping_pong_test.cc | 12 +++-----
test/cpp/qps/async_unary_ping_pong_test.cc | 12 +++-----
test/cpp/qps/qps_driver.cc | 16 ++++------
test/cpp/qps/qps_test.cc | 11 +++----
test/cpp/qps/report.cc | 30 +++++++++++++++++++
test/cpp/qps/report.h | 20 +++++++++++++
test/cpp/qps/sync_streaming_ping_pong_test.cc | 12 +++-----
test/cpp/qps/sync_unary_ping_pong_test.cc | 12 +++-----
test/cpp/util/benchmark_config.cc | 14 ++++++---
test/cpp/util/benchmark_config.h | 8 ++++-
10 files changed, 93 insertions(+), 54 deletions(-)
diff --git a/test/cpp/qps/async_streaming_ping_pong_test.cc b/test/cpp/qps/async_streaming_ping_pong_test.cc
index 8cb0949bb4..dc972443ff 100644
--- a/test/cpp/qps/async_streaming_ping_pong_test.cc
+++ b/test/cpp/qps/async_streaming_ping_pong_test.cc
@@ -47,8 +47,7 @@ namespace testing {
static const int WARMUP = 5;
static const int BENCHMARK = 10;
-static void RunAsyncStreamingPingPong(
- const std::vector<std::unique_ptr<Reporter> >& reporters) {
+static void RunAsyncStreamingPingPong() {
gpr_log(GPR_INFO, "Running Async Streaming Ping Pong");
ClientConfig client_config;
@@ -68,10 +67,8 @@ static void RunAsyncStreamingPingPong(
const auto result =
RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
- for (const auto& reporter : reporters) {
- reporter->ReportQPS(result);
- reporter->ReportLatency(result);
- }
+ GetReporter()->ReportQPS(result);
+ GetReporter()->ReportLatency(result);
}
} // namespace testing
@@ -79,9 +76,8 @@ static void RunAsyncStreamingPingPong(
int main(int argc, char** argv) {
grpc::testing::InitBenchmark(&argc, &argv, true);
- const auto& reporters = grpc::testing::InitBenchmarkReporters();
signal(SIGPIPE, SIG_IGN);
- grpc::testing::RunAsyncStreamingPingPong(reporters);
+ grpc::testing::RunAsyncStreamingPingPong();
return 0;
}
diff --git a/test/cpp/qps/async_unary_ping_pong_test.cc b/test/cpp/qps/async_unary_ping_pong_test.cc
index 997cbced30..05bc08a320 100644
--- a/test/cpp/qps/async_unary_ping_pong_test.cc
+++ b/test/cpp/qps/async_unary_ping_pong_test.cc
@@ -47,8 +47,7 @@ namespace testing {
static const int WARMUP = 5;
static const int BENCHMARK = 10;
-static void RunAsyncUnaryPingPong(
- const std::vector<std::unique_ptr<Reporter> >& reporters) {
+static void RunAsyncUnaryPingPong() {
gpr_log(GPR_INFO, "Running Async Unary Ping Pong");
ClientConfig client_config;
@@ -68,19 +67,16 @@ static void RunAsyncUnaryPingPong(
const auto result =
RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
- for (const auto& reporter : reporters) {
- reporter->ReportQPS(result);
- reporter->ReportLatency(result);
- }
+ GetReporter()->ReportQPS(result);
+ GetReporter()->ReportLatency(result);
}
} // namespace testing
} // namespace grpc
int main(int argc, char** argv) {
grpc::testing::InitBenchmark(&argc, &argv, true);
- const auto& reporters = grpc::testing::InitBenchmarkReporters();
signal(SIGPIPE, SIG_IGN);
- grpc::testing::RunAsyncUnaryPingPong(reporters);
+ grpc::testing::RunAsyncUnaryPingPong();
return 0;
}
diff --git a/test/cpp/qps/qps_driver.cc b/test/cpp/qps/qps_driver.cc
index 1f17424fad..2e491eeb05 100644
--- a/test/cpp/qps/qps_driver.cc
+++ b/test/cpp/qps/qps_driver.cc
@@ -74,8 +74,7 @@ using grpc::testing::ResourceUsage;
namespace grpc {
namespace testing {
-static void QpsDriver(
- const std::vector<std::unique_ptr<Reporter> >& reporters) {
+static void QpsDriver() {
RpcType rpc_type;
GPR_ASSERT(RpcType_Parse(FLAGS_rpc_type, &rpc_type));
@@ -112,12 +111,10 @@ static void QpsDriver(
client_config, FLAGS_num_clients, server_config, FLAGS_num_servers,
FLAGS_warmup_seconds, FLAGS_benchmark_seconds, FLAGS_local_workers);
- for (const auto& reporter : reporters) {
- reporter->ReportQPS(result);
- reporter->ReportQPSPerCore(result, server_config);
- reporter->ReportLatency(result);
- reporter->ReportTimes(result);
- }
+ GetReporter()->ReportQPS(result);
+ GetReporter()->ReportQPSPerCore(result, server_config);
+ GetReporter()->ReportLatency(result);
+ GetReporter()->ReportTimes(result);
}
} // namespace testing
@@ -125,10 +122,9 @@ static void QpsDriver(
int main(int argc, char** argv) {
grpc::testing::InitBenchmark(&argc, &argv, true);
- const auto& reporters = grpc::testing::InitBenchmarkReporters();
signal(SIGPIPE, SIG_IGN);
- grpc::testing::QpsDriver(reporters);
+ grpc::testing::QpsDriver();
return 0;
}
diff --git a/test/cpp/qps/qps_test.cc b/test/cpp/qps/qps_test.cc
index 92940795a7..03c9c2c423 100644
--- a/test/cpp/qps/qps_test.cc
+++ b/test/cpp/qps/qps_test.cc
@@ -47,7 +47,7 @@ namespace testing {
static const int WARMUP = 5;
static const int BENCHMARK = 10;
-static void RunQPS(const std::vector<std::unique_ptr<Reporter> >& reporters) {
+static void RunQPS() {
gpr_log(GPR_INFO, "Running QPS test");
ClientConfig client_config;
@@ -67,10 +67,8 @@ static void RunQPS(const std::vector<std::unique_ptr<Reporter> >& reporters) {
const auto result =
RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
- for (const auto& reporter : reporters) {
- reporter->ReportQPSPerCore(result, server_config);
- reporter->ReportLatency(result);
- }
+ GetReporter()->ReportQPSPerCore(result, server_config);
+ GetReporter()->ReportLatency(result);
}
} // namespace testing
@@ -78,10 +76,9 @@ static void RunQPS(const std::vector<std::unique_ptr<Reporter> >& reporters) {
int main(int argc, char** argv) {
grpc::testing::InitBenchmark(&argc, &argv, true);
- const auto& reporters = grpc::testing::InitBenchmarkReporters();
signal(SIGPIPE, SIG_IGN);
- grpc::testing::RunQPS(reporters);
+ grpc::testing::RunQPS();
return 0;
}
diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc
index 9c4bb0d954..e116175e3b 100644
--- a/test/cpp/qps/report.cc
+++ b/test/cpp/qps/report.cc
@@ -39,6 +39,36 @@
namespace grpc {
namespace testing {
+void CompositeReporter::add(std::unique_ptr<Reporter> reporter) {
+ reporters_.emplace_back(std::move(reporter));
+}
+
+void CompositeReporter::ReportQPS(const ScenarioResult& result) const {
+ for (size_t i = 0; i < reporters_.size(); ++i) {
+ reporters_[i]->ReportQPS(result);
+ }
+}
+
+void CompositeReporter::ReportQPSPerCore(const ScenarioResult& result,
+ const ServerConfig& config) const {
+ for (size_t i = 0; i < reporters_.size(); ++i) {
+ reporters_[i]->ReportQPSPerCore(result, config);
+ }
+}
+
+void CompositeReporter::ReportLatency(const ScenarioResult& result) const {
+ for (size_t i = 0; i < reporters_.size(); ++i) {
+ reporters_[i]->ReportLatency(result);
+ }
+}
+
+void CompositeReporter::ReportTimes(const ScenarioResult& result) const {
+ for (size_t i = 0; i < reporters_.size(); ++i) {
+ reporters_[i]->ReportTimes(result);
+ }
+}
+
+
void GprLogReporter::ReportQPS(const ScenarioResult& result) const {
gpr_log(GPR_INFO, "QPS: %.1f",
result.latencies.Count() /
diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h
index 32b948c34f..3712f67a47 100644
--- a/test/cpp/qps/report.h
+++ b/test/cpp/qps/report.h
@@ -51,6 +51,8 @@ class Reporter {
/** Construct a reporter with the given \a name. */
Reporter(const string& name) : name_(name) {}
+ virtual ~Reporter() {}
+
/** Returns this reporter's name.
*
* Names are constants, set at construction time. */
@@ -73,6 +75,24 @@ class Reporter {
const string name_;
};
+/** A composite for all reporters to be considered. */
+class CompositeReporter : public Reporter {
+ public:
+ CompositeReporter() : Reporter("CompositeReporter") {}
+
+ /** Adds a \a reporter to the composite. */
+ void add(std::unique_ptr<Reporter> reporter);
+
+ void ReportQPS(const ScenarioResult& result) const GRPC_OVERRIDE;
+ void ReportQPSPerCore(const ScenarioResult& result,
+ const ServerConfig& config) const GRPC_OVERRIDE;
+ void ReportLatency(const ScenarioResult& result) const GRPC_OVERRIDE;
+ void ReportTimes(const ScenarioResult& result) const GRPC_OVERRIDE;
+
+ private:
+ std::vector<std::unique_ptr<Reporter> > reporters_;
+};
+
/** Reporter to gpr_log(GPR_INFO). */
class GprLogReporter : public Reporter {
public:
diff --git a/test/cpp/qps/sync_streaming_ping_pong_test.cc b/test/cpp/qps/sync_streaming_ping_pong_test.cc
index 6da107aa73..776fbdde1c 100644
--- a/test/cpp/qps/sync_streaming_ping_pong_test.cc
+++ b/test/cpp/qps/sync_streaming_ping_pong_test.cc
@@ -47,8 +47,7 @@ namespace testing {
static const int WARMUP = 5;
static const int BENCHMARK = 10;
-static void RunSynchronousStreamingPingPong(
- const std::vector<std::unique_ptr<Reporter> >& reporters) {
+static void RunSynchronousStreamingPingPong() {
gpr_log(GPR_INFO, "Running Synchronous Streaming Ping Pong");
ClientConfig client_config;
@@ -67,20 +66,17 @@ static void RunSynchronousStreamingPingPong(
const auto result =
RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
- for (const auto& reporter : reporters) {
- reporter->ReportQPS(result);
- reporter->ReportLatency(result);
- }
+ GetReporter()->ReportQPS(result);
+ GetReporter()->ReportLatency(result);
}
} // namespace testing
} // namespace grpc
int main(int argc, char** argv) {
grpc::testing::InitBenchmark(&argc, &argv, true);
- const auto& reporters = grpc::testing::InitBenchmarkReporters();
signal(SIGPIPE, SIG_IGN);
- grpc::testing::RunSynchronousStreamingPingPong(reporters);
+ grpc::testing::RunSynchronousStreamingPingPong();
return 0;
}
diff --git a/test/cpp/qps/sync_unary_ping_pong_test.cc b/test/cpp/qps/sync_unary_ping_pong_test.cc
index eb930def2a..e79b820ce3 100644
--- a/test/cpp/qps/sync_unary_ping_pong_test.cc
+++ b/test/cpp/qps/sync_unary_ping_pong_test.cc
@@ -47,8 +47,7 @@ namespace testing {
static const int WARMUP = 5;
static const int BENCHMARK = 10;
-static void RunSynchronousUnaryPingPong(
- const std::vector<std::unique_ptr<Reporter> >& reporters) {
+static void RunSynchronousUnaryPingPong() {
gpr_log(GPR_INFO, "Running Synchronous Unary Ping Pong");
ClientConfig client_config;
@@ -67,10 +66,8 @@ static void RunSynchronousUnaryPingPong(
const auto result =
RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
- for (const auto& reporter : reporters) {
- reporter->ReportQPS(result);
- reporter->ReportLatency(result);
- }
+ GetReporter()->ReportQPS(result);
+ GetReporter()->ReportLatency(result);
}
} // namespace testing
@@ -78,10 +75,9 @@ static void RunSynchronousUnaryPingPong(
int main(int argc, char** argv) {
grpc::testing::InitBenchmark(&argc, &argv, true);
- const auto& reporters = grpc::testing::InitBenchmarkReporters();
signal(SIGPIPE, SIG_IGN);
- grpc::testing::RunSynchronousUnaryPingPong(reporters);
+ grpc::testing::RunSynchronousUnaryPingPong();
return 0;
}
diff --git a/test/cpp/util/benchmark_config.cc b/test/cpp/util/benchmark_config.cc
index 914afa91ca..1b15ddcbcc 100644
--- a/test/cpp/util/benchmark_config.cc
+++ b/test/cpp/util/benchmark_config.cc
@@ -51,12 +51,18 @@ void InitBenchmark(int* argc, char*** argv, bool remove_flags) {
ParseCommandLineFlags(argc, argv, remove_flags);
}
-std::vector<std::unique_ptr<Reporter> > InitBenchmarkReporters() {
- std::vector<std::unique_ptr<Reporter> > reporters;
+static std::shared_ptr<Reporter> InitBenchmarkReporters() {
+ auto* composite_reporter = new CompositeReporter;
if (FLAGS_enable_log_reporter) {
- reporters.emplace_back(new GprLogReporter("LogReporter"));
+ composite_reporter->add(
+ std::unique_ptr<Reporter>(new GprLogReporter("LogReporter")));
}
- return reporters;
+ return std::shared_ptr<Reporter>(composite_reporter);
+}
+
+const std::shared_ptr<Reporter>& GetReporter() {
+ static std::shared_ptr<Reporter> reporter(InitBenchmarkReporters());
+ return reporter;
}
} // namespace testing
diff --git a/test/cpp/util/benchmark_config.h b/test/cpp/util/benchmark_config.h
index efbcad88f0..3a3a6d61ae 100644
--- a/test/cpp/util/benchmark_config.h
+++ b/test/cpp/util/benchmark_config.h
@@ -43,7 +43,13 @@ namespace grpc {
namespace testing {
void InitBenchmark(int* argc, char*** argv, bool remove_flags);
-std::vector<std::unique_ptr<Reporter> > InitBenchmarkReporters();
+
+/** Returns the benchmark Reporter instance.
+ *
+ * The returned instane will take care of generating reports for all the actual
+ * reporters configured via the "enable_*_reporter" command line flags (see
+ * benchmark_config.cc). */
+const std::shared_ptr<Reporter>& GetReporter();
} // namespace testing
} // namespace grpc
--
GitLab