Re: [Tarantool-patches] [PATCH v1 luajit 38/41] perf: add aggregator helper for bench statistics

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the review.
Fixed your comments.

On 18.11.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! See my comments.
> 
> Sergey
> 
> On 10/24/25 14:00, Sergey Kaplun wrote:
> > This patch adds a helper script to aggregate the benchmark results from
> > JSON files to the format parsable by the InfluxDB line protocol [1].
> 
> format cannot be parsed by protocol, please rephrase.
> 
> Something like "the format compatible with the InfluxDB line protocol"

Rephrased as you suggested.

> 
> >
> > All JSON files from each suite in the <perf/output> directory are
> > considered as the benchmark results and aggregated into the
> > <perf/output/summary.txt> file that can be posted to the InfluxDB. The
> > results are aggregated via the new target LuaJIT-perf-aggregate.
> may be say that cjson is required?

Added about it and git.

> >
> > [1]:https://docs.influxdata.com/influxdb/v2/reference/syntax/line-protocol/

The updated commit message is the following:

| perf: add aggregator helper for bench statistics
|
| This patch adds a helper script to aggregate the benchmark results from
| JSON files to the format compatible with the InfluxDB line protocol [1].
|
| The script takes 2 command line arguments:
| | luajit aggregate.lua output_file [input_dir]
|
| If `input_dir` isn't given, it uses the current directory by default.
|
| The script requires the `git` command or specified the `PERF_COMMIT`,
| `PERF_BRANCH` environment variables. Also, it requires the `cjson`
| module.
|
| All JSON files from each suite in the <perf/output> directory are
| considered as the benchmark results and aggregated into the
| <perf/output/summary.txt> file that can be posted to the InfluxDB. The
| results are aggregated via the new target LuaJIT-perf-aggregate.
|
| [1]: https://docs.influxdata.com/influxdb/v2/reference/syntax/line-protocol/

> > ---
> >   perf/CMakeLists.txt        |  13 ++++
> >   perf/helpers/aggregate.lua | 124 +++++++++++++++++++++++++++++++++++++
> >   2 files changed, 137 insertions(+)
> >   create mode 100644 perf/helpers/aggregate.lua
> >
> > diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt
> > index cc3c312f..68e561fd 100644
> > --- a/perf/CMakeLists.txt
> > +++ b/perf/CMakeLists.txt

<snipped>

> > diff --git a/perf/helpers/aggregate.lua b/perf/helpers/aggregate.lua
> > new file mode 100644
> > index 00000000..12a8ab89
> > --- /dev/null
> > +++ b/perf/helpers/aggregate.lua
> > @@ -0,0 +1,124 @@
> > +local json = require('cjson')
> What if cjson is absent? Do we want to handle error?

No, it is required. Mentioned it in the commit message and in the
comment below as you suggested.

> > +
> > +-- File to aggregate the benchmark results from JSON files to the
> > +-- format parsable by the InfluxDB line protocol [1]:
> > +-- <measurement>,<tag_set> <field_set> <timestamp>
> > +--
> > +-- <tag_set> and <field_set> have the following format:
> > +-- <key1>=<value1>,<key2>=<value2>
> > +--
> > +-- The reported tag set is a set of values that can be used for
> > +-- filtering data (i.e., branch or benchmark name).
> > +--
> > +-- luacheck: push no max comment line length
> > +--
> > +-- [1]:https://docs.influxdata.com/influxdb/v2/reference/syntax/line-protocol/
> > +--
> > +-- luacheck: pop
> 
> I propose to document command-line options
> 
> (1st arg is output file, 2nd arg is a dir, "current dir by default"),
> 
> env variables (PERF_COMMIT, PERF_BRANCH) and requirements
> 
> (git is an optional requirement, cjson Lua module is mandatory).

Added the comment:

===================================================================
diff --git a/perf/helpers/aggregate.lua b/perf/helpers/aggregate.lua
index 12a8ab89..01410a47 100644
--- a/perf/helpers/aggregate.lua
+++ b/perf/helpers/aggregate.lua
@@ -15,6 +15,14 @@ local json = require('cjson')
 -- [1]: https://docs.influxdata.com/influxdb/v2/reference/syntax/line-protocol/
 --
 -- luacheck: pop
+--
+-- The script takes 2 command line arguments:
+-- | luajit aggregate.lua output_file [input_dir]
+-- If `input_dir` isn't given, it uses the current directory by
+-- default.
+-- The script requires the `git` command or specified
+-- `PERF_COMMIT`, `PERF_BRANCH` environment variables. Also, it
+-- requires the `cjson` module.
 
 local output = assert(arg[1], 'Output file is required as the first argument')
 local input_dir = arg[2] or '.'
===================================================================

> 
> > +
> > +local output = assert(arg[1], 'Output file is required as the first argument')
> > +local input_dir = arg[2] or '.'
> > +
> > +local out_fh = assert(io.open(output, 'w+'))
> > +
> > +local function exec(cmd)
> > +  return io.popen(cmd):read('*all'):gsub('%s+$', '')
> > +end
> > +
> > +local commit = os.getenv('PERF_COMMIT') or exec('git rev-parse --short HEAD')
> > +assert(commit, 'can not determine the commit')
> > +
> > +local branch = os.getenv('PERF_BRANCH') or
> > +  exec('git rev-parse --abbrev-ref HEAD')
> > +assert(branch, 'can not determine the branch')
> > +

<snipped>

-- 
Best regards,
Sergey Kaplun