From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 E2F2B469719 for ; Wed, 7 Oct 2020 23:09:54 +0300 (MSK) Date: Wed, 7 Oct 2020 23:09:30 +0300 From: Sergey Kaplun Message-ID: <20201007200930.GA11972@root> References: <87bf84f266e16cd9401394c8276b1cd97f91a82d.1601878708.git.skaplun@tarantool.org> <20201006221721.GN18920@tarantool.org> <20201007143500.GA27749@root> <20201007182335.GQ18920@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201007182335.GQ18920@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 Thanks, again! Great comments as always! On 07.10.20, Igor Munkin wrote: > Sergey, > > Thanks for the fixes! The patch LGTM, except two nits below. > > On 07.10.20, Sergey Kaplun wrote: > > Hi Igor! Thanks for the review! > > > > On 07.10.20, Igor Munkin wrote: > > > Sergey, > > > > > > Thanks for the patch! Please consider my comments below. > > > > > 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) > > This line overrides the current . Sure! 10x! > > > > > > > > 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 > > > > > @@ -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); > > There is one more excess call at the end of this function. Thanks! Nice catch! > > > /* Generate traces. */ > > lua_call(L, 0, 1); > > n = lua_gettop(L); > > > > > =================================================================== > > > > -- > > Best regards, > > Sergey Kaplun > > -- > Best regards, > IM See iterative patch in the bottom. Branch force-pushed. =================================================================== diff --git a/test/misclib-getmetrics-capi.test.lua b/test/misclib-getmetrics-capi.test.lua index f133274..1ad6958 100755 --- a/test/misclib-getmetrics-capi.test.lua +++ b/test/misclib-getmetrics-capi.test.lua @@ -2,7 +2,7 @@ local path = arg[0]:gsub('%.test%.lua', '') local suffix = package.cpath:match('?.(%a+);') -package.cpath = ('%s/?.%s'):format(path, suffix) +package.cpath = ('%s/?.%s;'):format(path, suffix)..package.cpath local jit_opt_default = { 3, -- level diff --git a/test/misclib-getmetrics-capi/testgetmetrics.c b/test/misclib-getmetrics-capi/testgetmetrics.c index 4a7355a..8844b17 100644 --- a/test/misclib-getmetrics-capi/testgetmetrics.c +++ b/test/misclib-getmetrics-capi/testgetmetrics.c @@ -222,7 +222,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); lua_pushboolean(L, 1); return 1; } =================================================================== -- Best regards, Sergey Kaplun