benchmark-action / github-action-benchmark

GitHub Action for continuous benchmarking to keep performance
https://benchmark-action.github.io/github-action-benchmark/dev/bench/
MIT License
1.02k stars 152 forks source link

Summary table loses some of the extra information from go benchmarks when running with multiple CPU scenarios #242

Open jaqx0r opened 6 months ago

jaqx0r commented 6 months ago

https://github.com/google/mtail/actions/runs/8951254978?pr=858

The table includes lines like

BenchmarkStore/Add 818.6 ns/op 821.1 ns/op 1.00 BenchmarkStore/Add-1 778.3 ns/op 780.2 ns/op 1.00 BenchmarkStore/Add-1 779.9 ns/op 780.2 ns/op 1.00

which is confusng because these are three different scenarios -- the number of procs per the -cpu flag to go test -bench.

This is being recorded in the JSON file found in https://github.com/google/mtail/actions/runs/8951254978/job/24587370213?pr=858

   {
      name: 'BenchmarkStore/Add',
      value: 818.6,
      unit: 'ns/op',
      extra: '1260484 times\n1 procs'
    },
    {
      name: 'BenchmarkStore/Add-1',
      value: 778.3,
      unit: 'ns/op',
      extra: '1538625 times\n2 procs'
    },
    {
      name: 'BenchmarkStore/Add-1',
      value: 779.9,
      unit: 'ns/op',
      extra: '1574536 times\n4 procs'

in this extra field. It would be useful if the name column included the "N procs" part as well to differentiate the names. Or it would be fine to leave the name alone as BenchmarkStore/Add-1-4 and not extract the procs count into a special "extra"

jaqx0r commented 6 months ago

There's a further bug here -- the name BenchmarkStore/Add loses the test name suffix. It looks like the benchmark-action is incorrectly assuming the suffix -1 is the number of procs, but go test -bench does not append a suffix for the default 1 CPU option.

So I don't think this attempt at parsing out N procs here is a useful function and should be removed. I'll offer a PR if you agree.