[Tarantool-patches] [PATCH] sysprof: enrich symtab on a new trace or a proto
Sergey Kaplun
skaplun at tarantool.org
Thu May 19 17:07:29 MSK 2022
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
<snipped>
> 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
<snipped>
> 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)
<snipped>
> 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
<snipped>
> 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 = {
<snipped>
> @@ -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
More information about the Tarantool-patches
mailing list