google / benchmark

A microbenchmark support library
Apache License 2.0
8.61k stars 1.57k forks source link

[FR] support flag --benchmark_list_tests work with --benchmark-format=json #1642

Open ylxxwx opened 11 months ago

ylxxwx commented 11 months ago

Is your feature request related to a problem? Please describe. For automation to collection the benchmark list easily, it is better to support use --benchmark_list_tests together with --benchmark-format=json.

Describe the solution you'd like to output the benchmark list in json format.

Describe alternatives you've considered

Additional context Add any other context or screenshots about the feature request here.

varshneydevansh commented 11 months ago

In this one do, we have to modify the behavior of RunSpecifiedBenchmarks to support outputting the list of benchmarks in JSON format when using the--benchmark_list_tests flag with --benchmark_format=json.

https://github.com/google/benchmark/blob/cbecc8ffc774d22b59d7ca2073827246807a5805/src/benchmark.cc#L540

In file bennchmark.cc and need some modification in the benchmark.h?


https://github.com/varshneydevansh/benchmark/blob/cbecc8ffc774d22b59d7ca2073827246807a5805/src/benchmark.cc#L595

  if (FLAGS_benchmark_list_tests) {
    if (FLAGS_benchmark_format == "json") {
      // is this the palce to modify?
    } else {
      for (auto const& benchmark : benchmarks)
        Out << benchmark.name().str() << "\n";
    }
  } else {
    internal::RunBenchmarks(benchmarks, display_reporter, file_reporter);
  }
dmah42 commented 11 months ago

the test lists are a bit of a hack right now, and don't use the reporters at all. as such, i think you'll need to create a new List method in BenchmarkReporter which takes the vector of BenchmarkInstance objects and correctly prints them.

varshneydevansh commented 11 months ago

This is what I added in the benchmark.h file -

// Interface for custom benchmark result printers.
// By default, benchmark reports are printed to stdout. However an application
// can control the destination of the reports by calling
// RunSpecifiedBenchmarks and passing it a custom reporter object.
// The reporter object must implement the following interface.
class BENCHMARK_EXPORT BenchmarkReporter {
 public:
//Remaining code
  /**
   * @brief Lists and describes the provided benchmarks.
   *
   * This virtual method is intended to be overridden by derived classes
   * to provide specific implementations for listing benchmarks.
   * It can be used for outputting, logging, or any other operation
   * needed to handle or display the benchmarks' names and metadata.
   *
   * @param benchmarks A vector containing names and details of benchmarks
   *                   that need to be listed or processed.
   */
  virtual void BenchmarksList(const std::vector<BenchmarkName>& benchmarks) = 0;

 private:
  std::ostream* output_stream_;
  std::ostream* error_stream_;
};

Now my question is about the implementation of this function, do I add that in the json_reporter.cc file or create a separate file so that in future ConsoleReporter or the deprecated CSVReporter can utilize this?

dmah42 commented 11 months ago

nit: it should be List not BenchmarksList .. we don't need the redundant naming.

the implementation would be in all the *_reporter.cc files. the existing loop -> Out implementation in benchmark.cc would go in the console_reporter.cc implementation, a new JSON one in JSON, and a "not implemented" error in the CSV one.

varshneydevansh commented 11 months ago

Tho I made the all changes which we are desiring -

example -

void JSONReporter::List(const std::vector<internal::BenchmarkInstance>& benchmarks) {
  std::ostream& Out = GetOutputStream();
  Out << "{\n  \"benchmarks\": [\n";

  for (size_t i = 0; i < benchmarks.size(); ++i) {
    Out << "    {\"name\": \"" << benchmarks[i].str() << "\"}";
    if (i != benchmarks.size() - 1) Out << ",";
    Out << "\n";
  }

  Out << "  ]\n}";
}
BENCHMARK_EXPORT
void ConsoleReporter::List(const std::vector<internal::BenchmarkInstance>& benchmarks) {
  std::ostream& Out = GetOutputStream();
  for (const auto& benchmark : benchmarks) {
    Out << benchmark.str() << "\n";
  }
}
void CSVReporter::List(std::vector<internal::BenchmarkInstance>& benchmarks){
    (void)benchmarks; // This is to prevent unused variable warning
    std::ostream& err = GetErrorStream();
    err << "List() method is not implemented for CSVReporter.\n";
}

in Benchmark.cc

  if (FLAGS_benchmark_list_tests) {
    if (FLAGS_benchmark_format == "json") {
      JSONReporter().List(benchmarks);
        } else {
      for (auto const& benchmark : benchmarks)
        Out << benchmark.name().str() << "\n";
    }
  } else {
    internal::RunBenchmarks(benchmarks, display_reporter, file_reporter);
  }

  return benchmarks.size();
}

image

But stuck to some errors. Maybe will try to do in the morning.

https://github.com/varshneydevansh/benchmark/blob/cbecc8ffc774d22b59d7ca2073827246807a5805/src/benchmark_register.cc#L197

https://github.com/varshneydevansh/benchmark/blob/cbecc8ffc774d22b59d7ca2073827246807a5805/src/benchmark.cc

varshneydevansh commented 11 months ago

virtual void BenchmarksList(const std::vector& benchmarks) = 0;

I made this into static void List(const std::vector<internal::BenchmarkInstance>& benchmarks);

because from the benchmark.cc it was giving error so modified the code to below -

  if (FLAGS_benchmark_list_tests) {
    if (FLAGS_benchmark_format == "json") {
      dynamic_cast<JSONReporter*>(display_reporter)->List(benchmarks);
        } else {
      //ConsoleReporter::List(benchmarks);
      dynamic_cast<ConsoleReporter*>(display_reporter)->List(benchmarks);
    }
  } else {
    internal::RunBenchmarks(benchmarks, display_reporter, file_reporter);
  }

image

image

image

I will look into this in the morning. Opening the PR so that you can better guide me over there.