From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6AEEC469719 for ; Wed, 7 Oct 2020 17:35:25 +0300 (MSK) Date: Wed, 7 Oct 2020 17:35:00 +0300 From: Sergey Kaplun Message-ID: <20201007143500.GA27749@root> References: <87bf84f266e16cd9401394c8276b1cd97f91a82d.1601878708.git.skaplun@tarantool.org> <20201006221721.GN18920@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201006221721.GN18920@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 2/2] misc: add C and Lua API for platform metrics List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Hi Igor! Thanks for the review! On 07.10.20, Igor Munkin wrote: > Sergey, > > Thanks for the patch! Please consider my comments below. > > > > +#define LJLIB_MODULE_misc > > + > > +LJLIB_CF(misc_getmetrics) > > +{ > > + struct luam_Metrics metrics; > > + lua_createtable(L, 0, 22); > > 19 slots are definitely enough. Nice catch! Forgot to change this line when was removing colors counting. > > > + GCtab *m = tabV(L->top - 1); > > Once more: please, declare the variables in the beginning of the block. Ooops. My bad. Fixed. > > > + > > + luaM_metrics(L, &metrics); > > + > > > > > diff --git a/src/lj_mapi.c b/src/lj_mapi.c > > new file mode 100644 > > index 0000000..7645a44 > > --- /dev/null > > +++ b/src/lj_mapi.c > > @@ -0,0 +1,61 @@ > > > > > + > > +LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics) > > +{ > > + lua_assert(metrics != NULL); > > Ditto. And this too. > > > + global_State *g = G(L); > > + GCState *gc = &g->gc; > > +#if LJ_HASJIT > > + jit_State *J = G2J(g); > > +#endif > > + > > +struct luam_Metrics { > > + /* Strings amount found in string hash instead of allocation of new one. */ > > If I don't know how this metric is collected, I'll never guess what this > comment mean. Let's reword it the following way: > | /* > | ** Number of strings being interned (i.e. the string with the same > | ** payload is found, so a new one is not created/allocated) > | */ > Thanks! Fixed. > > + size_t strhash_hit; > > + /* Strings amount allocated and put into string hash. */ > > Well, does it mean this metric has the same value as gc_strnum? > Not indeed. This is a number of the strings Ok. I rewrote this in the following way: | /* Total number of strings allocations during the platform lifetime. */ > > > + size_t strhash_miss; > > + > > + /* > > + ** Overall number of snap restores (amount of guard assertions > > + ** leading to stopping trace executions and trace exits, > > + ** that are not stitching with other traces). > > Minor: Strictly saying, it's not a stitching in LuaJIT terms. Removed misleading comments. > > > + */ > > + size_t jit_snap_restore; > > + /* Overall number of abort traces. */ > > + size_t jit_trace_abort; > > + /* Total size of all allocated machine code areas. */ > > + size_t jit_mcode_size; > > + /* Amount of JIT traces. */ > > + unsigned int jit_trace_num; > > +}; > > > > > diff --git a/test/clib-misclib-getmetrics.test.lua b/test/clib-misclib-getmetrics.test.lua > > Add Github issue to the file name. Here and below: I thought we use gh-xxxx style for bugfix tests. Tests for new features are named the same as a feature. Is `misclib-getmetrics-{capi,lapi}.test.lua` good enough? > > > new file mode 100755 > > index 0000000..34adaba > > --- /dev/null > > +++ b/test/clib-misclib-getmetrics.test.lua > > @@ -0,0 +1,174 @@ > > +#!/usr/bin/env tarantool > > + > > +local file = debug.getinfo(1, "S").source:sub(2) > > +local filepath = file:match("(.*/)") > > +local soext = jit.os == "OSX" and "dylib" or "so" > > +package.cpath = filepath..'clib-misclib-getmetrics/?.'..soext..";" > > + ..package.cpath > > I've already solved this issue, so this part can be easily borrowed from > utils.lua[1]. Consider the following: > | local path = arg[0]:gsub('%.test%.lua', '') > | local suffix = package.cpath:match('?.(%a+);') > | package.cpath = ('%s/?.%s;%s'):format(path, suffix, package.cpath) > Yep. See now. Thanks! We should add one small function for this to reuse in other tests later. > > + > > +local jit_opt_default_hotloop = 56 > > +local jit_opt_default_hotexit = 10 > > +local jit_opt_default_level = 3 > > +local jit_opt_default_minstitch = 0 > > Well, this can be done in a much more elegant way: > | local jit_opt_default = { > | 3, -- level > | "hotloop=56", > | "hotexit=10", > | "minstitch=0", > | } > | > | > | > | jit.opt.start(unpack(jit_opt_default)) > Yep. Really looks better. > > + > > + for i = 1, 1000 do > > + table.insert(placeholder.tab, table_new(i, 0)) > > This can be done via creating { i }, so table.new is excess here. Yes! Thanks! Added here and below. > > > + end > > > > > +-- Compiled loop with a direct exit to the interpreter. > > +test:ok(testgetmetrics.snap_restores(function() > > + jit.opt.start(2, "hotloop=2") > > Why hotloop is 2? It has no any sacral significance. To avoid confusion, I've changed the value to 1 here and below. > > > > diff --git a/test/clib-misclib-getmetrics/testgetmetrics.c b/test/clib-misclib-getmetrics/testgetmetrics.c > > > > +static int base(lua_State *L) > > +{ > > + struct luam_Metrics metrics; > > + luaM_metrics(L, &metrics); > > + > > + /* Just check API. */ > > Minor: it worths to drop a comment why there is a here. It's > quite obvious, but has come to my mind only in a few seconds. Sure! No problem! > > > + assert((ssize_t)metrics.strhash_hit >= 0); > > > > > +} > > + > > +static int gc_allocated_freed(lua_State *L) > > +{ > > + 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")) > > + luaL_error(L, "failed to translate Lua code snippet"); > > Side note: OK, this is a funny example: the code to be executed doesn't > generate the garbage per se, but the machinery running it does. This is Yes, that was the idea. > nice, since AFAICS you can calculate the exact amount to allocated and > freed, can't you? Feel free to ignore this comment or consider it as > an extracurricular activity. I didn't set the exact value on purpose. It seemed to me that we should not be tied to the behavior of the corresponding machinery, that can to be changed in future. > > > + lua_gc(L, LUA_GCCOLLECT, 0); > > + luaM_metrics(L, &newm); > > + assert(newm.gc_allocated - oldm.gc_allocated > 0); > > + assert(newm.gc_freed - oldm.gc_freed > 0); > > + > > + lua_pushboolean(L, 1); > > + return 1; > > +} > > + > > +static int gc_steps(lua_State *L) > > +{ > > + struct luam_Metrics oldm, newm; > > + /* > > + * Some garbage has already happened before the next line, > > + * i.e. during fronted processing lua test chunk. > > Typo: s/fronted/frontend/. > Typo: s/lua/Lua/. Fixed here and below. > > > + * 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): > > + */ > > + luaM_metrics(L, &oldm); > > + lua_gc(L, LUA_GCCOLLECT, 0); > > OK, this is a misguiding part. At first I was confused with the comment > below that *new values are the same*. Why the difference doesn't equals > to zero if they are the same? Later I found the call after > initialization *but* prior to these asserts. I guess the next > reordering definitely make the code clearer: > | luaM_metrics(L, &oldm); > | assert(oldm.gc_steps_pause > 0); > | > | lua_gc(L, LUA_GCCOLLECT, 0); > > > + assert(oldm.gc_steps_pause > 0); > > > > > + /* > > + * As long as we don't create new Lua objects > > + * consequent call should return the same values: > > How come? They *are not* the same considering the asserts below (except > ). The comment is misguiding. > This is bad mistake of mine. Here I want to check that results are the same after two consecutive calls. Fixed. Thanks for your question! > > + */ > > + luaM_metrics(L, &newm); > > + assert(newm.gc_steps_pause - oldm.gc_steps_pause > 0); > > > > > +} > > + > > +static int objcount(lua_State *L) > > +{ > > + struct luam_Metrics oldm, newm; > > + int n = lua_gettop(L); > > + if (n != 1 || !lua_isfunction(L, 1)) > > + luaL_error(L, "incorrect argument: 1 function is required"); > > + > > + /* Force up garbage collect all dead objects. */ > > + lua_gc(L, LUA_GCCOLLECT, 0); > > + > > + luaM_metrics(L, &oldm); > > + /* Generate garbage. */ > > + lua_call(L, 0, 0); > > If loop optimization is disabled, then you can pass the number of > iterations as an argument to the function and check the metrics values > against it. Thoughts? But there are more objects besides base generated one (for example strings like "uint64_t", "ffi"). But we can steal set number of iterations here this makes test more flexible. I've added it to patch. > > > + lua_gc(L, LUA_GCCOLLECT, 0); > > + luaM_metrics(L, &newm); > > > > > +} > > > +static int tracenum_base(lua_State *L) > > +{ > > + struct luam_Metrics metrics; > > + int n = lua_gettop(L); > > + if (n != 1 || !lua_isfunction(L, 1)) > > + luaL_error(L, "incorrect arguments: 1 function is required"); > > + > > + luaJIT_setmode(L, 0, LUAJIT_MODE_OFF); > > This line looks excess. Removed here and below. > > > + 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(metrics.jit_trace_num == 0); > > + > > + luaJIT_setmode(L, 0, LUAJIT_MODE_ON); > > > > > + > > + 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(metrics.jit_trace_num == 0); > > + > > + luaJIT_setmode(L, 0, LUAJIT_MODE_ON); > > This line looks excess. Removed here and below. > > > + lua_pushboolean(L, 1); > > + return 1; > > +} > > > > > diff --git a/test/lib-misc-getmetrics.test.lua b/test/lib-misc-getmetrics.test.lua > > Add Github issue to the file name. > > > new file mode 100755 > > index 0000000..2859e26 > > --- /dev/null > > +++ b/test/lib-misc-getmetrics.test.lua > > @@ -0,0 +1,418 @@ > > +#!/usr/bin/env tarantool > > + > > +-- This is a part of tarantool/luajit testing suite. > > +-- Major portions taken verbatim or adapted from the LuaVela testing suit. > > Typo: s/suit/suite/. Thanks! Fixed. > > > +-- Copyright (C) 2015-2019 IPONWEB Ltd. > > > > > +test:test("gc-allocated-freed", function(subtest) > > + subtest:plan(1) > > + > > + -- Force up garbage collect all dead objects. > > + collectgarbage("collect") > > + > > + -- Bump getmetrincs table and string keys allocation. > > Typo: s/getmetrincs/getmetrics/. Thanks! Fixed! > > > + local old_metrics = misc.getmetrics() > > + > > + -- Remember allocated size for getmetrics table. > > + old_metrics = misc.getmetrics() > > + > > + collectgarbage("collect") > > + > > + local new_metrics = misc.getmetrics() > > + > > + local diff_alloc = new_metrics.gc_allocated - old_metrics.gc_allocated > > + local getmetrics_alloc = diff_alloc > > Why the difference can't be assigned right to ? Yep, it will be more clear. Thanks! > > > > + -- Bump strings and table creation. > > + local old_metrics = misc.getmetrics() > > OK, I finally got the math you explained to me offline, but it's too > complex for such simple case. Let's consider the newly allocated table > right in the test assertion. > Yep, this looks confusing. Fixed. > > + old_metrics = misc.getmetrics() > > > +test:test("tracenum-base", function(subtest) > > + subtest:plan(3) > > + > > + jit.off() > > This line looks excess. Yep. Removed. > > > + jit.flush() > > + collectgarbage("collect") > > + local metrics = misc.getmetrics() > > + subtest:is(metrics.jit_trace_num, 0) > > + > > + jit.on() And this too. > > + local sum = 0 > > + for i = 1, 100 do > > + sum = sum + i > > + end > > + > > + metrics = misc.getmetrics() > > + subtest:is(metrics.jit_trace_num, 1) > > + > > + jit.off() > > This line looks excess. Yep. Removed. > > > + jit.flush() > > + collectgarbage("collect") > > + > > + metrics = misc.getmetrics() > > + subtest:is(metrics.jit_trace_num, 0) > > + > > + jit.on() And this too. > > +end) > > + > > +os.exit(test:check() and 0 or 1) > > -- > > 2.28.0 > > > > [1]: https://github.com/tarantool/luajit/blob/tarantool/test/utils.lua > > -- > Best regards, > IM See iterative patch in the bottom. Branch force-pushed. =================================================================== diff --git a/src/lib_misc.c b/src/lib_misc.c index ef11237..6f7b9a9 100644 --- a/src/lib_misc.c +++ b/src/lib_misc.c @@ -29,8 +29,10 @@ static LJ_AINLINE void setnumfield(struct lua_State *L, GCtab *t, LJLIB_CF(misc_getmetrics) { struct luam_Metrics metrics; - lua_createtable(L, 0, 22); - GCtab *m = tabV(L->top - 1); + GCtab *m; + + lua_createtable(L, 0, 19); + m = tabV(L->top - 1); luaM_metrics(L, &metrics); diff --git a/src/lj_mapi.c b/src/lj_mapi.c index 7645a44..13737e0 100644 --- a/src/lj_mapi.c +++ b/src/lj_mapi.c @@ -17,13 +17,14 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics) { - lua_assert(metrics != NULL); global_State *g = G(L); GCState *gc = &g->gc; #if LJ_HASJIT jit_State *J = G2J(g); #endif + lua_assert(metrics != NULL); + metrics->strhash_hit = g->strhash_hit; metrics->strhash_miss = g->strhash_miss; diff --git a/src/lmisclib.h b/src/lmisclib.h index e2d1909..0c07707 100644 --- a/src/lmisclib.h +++ b/src/lmisclib.h @@ -13,9 +13,12 @@ /* API for obtaining various platform metrics. */ struct luam_Metrics { - /* Strings amount found in string hash instead of allocation of new one. */ + /* + ** Number of strings being interned (i.e. the string with the + ** same payload is found, so a new one is not created/allocated). + */ size_t strhash_hit; - /* Strings amount allocated and put into string hash. */ + /* Total number of strings allocations during the platform lifetime. */ size_t strhash_miss; /* Amount of allocated string objects. */ @@ -44,8 +47,7 @@ struct luam_Metrics { /* ** Overall number of snap restores (amount of guard assertions - ** leading to stopping trace executions and trace exits, - ** that are not stitching with other traces). + ** leading to stopping trace executions). */ size_t jit_snap_restore; /* Overall number of abort traces. */ diff --git a/test/misclib-getmetrics-capi.test.lua b/test/misclib-getmetrics-capi.test.lua index 34adaba..f133274 100755 --- a/test/misclib-getmetrics-capi.test.lua +++ b/test/misclib-getmetrics-capi.test.lua @@ -1,15 +1,15 @@ #!/usr/bin/env tarantool -local file = debug.getinfo(1, "S").source:sub(2) -local filepath = file:match("(.*/)") -local soext = jit.os == "OSX" and "dylib" or "so" -package.cpath = filepath..'clib-misclib-getmetrics/?.'..soext..";" - ..package.cpath +local path = arg[0]:gsub('%.test%.lua', '') +local suffix = package.cpath:match('?.(%a+);') +package.cpath = ('%s/?.%s'):format(path, suffix) -local jit_opt_default_hotloop = 56 -local jit_opt_default_hotexit = 10 -local jit_opt_default_level = 3 -local jit_opt_default_minstitch = 0 +local jit_opt_default = { + 3, -- level + "hotloop=56", + "hotexit=10", + "minstitch=0", +} local tap = require('tap') @@ -22,8 +22,7 @@ test:ok(testgetmetrics.base()) test:ok(testgetmetrics.gc_allocated_freed()) test:ok(testgetmetrics.gc_steps()) -test:ok(testgetmetrics.objcount(function() - local table_new = require("table.new") +test:ok(testgetmetrics.objcount(function(iterations) local ffi = require("ffi") jit.opt.start(0) @@ -36,36 +35,36 @@ test:ok(testgetmetrics.objcount(function() } -- Separate objects creations to separate jit traces. - for i = 1, 1000 do + for i = 1, iterations do table.insert(placeholder.str, tostring(i)) end - for i = 1, 1000 do - table.insert(placeholder.tab, table_new(i, 0)) + for i = 1, iterations do + table.insert(placeholder.tab, {i}) end - for i = 1, 1000 do + for i = 1, iterations do table.insert(placeholder.udata, newproxy()) end - for i = 1, 1000 do + for i = 1, iterations do -- Check counting of VLA/VLS/aligned cdata. table.insert(placeholder.cdata, ffi.new("char[?]", 4)) end - for i = 1, 1000 do + for i = 1, iterations do -- Check counting of non-VLA/VLS/aligned cdata. table.insert(placeholder.cdata, ffi.new("uint64_t", i)) end placeholder = nil -- Restore default jit settings. - jit.opt.start(jit_opt_default_level) + jit.opt.start(unpack(jit_opt_default)) end)) -- Compiled loop with a direct exit to the interpreter. test:ok(testgetmetrics.snap_restores(function() - jit.opt.start(0, "hotloop=2") + jit.opt.start(0, "hotloop=1") local old_metrics = misc.getmetrics() @@ -77,7 +76,7 @@ test:ok(testgetmetrics.snap_restores(function() local new_metrics = misc.getmetrics() -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop) + jit.opt.start(unpack(jit_opt_default)) -- A single snapshot restoration happened on loop finish. return 1 @@ -85,7 +84,7 @@ end)) -- Compiled loop with a side exit which does not get compiled. test:ok(testgetmetrics.snap_restores(function() - jit.opt.start(0, "hotloop=2", "hotexit=2", "minstitch=15") + jit.opt.start(0, "hotloop=1", "hotexit=2", "minstitch=15") local function foo(i) -- math.fmod is not yet compiled! @@ -98,9 +97,7 @@ test:ok(testgetmetrics.snap_restores(function() end -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop, - "hotexit="..jit_opt_default_hotexit, - "minstitch="..jit_opt_default_minstitch) + jit.opt.start(unpack(jit_opt_default)) -- Side exits from the root trace could not get compiled. return 5 @@ -110,7 +107,7 @@ end)) test:ok(testgetmetrics.snap_restores(function() -- Optimization level is important here as `loop` optimization -- may unroll the loop body and insert +1 side exit. - jit.opt.start(0, "hotloop=5", "hotexit=5") + jit.opt.start(0, "hotloop=1", "hotexit=5") local function foo(i) return i <= 10 and i or tostring(i) @@ -122,8 +119,7 @@ test:ok(testgetmetrics.snap_restores(function() end -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop, - "hotexit="..jit_opt_default_hotexit) + jit.opt.start(unpack(jit_opt_default)) -- 5 side exits to the interpreter before trace gets hot -- and compiled @@ -157,8 +153,8 @@ test:ok(testgetmetrics.snap_restores(function() foo(21) -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop, - "hotexit="..jit_opt_default_hotexit) + jit.opt.start(unpack(jit_opt_default)) + return 2 end)) diff --git a/test/misclib-getmetrics-capi/testgetmetrics.c b/test/misclib-getmetrics-capi/testgetmetrics.c index 32802d2..4a7355a 100644 --- a/test/misclib-getmetrics-capi/testgetmetrics.c +++ b/test/misclib-getmetrics-capi/testgetmetrics.c @@ -11,7 +11,10 @@ static int base(lua_State *L) struct luam_Metrics metrics; luaM_metrics(L, &metrics); - /* Just check API. */ + /* + * Just check API. + * (ssize_t) cast need for not always-true unsigned >= 0 check. + */ assert((ssize_t)metrics.strhash_hit >= 0); assert((ssize_t)metrics.strhash_miss >= 0); @@ -64,13 +67,14 @@ static int gc_steps(lua_State *L) struct luam_Metrics oldm, newm; /* * Some garbage has already happened before the next line, - * i.e. during fronted processing lua test chunk. + * 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): */ - luaM_metrics(L, &oldm); lua_gc(L, LUA_GCCOLLECT, 0); + + luaM_metrics(L, &oldm); assert(oldm.gc_steps_pause > 0); assert(oldm.gc_steps_propagate > 0); assert(oldm.gc_steps_atomic > 0); @@ -84,11 +88,11 @@ static int gc_steps(lua_State *L) * consequent call should return the same values: */ luaM_metrics(L, &newm); - assert(newm.gc_steps_pause - oldm.gc_steps_pause > 0); - assert(newm.gc_steps_propagate - oldm.gc_steps_propagate > 0); - assert(newm.gc_steps_atomic - oldm.gc_steps_atomic > 0); - assert(newm.gc_steps_sweepstring - oldm.gc_steps_sweepstring > 0); - assert(newm.gc_steps_sweep - oldm.gc_steps_sweep > 0); + assert(newm.gc_steps_pause - oldm.gc_steps_pause == 0); + assert(newm.gc_steps_propagate - oldm.gc_steps_propagate == 0); + assert(newm.gc_steps_atomic - oldm.gc_steps_atomic == 0); + assert(newm.gc_steps_sweepstring - oldm.gc_steps_sweepstring == 0); + assert(newm.gc_steps_sweep - oldm.gc_steps_sweep == 0); /* Nothing to finalize, skipped. */ assert(newm.gc_steps_finalize == 0); oldm = newm; @@ -110,7 +114,7 @@ static int gc_steps(lua_State *L) /* * Now let's run three GC cycles to ensure that - * zero-to-one transition was not a lucky coincidence. + * increment was not a lucky coincidence. */ lua_gc(L, LUA_GCCOLLECT, 0); lua_gc(L, LUA_GCCOLLECT, 0); @@ -139,8 +143,9 @@ static int objcount(lua_State *L) lua_gc(L, LUA_GCCOLLECT, 0); luaM_metrics(L, &oldm); - /* Generate garbage. */ - lua_call(L, 0, 0); + /* 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(newm.gc_strnum - oldm.gc_strnum == 0); @@ -195,7 +200,6 @@ static int tracenum_base(lua_State *L) if (n != 1 || !lua_isfunction(L, 1)) luaL_error(L, "incorrect arguments: 1 function is required"); - luaJIT_setmode(L, 0, LUAJIT_MODE_OFF); luaJIT_setmode(L, 0, LUAJIT_MODE_FLUSH); /* Force up garbage collect all dead objects. */ lua_gc(L, LUA_GCCOLLECT, 0); @@ -203,7 +207,6 @@ static int tracenum_base(lua_State *L) luaM_metrics(L, &metrics); assert(metrics.jit_trace_num == 0); - luaJIT_setmode(L, 0, LUAJIT_MODE_ON); /* Generate traces. */ lua_call(L, 0, 1); n = lua_gettop(L); diff --git a/test/misclib-getmetrics-lapi.test.lua b/test/misclib-getmetrics-lapi.test.lua index 2859e26..3b3d1f8 100755 --- a/test/misclib-getmetrics-lapi.test.lua +++ b/test/misclib-getmetrics-lapi.test.lua @@ -1,7 +1,7 @@ #!/usr/bin/env tarantool -- This is a part of tarantool/luajit testing suite. --- Major portions taken verbatim or adapted from the LuaVela testing suit. +-- Major portions taken verbatim or adapted from the LuaVela testing suite. -- Copyright (C) 2015-2019 IPONWEB Ltd. local tap = require('tap') @@ -9,10 +9,12 @@ local tap = require('tap') local test = tap.test("lib-misc-getmetrics") test:plan(10) -local jit_opt_default_hotloop = 56 -local jit_opt_default_hotexit = 10 -local jit_opt_default_level = 3 -local jit_opt_default_minstitch = 0 +local jit_opt_default = { + 3, -- level + "hotloop=56", + "hotexit=10", + "minstitch=0", +} -- Test Lua API. test:test("base", function(subtest) @@ -49,7 +51,7 @@ test:test("gc-allocated-freed", function(subtest) -- Force up garbage collect all dead objects. collectgarbage("collect") - -- Bump getmetrincs table and string keys allocation. + -- Bump getmetrics table and string keys allocation. local old_metrics = misc.getmetrics() -- Remember allocated size for getmetrics table. @@ -59,8 +61,7 @@ test:test("gc-allocated-freed", function(subtest) local new_metrics = misc.getmetrics() - local diff_alloc = new_metrics.gc_allocated - old_metrics.gc_allocated - local getmetrics_alloc = diff_alloc + local getmetrics_alloc = new_metrics.gc_allocated - old_metrics.gc_allocated -- Do not use test:ok to avoid extra allocated/freed objects. assert(getmetrics_alloc > 0, "count allocated table for getmetrics") @@ -75,7 +76,7 @@ test:test("gc-allocated-freed", function(subtest) new_metrics = misc.getmetrics() - diff_alloc = new_metrics.gc_allocated - old_metrics.gc_allocated + local diff_alloc = new_metrics.gc_allocated - old_metrics.gc_allocated local diff_freed = new_metrics.gc_freed - old_metrics.gc_freed assert(diff_alloc > getmetrics_alloc, @@ -113,7 +114,7 @@ test:test("gc-steps", function(subtest) subtest:plan(24) -- Some garbage has already created before the next line, - -- i.e. during fronted processing this chunk. + -- i.e. during frontend processing this 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): @@ -154,8 +155,8 @@ test:test("gc-steps", function(subtest) subtest:is(newm.gc_steps_finalize, 0) oldm = newm - -- Now let's run three GC cycles to ensure that zero-to-one - -- transition was not a lucky coincidence. + -- Now let's run three GC cycles to ensure that increment + -- was not a lucky coincidence. collectgarbage("collect") collectgarbage("collect") collectgarbage("collect") @@ -172,7 +173,6 @@ end) test:test("objcount", function(subtest) subtest:plan(4) - local table_new = require("table.new") local ffi = require("ffi") jit.opt.start(0) @@ -180,9 +180,7 @@ test:test("objcount", function(subtest) -- Remove all dead objects. collectgarbage("collect") - -- Bump strings and table creation. local old_metrics = misc.getmetrics() - old_metrics = misc.getmetrics() local placeholder = { str = {}, @@ -197,7 +195,7 @@ test:test("objcount", function(subtest) end for i = 1, 1000 do - table.insert(placeholder.tab, table_new(i, 0)) + table.insert(placeholder.tab, {i}) end for i = 1, 1000 do @@ -221,7 +219,12 @@ test:test("objcount", function(subtest) -- Check that amount of objects not increased. subtest:is(new_metrics.gc_strnum, old_metrics.gc_strnum, "strnum don't change") - subtest:is(new_metrics.gc_tabnum, old_metrics.gc_tabnum, + -- When we call getmetrics, we create table for metrics first. + -- So, when we save old_metrics there are x + 1 tables, + -- when we save new_metrics there are x + 2 tables, because + -- old table hasn't been collected yet (it is still + -- reachable). + subtest:is(new_metrics.gc_tabnum - old_metrics.gc_tabnum, 1, "tabnum don't change") subtest:is(new_metrics.gc_udatanum, old_metrics.gc_udatanum, "udatanum don't change") @@ -229,14 +232,14 @@ test:test("objcount", function(subtest) "cdatanum don't change") -- Restore default jit settings. - jit.opt.start(jit_opt_default_level) + jit.opt.start(unpack(jit_opt_default)) end) test:test("snap-restores-direct-loop", function(subtest) -- Compiled loop with a direct exit to the interpreter. subtest:plan(1) - jit.opt.start(0, "hotloop=2") + jit.opt.start(0, "hotloop=1") local old_metrics = misc.getmetrics() @@ -251,14 +254,14 @@ test:test("snap-restores-direct-loop", function(subtest) subtest:is(new_metrics.jit_snap_restore - old_metrics.jit_snap_restore, 1) -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop) + jit.opt.start(unpack(jit_opt_default)) end) test:test("snap-restores-loop-side-exit-non-compiled", function(subtest) -- Compiled loop with a side exit which does not get compiled. subtest:plan(1) - jit.opt.start(0, "hotloop=2", "hotexit=2", "minstitch=15") + jit.opt.start(0, "hotloop=1", "hotexit=2", "minstitch=15") local function foo(i) -- math.fmod is not yet compiled! @@ -277,9 +280,7 @@ test:test("snap-restores-loop-side-exit-non-compiled", function(subtest) subtest:is(new_metrics.jit_snap_restore - old_metrics.jit_snap_restore, 5) -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop, - "hotexit="..jit_opt_default_hotexit, - "minstitch="..jit_opt_default_minstitch) + jit.opt.start(unpack(jit_opt_default)) end) test:test("snap-restores-loop-side-exit", function(subtest) @@ -288,7 +289,7 @@ test:test("snap-restores-loop-side-exit", function(subtest) -- Optimization level is important here as `loop` optimization -- may unroll the loop body and insert +1 side exit. - jit.opt.start(0, "hotloop=5", "hotexit=5") + jit.opt.start(0, "hotloop=1", "hotexit=5") local function foo(i) return i <= 10 and i or tostring(i) @@ -308,8 +309,7 @@ test:test("snap-restores-loop-side-exit", function(subtest) subtest:is(new_metrics.jit_snap_restore - old_metrics.jit_snap_restore, 6) -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop, - "hotexit="..jit_opt_default_hotexit) + jit.opt.start(unpack(jit_opt_default)) end) test:test("snap-restores-scalar", function(subtest) @@ -352,8 +352,7 @@ test:test("snap-restores-scalar", function(subtest) subtest:is(new_metrics.jit_snap_restore - old_metrics.jit_snap_restore, 2) -- Restore default jit settings. - jit.opt.start(jit_opt_default_level, "hotloop="..jit_opt_default_hotloop, - "hotexit="..jit_opt_default_hotexit) + jit.opt.start(unpack(jit_opt_default)) end) test:test("strhash", function(subtest) @@ -390,13 +389,11 @@ end) test:test("tracenum-base", function(subtest) subtest:plan(3) - jit.off() jit.flush() collectgarbage("collect") local metrics = misc.getmetrics() subtest:is(metrics.jit_trace_num, 0) - jit.on() local sum = 0 for i = 1, 100 do sum = sum + i @@ -405,14 +402,11 @@ test:test("tracenum-base", function(subtest) metrics = misc.getmetrics() subtest:is(metrics.jit_trace_num, 1) - jit.off() jit.flush() collectgarbage("collect") metrics = misc.getmetrics() subtest:is(metrics.jit_trace_num, 0) - - jit.on() end) os.exit(test:check() and 0 or 1) =================================================================== -- Best regards, Sergey Kaplun