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 D98936EFDA; Thu, 19 May 2022 17:09:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D98936EFDA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1652969389; bh=FyCsaXeJZGkimBUoXm24eaPAk8FDljkDHZeR1Bpk/LQ=; 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=qEpeCCD9H462KIdCJTKBNGgUJ9mmA6R58HOSdfoKBhZoFzwSAubLfwD4KJ3Nb/Md8 2RhzHSlXVSxB2PXBJBfWVzp1Gue8e9ZFEQt5ionlAUeQ5c65PgY6nWGA722cmPMQOb uLwuIyjkUDPGhFlreAVPQHvpqHIOjEM99iWsLyeQ= Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 486246EFDA for ; Thu, 19 May 2022 17:09:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 486246EFDA Received: by smtp53.i.mail.ru with esmtpa (envelope-from ) id 1nrgqY-0006he-E2; Thu, 19 May 2022 17:09:46 +0300 Date: Thu, 19 May 2022 17:07:29 +0300 To: Maxim Kokryashkin Message-ID: References: <20220511082521.389687-1-m.kokryashkin@tarantool.org> <20220511082521.389687-3-m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220511082521.389687-3-m.kokryashkin@tarantool.org> X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9F33B3806BF114D159E7BD6746107077164F3C96AD71A0548182A05F5380850404C228DA9ACA6FE27AF8B22E40734E8C71A9007B65E06B57538BD6F911BD5927FDCD1A2133591F728 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70240200614025CBBC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7648F5E4671A60CF6EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BEBC5CAB6D411FFA695AC9C66447FDA0568837BF9ED69AE3CCC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC82FFDA4F57982C5F4F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C672DC5A730DF09D22D242C3BD2E3F4C64AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C39415BE9523AAFE76BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7B2B7C64F398C7410731C566533BA786AA5CC5B56E945C8DA X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A5F8FB171D95EA87D478FDA7E27860B4AEF57A286DBBAAFC26D59269BC5F550898D99A6476B3ADF6B4886A5961035A09600383DAD389E261318FB05168BE4CE3AF X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34AEC7C2AC3C44791D1A2F7B9F59320C2AF0A6B7F44F7BCFDA18872752241E441337C6AA88C0CC49871D7E09C32AA3244CFDEC314493C1C1B3D23D5B24361C5E6997FE24653F78E668927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojraYEVw8+kXuPlzokcjjEzg== X-Mailru-Sender: 583F1D7ACE8F49BD67330789FF5664DBF85540D552B2F9A7D042A8C5A8F9884D932997F95CA482A3525762887713E5F1475755348978188EF9D3679FA3DE6E791CC59163FFD68303B0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] sysprof: enrich symtab on a new trace or a proto 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: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maxim! Thanks for the patch! LGTM, except a few nits. On 11.05.22, Maxim Kokryashkin wrote: > This commit adds functionality introduced > in 0847e71abf7db2559e2bfa35f147ccf0112035e3 ('memprof: enrich symtab when > meeting new prototype') and 0243fb72b05c7c63481c50151c65bef6b04e3372 ('memprof: > enrich symtab when new trace is compiled') to sysprof. > > Both the proto and the trace symtab extensions cannot be run from the > sysprof's signal handler, so it is required to prevent sysprof from > dumping anything in its signal handler during the process to keep the > event stream intact. That is achieved with setting the sysprof's internal state Typo? s/achieved with/achieved by/ > to `SPS_IDLE`. > > Part of tarantool/tarantool#781 > --- > Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci > > src/lj_bcread.c | 7 +++++++ > src/lj_parse.c | 6 ++++++ > src/lj_sysprof.c | 44 ++++++++++++++++++++++++++++++++++++++++- > src/lj_sysprof.h | 9 ++++++++- > src/lj_trace.c | 7 +++++++ > tools/sysprof/parse.lua | 18 ++++++++++++----- Nit: please update Makefile.dep.original as well. > 6 files changed, 84 insertions(+), 7 deletions(-) > > diff --git a/src/lj_bcread.c b/src/lj_bcread.c > index cb08599d..f6c7ad25 100644 > --- a/src/lj_bcread.c > +++ b/src/lj_bcread.c > diff --git a/src/lj_parse.c b/src/lj_parse.c > index 30b0caa0..af0dc53f 100644 > --- a/src/lj_parse.c > +++ b/src/lj_parse.c > diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c > index 28d7d229..23947315 100644 > --- a/src/lj_sysprof.c > +++ b/src/lj_sysprof.c > @@ -238,7 +238,7 @@ static void stream_guest(struct sysprof *sp, uint32_t vmstate) > diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h > index 2978bbd8..c31d61d8 100644 > --- a/src/lj_sysprof.h > +++ b/src/lj_sysprof.h > @@ -15,6 +15,7 @@ > #ifndef _LJ_SYSPROF_H > #define _LJ_SYSPROF_H > Nit: We need to check that LuaJIT is build with JIT here. | #if LJ_HASJIT > +#include "lj_jit.h" > #include "lj_obj.h" > #include "lmisclib.h" > > @@ -80,7 +81,9 @@ > #define LJP_FRAME_LUA_LAST 0x80 > #define LJP_FRAME_HOST_LAST NULL > > -#define LJP_SYMTAB_EVENT ((uint8_t)10) > +#define LJP_SYMTAB_LFUNC_EVENT ((uint8_t)10) > +#define LJP_SYMTAB_CFUNC_EVENT ((uint8_t)11) > +#define LJP_SYMTAB_TRACE_EVENT ((uint8_t)12) This means we should increase LJP_FORMAT_VERSION as well. > #define LJP_EPILOGUE_BYTE 0x80 > > int lj_sysprof_configure(const struct luam_Sysprof_Config *config); > @@ -91,4 +94,8 @@ int lj_sysprof_stop(lua_State *L); > > int lj_sysprof_report(struct luam_Sysprof_Counters *counters); > > +void lj_sysprof_add_proto(const struct GCproto *pt); > + > +void lj_sysprof_add_trace(const struct GCtrace *tr); Nit: We need to check that LuaJIT is build with JIT here. | #if LJ_HASJIT > + > #endif > diff --git a/src/lj_trace.c b/src/lj_trace.c > index 84b957c6..d7a78d4d 100644 > --- a/src/lj_trace.c > +++ b/src/lj_trace.c > diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua > index cb271784..555b6b3b 100755 > --- a/tools/sysprof/parse.lua > +++ b/tools/sysprof/parse.lua > @@ -33,7 +33,9 @@ M.FRAME = { > @@ -128,8 +130,14 @@ local function parse_trace(reader, event, symbols) > -- parse_lua_callchain(reader, event) > end > > -local function parse_symtab(reader, symbols) > - symtab.parse_sym_cfunc(reader, symbols) > +local function parse_symtab(reader, symbols, vmstate) > + if vmstate == SYMTAB_LFUNC_EVENT then > + symtab.parse_sym_lfunc(reader, symbols) > + elseif vmstate == SYMTAB_CFUNC_EVENT then > + symtab.parse_sym_cfunc(reader, symbols) > + elseif vmstate == SYMTAB_TRACE_EVENT then > + symtab.parse_sym_trace(reader, symbols) > + end Nit: may be we should add | else | error('Unknown symtab event') | end check here. Feel free to ignore. > end > > local event_parsers = { > @@ -152,8 +160,8 @@ local function parse_event(reader, events, symbols) > if vmstate == STREAM_END then > -- TODO: samples & overruns > return false > - elseif vmstate == SYMTAB_EVENT then > - parse_symtab(reader, symbols) > + elseif SYMTAB_LFUNC_EVENT <= vmstate and vmstate <= SYMTAB_TRACE_EVENT then > + parse_symtab(reader, symbols, vmstate) > return true > end > > -- > 2.35.1 > -- Best regards, Sergey Kaplun