Simplified code based on comments and fixed build.json

Makefile

@@ -3199,6 +3199,7 @@ endif
 LIBGRPC++_BENCHMARK_CONFIG_SRC = \
+    test/cpp/qps/report.cc \
     test/cpp/util/benchmark_config.cc \
@@ -3756,7 +3757,6 @@ LIBQPS_SRC = \
     test/cpp/qps/client_sync.cc \
     test/cpp/qps/driver.cc \
     test/cpp/qps/qps_worker.cc \
-    test/cpp/qps/report.cc \
     test/cpp/qps/server_async.cc \
     test/cpp/qps/server_sync.cc \
     test/cpp/qps/timer.cc \
@@ -3807,7 +3807,6 @@ $(OBJDIR)/$(CONFIG)/test/cpp/qps/client_async.o: $(GENDIR)/test/cpp/qps/qpstest.
 $(OBJDIR)/$(CONFIG)/test/cpp/qps/client_sync.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc
 $(OBJDIR)/$(CONFIG)/test/cpp/qps/driver.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc
 $(OBJDIR)/$(CONFIG)/test/cpp/qps/qps_worker.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc
-$(OBJDIR)/$(CONFIG)/test/cpp/qps/report.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc
 $(OBJDIR)/$(CONFIG)/test/cpp/qps/server_async.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc
 $(OBJDIR)/$(CONFIG)/test/cpp/qps/server_sync.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc
 $(OBJDIR)/$(CONFIG)/test/cpp/qps/timer.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc

build.json

@@ -530,6 +530,7 @@
       "build": "private",
       "language": "c++",
       "src": [
+        "test/cpp/qps/report.cc",
         "test/cpp/util/benchmark_config.cc"
       ]
     },
@@ -701,7 +702,6 @@
         "test/cpp/qps/client_sync.cc",
         "test/cpp/qps/driver.cc",
         "test/cpp/qps/qps_worker.cc",
-        "test/cpp/qps/report.cc",
         "test/cpp/qps/server_async.cc",
         "test/cpp/qps/server_sync.cc",
         "test/cpp/qps/timer.cc"

test/cpp/qps/async_streaming_ping_pong_test.cc

@@ -68,11 +68,9 @@ static void RunAsyncStreamingPingPong(
   const auto result =
       RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
 
-  std::set<ReportType> types;
-  types.insert(grpc::testing::ReportType::REPORT_QPS);
-  types.insert(grpc::testing::ReportType::REPORT_LATENCY);
   for (const auto& reporter : reporters) {
-    reporter->Report({client_config, server_config, result}, types);
+    reporter->ReportQPS(result);
+    reporter->ReportLatency(result);
   }
 }
 

test/cpp/qps/async_unary_ping_pong_test.cc

@@ -68,11 +68,9 @@ static void RunAsyncUnaryPingPong(
   const auto result =
       RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
 
-  std::set<ReportType> types;
-  types.insert(grpc::testing::ReportType::REPORT_QPS);
-  types.insert(grpc::testing::ReportType::REPORT_LATENCY);
   for (const auto& reporter : reporters) {
-    reporter->Report({client_config, server_config, result}, types);
+    reporter->ReportQPS(result);
+    reporter->ReportLatency(result);
   }
 }
 }  // namespace testing

test/cpp/qps/qps_driver.cc

@@ -70,7 +70,6 @@ using grpc::testing::ClientType;
 using grpc::testing::ServerType;
 using grpc::testing::RpcType;
 using grpc::testing::ResourceUsage;
-using grpc::testing::ReportType;
 
 namespace grpc {
 namespace testing {
@@ -113,10 +112,11 @@ static void QpsDriver(
       client_config, FLAGS_num_clients, server_config, FLAGS_num_servers,
       FLAGS_warmup_seconds, FLAGS_benchmark_seconds, FLAGS_local_workers);
 
-  std::set<ReportType> types;
-  types.insert(grpc::testing::ReportType::REPORT_ALL);
   for (const auto& reporter : reporters) {
-    reporter->Report({client_config, server_config, result}, types);
+    reporter->ReportQPS(result);
+    reporter->ReportQPSPerCore(result, server_config);
+    reporter->ReportLatency(result);
+    reporter->ReportTimes(result);
   }
 }
 

test/cpp/qps/qps_test.cc

@@ -67,11 +67,9 @@ static void RunQPS(const std::vector<std::unique_ptr<Reporter> >& reporters) {
   const auto result =
       RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
 
-  std::set<ReportType> types;
-  types.insert(grpc::testing::ReportType::REPORT_QPS_PER_CORE);
-  types.insert(grpc::testing::ReportType::REPORT_LATENCY);
   for (const auto& reporter : reporters) {
-    reporter->Report({client_config, server_config, result}, types);
+    reporter->ReportQPSPerCore(result, server_config);
+    reporter->ReportLatency(result);
   }
 }
 

test/cpp/qps/report.cc

@@ -39,32 +39,6 @@
 namespace grpc {
 namespace testing {
 
-// Reporter implementation.
-void Reporter::Report(const ReportData& data,
-                      const std::set<ReportType>& types) const {
-  for (ReportType rtype : types) {
-    bool all = false;
-    switch (rtype) {
-      case REPORT_ALL:
-        all = true;
-      case REPORT_QPS:
-        ReportQPS(data.scenario_result);
-        if (!all) break;
-      case REPORT_QPS_PER_CORE:
-        ReportQPSPerCore(data.scenario_result, data.server_config);
-        if (!all) break;
-      case REPORT_LATENCY:
-        ReportLatency(data.scenario_result);
-        if (!all) break;
-      case REPORT_TIMES:
-        ReportTimes(data.scenario_result);
-        if (!all) break;
-    }
-    if (all) break;
-  }
-}
-
-// GprLogReporter implementation.
 void GprLogReporter::ReportQPS(const ScenarioResult& result) const {
   gpr_log(GPR_INFO, "QPS: %.1f",
           result.latencies.Count() /

test/cpp/qps/report.h

@@ -45,32 +45,6 @@
 namespace grpc {
 namespace testing {
 
-/** General set of data required for report generation. */
-struct ReportData {
-  const ClientConfig& client_config;
-  const ServerConfig& server_config;
-  const ScenarioResult& scenario_result;
-};
-
-/** Specifies the type of performance report we are interested in.
- *
- *  \note The special type \c REPORT_ALL is equivalent to specifying all the
- *  other fields. */
-enum ReportType {
-  /** Equivalent to the combination of all other fields. */
-  REPORT_ALL,
-  /** Report only QPS information. */
-  REPORT_QPS,
-  /** Report only QPS per core information. */
-  REPORT_QPS_PER_CORE,
-  /** Report latency info for the 50, 90, 95, 99 and 99.9th percentiles. */
-  REPORT_LATENCY,
-  /** Report user and system time. */
-  REPORT_TIMES
-};
-
-class Reporter;
-
 /** Interface for all reporters. */
 class Reporter {
  public:
@@ -82,10 +56,6 @@ class Reporter {
    * Names are constants, set at construction time. */
   string name() const { return name_; }
 
-  /** Template method responsible for the generation of the requested types. */
-  void Report(const ReportData& data, const std::set<ReportType>& types) const;
-
- protected:
   /** Reports QPS for the given \a result. */
   virtual void ReportQPS(const ScenarioResult& result) const = 0;
 
@@ -103,9 +73,6 @@ class Reporter {
   const string name_;
 };
 
-
-// Reporters.
-
 /** Reporter to gpr_log(GPR_INFO). */
 class GprLogReporter : public Reporter {
  public:

test/cpp/qps/sync_streaming_ping_pong_test.cc

@@ -67,11 +67,9 @@ static void RunSynchronousStreamingPingPong(
   const auto result =
       RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
 
-  std::set<ReportType> types;
-  types.insert(grpc::testing::ReportType::REPORT_QPS);
-  types.insert(grpc::testing::ReportType::REPORT_LATENCY);
   for (const auto& reporter : reporters) {
-    reporter->Report({client_config, server_config, result}, types);
+    reporter->ReportQPS(result);
+    reporter->ReportLatency(result);
   }
 }
 }  // namespace testing

test/cpp/qps/sync_unary_ping_pong_test.cc

@@ -51,9 +51,6 @@ static void RunSynchronousUnaryPingPong(
     const std::vector<std::unique_ptr<Reporter> >& reporters) {
   gpr_log(GPR_INFO, "Running Synchronous Unary Ping Pong");
 
-  ReportersRegistry reporters_registry;
-  reporters_registry.Register(new GprLogReporter("LogReporter"));
-
   ClientConfig client_config;
   client_config.set_client_type(SYNCHRONOUS_CLIENT);
   client_config.set_enable_ssl(false);
@@ -70,11 +67,9 @@ static void RunSynchronousUnaryPingPong(
   const auto result =
       RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2);
 
-  std::set<ReportType> types;
-  types.insert(grpc::testing::ReportType::REPORT_QPS);
-  types.insert(grpc::testing::ReportType::REPORT_LATENCY);
   for (const auto& reporter : reporters) {
-    reporter->Report({client_config, server_config, result}, types);
+    reporter->ReportQPS(result);
+    reporter->ReportLatency(result);
   }
 }
 

test/cpp/util/benchmark_config.cc

@@ -34,7 +34,7 @@
 #include <gflags/gflags.h>
 #include "test/cpp/util/benchmark_config.h"
 
-DEFINE_bool(enable_log_reporter, false,
+DEFINE_bool(enable_log_reporter, true,
             "Enable reporting of benchmark results through GprLog");
 
 // In some distros, gflags is in the namespace google, and in some others,