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 B15576EFDA; Thu, 19 May 2022 09:59:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B15576EFDA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1652943551; bh=BPdqe4pf0FNtDPAkbBEKX/hDn+2ZOfM29i5iIssu3C0=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=VjV8ssW17iuXTW30v4pBrONeZjEFiA0mlY6UkPFl16MYcsskkFq3rXUUlMKFPp/8k iqGsnJWSx3dj1MD9YJG0cYOUYWVQck3II4JxnV0B7ufkRZSEYp4GepqzB1840XipaX +lPrgSAfTl+sgPFDIPlDxShBjzw50Dkcp/hlV4p8= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102]) (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 CBBE96EFDA for ; Thu, 19 May 2022 09:59:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CBBE96EFDA Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1nra7n-0006jA-Q4; Thu, 19 May 2022 09:59:08 +0300 Date: Thu, 19 May 2022 09:56:50 +0300 To: Igor Munkin Message-ID: References: <20220511082521.389687-1-m.kokryashkin@tarantool.org> <20220511082521.389687-2-m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD959DE7DA6C9DEA5D539C636C63CFFC55D88E4F46E2BDBD881182A05F53808504015CA2EF64F2C3A2DA27F64B571CB507B164180DD81387EFD68B85B50C7A7D202 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE76D34FAA3D8B31588C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE79A02CDD1178524C2EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BEBC5CAB6D411FFA68B2906F343D8111C68B742B4E1B3361BCC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC82FFDA4F57982C5F4F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947CA85A14DF5F041C996136E347CC761E074AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C3337480BD08FF08ECBA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7B2B7C64F398C7410731C566533BA786AA5CC5B56E945C8DA X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A55F2F65EDEF590F7BE7980E08C52E574D9332EC5E9B6FAADAD59269BC5F550898D99A6476B3ADF6B4886A5961035A09600383DAD389E261318FB05168BE4CE3AF X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34AAC3D1FDB34D0488C9933ADC230165B957FB004906899068A4E25538A64D46871E94C51587CF4CDE1D7E09C32AA3244C096CE0BD886B7231C7B89628D9EE2ED07101BF96129E4011927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojraYEVw8+kXtXuoGjqiO27Q== X-Mailru-Sender: 583F1D7ACE8F49BD955789E124DBF952901B872A12BE26BB524670258D960393834DE321BC2006601057052554D19BDA5CCB423FF989BC332B919154812A68BF37430C57CC4355E80F27244EEAA5B9A50D4ABDE8C577C2ED X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] sysprof: change C configuration API 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 Cc: Maxim Kokryashkin , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maxim! Thanks for the patch! LGTM, after fixes mentioned by Igor below. On 18.05.22, Igor Munkin wrote: > Max, > > Thanks for the patch! See no critical flaws, but please consider my > comments below. > > On 11.05.22, Maxim Kokryashkin wrote: > > This patch replaces the `lj_sysprof_configure` with individual > > configuration handles for each config entry. Also, it chnages the > > Typo: s/chnages/changes/. > > > internal sysprof checks to let users run sysprof in default mode without > > configuratons. > > > > Part of tarantool/tarantool#781 > > --- > > Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci > > PR with Tarantool integration: > > https://github.com/tarantool/tarantool/pull/7123 > > > > src/lib_misc.c | 20 +--- > > src/lj_mapi.c | 13 ++- > > src/lj_sysprof.c | 91 +++++++++++++------ > > src/lj_sysprof.h | 6 +- > > src/lmisclib.h | 54 +++++------ > > .../misclib-sysprof-capi/testsysprof.c | 17 +--- > > .../misclib-sysprof-lapi.test.lua | 2 +- > > 7 files changed, 116 insertions(+), 87 deletions(-) > > > > diff --git a/src/lib_misc.c b/src/lib_misc.c > > index e07f1b32..370307a7 100644 > > --- a/src/lib_misc.c > > +++ b/src/lib_misc.c > > > > > @@ -454,9 +441,12 @@ LJLIB_CF(misc_memprof_stop) > > > > LUALIB_API int luaopen_misc(struct lua_State *L) > > { > > + luaM_sysprof_set_writer(buffer_writer_default); > > + luaM_sysprof_set_on_stop(on_stop_cb_default); > > + luaM_sysprof_set_backtracer(NULL); > > Why default backtracer is set to NULL here? > > Besides, why do you need to use public API if you can use the same > internal interfaces here? Otherwise, what is the reason to have these > lj_sysprof_* functions? > > > + > > LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc); > > LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof); > > LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof); > > - > > return 1; > > } > > > > > diff --git a/src/lmisclib.h b/src/lmisclib.h > > index b41f7f59..3ebf18ff 100644 > > --- a/src/lmisclib.h > > +++ b/src/lmisclib.h > > @@ -63,31 +63,29 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics); > > /* --- Sysprof - platform and lua profiler -------------------------------- */ > > > > /* Profiler configurations. */ > > > > > +/* > > +** Writer function for profile events. Must be async-safe, see also > > +** `man 7 signal-safety`. > > +** Should return amount of written bytes on success or zero in case of error. > > +** Setting *data to NULL means end of profiling. > > +** For details see . > > +*/ > > +typedef size_t (*sp_writer)(const void **data, size_t len, void *ctx); > > Since this is a public type, please use a proper prefix for it. > > > +/* > > +** Callback on profiler stopping. Required for correctly cleaning > > +** at VM finalization when profiler is still running. > > +** Returns zero on success. > > +*/ > > +typedef int (*sp_on_stop)(void *ctx, uint8_t *buf); > > Ditto. > > > +/* > > +** Backtracing function for the host stack. Should call `frame_writer` on > > +** each frame in the stack in the order from the stack top to the stack > > +** bottom. The `frame_writer` function is implemented inside the sysprof > > +** and will be passed to the `backtracer` function. If `frame_writer` returns > > +** NULL, backtracing should be stopped. If `frame_writer` returns not NULL, > > +** the backtracing should be continued if there are frames left. > > +*/ > > +typedef void (*sp_backtracer)(void *(*frame_writer)(int frame_no, void *addr)); > > Ditto. > > > > > /* > > ** DEFAULT mode collects only data for luam_sysprof_counters, which is stored > > > > > -- > > 2.35.1 > > > > -- > Best regards, > IM -- Best regards, Sergey Kaplun