Re: [Tarantool-patches] [PATCH luajit 5/7] ci: add workflow with disabled profilers

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

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

Hi, Sergey,

thanks for review!

On 18.02.2025 15:10, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, with a minor nits below.
>
> On 13.02.25, Sergey Bronnikov wrote:
>> Needed for the following commit.
>> ---
>>   .github/workflows/exotic-builds-testing.yml       |  6 +++++-
>>   test/tarantool-tests/CMakeLists.txt               |  4 ++++
>>   .../gh-5994-memprof-human-readable.test.lua       |  1 +
>>   .../profilers/misclib-memprof-lapi.test.lua       | 15 ++++++++-------
> I suggest to split this commit into 2 separate (both of them are LGTM
> though):
> 1) Introduce `LUAJIT_DISABLE_MEMPROF` flag in cmake and tests.
> 2) Add the corresponding exotic workflow.

Separated to commits:

  -   ci: add workflow with disabled profilers
  -   test: introduce flag LUAJIT_DISABLE_MEMPROF

>
>>   4 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml
>> index 374c879b..70b55423 100644
>> --- a/.github/workflows/exotic-builds-testing.yml
>> +++ b/.github/workflows/exotic-builds-testing.yml
>> @@ -34,7 +34,7 @@ jobs:
>>           BUILDTYPE: [Debug, Release]
>>           ARCH: [ARM64, x86_64]
>>           GC64: [ON, OFF]
>> -        FLAVOR: [checkhook, dualnum, dynamic, gdbjit, nojit, nounwind, tablebump]
>> +        FLAVOR: [checkhook, dualnum, dynamic, gdbjit, nojit, nounwind, tablebump, noprof]
> Please sort entries alphabetically.
> Here and below.
Fixed.
>
>>           include:
>>             - BUILDTYPE: Debug
>>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>> @@ -54,6 +54,8 @@ jobs:
>>               FLAVORFLAGS: -DLUAJIT_NO_UNWIND=ON
>>             - FLAVOR: tablebump
>>               FLAVORFLAGS: -DLUAJIT_ENABLE_TABLE_BUMP=ON
>> +          - FLAVOR: noprof
>> +            FLAVORFLAGS: -DLUAJIT_DISABLE_MEMPROF=ON -DLUAJIT_DISABLE_SYSPROF=ON
>>           exclude:
>>             - ARCH: ARM64
>>               GC64: OFF
>> @@ -72,6 +74,8 @@ jobs:
>>             # (`DASM_S_RANGE_I`).
>>             - FLAVOR: tablebump
>>               ARCH: ARM64
>> +          - FLAVOR: noprof
>> +            ARCH: ARM64
>>       runs-on: [self-hosted, regular, Linux, '${{ matrix.ARCH }}']
>>       name: >
>>         LuaJIT ${{ matrix.FLAVOR }}
>> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
>> index 9bacac88..a6d09dc7 100644
>> --- a/test/tarantool-tests/CMakeLists.txt
>> +++ b/test/tarantool-tests/CMakeLists.txt
> <snipped>
>
>> index f3041779..0ab9c57c 100644
>> --- a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
>> +++ b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
>> @@ -7,6 +7,7 @@ local test = tap.test('gh-5994-memprof-human-readable'):skipcond({
>>     ['No profile tools CLI option integration'] = _TARANTOOL,
>>     -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
>>     ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
>> +  ["Memprof is disabled"] = os.getenv('LUAJIT_DISABLE_MEMPROF'),
> Please use single quotes for this file (for the consistency).

Fixed, thanks.


>>   })
>>   
>>   local utils = require('utils')
>> diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
>> index ce41e4d5..33925c7f 100644
>> --- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
>> +++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
>> @@ -8,6 +8,7 @@ local test = tap.test("misc-memprof-lapi"):skipcond({
>>                                                  jit.arch ~= "x64",
>>     -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
>>     ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
>> +  ["Memprof is disabled"] = os.getenv('LUAJIT_DISABLE_MEMPROF'),
> Please use dougle quotes for this file (for the consistency).
Fixed.
>
>>   })
>>   
> <snipped>
>
>> -- 
>> 2.34.1
>>

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