sysprof: enrich symtab on a new trace or a proto

mkokryashkin · igormunkin · commit 9d78aa1d5964 · 2022-05-20T08:03:48.000+03:00
This commit adds functionality introduced in 0847e71 ('memprof: enrich symtab when meeting new prototype') and 0243fb7 ('memprof: enrich symtab when new trace is compiled') for 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 by setting the sysprof's internal state to `SPS_IDLE`. Follows up tarantool/tarantool#781 Reviewed-by: Sergey Kaplun <skaplun@tarantool.org> Reviewed-by: Igor Munkin <imun@tarantool.org> Signed-off-by: Igor Munkin <imun@tarantool.org>
diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original
@@ -59,7 +59,8 @@ lj_bc.o: lj_bc.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h lj_bc.h \
 lj_bcread.o: lj_bcread.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
  lj_gc.h lj_err.h lj_errmsg.h lj_buf.h lj_str.h lj_tab.h lj_bc.h \
  lj_ctype.h lj_cdata.h lualib.h lj_lex.h lj_bcdump.h lj_state.h \
- lj_strfmt.h lj_memprof.h lj_wbuf.h
+ lj_strfmt.h lj_memprof.h lj_wbuf.h lmisclib.h lj_sysprof.h lj_jit.h \
+ lj_ir.h
 lj_bcwrite.o: lj_bcwrite.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
  lj_gc.h lj_buf.h lj_str.h lj_bc.h lj_ctype.h lj_dispatch.h lj_jit.h \
  lj_ir.h lj_strfmt.h lj_bcdump.h lj_lex.h lj_err.h lj_errmsg.h lj_vm.h
@@ -175,7 +176,8 @@ lj_opt_split.o: lj_opt_split.c lj_obj.h lua.h luaconf.h lj_def.h \
 lj_parse.o: lj_parse.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
  lj_gc.h lj_err.h lj_errmsg.h lj_debug.h lj_buf.h lj_str.h lj_tab.h \
  lj_func.h lj_state.h lj_bc.h lj_ctype.h lj_strfmt.h lj_lex.h lj_parse.h \
- lj_vm.h lj_vmevent.h lj_memprof.h lj_wbuf.h
+ lj_vm.h lj_vmevent.h lj_memprof.h lj_wbuf.h lmisclib.h lj_sysprof.h \
+ lj_jit.h lj_ir.h
 lj_profile.o: lj_profile.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
  lj_buf.h lj_gc.h lj_str.h lj_frame.h lj_bc.h lj_debug.h lj_dispatch.h \
  lj_jit.h lj_ir.h lj_trace.h lj_traceerr.h lj_profile.h \
@@ -208,16 +210,17 @@ lj_strscan.o: lj_strscan.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
 lj_symtab.o: lj_symtab.c lj_symtab.h lj_wbuf.h lj_def.h lua.h luaconf.h \
  lj_obj.h lj_arch.h
 lj_sysprof.o: lj_sysprof.c lj_arch.h lua.h luaconf.h lj_sysprof.h \
- lj_obj.h lj_def.h lmisclib.h lj_debug.h lj_dispatch.h lj_bc.h lj_jit.h \
- lj_ir.h lj_frame.h lj_trace.h lj_traceerr.h lj_wbuf.h lj_profile_timer.h \
+ lj_jit.h lj_obj.h lj_def.h lj_ir.h lmisclib.h lj_debug.h lj_dispatch.h \
+ lj_bc.h lj_frame.h lj_trace.h lj_traceerr.h lj_wbuf.h lj_profile_timer.h \
  lj_symtab.h
 lj_tab.o: lj_tab.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h lj_gc.h \
  lj_err.h lj_errmsg.h lj_tab.h
 lj_trace.o: lj_trace.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
  lj_gc.h lj_err.h lj_errmsg.h lj_debug.h lj_str.h lj_frame.h lj_bc.h \
  lj_state.h lj_ir.h lj_jit.h lj_iropt.h lj_mcode.h lj_trace.h \
  lj_dispatch.h lj_traceerr.h lj_snap.h lj_gdbjit.h lj_record.h lj_asm.h \
- lj_vm.h lj_vmevent.h lj_target.h lj_target_*.h lj_memprof.h lj_wbuf.h
+ lj_vm.h lj_vmevent.h lj_target.h lj_target_*.h lj_memprof.h lj_wbuf.h \
+ lmisclib.h lj_sysprof.h
 lj_udata.o: lj_udata.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
  lj_gc.h lj_udata.h
 lj_utils_leb128.o: lj_utils_leb128.c lj_utils.h lj_def.h lua.h luaconf.h
diff --git a/src/lj_bcread.c b/src/lj_bcread.c
@@ -25,6 +25,9 @@
 #if LJ_HASMEMPROF
 #include "lj_memprof.h"
 #endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
 
 /* Reuse some lexer fields for our own purposes. */
 #define bcread_flags(ls)	ls->level
@@ -390,6 +393,10 @@ GCproto *lj_bcread_proto(LexState *ls)
   lj_memprof_add_proto(pt);
 #endif
 
+#if LJ_HASSYSPROF
+  lj_sysprof_add_proto(pt);
+#endif
+
   return pt;
 }
 
diff --git a/src/lj_parse.c b/src/lj_parse.c
@@ -30,6 +30,9 @@
 #if LJ_HASMEMPROF
 #include "lj_memprof.h"
 #endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
 
 /* -- Parser structures and definitions ----------------------------------- */
 
@@ -1598,6 +1601,9 @@ static GCproto *fs_finish(LexState *ls, BCLine line)
 #if LJ_HASMEMPROF
   lj_memprof_add_proto(pt);
 #endif
+#if LJ_HASSYSPROF
+  lj_sysprof_add_proto(pt);
+#endif
 
   L->top--;  /* Pop table of constants. */
   ls->vtop = fs->vbase;  /* Reset variable stack. */
diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
@@ -236,7 +236,7 @@ static void stream_guest(struct sysprof *sp, uint32_t vmstate)
 static void stream_host(struct sysprof *sp, uint32_t vmstate)
 {
   struct lua_State *L = gco2th(gcref(sp->g->cur_L));
-  lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_EVENT, L);
+  lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_CFUNC_EVENT, L);
   lj_wbuf_addbyte(&sp->out, (uint8_t)vmstate);
   stream_backtrace_host(sp);
 }
@@ -494,6 +494,40 @@ int lj_sysprof_report(struct luam_Sysprof_Counters *counters)
   return PROFILE_SUCCESS;
 }
 
+void lj_sysprof_add_proto(const struct GCproto *pt)
+{
+  struct sysprof *sp = &sysprof;
+
+  if (sp->state != SPS_PROFILE)
+    return;
+
+  /*
+  ** XXX: Avoid sampling during the symtab extension. That shouldn't have any
+  ** significant effect on profile precision, but if it does, it's better to
+  ** implement an async-safe queue for the symtab events.
+  */
+  sp->state = SPS_IDLE;
+  lj_wbuf_addbyte(&sp->out, LJP_SYMTAB_LFUNC_EVENT);
+  lj_symtab_dump_proto(&sp->out, pt);
+  sp->state = SPS_PROFILE;
+}
+
+#if LJ_HASJIT
+void lj_sysprof_add_trace(const struct GCtrace *tr)
+{
+  struct sysprof *sp = &sysprof;
+
+  if (sp->state != SPS_PROFILE)
+    return;
+
+  /* See the comment about the sysprof state above. */
+  sp->state = SPS_IDLE;
+  lj_wbuf_addbyte(&sp->out, LJP_SYMTAB_TRACE_EVENT);
+  lj_symtab_dump_trace(&sp->out, tr);
+  sp->state = SPS_PROFILE;
+}
+#endif /* LJ_HASJIT */
+
 #else /* LJ_HASSYSPROF */
 
 int lj_sysprof_configure(const struct luam_Sysprof_Config *config)
@@ -520,4 +554,16 @@ int lj_sysprof_report(struct luam_Sysprof_Counters *counters)
   return PROFILE_ERRUSE;
 }
 
+void lj_sysprof_add_proto(const struct GCproto *pt)
+{
+  UNUSED(pt);
+}
+
+#if LJ_HASJIT
+void lj_sysprof_add_trace(const struct GCtrace *tr)
+{
+  UNUSED(tr);
+}
+#endif /* LJ_HASJIT */
+
 #endif /* LJ_HASSYSPROF */
diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h
@@ -15,10 +15,13 @@
 #ifndef _LJ_SYSPROF_H
 #define _LJ_SYSPROF_H
 
+#if LJ_HASJIT
+#include "lj_jit.h"
+#endif
 #include "lj_obj.h"
 #include "lmisclib.h"
 
-#define LJP_FORMAT_VERSION 0x1
+#define LJP_FORMAT_VERSION 0x2
 
 /*
 ** Event stream format:
@@ -80,9 +83,13 @@
 #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)
 #define LJP_EPILOGUE_BYTE 0x80
 
+LJ_STATIC_ASSERT(LJ_VMST__MAX <= LJP_SYMTAB_LFUNC_EVENT);
+
 int lj_sysprof_configure(const struct luam_Sysprof_Config *config);
 
 int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt);
@@ -91,4 +98,10 @@ 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);
+
+#if LJ_HASJIT
+void lj_sysprof_add_trace(const struct GCtrace *tr);
+#endif /* LJ_HASJIT */
+
 #endif
diff --git a/src/lj_trace.c b/src/lj_trace.c
@@ -33,6 +33,9 @@
 #if LJ_HASMEMPROF
 #include "lj_memprof.h"
 #endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
 
 /* -- Error handling ------------------------------------------------------ */
 
@@ -171,6 +174,10 @@ static void trace_save(jit_State *J, GCtrace *T)
 #if LJ_HASMEMPROF
   lj_memprof_add_trace(T);
 #endif
+
+#if LJ_HASSYSPROF
+  lj_sysprof_add_trace(T);
+#endif
 }
 
 void LJ_FASTCALL lj_trace_free(global_State *g, GCtrace *T)
diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
@@ -6,7 +6,7 @@ local symtab = require "utils.symtab"
 local string_format = string.format
 
 local LJP_MAGIC = "ljp"
-local LJP_CURRENT_VERSION = 1
+local LJP_CURRENT_VERSION = 2
 
 local M = {}
 
@@ -33,7 +33,9 @@ M.FRAME = {
 }
 
 local STREAM_END = 0x80
-local SYMTAB_EVENT = 10
+local SYMTAB_LFUNC_EVENT = 10
+local SYMTAB_CFUNC_EVENT = 11
+local SYMTAB_TRACE_EVENT = 12
 
 local function new_event()
   return {
@@ -128,8 +130,16 @@ 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)
+  else
+    error("Unknown symtab event")
+  end
 end
 
 local event_parsers = {
@@ -152,8 +162,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
 
