From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id ABD43437874; Thu, 12 Mar 2026 20:58:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ABD43437874 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1773338304; bh=B2m8SxgnLRgw3eToVDqseyU0EPYDG37lKZlfrIJQYqE=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=FKrIA1tNDUd7gijV8G+qb4cuytDTrX2ilJfPLHUbSN1rhHwIwMUgr/jdhsTYeQ9jN AGbnEKmDrV1E6Dhv08japt3xEftmsNMkZtWGs8rkofCvLa0uSyfekg9v2TPGbp0ena f46dmAQnTAxMYHjNkHt65H/7W2acyb7TEOKCGXgk= Received: from send217.i.mail.ru (send217.i.mail.ru [95.163.59.56]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 293BD437874 for ; Thu, 12 Mar 2026 20:58:23 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 293BD437874 Received: by exim-smtp-64cdfc6c8d-l7cm9 with esmtpa (envelope-from ) id 1w0kIg-00000000Tgx-1JoK; Thu, 12 Mar 2026 20:58:22 +0300 Date: Thu, 12 Mar 2026 20:59:16 +0300 To: Sergey Bronnikov Cc: Sergey Bronnikov , tarantool-patches@dev.tarantool.org Message-ID: References: <7f14d93b-f295-434f-898c-3409daee3053@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7f14d93b-f295-434f-898c-3409daee3053@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD91ABAE9865AC7DC8857B77F625E8FF4B706F175266ECB635F182A05F5380850403765DAD4440062EC3DE06ABAFEAF6705237D29F09955E9B4A11DA5EC286225E1D2FD38C2D22A140C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE77BF46084C0059042EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB5533756627194F95AE6E7C2D8B1F9A3B1397982D73D01A8175359D90BDE1B49122A32F77389733CBF5DBD5E913377AFFFEAFD269A417C69337E82CC2CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD269176DF2183F8FC7C0766A71399C18A7667B076A6E789B0E97A8DF7F3B2552694AD5FFEEA1DED7F25D49FD398EE364050F0AC5B80A05675ACD7C6FCE95544A9834B3661434B16C20ACC84D3B47A649675FE827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BB07C9E286C61B7F975ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A58B068721F777A90D5002B1117B3ED696A37D6C7C1DA67DA9AD0703CEB2EF9A27823CB91A9FED034534781492E4B8EEAD0942DC5495D0595EBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE191716CD42B3DD1D34CAB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D659E130D33CC69FDA8F231CDA72B0A9A518052955EA0028CE8CEA5D3FE3E5BE7AD293D994AB5F662C1BB8341EE9D5BE9A0A81B82CE3B4F84B9A4B51B2A63FEF6BF0743DB74E99CDEAD46536EB022892E5344C41F94D744909CECFA6C6B0C050A61A8CAF69B82BA93681CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVdbVVJCphTR/gB+VWhX5pSc= X-Mailru-Sender: 583F1D7ACE8F49BDD951BA70C165859E716A5BD083E2CD41EDE2FBCE1621770AB11A67E06DA813750228B881135CB80AF2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A84198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] misc: introduce flags with profiler support status X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! See my answers below. On 12.03.26, Sergey Bronnikov wrote: > Hi, Sergey, > > thanks for review! See my comments. > > Sergey > > On 3/12/26 15:00, Sergey Kaplun via Tarantool-patches wrote: > > Hi, Sergey! > > Thanks for the patch! > > Please consider my comments below. > > > > On 22.01.26, Sergey Bronnikov wrote: > >> The patch introduce flags in module "misc" with support status > >> for sysprof and memprof: `misc.sysprof.enabled` and > >> `misc.memprof.enabled`. Both flags are boolean and always > > Let's rename it to `available` instead. The `enabled` may be interpreted > > as `is_running`, and confuse the user then. > I propose using `is_available` instead. The Lua API has 0 functions in the snake_case. Even `funcinfo`, `traceexitstub` from lib_jit.c have this naming style. Lets be consistent with it, especially, since `is_` prefix doesn't change the semantics. > > > >> available on platforms supported by profilers (Windows is not > >> supported). > >> > >> Closes tarantool/tarantool#12215 > > Minor: Should be Resolves, since it is closed when we bump LuaJIT in the > > Tarantool repository. > Ok, updated. > > > >> --- > >> src/lib_misc.c | 4 ++++ > >> .../profilers/misclib-memprof-lapi-disabled.test.lua | 5 ++++- > >> test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua | 5 ++++- > >> .../profilers/misclib-sysprof-lapi-disabled.test.lua | 5 ++++- > >> test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 5 ++++- > >> 5 files changed, 20 insertions(+), 4 deletions(-) > >> > >> diff --git a/src/lib_misc.c b/src/lib_misc.c > >> index 034ff878..6b2278c1 100644 > >> --- a/src/lib_misc.c > >> +++ b/src/lib_misc.c > >> @@ -478,7 +478,11 @@ LUALIB_API int luaopen_misc(struct lua_State *L) > >> LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc); > >> #if !LJ_TARGET_WINDOWS > >> LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof); > >> + lua_pushboolean(L, LJ_HASMEMPROF); > >> + lua_setfield(L, -2, "enabled"); > >> LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof); > >> + lua_pushboolean(L, LJ_HASSYSPROF); > >> + lua_setfield(L, -2, "enabled"); > > Is it possible to use standard `LJLIB_PUSH() LJLIB_SET()` machinery > > instead? > > I didn't get what is a macros `LJLIB_SET`. > > Also, what are the benefits with using mentioned macros instead more > standard Lua API functions? It is more consistent with the rest of the code base. Also, it helps to avoid table rehasing on library initialization. You may see details in buildvm_lib.c > > > > > >> #endif /* !LJ_TARGET_WINDOWS */ > >> return 1; > >> } > >> diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua > >> index de0aa136..f867cfc6 100644 > >> --- a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua > >> +++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua > >> @@ -3,7 +3,7 @@ local test = tap.test('misclib-memprof-lapi-disabled'):skipcond({ > >> ['Memprof is enabled'] = not os.getenv('LUAJIT_DISABLE_MEMPROF'), > >> }) > >> > >> -test:plan(6) > >> +test:plan(8) > >> > >> -- Attempt to start memprof when it is disabled. > >> local res, err, errno = misc.memprof.start() > >> @@ -19,4 +19,7 @@test:ok(err:match('profiler is disabled'), > >> 'error on stop when memprof is disabled') > >> test:ok(type(errno) == 'number', 'errno on start when memprof is disabled') > >> > >> +test:ok(type(misc.memprof.enabled) == 'boolean', 'misc.memprof.enabled exists') > > I suppose that > > |test:is(misc.memprof.available, false, 'misc.memprof.enabled correct') > > is enough. > > > > Same for other tests below. > if "misc.memprof" is not a table the error will be "attempt to index field" Don't get the point here: 1) We will see this error much earlier in that case. 2) We will see with your approach as well. I don't get the reason for the `type()` call if the check below will fail with a different type as well (since Lua checks types first). I see no reason in double-checking that means nothing. Also, `is()` check is more verbose in case of failure -- it prints got and expected values. > > > > > >> +test:ok(misc.memprof.enabled == false, 'misc.memprof.enabled is false') > >> + > >> test:done(true) > >> diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua > >> index cd675864..44ba8b08 100644 > >> --- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua > >> +++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua > > > > > >> diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua > >> index 2a9ce796..c023d8f1 100644 > >> --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua > >> +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua > > > > > >> diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua > >> index 701d58e4..41ed43e0 100644 > >> --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua > >> +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua > > > > > >> -- > >> 2.43.0 > >> -- Best regards, Sergey Kaplun