Re: [Tarantool-patches] [PATCH luajit][v1] sysprof: allow calling sysprof.report before stopping

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

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

Hello, Sergey,

On 6/4/25 16:13, Sergey Kaplun wrote:


<snipped>

>>>> +    interval = 1,
>>>> +    path = "/dev/null",
>>>> +}
>>>> +test:is(res, true, "res is correct")
>>>> +test:is(err, nil, "no error")
>>>> +test:is(errno, nil, "no errno")
>>> I suppose that 2 last checks are excess. The first one is enough to be
>>> sure that the profiler is started. Also, we may use `assert()` here
>>> instead of `test:is()` check, since we don't want to _test_ the starting of
>>> the profiler only to _assert_ that the sysprof has been started.
>>>
>> last two checks were removed andtest:is() replaced with assert()
> I would rather use
> | assert(misc.sysprof.start({...})
> and
> | assert(misc.sysprof.stop())
> instead, for simplicity and to avoid the irrelevant local variables.
>
> Feel free to ignore.
Fixed.
>>>> +
>>>> +local report = misc.sysprof.report()
>>>> +test:ok(report.samples == 0, "total number of samples is non-zero")
>>> I'm not sure that this will always be true (for example, in coverage
>>> workflow). I suggest increasing the interval dramatically to avoid false
>>> positives here.
>> Updated.
> Side note: Checking the non-0 samples for default payload instead.
>
>>>> +misc.sysprof.stop()
>>>> +
>>> <snipped>
>>>
>>>> 2.43.0
>>>>

[-- Attachment #2: Type: text/html, Size: 3030 bytes --]