[Tarantool-patches] [PATCH luajit v2 0/3] memprof: enrich symtab when meeting new prototype

[Tarantool-patches] [PATCH luajit v2 0/3] memprof: enrich symtab when meeting new prototype

[Mikhail Shishatskiy via Tarantool-patches]

Patchset v2 with some adjustments based on previous patch series,
and several fixes with improvements:

- Removed "change order of modules patch";
- Changed order of patches so that patch with symtab enriching
  works correctly straingt away, and not after applying "on the spot
  demangling" patch;
- Changed flag representing that object has been dumped to an epoch
  counter to respect consequent memprof launches.

Issue: https://github.com/tarantool/tarantool/issues/5815
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated

Mikhail Shishatskiy (3):
  memprof: add symbol epochs
  memprof: enrich symtab when meeting new prototype
  memprof: substitute long proto names with aliases

 src/lj_bcread.c                               |  3 +
 src/lj_memprof.c                              | 35 +++++++---
 src/lj_memprof.h                              | 13 +++-
 src/lj_obj.h                                  | 10 +++
 src/lj_parse.c                                |  3 +
 src/lj_state.c                                |  3 +
 .../misclib-memprof-lapi.test.lua             | 55 ++++++++++++---
 tools/memprof.lua                             |  1 +
 tools/memprof/humanize.lua                    | 14 ++++
 tools/memprof/parse.lua                       | 51 ++++++++------
 tools/utils/symtab.lua                        | 68 ++++++++++++++-----
 11 files changed, 196 insertions(+), 60 deletions(-)

-- 
2.33.1

[Tarantool-patches] [PATCH luajit v2 1/3] memprof: add symbol epochs

[Mikhail Shishatskiy via Tarantool-patches]

There may be situations, when new prototype or trace has the same
address than the previously dumped one. In this case, the new symtab
entry will shadow the preceeding, and if we demangle symbols when all
the events are parsed, we can get irrelevant information.

For this reason, the symtab now stores not a single entry, but array of
entries for each location. The structure <loc> was extended by the field
<epoch> representing the generation of symtab entry associated with this
particular location entry. Also, the <utils/symtab.lua> API has extended
by function symtab.new_loc(symbols, addr, line, traceno) so that it can
assign an epoch for a new location.

Part of tarantool/tarantool#5815
---

Issue: https://github.com/tarantool/tarantool/issues/5815
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated

 .../misclib-memprof-lapi.test.lua             | 12 ++--
 tools/memprof/parse.lua                       | 41 +++++++-------
 tools/utils/symtab.lua                        | 55 +++++++++++++------
 3 files changed, 67 insertions(+), 41 deletions(-)

diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 7919c61f..dd973f2a 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -70,7 +70,7 @@ local function generate_parsed_output(payload)
 
   local reader = bufread.new(TMP_BINFILE)
   local symbols = symtab.parse(reader)
-  local events = memprof.parse(reader)
+  local events = memprof.parse(reader, symbols)
 
   -- We don't need it any more.
   os.remove(TMP_BINFILE)
@@ -86,13 +86,15 @@ local function fill_ev_type(events, symbols, event_type)
   for _, event in pairs(events[event_type]) do
     local addr = event.loc.addr
     local traceno = event.loc.traceno
+    local epoch = event.loc.epoch
 
     if traceno ~= 0 and symbols.trace[traceno] then
-      local trace_loc = symbols.trace[traceno].start
+      local trace_loc = symbols.trace[traceno][epoch].start
       addr = trace_loc.addr
+      epoch = trace_loc.epoch
       ev_type.trace[traceno] = {
         name = string.format("TRACE [%d] %s:%d",
-          traceno, symbols.lfunc[addr].source, trace_loc.line
+          traceno, symbols.lfunc[addr][epoch].source, trace_loc.line
         ),
         num = event.num,
       }
@@ -104,7 +106,9 @@ local function fill_ev_type(events, symbols, event_type)
     elseif symbols.lfunc[addr] then
       ev_type.line[event.loc.line] = {
         name = string.format(
-          "%s:%d", symbols.lfunc[addr].source, symbols.lfunc[addr].linedefined
+          "%s:%d",
+          symbols.lfunc[addr][epoch].source,
+          symbols.lfunc[addr][epoch].linedefined
         ),
         num = event.num,
       }
diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index 47dbaee4..be5844a4 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -64,29 +64,28 @@ local function link_to_previous(heap_chunk, e, nsize)
   end
 end
 
-local function parse_location(reader, asource)
-  local loc = {
-    addr = 0,
-    line = 0,
-    traceno = 0,
-  }
+local function parse_location(reader, asource, symbols)
+  local addr = 0
+  local line = 0
+  local traceno = 0
   if asource == ASOURCE_INT then -- luacheck: ignore
   elseif asource == ASOURCE_CFUNC then
-    loc.addr = reader:read_uleb128()
+    addr = reader:read_uleb128()
   elseif asource == ASOURCE_LFUNC then
-    loc.addr = reader:read_uleb128()
-    loc.line = reader:read_uleb128()
+    addr = reader:read_uleb128()
+    line = reader:read_uleb128()
   elseif asource == ASOURCE_TRACE then
-    loc.traceno = reader:read_uleb128()
-    loc.addr = reader:read_uleb128()
+    traceno = reader:read_uleb128()
+    addr = reader:read_uleb128()
   else
     error("Unknown asource "..asource)
   end
+  local loc = symtab.new_loc(symbols, addr, line, traceno)
   return symtab.id(loc), loc
 end
 
-local function parse_alloc(reader, asource, events, heap)
-  local id, loc = parse_location(reader, asource)
+local function parse_alloc(reader, asource, events, heap, symbols)
+  local id, loc = parse_location(reader, asource, symbols)
 
   local naddr = reader:read_uleb128()
   local nsize = reader:read_uleb128()
@@ -101,8 +100,8 @@ local function parse_alloc(reader, asource, events, heap)
   heap[naddr] = {nsize, id, loc}
 end
 
-local function parse_realloc(reader, asource, events, heap)
-  local id, loc = parse_location(reader, asource)
+local function parse_realloc(reader, asource, events, heap, symbols)
+  local id, loc = parse_location(reader, asource, symbols)
 
   local oaddr = reader:read_uleb128()
   local osize = reader:read_uleb128()
@@ -123,8 +122,8 @@ local function parse_realloc(reader, asource, events, heap)
   heap[naddr] = {nsize, id, loc}
 end
 
-local function parse_free(reader, asource, events, heap)
-  local id, loc = parse_location(reader, asource)
+local function parse_free(reader, asource, events, heap, symbols)
+  local id, loc = parse_location(reader, asource, symbols)
 
   local oaddr = reader:read_uleb128()
   local osize = reader:read_uleb128()
@@ -157,7 +156,7 @@ local function ev_header_split(evh)
   return band(evh, AEVENT_MASK), band(evh, ASOURCE_MASK)
 end
 
-local function parse_event(reader, events)
+local function parse_event(reader, events, symbols)
   local ev_header = reader:read_octet()
 
   assert(ev_header_is_valid(ev_header), "Bad ev_header "..ev_header)
@@ -171,12 +170,12 @@ local function parse_event(reader, events)
 
   assert(parser, "Bad aevent "..aevent)
 
-  parser.parse(reader, asource, events[parser.evname], events.heap)
+  parser.parse(reader, asource, events[parser.evname], events.heap, symbols)
 
   return true
 end
 
-function M.parse(reader)
+function M.parse(reader, symbols)
   local events = {
     alloc = {},
     realloc = {},
@@ -201,7 +200,7 @@ function M.parse(reader)
     ))
   end
 
-  while parse_event(reader, events) do
+  while parse_event(reader, events, symbols) do
     -- Empty body.
   end
 
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index c7fcf77c..c758b67e 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -19,16 +19,37 @@ local SYMTAB_TRACE = 1
 
 local M = {}
 
+function M.new_loc(symtab, addr, line, traceno)
+  local loc = {
+    addr = addr,
+    line = line,
+    traceno = traceno,
+    epoch = 1,
+  }
+
+  if traceno ~= 0 and symtab.trace[traceno] then
+    loc.epoch = #symtab.trace[traceno]
+  elseif symtab.lfunc[addr] then
+    loc.epoch = #symtab.lfunc[addr]
+  end
+
+  return loc
+end
+
 -- Parse a single entry in a symtab: lfunc symbol.
 local function parse_sym_lfunc(reader, symtab)
   local sym_addr = reader:read_uleb128()
   local sym_chunk = reader:read_string()
   local sym_line = reader:read_uleb128()
 
-  symtab.lfunc[sym_addr] = {
+  if not symtab.lfunc[sym_addr] then
+    symtab.lfunc[sym_addr] = {}
+  end
+
+  table.insert(symtab.lfunc[sym_addr], {
     source = sym_chunk,
     linedefined = sym_line,
-  }
+  })
 end
 
 local function parse_sym_trace(reader, symtab)
@@ -37,17 +58,14 @@ local function parse_sym_trace(reader, symtab)
   local sym_addr = reader:read_uleb128()
   local sym_line = reader:read_uleb128()
 
-  symtab.trace[traceno] = {
+  if not symtab.trace[traceno] then
+    symtab.trace[traceno] = {}
+  end
+
+  table.insert(symtab.trace[traceno], {
     addr = trace_addr,
-    -- The structure <start> is the same as the one
-    -- yielded from the <parse_location> function
-    -- in the <memprof/parse.lua> module.
-    start = {
-      addr = sym_addr,
-      line = sym_line,
-      traceno = 0,
-    },
-  }
+    start = M.new_loc(symtab, sym_addr, sym_line, 0)
+  })
 end
 
 local parsers = {
@@ -98,7 +116,9 @@ function M.parse(reader)
 end
 
 function M.id(loc)
-  return string_format("f%#xl%dt%d", loc.addr, loc.line, loc.traceno)
+  return string_format(
+    "f%#xl%dt%de%d", loc.addr, loc.line, loc.traceno, loc.epoch
+  )
 end
 
 local function demangle_trace(symtab, loc)
@@ -108,7 +128,8 @@ local function demangle_trace(symtab, loc)
   assert(traceno ~= 0, "Location is a trace")
 
   local trace_str = string_format("TRACE [%d] %#x", traceno, addr)
-  local trace = symtab.trace[traceno]
+  local epochs = symtab.trace[traceno]
+  local trace = epochs and epochs[loc.epoch]
 
   -- If trace, which was remembered in the symtab, has not
   -- been flushed, associate it with a proto, where trace
@@ -131,8 +152,10 @@ function M.demangle(symtab, loc)
     return "INTERNAL"
   end
 
-  if symtab.lfunc[addr] then
-    return string_format("%s:%d", symtab.lfunc[addr].source, loc.line)
+  if symtab.lfunc[addr] and symtab.lfunc[addr][loc.epoch] then
+    return string_format(
+      "%s:%d", symtab.lfunc[addr][loc.epoch].source, loc.line
+    )
   end
 
   return string_format("CFUNC %#x", addr)
-- 
2.33.1

Re: [Tarantool-patches] [PATCH luajit v2 1/3] memprof: add symbol epochs

[Sergey Kaplun via Tarantool-patches]

Hi!

Thanks for the patch!
Please, sorry for such long response.
LGTM, except a few nits regarding the commit message.

Also, maybe we should add some tests to the patch?
Case for traces looks easy, doesn't it?

On 02.12.21, Mikhail Shishatskiy wrote:
> There may be situations, when new prototype or trace has the same
> address than the previously dumped one. In this case, the new symtab
> entry will shadow the preceeding, and if we demangle symbols when all

Typo: s/preceeding/preceding one/

> the events are parsed, we can get irrelevant information.
> 
> For this reason, the symtab now stores not a single entry, but array of

Typo: s/array/the array/

> entries for each location. The structure <loc> was extended by the field
> <epoch> representing the generation of symtab entry associated with this
> particular location entry. Also, the <utils/symtab.lua> API has extended
> by function symtab.new_loc(symbols, addr, line, traceno) so that it can
> assign an epoch for a new location.
> 
> Part of tarantool/tarantool#5815
> ---

-- 
Best regards,
Sergey Kaplun

[Tarantool-patches] [PATCH luajit v2 2/3] memprof: enrich symtab when meeting new prototype

[Mikhail Shishatskiy via Tarantool-patches]

Previously, the profiler dumped all the function prototypes only
at the start of profiling. It was enough for most cases, but
sometimes we may want to investigate the memory profile of module
loading. In such a case, we expect memprof to dump new prototypes
from parsed source code.

This patch extends memprof's streaming format with entries providing
extra information about allocation source's symbols if they were not
streamed at the start of profiling. This event is called <event-symtab>
and precedes the allocation-related event. The format is the following:

| event-symtab := event-header sym?
| sym          := sym-lua
| sym-lua      := sym-addr sym-chunk sym-line

The `sym-addr`, `sym-chunk` and `sym-line` are the prototype's address,
its chunk name and line where it was defined.

To be able determine, if the allocation source is present in the symtab
or not, the `prof_epoch` counter added to the `GCproto` structure, which
can be compared to the `prof_epoch` added to the VM global state.
If object's epoch is less than global, the memprof will dymp symtab
entry with <event-symtab> and symchronize the object's counter with
the global.

Also, the profiler parser is adjusted to recognize entries described
above and enrich the symtab on the fly while parsing events. For this
reason, the <utils/symtab.lua> API has been changed: the function
`parse_sym_lfunc` became public to be available for the parser module.

Resolves tarantool/tarantool#5815
---

Issue: https://github.com/tarantool/tarantool/issues/5815
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated

 src/lj_bcread.c                               |  3 ++
 src/lj_memprof.c                              | 35 +++++++++++----
 src/lj_memprof.h                              | 13 +++++-
 src/lj_obj.h                                  | 10 +++++
 src/lj_parse.c                                |  3 ++
 src/lj_state.c                                |  3 ++
 .../misclib-memprof-lapi.test.lua             | 43 ++++++++++++++++---
 tools/memprof/parse.lua                       | 10 ++++-
 tools/utils/symtab.lua                        |  4 +-
 9 files changed, 105 insertions(+), 19 deletions(-)

diff --git a/src/lj_bcread.c b/src/lj_bcread.c
index 48c5e7c7..4e2c141b 100644
--- a/src/lj_bcread.c
+++ b/src/lj_bcread.c
@@ -353,6 +353,9 @@ GCproto *lj_bcread_proto(LexState *ls)
   pt->sizeuv = (uint8_t)sizeuv;
   pt->flags = (uint8_t)flags;
   pt->trace = 0;
+#if LJ_HASMEMPROF
+  pt->prof_epoch = 0;
+#endif
   setgcref(pt->chunkname, obj2gco(ls->chunkname));
 
   /* Close potentially uninitialized gap between bc and kgc. */
diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 2d779983..c154de93 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -66,10 +66,19 @@ static void dump_symtab_trace(struct lj_wbuf *out, const GCtrace *trace)
 
 #endif
 
+static void dump_symtab_proto(struct lj_wbuf *out, GCproto *pt,
+			      const global_State *g)
+{
+  lj_wbuf_addu64(out, (uintptr_t)pt);
+  lj_wbuf_addstring(out, proto_chunknamestr(pt));
+  lj_wbuf_addu64(out, (uint64_t)pt->firstline);
+  pt->prof_epoch = g->prof_epoch;
+}
+
 static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
 {
   const GCRef *iter = &g->gc.root;
-  const GCobj *o;
+  GCobj *o;
   const size_t ljs_header_len = sizeof(ljs_header) / sizeof(ljs_header[0]);
 
   /* Write prologue. */
@@ -78,11 +87,9 @@ static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
   while ((o = gcref(*iter)) != NULL) {
     switch (o->gch.gct) {
     case (~LJ_TPROTO): {
-      const GCproto *pt = gco2pt(o);
+      GCproto *pt = gco2pt(o);
       lj_wbuf_addbyte(out, SYMTAB_LFUNC);
-      lj_wbuf_addu64(out, (uintptr_t)pt);
-      lj_wbuf_addstring(out, proto_chunknamestr(pt));
-      lj_wbuf_addu64(out, (uint64_t)pt->firstline);
+      dump_symtab_proto(out, pt, g);
       break;
     }
     case (~LJ_TTRACE): {
@@ -140,6 +147,7 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
   */
   const BCLine line = lj_debug_frameline(L, fn, nextframe);
+  GCproto *pt = funcproto(fn);
 
   if (line < 0) {
     /*
@@ -151,11 +159,17 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
     ** We report such allocations as internal in order not to confuse users.
     */
     lj_wbuf_addbyte(out, aevent | ASOURCE_INT);
-  } else {
-    lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
-    lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
-    lj_wbuf_addu64(out, (uint64_t)line);
+    return;
   }
+
+  if (LJ_UNLIKELY(pt->prof_epoch != memprof.g->prof_epoch)) {
+    lj_wbuf_addbyte(out, AEVENT_SYMTAB | ASOURCE_LFUNC);
+    dump_symtab_proto(out, pt, memprof.g);
+  }
+
+  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
+  lj_wbuf_addu64(out, (uintptr_t)pt);
+  lj_wbuf_addu64(out, (uint64_t)line);
 }
 
 static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
@@ -329,6 +343,9 @@ int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt)
   mp->g = G(L);
   mp->state = MPS_PROFILE;
 
+  /* Increment the profiling epoch. */
+  mp->g->prof_epoch++;
+
   /* Init output. */
   lj_wbuf_init(&mp->out, mp_opt->writer, mp_opt->ctx, mp_opt->buf, mp_opt->len);
   dump_symtab(&mp->out, mp->g);
diff --git a/src/lj_memprof.h b/src/lj_memprof.h
index 395fb429..150e6b32 100644
--- a/src/lj_memprof.h
+++ b/src/lj_memprof.h
@@ -57,7 +57,7 @@
 #define SYMTAB_TRACE ((uint8_t)1)
 #define SYMTAB_FINAL ((uint8_t)0x80)
 
-#define LJM_CURRENT_FORMAT_VERSION 0x02
+#define LJM_CURRENT_FORMAT_VERSION 0x03
 
 /*
 ** Event stream format:
@@ -68,16 +68,21 @@
 ** prologue       := 'l' 'j' 'm' version reserved
 ** version        := <BYTE>
 ** reserved       := <BYTE> <BYTE> <BYTE>
-** event          := event-alloc | event-realloc | event-free
+** event          := event-alloc | event-realloc | event-free | event-symtab
 ** event-alloc    := event-header loc? naddr nsize
 ** event-realloc  := event-header loc? oaddr osize naddr nsize
 ** event-free     := event-header loc? oaddr osize
+** event-symtab   := event-header sym?
 ** event-header   := <BYTE>
 ** loc            := loc-lua | loc-c | loc-trace
 ** loc-lua        := sym-addr line-no
 ** loc-c          := sym-addr
 ** loc-trace      := trace-no trace-addr
+** sym            := sym-lua
+** sym-lua        := sym-addr sym-chunk sym-line
 ** sym-addr       := <ULEB128>
+** sym-chunk      := string
+** sym-line       := <ULEB128>
 ** line-no        := <ULEB128>
 ** trace-no       := <ULEB128>
 ** trace-addr     := <ULEB128>
@@ -85,6 +90,9 @@
 ** naddr          := <ULEB128>
 ** osize          := <ULEB128>
 ** nsize          := <ULEB128>
+** string         := string-len string-payload
+** string-len     := <ULEB128>
+** string-payload := <BYTE> {string-len}
 ** epilogue       := event-header
 **
 ** <BYTE>   :  A single byte (no surprises here)
@@ -104,6 +112,7 @@
 */
 
 /* Allocation events. */
+#define AEVENT_SYMTAB  ((uint8_t)0)
 #define AEVENT_ALLOC   ((uint8_t)1)
 #define AEVENT_FREE    ((uint8_t)2)
 #define AEVENT_REALLOC ((uint8_t)(AEVENT_ALLOC | AEVENT_FREE))
diff --git a/src/lj_obj.h b/src/lj_obj.h
index d26e60be..283f30f6 100644
--- a/src/lj_obj.h
+++ b/src/lj_obj.h
@@ -385,6 +385,13 @@ typedef struct GCproto {
   MRef lineinfo;	/* Compressed map from bytecode ins. to source line. */
   MRef uvinfo;		/* Upvalue names. */
   MRef varinfo;		/* Names and compressed extents of local variables. */
+#if LJ_HASMEMPROF
+  /*
+  ** Epoch indicating if this proto was dumped to the symbol table for the
+  ** current profiling session.
+  */
+  uint8_t prof_epoch;
+#endif
 } GCproto;
 
 /* Flags for prototype. */
@@ -674,6 +681,9 @@ typedef struct global_State {
   MRef jit_base;	/* Current JIT code L->base or NULL. */
   MRef ctype_state;	/* Pointer to C type state. */
   GCRef gcroot[GCROOT_MAX];  /* GC roots. */
+#if LJ_HASMEMPROF
+  uint8_t prof_epoch;	/* Current profiling epoch for this VM. */
+#endif
 } global_State;
 
 #define mainthread(g)	(&gcref(g->mainthref)->th)
diff --git a/src/lj_parse.c b/src/lj_parse.c
index a6325a76..9415665a 100644
--- a/src/lj_parse.c
+++ b/src/lj_parse.c
@@ -1577,6 +1577,9 @@ static GCproto *fs_finish(LexState *ls, BCLine line)
   pt->flags = (uint8_t)(fs->flags & ~(PROTO_HAS_RETURN|PROTO_FIXUP_RETURN));
   pt->numparams = fs->numparams;
   pt->framesize = fs->framesize;
+#if LJ_HASMEMPROF
+  pt->prof_epoch = 0;
+#endif
   setgcref(pt->chunkname, obj2gco(ls->chunkname));
 
   /* Close potentially uninitialized gap between bc and kgc. */
diff --git a/src/lj_state.c b/src/lj_state.c
index f82b1b5b..b4bfd573 100644
--- a/src/lj_state.c
+++ b/src/lj_state.c
@@ -221,6 +221,9 @@ LUA_API lua_State *lua_newstate(lua_Alloc f, void *ud)
   g->strempty.gct = ~LJ_TSTR;
   g->allocf = f;
   g->allocd = ud;
+#if LJ_HASMEMPROF
+  g->prof_epoch = 0;
+#endif
   setgcref(g->mainthref, obj2gco(L));
   setgcref(g->uvhead.prev, obj2gco(&g->uvhead));
   setgcref(g->uvhead.next, obj2gco(&g->uvhead));
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index dd973f2a..1c74b4d7 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -7,7 +7,7 @@ require("utils").skipcond(
 local tap = require("tap")
 
 local test = tap.test("misc-memprof-lapi")
-test:plan(4)
+test:plan(5)
 
 local jit_opt_default = {
     3, -- level
@@ -117,8 +117,9 @@ local function fill_ev_type(events, symbols, event_type)
   return ev_type
 end
 
-local function form_source_line(line)
-  return string.format("@%s:%d", arg[0], line)
+local function form_source_line(line, source)
+  source = source or ("@"..arg[0])
+  return string.format("%s:%d", source, line)
 end
 
 local function check_alloc_report(alloc, location, nevents)
@@ -126,10 +127,10 @@ local function check_alloc_report(alloc, location, nevents)
   local traceno = location.traceno
   if traceno then
     expected_name = string.format("TRACE [%d] ", traceno)..
-                    form_source_line(location.line)
+                    form_source_line(location.line, location.source)
     event = alloc.trace[traceno]
   else
-    expected_name = form_source_line(location.linedefined)
+    expected_name = form_source_line(location.linedefined, location.source)
     event = alloc.line[location.line]
   end
   assert(expected_name == event.name, ("got='%s', expected='%s'"):format(
@@ -223,6 +224,38 @@ test:test("stack-resize", function(subtest)
   misc.memprof.stop()
 end)
 
+-- Test for extending symtab with function prototypes
+-- while profiler is running.
+test:test("symtab-enriching", function(subtest)
+  subtest:plan(2)
+
+  local payload_str = [[
+  local M = {
+    tmp = string.rep("1", 100)  -- line 2.
+  }
+
+  function M.payload()
+    local str = string.rep("42", 100)  -- line 6.
+  end
+
+  return M
+  ]]
+
+  local symbols, events = generate_parsed_output(function()
+    local str_chunk = assert(loadstring(payload_str, 'str_chunk'))()
+    str_chunk.payload()
+  end)
+
+  local alloc = fill_ev_type(events, symbols, "alloc")
+
+  subtest:ok(check_alloc_report(
+    alloc, { source = 'str_chunk', line = 6, linedefined = 5 }, 1)
+  )
+  subtest:ok(check_alloc_report(
+    alloc, { source = 'str_chunk', line = 2, linedefined = 0 }, 1)
+  )
+end)
+
 -- Test profiler with enabled JIT.
 jit.on()
 
diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index be5844a4..38f76f00 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -13,10 +13,11 @@ local string_format = string.format
 local symtab = require "utils.symtab"
 
 local LJM_MAGIC = "ljm"
-local LJM_CURRENT_VERSION = 0x02
+local LJM_CURRENT_VERSION = 0x03
 
 local LJM_EPILOGUE_HEADER = 0x80
 
+local AEVENT_SYMTAB = 0
 local AEVENT_ALLOC = 1
 local AEVENT_FREE = 2
 local AEVENT_REALLOC = 3
@@ -140,7 +141,14 @@ local function parse_free(reader, asource, events, heap, symbols)
   heap[oaddr] = nil
 end
 
+local function parse_symtab(reader, asource, _, _, symbols)
+  if asource == ASOURCE_LFUNC then
+    symtab.parse_sym_lfunc(reader, symbols)
+  end
+end
+
 local parsers = {
+  [AEVENT_SYMTAB] = {evname = "symtab", parse = parse_symtab},
   [AEVENT_ALLOC] = {evname = "alloc", parse = parse_alloc},
   [AEVENT_FREE] = {evname = "free", parse = parse_free},
   [AEVENT_REALLOC] = {evname = "realloc", parse = parse_realloc},
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index c758b67e..00bab03a 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -37,7 +37,7 @@ function M.new_loc(symtab, addr, line, traceno)
 end
 
 -- Parse a single entry in a symtab: lfunc symbol.
-local function parse_sym_lfunc(reader, symtab)
+function M.parse_sym_lfunc(reader, symtab)
   local sym_addr = reader:read_uleb128()
   local sym_chunk = reader:read_string()
   local sym_line = reader:read_uleb128()
@@ -69,7 +69,7 @@ local function parse_sym_trace(reader, symtab)
 end
 
 local parsers = {
-  [SYMTAB_LFUNC] = parse_sym_lfunc,
+  [SYMTAB_LFUNC] = M.parse_sym_lfunc,
   [SYMTAB_TRACE] = parse_sym_trace,
 }
 
-- 
2.33.1

Re: [Tarantool-patches] [PATCH luajit v2 2/3] memprof: enrich symtab when meeting new prototype

[Mikhail Shishatskiy via Tarantool-patches]

Updated patch in line with what we discussed with Sergey offline:

New commit message:
===========================================================
memprof: enrich symtab when meeting new prototype

Previously, the profiler dumped all the function prototypes only
at the start of profiling. It was enough for most cases, but
sometimes we may want to investigate the memory profile of module
loading. In such a case, we expect memprof to dump new prototypes
from parsed source code.

This patch extends memprof's streaming format with entries providing
extra information about allocation source's symbols if they were not
streamed at the start of profiling. This event is called <event-symtab>
and precedes the allocation-related event. The format is the following:

| event-symtab := event-header sym?
| sym          := sym-lua
| sym-lua      := sym-addr sym-chunk sym-line

The `sym-addr`, `sym-chunk` and `sym-line` are the prototype's address,
its chunk name and line where it was defined.

The profiler dumps new prototypes to the symbol table right after the
creation of a GCproto object.

Also, the profiler parser is adjusted to recognize entries described
above and enrich the symtab on the fly while parsing events. For this
reason, the <utils/symtab.lua> API has been changed: the function
`parse_sym_lfunc` became public to be available for the parser module.

Resolves tarantool/tarantool#5815
===========================================================

New diff:
===========================================================
diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original
index faa44a0b..82f34315 100644
--- a/src/Makefile.dep.original
+++ b/src/Makefile.dep.original
@@ -59,7 +59,7 @@ 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_strfmt.h lj_memprof.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 +175,7 @@ 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_vm.h lj_vmevent.h lj_memprof.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 luajit.h
diff --git a/src/lj_bcread.c b/src/lj_bcread.c
index 48c5e7c7..cb08599d 100644
--- a/src/lj_bcread.c
+++ b/src/lj_bcread.c
@@ -22,6 +22,9 @@
  #include "lj_bcdump.h"
  #include "lj_state.h"
  #include "lj_strfmt.h"
+#if LJ_HASMEMPROF
+#include "lj_memprof.h"
+#endif

  /* Reuse some lexer fields for our own purposes. */
  #define bcread_flags(ls)	ls->level
@@ -381,6 +384,12 @@ GCproto *lj_bcread_proto(LexState *ls)
      setmref(pt->uvinfo, NULL);
      setmref(pt->varinfo, NULL);
    }
+
+  /* Add a new prototype to the profiler. */
+#if LJ_HASMEMPROF
+  lj_memprof_add_proto(pt);
+#endif
+
    return pt;
  }

diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 2d779983..9bb2483e 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -66,6 +66,14 @@ static void dump_symtab_trace(struct lj_wbuf *out, const GCtrace *trace)

  #endif

+static void dump_symtab_proto(struct lj_wbuf *out, const GCproto *pt,
+			      const global_State *g)
+{
+  lj_wbuf_addu64(out, (uintptr_t)pt);
+  lj_wbuf_addstring(out, proto_chunknamestr(pt));
+  lj_wbuf_addu64(out, (uint64_t)pt->firstline);
+}
+
  static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
  {
    const GCRef *iter = &g->gc.root;
@@ -80,9 +88,7 @@ static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
      case (~LJ_TPROTO): {
        const GCproto *pt = gco2pt(o);
        lj_wbuf_addbyte(out, SYMTAB_LFUNC);
-      lj_wbuf_addu64(out, (uintptr_t)pt);
-      lj_wbuf_addstring(out, proto_chunknamestr(pt));
-      lj_wbuf_addu64(out, (uint64_t)pt->firstline);
+      dump_symtab_proto(out, pt, g);
        break;
      }
      case (~LJ_TTRACE): {
@@ -140,6 +146,7 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
    ** -DLUAJIT_DISABLE_DEBUGINFO flag.
    */
    const BCLine line = lj_debug_frameline(L, fn, nextframe);
+  const GCproto *pt = funcproto(fn);

    if (line < 0) {
      /*
@@ -151,11 +158,17 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
      ** We report such allocations as internal in order not to confuse users.
      */
      lj_wbuf_addbyte(out, aevent | ASOURCE_INT);
-  } else {
-    lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
-    lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
-    lj_wbuf_addu64(out, (uint64_t)line);
+    return;
    }
+
+  /*
+  ** As a prototype is a source of an allocation, it has
+  ** already been inserted into the symtab: on the start
+  ** of the profiling or right after the creation.
+  */
+  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
+  lj_wbuf_addu64(out, (uintptr_t)pt);
+  lj_wbuf_addu64(out, (uint64_t)line);
  }

  static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
@@ -414,6 +427,17 @@ errio:
    return PROFILE_ERRIO;
  }

+void lj_memprof_add_proto(const struct GCproto *pt)
+{
+  struct memprof *mp = &memprof;
+
+  if (mp->state != MPS_PROFILE)
+    return;
+
+  lj_wbuf_addbyte(&mp->out, AEVENT_SYMTAB | ASOURCE_LFUNC);
+  dump_symtab_proto(&mp->out, pt, mp->g);
+}
+
  #else /* LJ_HASMEMPROF */

  int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt)
@@ -430,4 +454,9 @@ int lj_memprof_stop(struct lua_State *L)
    return PROFILE_ERRUSE;
  }

+void lj_memprof_add_proto(const struct GCproto *pt)
+{
+  UNUSED(pt);
+}
+
  #endif /* LJ_HASMEMPROF */
diff --git a/src/lj_memprof.h b/src/lj_memprof.h
index 395fb429..eda8ca0c 100644
--- a/src/lj_memprof.h
+++ b/src/lj_memprof.h
@@ -57,7 +57,7 @@
  #define SYMTAB_TRACE ((uint8_t)1)
  #define SYMTAB_FINAL ((uint8_t)0x80)

-#define LJM_CURRENT_FORMAT_VERSION 0x02
+#define LJM_CURRENT_FORMAT_VERSION 0x03

  /*
  ** Event stream format:
@@ -68,16 +68,21 @@
  ** prologue       := 'l' 'j' 'm' version reserved
  ** version        := <BYTE>
  ** reserved       := <BYTE> <BYTE> <BYTE>
-** event          := event-alloc | event-realloc | event-free
+** event          := event-alloc | event-realloc | event-free | event-symtab
  ** event-alloc    := event-header loc? naddr nsize
  ** event-realloc  := event-header loc? oaddr osize naddr nsize
  ** event-free     := event-header loc? oaddr osize
+** event-symtab   := event-header sym?
  ** event-header   := <BYTE>
  ** loc            := loc-lua | loc-c | loc-trace
  ** loc-lua        := sym-addr line-no
  ** loc-c          := sym-addr
  ** loc-trace      := trace-no trace-addr
+** sym            := sym-lua
+** sym-lua        := sym-addr sym-chunk sym-line
  ** sym-addr       := <ULEB128>
+** sym-chunk      := string
+** sym-line       := <ULEB128>
  ** line-no        := <ULEB128>
  ** trace-no       := <ULEB128>
  ** trace-addr     := <ULEB128>
@@ -85,6 +90,9 @@
  ** naddr          := <ULEB128>
  ** osize          := <ULEB128>
  ** nsize          := <ULEB128>
+** string         := string-len string-payload
+** string-len     := <ULEB128>
+** string-payload := <BYTE> {string-len}
  ** epilogue       := event-header
  **
  ** <BYTE>   :  A single byte (no surprises here)
@@ -104,6 +112,7 @@
  */

  /* Allocation events. */
+#define AEVENT_SYMTAB  ((uint8_t)0)
  #define AEVENT_ALLOC   ((uint8_t)1)
  #define AEVENT_FREE    ((uint8_t)2)
  #define AEVENT_REALLOC ((uint8_t)(AEVENT_ALLOC | AEVENT_FREE))
@@ -148,6 +157,7 @@ struct lj_memprof_options {

  /* Avoid to provide additional interfaces described in other headers. */
  struct lua_State;
+struct GCproto;

  /*
  ** Starts profiling. Returns PROFILE_SUCCESS on success and one of
@@ -164,4 +174,10 @@ int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt);
  */
  int lj_memprof_stop(struct lua_State *L);

+/*
+** Enriches profiler symbol table with a new proto, if profiler
+** is running.
+*/
+void lj_memprof_add_proto(const struct GCproto *pt);
+
  #endif
diff --git a/src/lj_parse.c b/src/lj_parse.c
index a6325a76..30b0caa0 100644
--- a/src/lj_parse.c
+++ b/src/lj_parse.c
@@ -27,6 +27,9 @@
  #include "lj_parse.h"
  #include "lj_vm.h"
  #include "lj_vmevent.h"
+#if LJ_HASMEMPROF
+#include "lj_memprof.h"
+#endif

  /* -- Parser structures and definitions ----------------------------------- */

@@ -1591,6 +1594,11 @@ static GCproto *fs_finish(LexState *ls, BCLine line)
      setprotoV(L, L->top++, pt);
    );

+  /* Add a new prototype to the profiler. */
+#if LJ_HASMEMPROF
+  lj_memprof_add_proto(pt);
+#endif
+
    L->top--;  /* Pop table of constants. */
    ls->vtop = fs->vbase;  /* Reset variable stack. */
    ls->fs = fs->prev;
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index dd973f2a..1c74b4d7 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -7,7 +7,7 @@ require("utils").skipcond(
  local tap = require("tap")

  local test = tap.test("misc-memprof-lapi")
-test:plan(4)
+test:plan(5)

  local jit_opt_default = {
      3, -- level
@@ -117,8 +117,9 @@ local function fill_ev_type(events, symbols, event_type)
    return ev_type
  end

-local function form_source_line(line)
-  return string.format("@%s:%d", arg[0], line)
+local function form_source_line(line, source)
+  source = source or ("@"..arg[0])
+  return string.format("%s:%d", source, line)
  end

  local function check_alloc_report(alloc, location, nevents)
@@ -126,10 +127,10 @@ local function check_alloc_report(alloc, location, nevents)
    local traceno = location.traceno
    if traceno then
      expected_name = string.format("TRACE [%d] ", traceno)..
-                    form_source_line(location.line)
+                    form_source_line(location.line, location.source)
      event = alloc.trace[traceno]
    else
-    expected_name = form_source_line(location.linedefined)
+    expected_name = form_source_line(location.linedefined, location.source)
      event = alloc.line[location.line]
    end
    assert(expected_name == event.name, ("got='%s', expected='%s'"):format(
@@ -223,6 +224,38 @@ test:test("stack-resize", function(subtest)
    misc.memprof.stop()
  end)

+-- Test for extending symtab with function prototypes
+-- while profiler is running.
+test:test("symtab-enriching", function(subtest)
+  subtest:plan(2)
+
+  local payload_str = [[
+  local M = {
+    tmp = string.rep("1", 100)  -- line 2.
+  }
+
+  function M.payload()
+    local str = string.rep("42", 100)  -- line 6.
+  end
+
+  return M
+  ]]
+
+  local symbols, events = generate_parsed_output(function()
+    local str_chunk = assert(loadstring(payload_str, 'str_chunk'))()
+    str_chunk.payload()
+  end)
+
+  local alloc = fill_ev_type(events, symbols, "alloc")
+
+  subtest:ok(check_alloc_report(
+    alloc, { source = 'str_chunk', line = 6, linedefined = 5 }, 1)
+  )
+  subtest:ok(check_alloc_report(
+    alloc, { source = 'str_chunk', line = 2, linedefined = 0 }, 1)
+  )
+end)
+
  -- Test profiler with enabled JIT.
  jit.on()

diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index be5844a4..38f76f00 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -13,10 +13,11 @@ local string_format = string.format
  local symtab = require "utils.symtab"

  local LJM_MAGIC = "ljm"
-local LJM_CURRENT_VERSION = 0x02
+local LJM_CURRENT_VERSION = 0x03

  local LJM_EPILOGUE_HEADER = 0x80

+local AEVENT_SYMTAB = 0
  local AEVENT_ALLOC = 1
  local AEVENT_FREE = 2
  local AEVENT_REALLOC = 3
@@ -140,7 +141,14 @@ local function parse_free(reader, asource, events, heap, symbols)
    heap[oaddr] = nil
  end

+local function parse_symtab(reader, asource, _, _, symbols)
+  if asource == ASOURCE_LFUNC then
+    symtab.parse_sym_lfunc(reader, symbols)
+  end
+end
+
  local parsers = {
+  [AEVENT_SYMTAB] = {evname = "symtab", parse = parse_symtab},
    [AEVENT_ALLOC] = {evname = "alloc", parse = parse_alloc},
    [AEVENT_FREE] = {evname = "free", parse = parse_free},
    [AEVENT_REALLOC] = {evname = "realloc", parse = parse_realloc},
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index c758b67e..00bab03a 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -37,7 +37,7 @@ function M.new_loc(symtab, addr, line, traceno)
  end

  -- Parse a single entry in a symtab: lfunc symbol.
-local function parse_sym_lfunc(reader, symtab)
+function M.parse_sym_lfunc(reader, symtab)
    local sym_addr = reader:read_uleb128()
    local sym_chunk = reader:read_string()
    local sym_line = reader:read_uleb128()
@@ -69,7 +69,7 @@ local function parse_sym_trace(reader, symtab)
  end

  local parsers = {
-  [SYMTAB_LFUNC] = parse_sym_lfunc,
+  [SYMTAB_LFUNC] = M.parse_sym_lfunc,
    [SYMTAB_TRACE] = parse_sym_trace,
  }
===========================================================

On 02.12.2021 14:03, Mikhail Shishatskiy wrote:
>Previously, the profiler dumped all the function prototypes only
>at the start of profiling. It was enough for most cases, but
>sometimes we may want to investigate the memory profile of module
>loading. In such a case, we expect memprof to dump new prototypes
>from parsed source code.
>
>This patch extends memprof's streaming format with entries providing
>extra information about allocation source's symbols if they were not
>streamed at the start of profiling. This event is called <event-symtab>
>and precedes the allocation-related event. The format is the following:
>
>| event-symtab := event-header sym?
>| sym          := sym-lua
>| sym-lua      := sym-addr sym-chunk sym-line
>
>The `sym-addr`, `sym-chunk` and `sym-line` are the prototype's address,
>its chunk name and line where it was defined.
>
>To be able determine, if the allocation source is present in the symtab
>or not, the `prof_epoch` counter added to the `GCproto` structure, which
>can be compared to the `prof_epoch` added to the VM global state.
>If object's epoch is less than global, the memprof will dymp symtab
>entry with <event-symtab> and symchronize the object's counter with
>the global.
>
>Also, the profiler parser is adjusted to recognize entries described
>above and enrich the symtab on the fly while parsing events. For this
>reason, the <utils/symtab.lua> API has been changed: the function
>`parse_sym_lfunc` became public to be available for the parser module.
>
>Resolves tarantool/tarantool#5815
>---
>
>Issue: https://github.com/tarantool/tarantool/issues/5815
>Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
>Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated
>
> src/lj_bcread.c                               |  3 ++
> src/lj_memprof.c                              | 35 +++++++++++----
> src/lj_memprof.h                              | 13 +++++-
> src/lj_obj.h                                  | 10 +++++
> src/lj_parse.c                                |  3 ++
> src/lj_state.c                                |  3 ++
> .../misclib-memprof-lapi.test.lua             | 43 ++++++++++++++++---
> tools/memprof/parse.lua                       | 10 ++++-
> tools/utils/symtab.lua                        |  4 +-
> 9 files changed, 105 insertions(+), 19 deletions(-)
>
>diff --git a/src/lj_bcread.c b/src/lj_bcread.c
>index 48c5e7c7..4e2c141b 100644
>--- a/src/lj_bcread.c
>+++ b/src/lj_bcread.c
>@@ -353,6 +353,9 @@ GCproto *lj_bcread_proto(LexState *ls)
>   pt->sizeuv = (uint8_t)sizeuv;
>   pt->flags = (uint8_t)flags;
>   pt->trace = 0;
>+#if LJ_HASMEMPROF
>+  pt->prof_epoch = 0;
>+#endif
>   setgcref(pt->chunkname, obj2gco(ls->chunkname));
>
>   /* Close potentially uninitialized gap between bc and kgc. */
>diff --git a/src/lj_memprof.c b/src/lj_memprof.c
>index 2d779983..c154de93 100644
>--- a/src/lj_memprof.c
>+++ b/src/lj_memprof.c
>@@ -66,10 +66,19 @@ static void dump_symtab_trace(struct lj_wbuf *out, const GCtrace *trace)
>
> #endif
>
>+static void dump_symtab_proto(struct lj_wbuf *out, GCproto *pt,
>+			      const global_State *g)
>+{
>+  lj_wbuf_addu64(out, (uintptr_t)pt);
>+  lj_wbuf_addstring(out, proto_chunknamestr(pt));
>+  lj_wbuf_addu64(out, (uint64_t)pt->firstline);
>+  pt->prof_epoch = g->prof_epoch;
>+}
>+
> static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
> {
>   const GCRef *iter = &g->gc.root;
>-  const GCobj *o;
>+  GCobj *o;
>   const size_t ljs_header_len = sizeof(ljs_header) / sizeof(ljs_header[0]);
>
>   /* Write prologue. */
>@@ -78,11 +87,9 @@ static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
>   while ((o = gcref(*iter)) != NULL) {
>     switch (o->gch.gct) {
>     case (~LJ_TPROTO): {
>-      const GCproto *pt = gco2pt(o);
>+      GCproto *pt = gco2pt(o);
>       lj_wbuf_addbyte(out, SYMTAB_LFUNC);
>-      lj_wbuf_addu64(out, (uintptr_t)pt);
>-      lj_wbuf_addstring(out, proto_chunknamestr(pt));
>-      lj_wbuf_addu64(out, (uint64_t)pt->firstline);
>+      dump_symtab_proto(out, pt, g);
>       break;
>     }
>     case (~LJ_TTRACE): {
>@@ -140,6 +147,7 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
>   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
>   */
>   const BCLine line = lj_debug_frameline(L, fn, nextframe);
>+  GCproto *pt = funcproto(fn);
>
>   if (line < 0) {
>     /*
>@@ -151,11 +159,17 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
>     ** We report such allocations as internal in order not to confuse users.
>     */
>     lj_wbuf_addbyte(out, aevent | ASOURCE_INT);
>-  } else {
>-    lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
>-    lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
>-    lj_wbuf_addu64(out, (uint64_t)line);
>+    return;
>   }
>+
>+  if (LJ_UNLIKELY(pt->prof_epoch != memprof.g->prof_epoch)) {
>+    lj_wbuf_addbyte(out, AEVENT_SYMTAB | ASOURCE_LFUNC);
>+    dump_symtab_proto(out, pt, memprof.g);
>+  }
>+
>+  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
>+  lj_wbuf_addu64(out, (uintptr_t)pt);
>+  lj_wbuf_addu64(out, (uint64_t)line);
> }
>
> static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
>@@ -329,6 +343,9 @@ int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt)
>   mp->g = G(L);
>   mp->state = MPS_PROFILE;
>
>+  /* Increment the profiling epoch. */
>+  mp->g->prof_epoch++;
>+
>   /* Init output. */
>   lj_wbuf_init(&mp->out, mp_opt->writer, mp_opt->ctx, mp_opt->buf, mp_opt->len);
>   dump_symtab(&mp->out, mp->g);
>diff --git a/src/lj_memprof.h b/src/lj_memprof.h
>index 395fb429..150e6b32 100644
>--- a/src/lj_memprof.h
>+++ b/src/lj_memprof.h
>@@ -57,7 +57,7 @@
> #define SYMTAB_TRACE ((uint8_t)1)
> #define SYMTAB_FINAL ((uint8_t)0x80)
>
>-#define LJM_CURRENT_FORMAT_VERSION 0x02
>+#define LJM_CURRENT_FORMAT_VERSION 0x03
>
> /*
> ** Event stream format:
>@@ -68,16 +68,21 @@
> ** prologue       := 'l' 'j' 'm' version reserved
> ** version        := <BYTE>
> ** reserved       := <BYTE> <BYTE> <BYTE>
>-** event          := event-alloc | event-realloc | event-free
>+** event          := event-alloc | event-realloc | event-free | event-symtab
> ** event-alloc    := event-header loc? naddr nsize
> ** event-realloc  := event-header loc? oaddr osize naddr nsize
> ** event-free     := event-header loc? oaddr osize
>+** event-symtab   := event-header sym?
> ** event-header   := <BYTE>
> ** loc            := loc-lua | loc-c | loc-trace
> ** loc-lua        := sym-addr line-no
> ** loc-c          := sym-addr
> ** loc-trace      := trace-no trace-addr
>+** sym            := sym-lua
>+** sym-lua        := sym-addr sym-chunk sym-line
> ** sym-addr       := <ULEB128>
>+** sym-chunk      := string
>+** sym-line       := <ULEB128>
> ** line-no        := <ULEB128>
> ** trace-no       := <ULEB128>
> ** trace-addr     := <ULEB128>
>@@ -85,6 +90,9 @@
> ** naddr          := <ULEB128>
> ** osize          := <ULEB128>
> ** nsize          := <ULEB128>
>+** string         := string-len string-payload
>+** string-len     := <ULEB128>
>+** string-payload := <BYTE> {string-len}
> ** epilogue       := event-header
> **
> ** <BYTE>   :  A single byte (no surprises here)
>@@ -104,6 +112,7 @@
> */
>
> /* Allocation events. */
>+#define AEVENT_SYMTAB  ((uint8_t)0)
> #define AEVENT_ALLOC   ((uint8_t)1)
> #define AEVENT_FREE    ((uint8_t)2)
> #define AEVENT_REALLOC ((uint8_t)(AEVENT_ALLOC | AEVENT_FREE))
>diff --git a/src/lj_obj.h b/src/lj_obj.h
>index d26e60be..283f30f6 100644
>--- a/src/lj_obj.h
>+++ b/src/lj_obj.h
>@@ -385,6 +385,13 @@ typedef struct GCproto {
>   MRef lineinfo;	/* Compressed map from bytecode ins. to source line. */
>   MRef uvinfo;		/* Upvalue names. */
>   MRef varinfo;		/* Names and compressed extents of local variables. */
>+#if LJ_HASMEMPROF
>+  /*
>+  ** Epoch indicating if this proto was dumped to the symbol table for the
>+  ** current profiling session.
>+  */
>+  uint8_t prof_epoch;
>+#endif
> } GCproto;
>
> /* Flags for prototype. */
>@@ -674,6 +681,9 @@ typedef struct global_State {
>   MRef jit_base;	/* Current JIT code L->base or NULL. */
>   MRef ctype_state;	/* Pointer to C type state. */
>   GCRef gcroot[GCROOT_MAX];  /* GC roots. */
>+#if LJ_HASMEMPROF
>+  uint8_t prof_epoch;	/* Current profiling epoch for this VM. */
>+#endif
> } global_State;
>
> #define mainthread(g)	(&gcref(g->mainthref)->th)
>diff --git a/src/lj_parse.c b/src/lj_parse.c
>index a6325a76..9415665a 100644
>--- a/src/lj_parse.c
>+++ b/src/lj_parse.c
>@@ -1577,6 +1577,9 @@ static GCproto *fs_finish(LexState *ls, BCLine line)
>   pt->flags = (uint8_t)(fs->flags & ~(PROTO_HAS_RETURN|PROTO_FIXUP_RETURN));
>   pt->numparams = fs->numparams;
>   pt->framesize = fs->framesize;
>+#if LJ_HASMEMPROF
>+  pt->prof_epoch = 0;
>+#endif
>   setgcref(pt->chunkname, obj2gco(ls->chunkname));
>
>   /* Close potentially uninitialized gap between bc and kgc. */
>diff --git a/src/lj_state.c b/src/lj_state.c
>index f82b1b5b..b4bfd573 100644
>--- a/src/lj_state.c
>+++ b/src/lj_state.c
>@@ -221,6 +221,9 @@ LUA_API lua_State *lua_newstate(lua_Alloc f, void *ud)
>   g->strempty.gct = ~LJ_TSTR;
>   g->allocf = f;
>   g->allocd = ud;
>+#if LJ_HASMEMPROF
>+  g->prof_epoch = 0;
>+#endif
>   setgcref(g->mainthref, obj2gco(L));
>   setgcref(g->uvhead.prev, obj2gco(&g->uvhead));
>   setgcref(g->uvhead.next, obj2gco(&g->uvhead));
>diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>index dd973f2a..1c74b4d7 100644
>--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
>+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>@@ -7,7 +7,7 @@ require("utils").skipcond(
> local tap = require("tap")
>
> local test = tap.test("misc-memprof-lapi")
>-test:plan(4)
>+test:plan(5)
>
> local jit_opt_default = {
>     3, -- level
>@@ -117,8 +117,9 @@ local function fill_ev_type(events, symbols, event_type)
>   return ev_type
> end
>
>-local function form_source_line(line)
>-  return string.format("@%s:%d", arg[0], line)
>+local function form_source_line(line, source)
>+  source = source or ("@"..arg[0])
>+  return string.format("%s:%d", source, line)
> end
>
> local function check_alloc_report(alloc, location, nevents)
>@@ -126,10 +127,10 @@ local function check_alloc_report(alloc, location, nevents)
>   local traceno = location.traceno
>   if traceno then
>     expected_name = string.format("TRACE [%d] ", traceno)..
>-                    form_source_line(location.line)
>+                    form_source_line(location.line, location.source)
>     event = alloc.trace[traceno]
>   else
>-    expected_name = form_source_line(location.linedefined)
>+    expected_name = form_source_line(location.linedefined, location.source)
>     event = alloc.line[location.line]
>   end
>   assert(expected_name == event.name, ("got='%s', expected='%s'"):format(
>@@ -223,6 +224,38 @@ test:test("stack-resize", function(subtest)
>   misc.memprof.stop()
> end)
>
>+-- Test for extending symtab with function prototypes
>+-- while profiler is running.
>+test:test("symtab-enriching", function(subtest)
>+  subtest:plan(2)
>+
>+  local payload_str = [[
>+  local M = {
>+    tmp = string.rep("1", 100)  -- line 2.
>+  }
>+
>+  function M.payload()
>+    local str = string.rep("42", 100)  -- line 6.
>+  end
>+
>+  return M
>+  ]]
>+
>+  local symbols, events = generate_parsed_output(function()
>+    local str_chunk = assert(loadstring(payload_str, 'str_chunk'))()
>+    str_chunk.payload()
>+  end)
>+
>+  local alloc = fill_ev_type(events, symbols, "alloc")
>+
>+  subtest:ok(check_alloc_report(
>+    alloc, { source = 'str_chunk', line = 6, linedefined = 5 }, 1)
>+  )
>+  subtest:ok(check_alloc_report(
>+    alloc, { source = 'str_chunk', line = 2, linedefined = 0 }, 1)
>+  )
>+end)
>+
> -- Test profiler with enabled JIT.
> jit.on()
>
>diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
>index be5844a4..38f76f00 100644
>--- a/tools/memprof/parse.lua
>+++ b/tools/memprof/parse.lua
>@@ -13,10 +13,11 @@ local string_format = string.format
> local symtab = require "utils.symtab"
>
> local LJM_MAGIC = "ljm"
>-local LJM_CURRENT_VERSION = 0x02
>+local LJM_CURRENT_VERSION = 0x03
>
> local LJM_EPILOGUE_HEADER = 0x80
>
>+local AEVENT_SYMTAB = 0
> local AEVENT_ALLOC = 1
> local AEVENT_FREE = 2
> local AEVENT_REALLOC = 3
>@@ -140,7 +141,14 @@ local function parse_free(reader, asource, events, heap, symbols)
>   heap[oaddr] = nil
> end
>
>+local function parse_symtab(reader, asource, _, _, symbols)
>+  if asource == ASOURCE_LFUNC then
>+    symtab.parse_sym_lfunc(reader, symbols)
>+  end
>+end
>+
> local parsers = {
>+  [AEVENT_SYMTAB] = {evname = "symtab", parse = parse_symtab},
>   [AEVENT_ALLOC] = {evname = "alloc", parse = parse_alloc},
>   [AEVENT_FREE] = {evname = "free", parse = parse_free},
>   [AEVENT_REALLOC] = {evname = "realloc", parse = parse_realloc},
>diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
>index c758b67e..00bab03a 100644
>--- a/tools/utils/symtab.lua
>+++ b/tools/utils/symtab.lua
>@@ -37,7 +37,7 @@ function M.new_loc(symtab, addr, line, traceno)
> end
>
> -- Parse a single entry in a symtab: lfunc symbol.
>-local function parse_sym_lfunc(reader, symtab)
>+function M.parse_sym_lfunc(reader, symtab)
>   local sym_addr = reader:read_uleb128()
>   local sym_chunk = reader:read_string()
>   local sym_line = reader:read_uleb128()
>@@ -69,7 +69,7 @@ local function parse_sym_trace(reader, symtab)
> end
>
> local parsers = {
>-  [SYMTAB_LFUNC] = parse_sym_lfunc,
>+  [SYMTAB_LFUNC] = M.parse_sym_lfunc,
>   [SYMTAB_TRACE] = parse_sym_trace,
> }
>
>-- 
>2.33.1
>

Re: [Tarantool-patches] [PATCH luajit v2 2/3] memprof: enrich symtab when meeting new prototype

[Sergey Kaplun via Tarantool-patches]

Hi, Mikhail!

Thanks for the patch!

LGTM, except a few comments below.

Also, please share Tarantool's branch for tests, I can't found it :(.

On 19.01.22, Mikhail Shishatskiy wrote:
> Updated patch in line with what we discussed with Sergey offline:
> 
> New commit message:
> ===========================================================
> memprof: enrich symtab when meeting new prototype
> 
> Previously, the profiler dumped all the function prototypes only
> at the start of profiling. It was enough for most cases, but
> sometimes we may want to investigate the memory profile of module
> loading. In such a case, we expect memprof to dump new prototypes
> from parsed source code.
> 
> This patch extends memprof's streaming format with entries providing
> extra information about allocation source's symbols if they were not
> streamed at the start of profiling. This event is called <event-symtab>
> and precedes the allocation-related event. The format is the following:
> 
> | event-symtab := event-header sym?
> | sym          := sym-lua
> | sym-lua      := sym-addr sym-chunk sym-line
> 
> The `sym-addr`, `sym-chunk` and `sym-line` are the prototype's address,
> its chunk name and line where it was defined.
> 
> The profiler dumps new prototypes to the symbol table right after the
> creation of a GCproto object.
> 
> Also, the profiler parser is adjusted to recognize entries described
> above and enrich the symtab on the fly while parsing events. For this
> reason, the <utils/symtab.lua> API has been changed: the function
> `parse_sym_lfunc` became public to be available for the parser module.
> 
> Resolves tarantool/tarantool#5815
> ===========================================================
> 
> New diff:

Did I understand correctly, that after this patch there is no need in
epoch labels for prototypes? So the previous commit can be removed?

> ===========================================================
> diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original
> index faa44a0b..82f34315 100644
> --- a/src/Makefile.dep.original
> +++ b/src/Makefile.dep.original
> @@ -59,7 +59,7 @@ 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_strfmt.h lj_memprof.h

You must mention lj_wbuf.h here too, as far as it is included by
lj_memprof.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 +175,7 @@ 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_vm.h lj_vmevent.h lj_memprof.h

Ditto.

>   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 luajit.h
> diff --git a/src/lj_bcread.c b/src/lj_bcread.c
> index 48c5e7c7..cb08599d 100644
> --- a/src/lj_bcread.c
> +++ b/src/lj_bcread.c
> @@ -22,6 +22,9 @@

<snipped>

> diff --git a/src/lj_memprof.c b/src/lj_memprof.c
> index 2d779983..9bb2483e 100644
> --- a/src/lj_memprof.c
> +++ b/src/lj_memprof.c
> @@ -66,6 +66,14 @@ static void dump_symtab_trace(struct lj_wbuf *out, const GCtrace *trace)
> 
>   #endif
> 
> +static void dump_symtab_proto(struct lj_wbuf *out, const GCproto *pt,
> +			      const global_State *g)

Looks like global_State is unused in this function.

> +{
> +  lj_wbuf_addu64(out, (uintptr_t)pt);
> +  lj_wbuf_addstring(out, proto_chunknamestr(pt));
> +  lj_wbuf_addu64(out, (uint64_t)pt->firstline);
> +}
> +
>   static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
>   {
>     const GCRef *iter = &g->gc.root;
> @@ -80,9 +88,7 @@ static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
>       case (~LJ_TPROTO): {
>         const GCproto *pt = gco2pt(o);
>         lj_wbuf_addbyte(out, SYMTAB_LFUNC);
> -      lj_wbuf_addu64(out, (uintptr_t)pt);
> -      lj_wbuf_addstring(out, proto_chunknamestr(pt));
> -      lj_wbuf_addu64(out, (uint64_t)pt->firstline);
> +      dump_symtab_proto(out, pt, g);
>         break;
>       }
>       case (~LJ_TTRACE): {
> @@ -140,6 +146,7 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,

The next chunk (*) seems like unnecessary changes, why do we need them?
We can just add the comment to the old chunk if you want.

>     ** -DLUAJIT_DISABLE_DEBUGINFO flag.
>     */
>     const BCLine line = lj_debug_frameline(L, fn, nextframe);
> +  const GCproto *pt = funcproto(fn);
> 
>     if (line < 0) {
>       /*
> @@ -151,11 +158,17 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
>       ** We report such allocations as internal in order not to confuse users.
>       */
>       lj_wbuf_addbyte(out, aevent | ASOURCE_INT);
> -  } else {
> -    lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> -    lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
> -    lj_wbuf_addu64(out, (uint64_t)line);
> +    return;
>     }
> +
> +  /*
> +  ** As a prototype is a source of an allocation, it has
> +  ** already been inserted into the symtab: on the start
> +  ** of the profiling or right after the creation.
> +  */
> +  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> +  lj_wbuf_addu64(out, (uintptr_t)pt);
> +  lj_wbuf_addu64(out, (uint64_t)line);
>   }
> 
>   static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,

(*) up to here.

> @@ -414,6 +427,17 @@ errio:

<snipped>

> diff --git a/src/lj_memprof.h b/src/lj_memprof.h
> index 395fb429..eda8ca0c 100644
> --- a/src/lj_memprof.h
> +++ b/src/lj_memprof.h

<snipped>

> @@ -164,4 +174,10 @@ int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt);
>   */
>   int lj_memprof_stop(struct lua_State *L);
> 
> +/*
> +** Enriches profiler symbol table with a new proto, if profiler

Typo: s/profiler/the profiler/

> +** is running.
> +*/
> +void lj_memprof_add_proto(const struct GCproto *pt);
> +
>   #endif
> diff --git a/src/lj_parse.c b/src/lj_parse.c
> index a6325a76..30b0caa0 100644
> --- a/src/lj_parse.c
> +++ b/src/lj_parse.c
> @@ -27,6 +27,9 @@

<snipped>

> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index dd973f2a..1c74b4d7 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -7,7 +7,7 @@ require("utils").skipcond(

<snipped>

> @@ -223,6 +224,38 @@ test:test("stack-resize", function(subtest)
>     misc.memprof.stop()
>   end)
> 
> +-- Test for extending symtab with function prototypes
> +-- while profiler is running.
> +test:test("symtab-enriching", function(subtest)
> +  subtest:plan(2)
> +
> +  local payload_str = [[
> +  local M = {
> +    tmp = string.rep("1", 100)  -- line 2.
> +  }
> +
> +  function M.payload()
> +    local str = string.rep("42", 100)  -- line 6.
> +  end
> +
> +  return M
> +  ]]
> +
> +  local symbols, events = generate_parsed_output(function()
> +    local str_chunk = assert(loadstring(payload_str, 'str_chunk'))()
> +    str_chunk.payload()
> +  end)
> +
> +  local alloc = fill_ev_type(events, symbols, "alloc")
> +
> +  subtest:ok(check_alloc_report(
> +    alloc, { source = 'str_chunk', line = 6, linedefined = 5 }, 1)
> +  )
> +  subtest:ok(check_alloc_report(
> +    alloc, { source = 'str_chunk', line = 2, linedefined = 0 }, 1)
> +  )

It is also good to test bcread case. Please, add the corresponding
test.

> +end)
> +
>   -- Test profiler with enabled JIT.
>   jit.on()
> 
> diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> index be5844a4..38f76f00 100644
> --- a/tools/memprof/parse.lua
> +++ b/tools/memprof/parse.lua
> @@ -13,10 +13,11 @@ local string_format = string.format

<snipped>

> diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
> index c758b67e..00bab03a 100644
> --- a/tools/utils/symtab.lua
> +++ b/tools/utils/symtab.lua

<snipped>

> ===========================================================
> 
> On 02.12.2021 14:03, Mikhail Shishatskiy wrote:

<snipped>

> >

-- 
Best regards,
Sergey Kaplun

[Tarantool-patches] [PATCH luajit v2 3/3] memprof: substitute long proto names with aliases

[Mikhail Shishatskiy via Tarantool-patches]

Sometimes a loaded chunk name can be multiline (actually, it
is the Lua code itself). In order not to burden memprof parser
output with big multiline names, aliases were introduced.

The chunk name is replaced by `function_alias_N` (where N is a unique id)
to be displayed in the allocation events report. All the aliases are
printed in the end of parser's output under the header "ALIASES".

Because of changes mentioned above, the API of <utils/symtab.lua>
changed: now symtab has additional `alias` assotiative table for
storing aliases: one can get alias string by sym_chunk key and sym_chunk
by alias index. The humanizer module now can display aliases with the
new function <aliases>.

Follows up tarantool/tarantool#5815
---

Issue: https://github.com/tarantool/tarantool/issues/5815
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated

 tools/memprof.lua          |  1 +
 tools/memprof/humanize.lua | 14 ++++++++++++++
 tools/utils/symtab.lua     | 17 +++++++++++++----
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/tools/memprof.lua b/tools/memprof.lua
index 18b44fdd..cf66dd9e 100644
--- a/tools/memprof.lua
+++ b/tools/memprof.lua
@@ -106,6 +106,7 @@ local function dump(inputfile)
   end
   local dheap = process.form_heap_delta(events, symbols)
   view.leak_info(dheap)
+  view.aliases(symbols)
   os.exit(0)
 end
 
diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
index 7771005d..d77c7132 100644
--- a/tools/memprof/humanize.lua
+++ b/tools/memprof/humanize.lua
@@ -81,4 +81,18 @@ function M.leak_info(dheap)
   print("")
 end
 
+function M.aliases(symbols)
+  if #symbols.alias == 0 then return end
+  print("ALIASES:")
+  for _, source in ipairs(symbols.alias) do
+    print(symbols.alias[source]..":")
+    local lineno = 1
+    for line in source:gmatch("(.-)\n") do
+      print(tostring(lineno).."\t| "..line)
+      lineno = lineno + 1
+    end
+    print("~\n")
+  end
+end
+
 return M
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index 00bab03a..133a0fc7 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -46,6 +46,13 @@ function M.parse_sym_lfunc(reader, symtab)
     symtab.lfunc[sym_addr] = {}
   end
 
+  if sym_chunk:find('\n') and symtab.alias[sym_chunk] == nil then
+    table.insert(symtab.alias, sym_chunk)
+    symtab.alias[sym_chunk] = string_format(
+      "function_alias_%d", #symtab.alias
+    )
+  end
+
   table.insert(symtab.lfunc[sym_addr], {
     source = sym_chunk,
     linedefined = sym_line,
@@ -77,6 +84,7 @@ function M.parse(reader)
   local symtab = {
     lfunc = {},
     trace = {},
+    alias = {},
   }
   local magic = reader:read_octets(3)
   local version = reader:read_octets(1)
@@ -147,15 +155,16 @@ function M.demangle(symtab, loc)
   end
 
   local addr = loc.addr
+  local epoch = loc.epoch
 
   if addr == 0 then
     return "INTERNAL"
   end
 
-  if symtab.lfunc[addr] and symtab.lfunc[addr][loc.epoch] then
-    return string_format(
-      "%s:%d", symtab.lfunc[addr][loc.epoch].source, loc.line
-    )
+  if symtab.lfunc[addr] and symtab.lfunc[addr][epoch] then
+    local source = symtab.lfunc[addr][epoch].source
+    local alias = symtab.alias[source]
+    return string_format("%s:%d", alias or source, loc.line)
   end
 
   return string_format("CFUNC %#x", addr)
-- 
2.33.1

Re: [Tarantool-patches] [PATCH luajit v2 3/3] memprof: substitute long proto names with aliases

[Sergey Kaplun via Tarantool-patches]

Hi, Mikhail!

Thanks for the patch!

May be it is more user-friendly to use the first line of alias instead
strict "function_alias_\d"? Something like:
| \d string "print(nil)..."#N:

\d here is the number of alias.
#N -- linenumber.

Also this simplifies reading for oneline functions without \n.

Thoughts?
But I'm OK with the current version.

LGTM, otherwise, except a few nits below.

On 02.12.21, Mikhail Shishatskiy wrote:
> Sometimes a loaded chunk name can be multiline (actually, it
> is the Lua code itself). In order not to burden memprof parser
> output with big multiline names, aliases were introduced.
> 
> The chunk name is replaced by `function_alias_N` (where N is a unique id)
> to be displayed in the allocation events report. All the aliases are
> printed in the end of parser's output under the header "ALIASES".

Typo: s/in/at/
Typo: s/parser's/the parsers/

> 
> Because of changes mentioned above, the API of <utils/symtab.lua>
> changed: now symtab has additional `alias` assotiative table for

Typo: s/assotiative/associative/

> storing aliases: one can get alias string by sym_chunk key and sym_chunk
> by alias index. The humanizer module now can display aliases with the
> new function <aliases>.
> 
> Follows up tarantool/tarantool#5815

Nit: "Part of" looks more properly.

> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/5815
> Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
> Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated
> 
>  tools/memprof.lua          |  1 +
>  tools/memprof/humanize.lua | 14 ++++++++++++++
>  tools/utils/symtab.lua     | 17 +++++++++++++----
>  3 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/memprof.lua b/tools/memprof.lua
> index 18b44fdd..cf66dd9e 100644
> --- a/tools/memprof.lua
> +++ b/tools/memprof.lua
> @@ -106,6 +106,7 @@ local function dump(inputfile)

<snipped>

> diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
> index 7771005d..d77c7132 100644
> --- a/tools/memprof/humanize.lua
> +++ b/tools/memprof/humanize.lua
> @@ -81,4 +81,18 @@ function M.leak_info(dheap)
>    print("")
>  end
>  
> +function M.aliases(symbols)
> +  if #symbols.alias == 0 then return end
> +  print("ALIASES:")
> +  for _, source in ipairs(symbols.alias) do
> +    print(symbols.alias[source]..":")
> +    local lineno = 1
> +    for line in source:gmatch("(.-)\n") do

The last line may be without \n symbol and will be skipped.
For example the following chunk

| $ src/luajit -e '
|   misc.memprof.start("/tmp/test_memprof.bin")
|   loadstring"\n\nfor i = 1, 1e3 do _ = {i = string.rep(i, 12)} end"()
|   misc.memprof.stop()
| '

will report:

| function_alias_1:
| 1       | 
| 2       | 
| ~

> +      print(tostring(lineno).."\t| "..line)
> +      lineno = lineno + 1
> +    end
> +    print("~\n")

Do we need this '~' symbol?

> +  end
> +end
> +
>  return M
> diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
> index 00bab03a..133a0fc7 100644
> --- a/tools/utils/symtab.lua
> +++ b/tools/utils/symtab.lua
> @@ -46,6 +46,13 @@ function M.parse_sym_lfunc(reader, symtab)
>      symtab.lfunc[sym_addr] = {}
>    end
>  
> +  if sym_chunk:find('\n') and symtab.alias[sym_chunk] == nil then

Nit: `and not symtab.alias[sym_chunk]` looks more in Lua way for me.
Fill free to ignore.

> +    table.insert(symtab.alias, sym_chunk)
> +    symtab.alias[sym_chunk] = string_format(
> +      "function_alias_%d", #symtab.alias
> +    )
> +  end
> +
>    table.insert(symtab.lfunc[sym_addr], {
>      source = sym_chunk,
>      linedefined = sym_line,
> @@ -77,6 +84,7 @@ function M.parse(reader)

<snipped>

> -- 
> 2.33.1
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit v2 0/3] memprof: enrich symtab when meeting new prototype

[Igor Munkin via Tarantool-patches]

Misha,

Thanks for the series!

I've fixed all vital comments by Sergey, and few critical issues I've
found by myself, LGTM otherwise. CI is green[1] (the topmost patch
relates to another thread[2]), so I've I've checked the patchset into
tarantool branch in tarantool/luajit and bumped a new version in master.

On 02.12.21, Mikhail Shishatskiy wrote:
> Patchset v2 with some adjustments based on previous patch series,
> and several fixes with improvements:
> 
> - Removed "change order of modules patch";
> - Changed order of patches so that patch with symtab enriching
>   works correctly straingt away, and not after applying "on the spot
>   demangling" patch;
> - Changed flag representing that object has been dumped to an epoch
>   counter to respect consequent memprof launches.
> 
> Issue: https://github.com/tarantool/tarantool/issues/5815
> Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated-v2
> Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated
> 
> Mikhail Shishatskiy (3):
>   memprof: add symbol epochs
>   memprof: enrich symtab when meeting new prototype
>   memprof: substitute long proto names with aliases
> 
>  src/lj_bcread.c                               |  3 +
>  src/lj_memprof.c                              | 35 +++++++---
>  src/lj_memprof.h                              | 13 +++-
>  src/lj_obj.h                                  | 10 +++
>  src/lj_parse.c                                |  3 +
>  src/lj_state.c                                |  3 +
>  .../misclib-memprof-lapi.test.lua             | 55 ++++++++++++---
>  tools/memprof.lua                             |  1 +
>  tools/memprof/humanize.lua                    | 14 ++++
>  tools/memprof/parse.lua                       | 51 ++++++++------
>  tools/utils/symtab.lua                        | 68 ++++++++++++++-----
>  11 files changed, 196 insertions(+), 60 deletions(-)
> 
> -- 
> 2.33.1
> 

[1]: https://github.com/tarantool/luajit/commit/11ab4c4
[2]: https://lists.tarantool.org/tarantool-patches/20211208172207.80148-1-m.shishatskiy@tarantool.org/

-- 
Best regards,
IM