Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: "Sergey Kaplun" <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches]  [PATCH v1 luajit 4/5] test: rewrite misclib-getmetrics-capi test in C
Date: Wed, 22 Mar 2023 03:07:25 +0300	[thread overview]
Message-ID: <1679443645.145929878@f300.i.mail.ru> (raw)
In-Reply-To: <e907f9c223d0ffb9bdb9cd7c6dd3be55fb40e40f.1678895861.git.skaplun@tarantool.org>

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


Hi!
Thanks for the patch!
Please consider my comments below.
 
> 
>>This patch rewrites the aforementioned test with usage libtest recently
>Typo: s/with usage libtest/with the usage of libtest,/
>>introduced. Since we still stand in need of a Lua helper script for
>>generation of traces, the original test file is reworked as a standalone
>>script, which returns the table with helper functions. Each helper
>>function is named the same as the test where it will be used. Now single
>>quotes are used according our Lua code style.
>Typo: s/according our/according to our/
>>
>>In C part all asserts from glibc are replaced with the corresponding
>>assert_{cond} from the libtest. Now tests return `TEST_EXIT_SUCCESS` at
>>the finish. Also, stack check for the amount of return results from
>Typo s:/stack check/the stack check/
>>helper function is slightly changed, since there is one more stack slot
>Typo: s/helper/the helper/
>>in use (table with these functions). `snap_restores()` C function
>>duplicates 4 times for each subtest. Common helper
>>`check_snap_restores()` is used for each of them. Each error throwing is
>>replaced with `bail_out()` call.
>>
>>NB: `lua_pop()` to clear the Lua stack after call should be done before
>Typo: s/after call/after a call/
>>any possible assertion, which exit from test leaving the stack
>Typo: s/which exit from test/which would exit from the test/
>>uncleaned.
>>
>>All skipconds use macros defined in <lj_arch.h>, so it is included in
>>the test. As far as this test initializes LuaJIT VM manually, there is
>Typo: s/LuaJIT/the LuaJIT
>>no need to check `jit.status()` result; check `LJ_HASJIT` is enough.
>Typo: s/check/checking/
>>Now, only JIT-related tests are skipped, when compiled without JIT.
>>Nevertheless, all tests are skipped for *BSD arches.
>>
>>Also, this patch sets the new CMake variable named `LUAJIT_LIBRARY`
>>equals to `LUAJIT_LIB` in `PARENT_SCOPE` to be used in tarantool-c-tests
>Typo: s/equals/equal/
>>linking.
>>
>>Also, .c_test suffix is added to the <.gitignore>.
>>
>>Part of tarantool/tarantool#7900
>>---
>> .gitignore | 1 +
>> src/CMakeLists.txt | 2 +
>> test/tarantool-c-tests/CMakeLists.txt | 32 +-
>> .../misclib-getmetrics-capi-script.lua} | 82 ++---
>> .../misclib-getmetrics-capi.test.c | 341 ++++++++++++++++++
>> test/tarantool-tests/CMakeLists.txt | 1 -
>> .../misclib-getmetrics-capi/CMakeLists.txt | 1 -
>> .../misclib-getmetrics-capi/testgetmetrics.c | 270 --------------
>> 8 files changed, 409 insertions(+), 321 deletions(-)
>> rename test/{tarantool-tests/misclib-getmetrics-capi.test.lua => tarantool-c-tests/misclib-getmetrics-capi-script.lua} (68%)
>> create mode 100644 test/tarantool-c-tests/misclib-getmetrics-capi.test.c
>> delete mode 100644 test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt
>> delete mode 100644 test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c
>>
>>diff --git a/.gitignore b/.gitignore
>>index b7908aee..dc5ea5fc 100644
>>--- a/.gitignore
>>+++ b/.gitignore
>>@@ -24,3 +24,4 @@ install_manifest.txt
>> luajit-parse-memprof
>> luajit-parse-sysprof
>> luajit.pc
>>+*.c_test
>>diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>>index dffc0a4d..58d83330 100644
>>--- a/src/CMakeLists.txt
>>+++ b/src/CMakeLists.txt
>>@@ -325,6 +325,8 @@ if(NOT BUILDMODE STREQUAL "dynamic")
>>   set(LUAJIT_BIN luajit_static)
>>   set(LUAJIT_LIB libluajit_static)
>> endif()
>>+# Need for test linking, so PARENT_SCOPE option is used.
>Typo: s/Need for test/Needed for the test/
>Typo: s/so PARENT_SCOPE/so the PARENT_SCOPE/
>>+set(LUAJIT_LIBRARY ${LUAJIT_LIB} PARENT_SCOPE)
>> set(LIBLUAJIT_DEPS ${LIBLUAJIT_STATIC_DEPS} ${LIBLUAJIT_SHARED_DEPS})
>> 
>> add_executable(${LUAJIT_BIN} EXCLUDE_FROM_ALL ${CLI_SOURCES})
>>diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
>>index 5ebea441..96beef1a 100644
>>--- a/test/tarantool-c-tests/CMakeLists.txt
>>+++ b/test/tarantool-c-tests/CMakeLists.txt
>>@@ -22,13 +22,37 @@ set_target_properties(libtest PROPERTIES
>>   LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
>> )
>> 
>>-# XXX: For now, just build libtest. The tests to be depended on
>>-# will be added at the next commit.
>>+# TARGET_C_FLAGS is required here to be sure that headers like
>>+# lj_arch.h in compiled test is consistent with the LuaJIT library
>Typo: s/is consistent/are consistent/
>>+# to link.
>>+AppendFlags(TESTS_C_FLAGS ${TARGET_C_FLAGS})
>>+
>>+set(CTEST_SRC_SUFFIX ".test.c")
>>+file(GLOB tests "${CMAKE_CURRENT_SOURCE_DIR}/*${CTEST_SRC_SUFFIX}")
>>+foreach(test_source ${tests})
>>+ string(REGEX REPLACE ".+/([^/]+)${CTEST_SRC_SUFFIX}" "\\1" exe ${test_source})
>>+ add_executable(${exe} EXCLUDE_FROM_ALL ${test_source})
>>+ target_include_directories(${exe} PRIVATE
>>+ ${CMAKE_CURRENT_SOURCE_DIR}
>>+ ${LUAJIT_SOURCE_DIR}
>>+ )
>>+ set_target_properties(${exe} PROPERTIES
>>+ # `__FILE__` macro may not represnt absolute path to the
>>+ # source file, depening on cmake version or
>>+ # -fmacro-prefix-map flag value.
>>+ # Use custom macro.
>>+ COMPILE_FLAGS "${TESTS_C_FLAGS} -D__ABS_FILENAME__='\"${test_source}\"'"
>>+ OUTPUT_NAME "${exe}${C_TEST_SUFFIX}"
>>+ RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
>>+ )
>>+ target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY})
>>+ LIST(APPEND TESTS_COMPILED ${exe})
>>+endforeach()
>>+
>> add_custom_target(tarantool-c-tests
>>- DEPENDS libluajit libtest
>>+ DEPENDS libluajit libtest ${TESTS_COMPILED}
>> )
>> 
>>-# XXX: For now, run 0 tests. Just verify that libtest was build.
>> add_custom_command(TARGET tarantool-c-tests
>>   COMMENT "Running Tarantool C tests"
>>   COMMAND
>>diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-c-tests/misclib-getmetrics-capi-script.lua
>>similarity index 68%
>>rename from test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>rename to test/tarantool-c-tests/misclib-getmetrics-capi-script.lua
>>index 654e5545..8fab485a 100644
>>--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>+++ b/test/tarantool-c-tests/misclib-getmetrics-capi-script.lua
>>@@ -1,32 +1,25 @@
>>-local tap = require('tap')
>>-local test = tap.test("clib-misc-getmetrics"):skipcond({
>>- ['Test requires JIT enabled'] = not jit.status(),
>>- ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>-})
>>+local ffi = require('ffi')
>> 
>>-test:plan(11)
>>+-- Auxiliary script to provide Lua functions to be used in tests
>>+-- for `getmetrics()` C API inside the test
>>+-- <misclib-getmetrics-capi.test.c>.
>>+local M = {}
>> 
>>-local path = arg[0]:gsub('%.test%.lua', '')
>>-local suffix = package.cpath:match('?.(%a+);')
>>-package.cpath = ('%s/?.%s;'):format(path, suffix)..package.cpath
>>+-- XXX: Max nins is limited by max IRRef, that equals to
>>+-- REF_DROP - REF_BIAS. Unfortunately, these constants are not
>>+-- provided to Lua space, so we ought to make some math:
>Maybe, it’s better to say `exposed` instead of `provided`. Feel free to ignore.
>>+-- * REF_DROP = 0xffff
>>+-- * REF_BIAS = 0x8000
>Path to file with their definition would be useful.
>>+local MAXNINS = 0xffff - 0x8000
>> 
>>-local MAXNINS = require('utils').const.maxnins
>> local jit_opt_default = {
>>     3, -- level
>>- "hotloop=56",
>>- "hotexit=10",
>>- "minstitch=0",
>>+ 'hotloop=56',
>>+ 'hotexit=10',
>>+ 'minstitch=0',
>> }
>> 
>>-local testgetmetrics = require("testgetmetrics")
>>-
>>-test:ok(testgetmetrics.base())
>>-test:ok(testgetmetrics.gc_allocated_freed())
>>-test:ok(testgetmetrics.gc_steps())
>>-
>>-test:ok(testgetmetrics.objcount(function(iterations)
>>- local ffi = require("ffi")
>>-
>>+M.objcount = function(iterations)
>>     jit.opt.start(0)
>> 
>>     local placeholder = {
>>@@ -51,35 +44,34 @@ test:ok(testgetmetrics.objcount(function(iterations)
>> 
>>     for _ = 1, iterations do
>>         -- Check counting of VLA/VLS/aligned cdata.
>>- table.insert(placeholder.cdata, ffi.new("char[?]", 4))
>>+ table.insert(placeholder.cdata, ffi.new('char[?]', 4))
>>     end
>> 
>>     for _ = 1, iterations do
>>         -- Check counting of non-VLA/VLS/aligned cdata.
>>- table.insert(placeholder.cdata, ffi.new("uint64_t", _))
>>+ table.insert(placeholder.cdata, ffi.new('uint64_t', _))
>>     end
>> 
>>     placeholder = nil -- luacheck: no unused
>>     -- Restore default jit settings.
>>     jit.opt.start(unpack(jit_opt_default))
>>-end))
>>+end
>> 
>>-test:ok(testgetmetrics.objcount_cdata_decrement(function()
>>+M.objcount_cdata_decrement = function()
>>     -- gc_cdatanum decrement test.
>>     -- See  https://github.com/tarantool/tarantool/issues/5820 .
>>- local ffi = require("ffi")
>>     local function nop() end
>>- ffi.gc(ffi.cast("void *", 0), nop)
>>+ ffi.gc(ffi.cast('void *', 0), nop)
>>     -- Does not collect the cdata, but resurrects the object and
>>     -- removes LJ_GC_CDATA_FIN flag.
>>     collectgarbage()
>>     -- Collects the cdata.
>>     collectgarbage()
>>-end))
>>+end
>> 
>> -- Compiled loop with a direct exit to the interpreter.
>>-test:ok(testgetmetrics.snap_restores(function()
>>- jit.opt.start(0, "hotloop=1")
>>+M.snap_restores_direct_exit = function()
>>+ jit.opt.start(0, 'hotloop=1')
>> 
>>     local sum = 0
>>     for i = 1, 20 do
>>@@ -91,11 +83,11 @@ test:ok(testgetmetrics.snap_restores(function()
>> 
>>     -- A single snapshot restoration happened on loop finish.
>>     return 1
>>-end))
>>+end
>> 
>> -- Compiled loop with a side exit which does not get compiled.
>>-test:ok(testgetmetrics.snap_restores(function()
>>- jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(MAXNINS))
>>+M.snap_restores_side_exit_not_compiled = function()
>>+ jit.opt.start(0, 'hotloop=1', 'hotexit=2', ('minstitch=%d'):format(MAXNINS))
>> 
>>     local function foo(i)
>>         -- math.fmod is not yet compiled!
>>@@ -112,13 +104,13 @@ test:ok(testgetmetrics.snap_restores(function()
>> 
>>     -- Side exits from the root trace could not get compiled.
>>     return 5
>>-end))
>>+end
>> 
>> -- Compiled loop with a side exit which gets compiled.
>>-test:ok(testgetmetrics.snap_restores(function()
>>+M.snap_restores_side_exit_compiled = function()
>>     -- Optimization level is important here as `loop` optimization
>>     -- may unroll the loop body and insert +1 side exit.
>>- jit.opt.start(0, "hotloop=1", "hotexit=5")
>>+ jit.opt.start(0, 'hotloop=1', 'hotexit=5')
>> 
>>     local function foo(i)
>>         return i <= 10 and i or tostring(i)
>>@@ -136,13 +128,13 @@ test:ok(testgetmetrics.snap_restores(function()
>>     -- and compiled
>>     -- 1 side exit on loop end
>>     return 6
>>-end))
>>+end
>> 
>> -- Compiled scalar trace with a direct exit to the interpreter.
>>-test:ok(testgetmetrics.snap_restores(function()
>>+M.snap_restores_direct_exit_scalar = function()
>>     -- For calls it will be 2 * hotloop (see lj_dispatch.{c,h}
>>     -- and hotcall@vm_*.dasc).
>>- jit.opt.start(3, "hotloop=2", "hotexit=3")
>>+ jit.opt.start(3, 'hotloop=2', 'hotexit=3')
>> 
>>     local function foo(i)
>>         return i <= 15 and i or tostring(i)
>>@@ -167,15 +159,15 @@ test:ok(testgetmetrics.snap_restores(function()
>>     jit.opt.start(unpack(jit_opt_default))
>> 
>>     return 2
>>-end))
>>+end
>> 
>>-test:ok(testgetmetrics.strhash())
>>-
>>-test:ok(testgetmetrics.tracenum_base(function()
>>+M.tracenum_base = function()
>>     local sum = 0
>>     for i = 1, 200 do
>>         sum = sum + i
>>     end
>>     -- Compiled only 1 loop as new trace.
>>     return 1
>>-end))
>>+end
>>+
>>+return M
>>diff --git a/test/tarantool-c-tests/misclib-getmetrics-capi.test.c b/test/tarantool-c-tests/misclib-getmetrics-capi.test.c
>>new file mode 100644
>>index 00000000..a1bafbe6
>>--- /dev/null
>>+++ b/test/tarantool-c-tests/misclib-getmetrics-capi.test.c
>>@@ -0,0 +1,341 @@
>>+#include "lua.h"
>>+#include "luajit.h"
>>+#include "lauxlib.h"
>>+#include "lmisclib.h"
>>+
>>+#include "test.h"
>>+#include "utils.h"
>>+
>>+/* Need for skipcond for BSD and JIT. */
>>+#include "lj_arch.h"
>Still don’t like the fact that we need to do that.
>>+
>>+static int base(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ struct luam_Metrics metrics;
>>+ luaM_metrics(L, &metrics);
>>+
>>+ /*
>>+ * Just check structure format, not values that fields
>>+ * contain.
>>+ */
>>+ (void)metrics.strhash_hit;
>>+ (void)metrics.strhash_miss;
>>+
>>+ (void)metrics.gc_strnum;
>>+ (void)metrics.gc_tabnum;
>>+ (void)metrics.gc_udatanum;
>>+ (void)metrics.gc_cdatanum;
>>+
>>+ (void)metrics.gc_total;
>>+ (void)metrics.gc_freed;
>>+ (void)metrics.gc_allocated;
>>+
>>+ (void)metrics.gc_steps_pause;
>>+ (void)metrics.gc_steps_propagate;
>>+ (void)metrics.gc_steps_atomic;
>>+ (void)metrics.gc_steps_sweepstring;
>>+ (void)metrics.gc_steps_sweep;
>>+ (void)metrics.gc_steps_finalize;
>>+
>>+ (void)metrics.jit_snap_restore;
>>+ (void)metrics.jit_trace_abort;
>>+ (void)metrics.jit_mcode_size;
>>+ (void)metrics.jit_trace_num;
>>+
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int gc_allocated_freed(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ struct luam_Metrics oldm, newm;
>>+ /* Force up garbage collect all dead objects. */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+
>>+ luaM_metrics(L, &oldm);
>>+ /* Simple garbage generation. */
>>+ if (luaL_dostring(L, "local i = 0 for j = 1, 10 do i = i + j end"))
>>+ bail_out("failed to translate Lua code snippet");
>Why `bail_out` and not an assertion? Here and below.
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ luaM_metrics(L, &newm);
>>+ assert_true(newm.gc_allocated - oldm.gc_allocated > 0);
>>+ assert_true(newm.gc_freed - oldm.gc_freed > 0);
>>+
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int gc_steps(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ struct luam_Metrics oldm, newm;
>>+ /*
>>+ * Some garbage has already happened before the next line,
>>+ * i.e. during frontend processing Lua test chunk.
>>+ * Let's put a full garbage collection cycle on top
>>+ * of that, and confirm that non-null values are reported
>>+ * (we are not yet interested in actual numbers):
>>+ */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+
>>+ luaM_metrics(L, &oldm);
>>+ assert_true(oldm.gc_steps_pause > 0);
>>+ assert_true(oldm.gc_steps_propagate > 0);
>>+ assert_true(oldm.gc_steps_atomic > 0);
>>+ assert_true(oldm.gc_steps_sweepstring > 0);
>>+ assert_true(oldm.gc_steps_sweep > 0);
>>+ /* Nothing to finalize, skipped. */
>>+ assert_true(oldm.gc_steps_finalize == 0);
>>+
>>+ /*
>>+ * As long as we don't create new Lua objects
>>+ * consequent call should return the same values:
>>+ */
>>+ luaM_metrics(L, &newm);
>>+ assert_sizet_equal(newm.gc_steps_pause, oldm.gc_steps_pause);
>>+ assert_sizet_equal(newm.gc_steps_propagate, oldm.gc_steps_propagate);
>>+ assert_sizet_equal(newm.gc_steps_atomic, oldm.gc_steps_atomic);
>>+ assert_sizet_equal(newm.gc_steps_sweepstring,
>>+ oldm.gc_steps_sweepstring);
>>+ assert_sizet_equal(newm.gc_steps_sweep, oldm.gc_steps_sweep);
>>+ /* Nothing to finalize, skipped. */
>>+ assert_true(newm.gc_steps_finalize == 0);
>>+ oldm = newm;
>>+
>>+ /*
>>+ * Now the last phase: run full GC once and make sure that
>>+ * everything is being reported as expected:
>>+ */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ luaM_metrics(L, &newm);
>>+ assert_true(newm.gc_steps_pause - oldm.gc_steps_pause == 1);
>>+ assert_true(newm.gc_steps_propagate - oldm.gc_steps_propagate >= 1);
>>+ assert_true(newm.gc_steps_atomic - oldm.gc_steps_atomic == 1);
>>+ assert_true(newm.gc_steps_sweepstring - oldm.gc_steps_sweepstring >= 1);
>>+ assert_true(newm.gc_steps_sweep - oldm.gc_steps_sweep >= 1);
>>+ /* Nothing to finalize, skipped. */
>>+ assert_true(newm.gc_steps_finalize == 0);
>>+ oldm = newm;
>>+
>>+ /*
>>+ * Now let's run three GC cycles to ensure that
>>+ * increment was not a lucky coincidence.
>>+ */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ luaM_metrics(L, &newm);
>>+ assert_true(newm.gc_steps_pause - oldm.gc_steps_pause == 3);
>>+ assert_true(newm.gc_steps_propagate - oldm.gc_steps_propagate >= 3);
>>+ assert_true(newm.gc_steps_atomic - oldm.gc_steps_atomic == 3);
>>+ assert_true(newm.gc_steps_sweepstring - oldm.gc_steps_sweepstring >= 3);
>>+ assert_true(newm.gc_steps_sweep - oldm.gc_steps_sweep >= 3);
>>+ /* Nothing to finalize, skipped. */
>>+ assert_true(newm.gc_steps_finalize == 0);
>>+
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int objcount(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ struct luam_Metrics oldm, newm;
>>+ if (!LJ_HASJIT)
>>+ skip("Test requires JIT enabled");
>>+
>>+ utils_get_aux_lfunc(L);
>>+
>>+ /* Force up garbage collect all dead objects. */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+
>>+ luaM_metrics(L, &oldm);
>>+ /* Generate garbage. Argument is iterations amount. */
>>+ lua_pushnumber(L, 1000);
>>+ lua_call(L, 1, 0);
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ luaM_metrics(L, &newm);
>>+ assert_sizet_equal(newm.gc_strnum, oldm.gc_strnum);
>>+ assert_sizet_equal(newm.gc_tabnum, oldm.gc_tabnum);
>>+ assert_sizet_equal(newm.gc_udatanum, oldm.gc_udatanum);
>>+ assert_sizet_equal(newm.gc_cdatanum, oldm.gc_cdatanum);
>>+
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int objcount_cdata_decrement(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ /*
>>+ * cdata decrement test.
>>+ * See  https://github.com/tarantool/tarantool/issues/5820 .
>>+ */
>>+ struct luam_Metrics oldm, newm;
>>+ utils_get_aux_lfunc(L);
>>+
>>+ /* Force up garbage collect all dead objects. */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+
>>+ luaM_metrics(L, &oldm);
>>+ /*
>>+ * The function generates and collects cdata with
>>+ * LJ_GC_CDATA_FIN flag.
>>+ */
>>+ lua_call(L, 0, 0);
>>+ luaM_metrics(L, &newm);
>>+ assert_sizet_equal(newm.gc_cdatanum, oldm.gc_cdatanum);
>>+
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+/*
>>+ * Get function to call to generate the corresponding snapshot
>>+ * restores on top of the Lua stack. Function returns the amount
>>+ * of snapshot restorations expected.
>>+ * Clear stack after call.
>>+ */
>>+static void check_snap_restores(lua_State *L)
>>+{
>>+ struct luam_Metrics oldm, newm;
>>+ luaM_metrics(L, &oldm);
>>+ /* Generate snapshots. */
>>+ lua_call(L, 0, 1);
>>+ int n = lua_gettop(L);
>>+ /*
>>+ * The first value is the table with functions,
>>+ * the second is number of snapshot restores.
>>+ */
>>+ if (n != 2 || !lua_isnumber(L, -1))
>>+ bail_out("incorrect return value: 1 number is required");
>>+ size_t snap_restores = lua_tonumber(L, -1);
>>+ luaM_metrics(L, &newm);
>>+ /*
>>+ * Remove `snap_restores` from stack.
>>+ * Must be done before potiential assert and exit from
>>+ * the test.
>>+ */
>>+ lua_pop(L, 1);
>>+ assert_true(newm.jit_snap_restore - oldm.jit_snap_restore
>>+ == snap_restores);
>>+}
>>+
>>+static int snap_restores_direct_exit(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ if (!LJ_HASJIT)
>>+ skip("Test requires JIT enabled");
>>+ utils_get_aux_lfunc(L);
>>+ check_snap_restores(L);
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int snap_restores_direct_exit_scalar(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ if (!LJ_HASJIT)
>>+ skip("Test requires JIT enabled");
>>+ utils_get_aux_lfunc(L);
>>+ check_snap_restores(L);
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int snap_restores_side_exit_compiled(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ if (!LJ_HASJIT)
>>+ skip("Test requires JIT enabled");
>>+ utils_get_aux_lfunc(L);
>>+ check_snap_restores(L);
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int snap_restores_side_exit_not_compiled(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ if (!LJ_HASJIT)
>>+ skip("Test requires JIT enabled");
>>+ utils_get_aux_lfunc(L);
>>+ check_snap_restores(L);
>>+ return TEST_EXIT_SUCCESS;
>>+}
>Maybe we should extend the testing utility, so it is possible to join test sets like
>those into a single group and skip them all at once?
>It would be much easier to maintain them for sure.
>>+
>>+static int strhash(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ struct luam_Metrics oldm, newm;
>>+ lua_pushstring(L, "strhash_hit");
>>+ luaM_metrics(L, &oldm);
>>+ lua_pushstring(L, "strhash_hit");
>>+ lua_pushstring(L, "new_str");
>>+ luaM_metrics(L, &newm);
>>+ /* Remove pushed strings. */
>>+ lua_pop(L, 3);
>>+ assert_true(newm.strhash_hit - oldm.strhash_hit == 1);
>>+ assert_true(newm.strhash_miss - oldm.strhash_miss == 1);
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+static int tracenum_base(void *test_state)
>>+{
>>+ lua_State *L = test_state;
>>+ if (!LJ_HASJIT)
>>+ skip("Test requires JIT enabled");
>>+ struct luam_Metrics metrics;
>>+ utils_get_aux_lfunc(L);
>>+
>>+ luaJIT_setmode(L, 0, LUAJIT_MODE_FLUSH);
>>+ /* Force up garbage collect all dead objects. */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+
>>+ luaM_metrics(L, &metrics);
>>+ assert_true(metrics.jit_trace_num == 0);
>>+
>>+ /* Generate traces. */
>>+ lua_call(L, 0, 1);
>>+ int n = lua_gettop(L);
>>+ /*
>>+ * The first value is the table with functions,
>>+ * the second is the amount of traces.
>>+ */
>>+ if (n != 2 || !lua_isnumber(L, -1))
>>+ bail_out("incorrect return value: 1 number is required");
>>+ size_t jit_trace_num = lua_tonumber(L, -1);
>>+ luaM_metrics(L, &metrics);
>>+ /* Remove `jit_trace_num` from Lua stack. */
>>+ lua_pop(L, 1);
>>+
>>+ assert_sizet_equal(metrics.jit_trace_num, jit_trace_num);
>>+
>>+ luaJIT_setmode(L, 0, LUAJIT_MODE_FLUSH);
>>+ /* Force up garbage collect all dead objects. */
>>+ lua_gc(L, LUA_GCCOLLECT, 0);
>>+ luaM_metrics(L, &metrics);
>>+ assert_true(metrics.jit_trace_num == 0);
>>+
>>+ return TEST_EXIT_SUCCESS;
>>+}
>>+
>>+int main(void)
>>+{
>>+ if (LUAJIT_OS == LUAJIT_OS_BSD)
>>+ skip_all("Disabled on *BSD due to #4819");
>>+
>>+ lua_State *L = utils_lua_init();
>>+
>>+ utils_load_aux_script(L);
>>+ const struct test_unit tgroup[] = {
>>+ test_unit_new(base),
>>+ test_unit_new(gc_allocated_freed),
>>+ test_unit_new(gc_steps),
>>+ test_unit_new(objcount),
>>+ test_unit_new(objcount_cdata_decrement),
>>+ test_unit_new(snap_restores_direct_exit),
>>+ test_unit_new(snap_restores_direct_exit_scalar),
>>+ test_unit_new(snap_restores_side_exit_compiled),
>>+ test_unit_new(snap_restores_side_exit_not_compiled),
>>+ test_unit_new(strhash),
>>+ test_unit_new(tracenum_base)
>I think that we need to make an effort and try move all skipconds into
>one place. The main functions seems like a good place for them. It
>would greatly increase readabilty and maintainability.
>>+ };
>>+ const int test_result = test_run_group(tgroup, L);
>>+ utils_lua_close(L);
>>+ return test_result;
>>+}
>>diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
>>index 38d6ae49..b4ce39d3 100644
>>--- a/test/tarantool-tests/CMakeLists.txt
>>+++ b/test/tarantool-tests/CMakeLists.txt
>>@@ -66,7 +66,6 @@ add_subdirectory(lj-416-xor-before-jcc)
>><snipped>
>>diff --git a/test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt b/test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt
>>deleted file mode 100644
>>index 60eb5bbb..00000000
>>--- a/test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt
>>+++ /dev/null
>>@@ -1 +0,0 @@
>>-BuildTestCLib(testgetmetrics testgetmetrics.c)
>>diff --git a/test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c b/test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c
>>deleted file mode 100644
>>index 67776338..00000000
>>--- a/test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c
>>+++ /dev/null
>>@@ -1,270 +0,0 @@
>><snipped>
>>--
>>2.34.1
>--
>Best regards,
>Maxim Kokryashkin
> 

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

  reply	other threads:[~2023-03-22  0:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-15 16:11 [Tarantool-patches] [PATCH v1 luajit 0/5] reworking C tests Sergey Kaplun via Tarantool-patches
2023-03-15 16:11 ` [Tarantool-patches] [PATCH v1 luajit 1/5] test: fix setting of {DY}LD_LIBRARY_PATH variables Sergey Kaplun via Tarantool-patches
2023-03-20 13:54   ` Maxim Kokryashkin via Tarantool-patches
2023-03-15 16:11 ` [Tarantool-patches] [PATCH v1 luajit 2/5] test: introduce module for C tests Sergey Kaplun via Tarantool-patches
2023-03-20 15:17   ` Maxim Kokryashkin via Tarantool-patches
2023-03-15 16:11 ` [Tarantool-patches] [PATCH v1 luajit 3/5] test: introduce utils.h helper " Sergey Kaplun via Tarantool-patches
2023-03-20 15:21   ` Maxim Kokryashkin via Tarantool-patches
2023-03-15 16:11 ` [Tarantool-patches] [PATCH v1 luajit 4/5] test: rewrite misclib-getmetrics-capi test in C Sergey Kaplun via Tarantool-patches
2023-03-22  0:07   ` Maxim Kokryashkin via Tarantool-patches [this message]
2023-03-15 16:11 ` [Tarantool-patches] [PATCH v1 luajit 5/5] test: rewrite misclib-sysprof-capi " Sergey Kaplun via Tarantool-patches
2023-03-20 16:24   ` Maxim Kokryashkin via Tarantool-patches
2023-03-20 13:50 ` [Tarantool-patches] [PATCH v1 luajit 0/5] reworking C tests Maxim Kokryashkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1679443645.145929878@f300.i.mail.ru \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches]  [PATCH v1 luajit 4/5] test: rewrite misclib-getmetrics-capi test in C' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox