[PATCH] D125226: [pseudo] Add benchmarks for pseudoparser.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/pseudo/benchmarks/Benchmark.cpp:49
+static void runParseGrammar(benchmark::State &State) {
+  std::string GrammarText = readFile(GrammarFile);
+  for (auto _ : State)
----------------
sammccall wrote:
> rather than have each benchmark read the inputs and compile the grammar, it seems a bit less repetitive to do this in main() or a single setup function called from there, and share the const inputs.
> 
> The only real downside I see is that some of the benchmarks don't require source, but source text could default to an empty string.
moved to a common setup which is invoked in the main function.


================
Comment at: clang-tools-extra/pseudo/benchmarks/Benchmark.cpp:64
+
+static void runGLRParse(benchmark::State &State) {
+  std::string GrammarText = readFile(GrammarFile);
----------------
sammccall wrote:
> if the benchmark is "runGLRParse" then everything apart from the glrParse call should be outside the test loop I think. (and such a benchmark is useful)
> 
> If the benchmark is intended to be end-to-end (also useful) then it should have a more generic name, and we should expect to include disambiguation, syntax-tree forming etc in the future.
> 
> I think we should probably have both!
yeah, my intent is to run an overall parse. Renamed to` runParseOverral`, and added `runGLRParse` for the specific `glrParse` benchmark.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125226/new/

https://reviews.llvm.org/D125226