Re: [Tarantool-patches] [PATCH v1 luajit 27/41] perf: adjust scimark-2010-12-20 in LuaJIT-benches

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

Hi, Sergey!
Thanks for the review.
Please, consider my answers below.

On 17.11.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! See my comments.
> 
> Sergey
> 
> On 10/24/25 13:50, Sergey Kaplun wrote:
> > This patch adjusts the aforementioned test to use the benchmark
> > framework introduced before. The default arguments are adjusted
> > according to the <PARAM_x86.txt> file. The arguments to the script still
> > can be provided in the command line run.
> >
> > The time for each subsequent benchmark is increased up to 4 seconds,
> > accoring the defaults in the "bench" framework. The main difference
> > between this test and others that will be added in next commits is
> > the usage of FFI arrays instead of plain Lua tables.
> > ---
> >   perf/LuaJIT-benches/scimark-2010-12-20.lua | 93 +++++++++++++---------
> >   1 file changed, 54 insertions(+), 39 deletions(-)
> >
> > diff --git a/perf/LuaJIT-benches/scimark-2010-12-20.lua b/perf/LuaJIT-benches/scimark-2010-12-20.lua
> > index 353acb7c..3fb627fa 100644
> > --- a/perf/LuaJIT-benches/scimark-2010-12-20.lua
> > +++ b/perf/LuaJIT-benches/scimark-2010-12-20.lua
> > @@ -9,25 +9,26 @@
> >   local SCIMARK_VERSION = "2010-12-10"
> >   local SCIMARK_COPYRIGHT = "Copyright (C) 2006-2010 Mike Pall"
> >   
> > -local MIN_TIME = 2.0
> > +local bench = require("bench").new(arg)
> > +
> >   local RANDOM_SEED = 101009 -- Must be odd.
> >   local SIZE_SELECT = "small"
> >   
> >   local benchmarks = {
> >     "FFT", "SOR", "MC", "SPARSE", "LU",
> >     small = {
> > -    FFT		= { 1024 },
> > -    SOR		= { 100 },
> > -    MC		= { },
> > -    SPARSE	= { 1000, 5000 },
> > -    LU		= { 100 },
> > +    FFT		= { params = { 1024 }, cycles = 50000, },
> > +    SOR		= { params = { 100 }, cycles = 50000, },
> > +    MC		= { params = { }, cycles = 15e7, },
> > +    SPARSE	= { params = { 1000, 5000 }, cycles = 15e4, },
> > +    LU		= { params = { 100 }, cycles = 5000, },
> >     },
> >     large = {
> > -    FFT		= { 1048576 },
> > -    SOR		= { 1000 },
> > -    MC		= { },
> > -    SPARSE	= { 100000, 1000000 },
> > -    LU		= { 1000 },
> > +    FFT		= { params = { 1048576 }, cycles = 25, },
> > +    SOR		= { params = { 1000 }, cycles = 500, },
> > +    MC		= { params = { }, cycles = 15e7, },
> > +    SPARSE	= { params = { 100000, 1000000 }, cycles = 1500, },
> > +    LU		= { params = { 1000 }, cycles = 50, },
> >     },
> >   }
> please add a comment about chosen parameters

It is the default for x86 arch. I've taken the values from PARAMS_x86,
since this is the most important architecture for the Tarantool, see the
commit message.

Others parameters are adjusted by comparing with the non-ffi version of
the benchmark.

> >   
> > @@ -342,48 +343,51 @@ local function fmtparams(p1, p2)
> >     return ""
> >   end
> >   
> > -local function measure(min_time, name, ...)
> > +local function measure(name, cycles, ...)
> >     array_init()
> >     rand_init(RANDOM_SEED)
> >     local run = benchmarks[name](...)
> > -  local cycles = 1
> > -  repeat
> > -    local tm = clock()
> > -    local flops = run(cycles, ...)
> > -    tm = clock() - tm
> > -    if tm >= min_time then
> > -      local res = flops / tm * 1.0e-6
> > -      local p1, p2 = ...
> > -      printf("%-7s %8.2f  %s\n", name, res, fmtparams(...))
> > -      return res
> > -    end
> > -    cycles = cycles * 2
> > -  until false
> > +  local flops = run(cycles, ...)
> > +  return flops
> >   end
> >   
> > -printf("Lua SciMark %s based on SciMark 2.0a. %s.\n\n",
> > -       SCIMARK_VERSION, SCIMARK_COPYRIGHT)
> > +-- printf("Lua SciMark %s based on SciMark 2.0a. %s.\n\n",
> > +--        SCIMARK_VERSION, SCIMARK_COPYRIGHT)
> >   
> 
> I propose to move this to a comment with test description.

Removed this line. The top of the file already contains the
description:

===================================================================
diff --git a/perf/LuaJIT-benches/scimark-2010-12-20.lua b/perf/LuaJIT-benches/scimark-2010-12-20.lua
index 3fb627fa..4b80ffe2 100644
--- a/perf/LuaJIT-benches/scimark-2010-12-20.lua
+++ b/perf/LuaJIT-benches/scimark-2010-12-20.lua
@@ -351,9 +351,6 @@ local function measure(name, cycles, ...)
   return flops
 end
 
--- printf("Lua SciMark %s based on SciMark 2.0a. %s.\n\n",
---        SCIMARK_VERSION, SCIMARK_COPYRIGHT)
-
 while arg and arg[1] do
   local a = table.remove(arg, 1)
   if a == "noffi" then
===================================================================

> 
> Something like:
> 
> The test runs the Lua version of SciMark 2.0a, which is a benchmark for 
> scientific and numerical computing developed by programmers at the NIST 
> (National Institute of Standards and Technology). This test is made up 
> of Fast Foruier Transform, Jacobi Successive Over-relaxation, Monte 
> Carlo, Sparse Matrix Multiply, and dense LU matrix factorization benchmarks.
> 
> plus description of available test-specific options (noffi, small, etc) 
> or just

This options are kinda obvious for the usage. I suppose there is no need
to document them somehow, since the benchmark is run in the one default
configuration in the CI.

> 
> a command-line that will show usage: ./scimark-2010-12-20.lua help

It already has the following ouptut:

| Usage: scimark [noffi] [small|large] [BENCH params...]
|
| BENCH   small         large
| ---------------------------------------
| FFT     [1024]        [1048576]
| SOR     [100]         [1000]
| MC
| SPARSE  [1000, 5000]  [100000, 1000000]
| LU      [100]         [1000]

> 
> >   while arg and arg[1] do

<snipped>

-- 
Best regards,
Sergey Kaplun