[Tarantool-patches] [PATCH luajit] test: disable jit tests at M1 for Tarantool
sergos
sergos at tarantool.org
Tue Aug 23 13:23:22 MSK 2022
Hi, Sergey!
Thanks for the patch! See some comments below.
> On 17 Aug 2022, at 12:24, Sergey Kaplun <skaplun at tarantool.org> wrote:
>
> During running JIT-related tests on Tarantool the trace assembling
> occasionally fails with the error "failed to allocate mcode memory"
> because there are no suitable addresses available.
I would rephrase to be more on the spot:
Trace assembling can fail with an error of mcode memory allocation due to
the branch offset limitation of +-32Mb on an ARM platform. For the time we’re
elaborating a correct fix we disable tests proven to be flaky because of this
reason.
I would propose to separate the refactor commit as a preparational, since
sysprof is out of this patch context.
>
> This patch disables those tests for Tarantool in a similar way as it is
> done for FreeBSD. Also, this patch refactors `utils.skipcond()` a bit
> to make it call readable with multiple conditions/reasons pairs.
>
> Relates to tarantool/tarantool#7571
> ---
>
> Tarantool PR: https://github.com/tarantool/tarantool/pull/7578
> Issue: https://github.com/tarantool/tarantool/issues/7571
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-7571-disable-jit-tests-m1-full-ci
>
> Side note: Looks like adding the trampoline, as Sergos suggested is the
> correct solution of this issue. For now just disable JIT related tests
> only for Tarantool.
I believe we can pre-allocate a 64Mb mcode buffer ahead of regular Tarantool
memory allocation activities, since it can’t be bigger on ARM anyways?
Trampoline will remove those limitations although.
>
> .../gh-4199-gc64-fuse.test.lua | 2 +-
> .../gh-4427-ffi-sandwich.test.lua | 7 ++++--
> .../gh-5813-resolving-of-c-symbols.test.lua | 10 ++++-----
> ...4-add-proto-trace-sysprof-default.test.lua | 12 +++++-----
> .../lj-430-maxirconst.test.lua | 8 +++++--
> ...lj-672-cdata-allocation-recording.test.lua | 8 +++++--
> .../lj-flush-on-trace.test.lua | 7 ++++--
> .../misclib-getmetrics-capi.test.lua | 8 +++++--
> .../misclib-getmetrics-lapi.test.lua | 8 +++++--
> .../misclib-memprof-lapi.test.lua | 17 +++++++-------
> .../misclib-sysprof-capi.test.lua | 11 +++++-----
> .../misclib-sysprof-lapi.test.lua | 11 +++++-----
> test/tarantool-tests/utils.lua | 22 ++++++++++++++-----
> 13 files changed, 83 insertions(+), 48 deletions(-)
>
> diff --git a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
> index 4d69250f..04950fd2 100644
> --- a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
> +++ b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
> @@ -1,6 +1,6 @@
> -- The test is GC64 only.
> local ffi = require('ffi')
> -require('utils').skipcond(not ffi.abi('gc64'), 'test is GC64 only')
> +require('utils').skipcond({not ffi.abi('gc64'), 'test is GC64 only'})
>
> local tap = require('tap')
> local test = tap.test('gh-4199-gc64-fuse')
> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> index dd02130c..fea1d181 100644
> --- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> @@ -1,7 +1,10 @@
> local utils = require('utils')
>
> --- Disabled on *BSD due to #4819.
> -utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +utils.skipcond(
> + {jit.os == 'BSD', 'Disabled due to #4819'},
> + --luacheck: no global
> + {_TARANTOOL and utils.is_M1(), 'Disabled due to #7571'}
> +)
>
> utils.selfrun(arg, {
> {
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> index d589dddf..ff0c48e6 100644
> --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> @@ -1,8 +1,8 @@
> --- Memprof is implemented for x86 and x64 architectures only.
> -require("utils").skipcond(
> - jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
> - jit.arch.." architecture or "..jit.os..
> - " OS is NIY for memprof c symbols resolving"
> +local utils = require("utils")
> +utils.skipcond(
> + {jit.arch ~= "x86" and jit.arch ~= "x64", jit.arch ..
> + " architecture is NIY for memprof c symbols resolving"},
> + {jit.os ~= "Linux", jit.os .. " OS is NIY for memprof c symbols resolving"}
> )
>
> local tap = require("tap")
> diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> index 15bd0a8b..332d4711 100644
> --- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> +++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> @@ -1,9 +1,9 @@
> --- Sysprof is implemented for x86 and x64 architectures only.
> -require('utils').skipcond(
> - jit.arch ~= 'x86' and jit.arch ~= 'x64' or jit.os ~= 'Linux'
> - or require('ffi').abi('gc64'),
> - jit.arch..' architecture or '..jit.os..
> - ' OS is NIY for sysprof'
> +local utils = require('utils')
> +utils.skipcond(
> + {jit.arch ~= 'x86' and jit.arch ~= 'x64', jit.arch ..
> + ' architecture is NIY for sysprof'},
> + {jit.os ~= 'Linux', jit.os .. ' OS is NIY for sysprof'},
> + {require('ffi').abi('gc64'), 'GC64 mode is NIY for sysprof'}
> )
>
> local tap = require('tap')
> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> index cd3587a8..b95aacbd 100644
> --- a/test/tarantool-tests/lj-430-maxirconst.test.lua
> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
> @@ -3,8 +3,12 @@
> jit.off()
> jit.flush()
>
> --- Disabled on *BSD due to #4819.
> -require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +local utils = require('utils')
> +utils.skipcond(
> + {jit.os == 'BSD', 'Disabled due to #4819'},
> + --luacheck: no global
> + {_TARANTOOL and utils.is_M1(), 'Disabled due to #7571'}
> +)
>
> local tap = require('tap')
> local traceinfo = require('jit.util').traceinfo
> diff --git a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
> index 1dc741d8..c72688d6 100644
> --- a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
> +++ b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
> @@ -1,5 +1,9 @@
> --- Disabled on *BSD due to #4819.
> -require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +local utils = require('utils')
> +utils.skipcond(
> + {jit.os == 'BSD', 'Disabled due to #4819'},
> + --luacheck: no global
> + {_TARANTOOL and utils.is_M1(), 'Disabled due to #7571'}
> +)
>
> local ffi = require('ffi')
> local traceinfo = require('jit.util').traceinfo
> diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> index c46b93f0..006c61f0 100644
> --- a/test/tarantool-tests/lj-flush-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
> @@ -1,7 +1,10 @@
> local utils = require('utils')
>
> --- Disabled on *BSD due to #4819.
> -utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +utils.skipcond(
> + {jit.os == 'BSD', 'Disabled due to #4819'},
> + --luacheck: no global
> + {_TARANTOOL and utils.is_M1(), 'Disabled due to #7571'}
> +)
>
> utils.selfrun(arg, {
> {
> diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> index 0cbc6cae..f747500b 100644
> --- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> +++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> @@ -1,5 +1,9 @@
> --- Disabled on *BSD due to #4819.
> -require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +local utils = require('utils')
> +utils.skipcond(
> + {jit.os == 'BSD', 'Disabled due to #4819'},
> + --luacheck: no global
> + {_TARANTOOL and utils.is_M1(), 'Disabled due to #7571'}
> +)
>
> local path = arg[0]:gsub('%.test%.lua', '')
> local suffix = package.cpath:match('?.(%a+);')
> diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> index 222e7d01..50a4fce3 100644
> --- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> @@ -2,8 +2,12 @@
> -- Major portions taken verbatim or adapted from the LuaVela testing suite.
> -- Copyright (C) 2015-2019 IPONWEB Ltd.
>
> --- Disabled on *BSD due to #4819.
> -require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +local utils = require('utils')
> +utils.skipcond(
> + {jit.os == 'BSD', 'Disabled due to #4819'},
> + --luacheck: no global
> + {_TARANTOOL and utils.is_M1(), 'Disabled due to #7571'}
> +)
>
> local tap = require('tap')
>
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index a11f0be1..66fb2108 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -1,7 +1,8 @@
> -- Memprof is implemented for x86 and x64 architectures only.
> -require("utils").skipcond(
> - jit.arch ~= "x86" and jit.arch ~= "x64",
> - jit.arch.." architecture is NIY for memprof"
> +local utils = require('utils')
> +utils.skipcond(
> + {jit.arch ~= "x86" and jit.arch ~= "x64",
> + jit.arch .. " architecture is NIY for memprof"}
> )
>
> local tap = require("tap")
> @@ -189,9 +190,9 @@ test:test("output", function(subtest)
> -- one is the number of allocations. 1 event - alocation of
> -- table by itself + 1 allocation of array part as far it is
> -- bigger than LJ_MAX_COLOSIZE (16).
> - subtest:ok(check_alloc_report(alloc, { line = 35, linedefined = 33 }, 2))
> + subtest:ok(check_alloc_report(alloc, { line = 36, linedefined = 34 }, 2))
> -- 20 strings allocations.
> - subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 20))
> + subtest:ok(check_alloc_report(alloc, { line = 41, linedefined = 34 }, 20))
>
> -- Collect all previous allocated objects.
> subtest:ok(free.INTERNAL.num == 22)
> @@ -291,10 +292,10 @@ test:test("jit-output", function(subtest)
> -- 10 allocations in interpreter mode, 1 allocation for a trace
> -- recording and assembling and next 9 allocations will happen
> -- while running the trace.
> - subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 11))
> - subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 38 }, 9))
> + subtest:ok(check_alloc_report(alloc, { line = 41, linedefined = 34 }, 11))
> + subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 39 }, 9))
> -- See same checks with jit.off().
> - subtest:ok(check_alloc_report(alloc, { line = 35, linedefined = 33 }, 2))
> + subtest:ok(check_alloc_report(alloc, { line = 36, linedefined = 34 }, 2))
>
> -- Restore default JIT settings.
> jit.opt.start(unpack(jit_opt_default))
> diff --git a/test/tarantool-tests/misclib-sysprof-capi.test.lua b/test/tarantool-tests/misclib-sysprof-capi.test.lua
> index afb99cf2..32f00ffd 100644
> --- a/test/tarantool-tests/misclib-sysprof-capi.test.lua
> +++ b/test/tarantool-tests/misclib-sysprof-capi.test.lua
> @@ -1,10 +1,11 @@
> -- Sysprof is implemented for x86 and x64 architectures only.
> local ffi = require("ffi")
> -require("utils").skipcond(
> - jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux"
> - or ffi.abi("gc64"),
> - jit.arch.." architecture or "..jit.os..
> - " OS is NIY for sysprof"
> +local utils = require("utils")
> +utils.skipcond(
> + {jit.arch ~= "x86" and jit.arch ~= "x64", jit.arch ..
> + " architecture is NIY for sysprof"},
> + {jit.os ~= "Linux", jit.os .. " OS is NIY for sysprof"},
> + {ffi.abi("gc64"), "GC64 mode is NIY for sysprof"}
> )
>
> local testsysprof = require("testsysprof")
> diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> index 9e0a8a77..e9ae182e 100644
> --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> @@ -1,10 +1,11 @@
> -- Sysprof is implemented for x86 and x64 architectures only.
> local ffi = require("ffi")
> -require("utils").skipcond(
> - jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux"
> - or ffi.abi("gc64"),
> - jit.arch.." architecture or "..jit.os..
> - " OS is NIY for sysprof"
> +local utils = require("utils")
> +utils.skipcond(
> + {jit.arch ~= "x86" and jit.arch ~= "x64", jit.arch ..
> + " architecture is NIY for sysprof"},
> + {jit.os ~= "Linux", jit.os .. " OS is NIY for sysprof"},
> + {ffi.abi("gc64"), "GC64 mode is NIY for sysprof"}
> )
>
> local tap = require("tap")
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index a1906498..d8412e03 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -20,6 +20,10 @@ ffi.cdef([[
> } GCHeader;
> ]])
>
> +function M.is_M1()
> + return jit.os == 'OSX' and jit.arch == 'arm64'
> +end
> +
> function M.gcisblack(obj)
> local objtype = type(obj)
> local address = objtype == 'string'
> @@ -95,12 +99,18 @@ function M.selfrun(arg, checks)
> os.exit(test:check() and 0 or 1)
> end
>
> -function M.skipcond(condition, message)
> - if not condition then return end
> - local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))
> - test:plan(1)
> - test:skip(message)
> - os.exit(test:check() and 0 or 1)
> +function M.skipcond(...)
Rather crucial change to the interface. Is it documented in some way?
Should it? Some in-line, or a readme? Nottin?..
> + local conditions = {...}
> + local ncond = #conditions
> + for i = 1, ncond do
> + local condition, message = conditions[i][1], conditions[i][2]
> + if condition then
> + local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))
> + test:plan(1)
> + test:skip(message)
> + os.exit(test:check() and 0 or 1)
> + end
> + end
> end
>
> function M.hasbc(f, bytecode)
> --
> 2.34.1
>
More information about the Tarantool-patches
mailing list