Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ylxxwx opened this issue Aug 8, 2023 · 6 comments · May be fixed by #1647
Open

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

ylxxwx opened this issue Aug 8, 2023 · 6 comments · May be fixed by #1647

Comments

@ylxxwx
Copy link

ylxxwx commented Aug 8, 2023

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
Copy link
Contributor

varshneydevansh commented Aug 11, 2023

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.

size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,

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
Copy link
Member

dmah42 commented Aug 11, 2023

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
Copy link
Contributor

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
Copy link
Member

dmah42 commented Aug 11, 2023

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
Copy link
Contributor

varshneydevansh commented Aug 11, 2023

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
Copy link
Contributor

varshneydevansh commented Aug 11, 2023

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.

@varshneydevansh varshneydevansh linked a pull request Aug 11, 2023 that will close this issue
4 tasks
varshneydevansh added a commit to varshneydevansh/benchmark that referenced this issue Aug 12, 2023
dmah42 added a commit to varshneydevansh/benchmark that referenced this issue Sep 26, 2023
varshneydevansh added a commit to varshneydevansh/benchmark that referenced this issue Feb 20, 2024
varshneydevansh added a commit to varshneydevansh/benchmark that referenced this issue May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants