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 6D7A66EFDA; Wed, 18 May 2022 17:23:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6D7A66EFDA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1652883832; bh=lo4jb3Dq7JoK/MbwF1oeB0JxqIt3C1f9O8g7pKKMBwI=; 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=X4j3/xDTQRRXH5/ifUwPnuOTHorGXaXuyM3V8hWj09z2nu3NHfglYXGSykGKfuizS BDJIYWEUefA332+zVrflHdw73n8Xh0xdClkGfioziz0aYdEtl+txWFVs11gBpxLtl3 lIy/SBqrvJhRM0jZs6/GVXF0DrYVS36F77/YP1OU= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (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 96C506EFDA for ; Wed, 18 May 2022 17:23:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 96C506EFDA Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1nrKac-0002vC-E7; Wed, 18 May 2022 17:23:50 +0300 Date: Wed, 18 May 2022 17:20:15 +0300 To: Maxim Kokryashkin 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=utf-8 Content-Disposition: inline In-Reply-To: <20220511082521.389687-2-m.kokryashkin@tarantool.org> X-Clacks-Overhead: GNU Terry Pratchett X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD959DE7DA6C9DEA5D5A526C9045BFBE159841E1B444D80DF13182A05F538085040ABD23013289EE9A79E43716E033400A4AC9158D0B0A469B2627CB6B6E4B03DBA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75DF2B1F23425CAE5EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637531CC3E3F637A59A8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8A10A2D61A00F2868505B650195870C16117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6BF3059D42242344A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A586D4996C5AC2B87212B5914AC1157FB62BF9D9A1CCEB3205D59269BC5F550898D99A6476B3ADF6B4886A5961035A09600383DAD389E261318FB05168BE4CE3AF X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34FACF503ACC83041BD6463E186A7C5F1C519AC79D8A358C5EA5B1A9184F1623C6C7CBCE104C4F258A1D7E09C32AA3244C836DBC51548A65B6223F8917A5334535F94338140B71B8EE83B48618A63566E0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojtNJAk7CcUJ2ZB0gqcaeGZg== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D675CFFA5C813D6CB5EF866EF456220E42A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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