Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mikhail Shishatskiy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, imun@tarantool.org,
	skaplun@tarantool.org
Subject: [Tarantool-patches] [PATCH luajit v4 4/4] memprof: add info about trace start to symtab
Date: Wed, 29 Sep 2021 23:07:58 +0300
Message-ID: <20210929200758.149446-5-m.shishatskiy@tarantool.org> (raw)
In-Reply-To: <20210929200758.149446-1-m.shishatskiy@tarantool.org>

Trace allocation sources, recorded by the memory profiler,
were reported as

| TRACE [<trace-no>] <trace-addr>

This approach is not descriptive enough to understand, where
exactly allocation took place, as we do not know the code
chunk, associated with the trace.

This patch fixes the problem described above by extending the
symbol table with <sym-trace> entries, consisting of a trace's
mcode starting address, trace number, address of function proto,
and line, where trace recording started:

| sym-trace  := sym-header trace-no trace-addr sym-addr sym-line
| trace-no   := <ULEB128>
| trace-addr := <ULEB128>

The memory profiler parser is adjusted to recognize the entries
mentioned above. On top of that, the API of <utils/symtab.lua> changed:
now table with symbols contains two tables: `lfunc` for Lua functions
symbols and `trace` for trace entries.

The demangler module has not changed, but the function
`describe_location` is added to the <memprof/humanize.lua> module,
which allows one to get a description of the trace location in the
format described below:

| TRACE [<trace-no>] <trace-addr> started at @<sym-chunk>:<sym-line>

Follows up tarantool/tarantool#5814
---

Issue: https://github.com/tarantool/tarantool/issues/5814
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number
CI: https://github.com/tarantool/tarantool/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number

 src/lj_memprof.c                              | 43 +++++++++++++++++++
 src/lj_memprof.h                              |  8 +++-
 .../misclib-memprof-lapi.test.lua             | 15 ++++---
 tools/memprof.lua                             |  4 +-
 tools/memprof/humanize.lua                    | 30 ++++++++++---
 tools/memprof/process.lua                     |  9 ++--
 tools/utils/symtab.lua                        | 31 ++++++++++---
 7 files changed, 118 insertions(+), 22 deletions(-)

diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 8702557f..e8b2ebbc 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -28,6 +28,45 @@
 static const unsigned char ljs_header[] = {'l', 'j', 's', LJS_CURRENT_VERSION,
 					   0x0, 0x0, 0x0};
 
+#if LJ_HASJIT
+
+static void dump_symtab_trace(struct lj_wbuf *out, const GCtrace *trace)
+{
+  GCproto *pt = &gcref(trace->startpt)->pt;
+  BCLine lineno = 0;
+
+  const BCIns *startpc = mref(trace->startpc, const BCIns);
+  lua_assert(startpc >= proto_bc(pt) &&
+             startpc < proto_bc(pt) + pt->sizebc);
+
+  lineno = lj_debug_line(pt, proto_bcpos(pt, startpc));
+  lua_assert(lineno >= 0);
+
+  lj_wbuf_addbyte(out, SYMTAB_TRACE);
+  lj_wbuf_addu64(out, (uint64_t)trace->traceno);
+  lj_wbuf_addu64(out, (uint64_t)trace->mcode);
+  /*
+  ** The information about the prototype, associated with the
+  ** trace's start has already been dumped, as it is anchored
+  ** via the trace and is not collected while the trace is alive.
+  ** For this reason, we do not need to repeat dumping the chunk
+  ** name for the prototype.
+  */
+  lj_wbuf_addu64(out, (uintptr_t)pt);
+  lj_wbuf_addu64(out, (uint64_t)lineno);
+}
+
+#else
+
+static void dump_symtab_trace(struct lj_wbuf *out, const GCtrace *trace)
+{
+  UNUSED(out);
+  UNUSED(trace);
+  lua_assert(0);
+}
+
+#endif
+
 static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
 {
   const GCRef *iter = &g->gc.root;
@@ -47,6 +86,10 @@ static void dump_symtab(struct lj_wbuf *out, const struct global_State *g)
       lj_wbuf_addu64(out, (uint64_t)pt->firstline);
       break;
     }
+    case (~LJ_TTRACE): {
+      dump_symtab_trace(out, gco2trace(o));
+      break;
+    }
     default:
       break;
     }
diff --git a/src/lj_memprof.h b/src/lj_memprof.h
index 47474a51..395fb429 100644
--- a/src/lj_memprof.h
+++ b/src/lj_memprof.h
@@ -16,7 +16,7 @@
 #include "lj_def.h"
 #include "lj_wbuf.h"
 
-#define LJS_CURRENT_VERSION 0x1
+#define LJS_CURRENT_VERSION 0x2
 
 /*
 ** symtab format:
@@ -25,13 +25,16 @@
 ** prologue       := 'l' 'j' 's' version reserved
 ** version        := <BYTE>
 ** reserved       := <BYTE> <BYTE> <BYTE>
-** sym            := sym-lua | sym-final
+** sym            := sym-lua | sym-trace | sym-final
 ** sym-lua        := sym-header sym-addr sym-chunk sym-line
+** sym-trace      := sym-header trace-no trace-addr sym-addr sym-line
 ** sym-header     := <BYTE>
 ** sym-addr       := <ULEB128>
 ** sym-chunk      := string
 ** sym-line       := <ULEB128>
 ** sym-final      := sym-header
+** trace-no       := <ULEB128>
+** trace-addr     := <ULEB128>
 ** string         := string-len string-payload
 ** string-len     := <ULEB128>
 ** string-payload := <BYTE> {string-len}
@@ -51,6 +54,7 @@
 */
 
 #define SYMTAB_LFUNC ((uint8_t)0)
+#define SYMTAB_TRACE ((uint8_t)1)
 #define SYMTAB_FINAL ((uint8_t)0x80)
 
 #define LJM_CURRENT_FORMAT_VERSION 0x02
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 3f4ffea0..b9edb80d 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -87,9 +87,13 @@ local function fill_ev_type(events, symbols, event_type)
     local addr = event.loc.addr
     local traceno = event.loc.traceno
 
-    if traceno ~= 0 then
+    if traceno ~= 0 and symbols.trace[traceno] then
+      local trace_loc = symbols.trace[traceno].start
+      addr = trace_loc.addr
       ev_type.trace[traceno] = {
-        name = string.format("TRACE [%d]", traceno),
+        name = string.format("TRACE [%d] %s:%d",
+          traceno, symbols.lfunc[addr].source, symbols.lfunc[addr].linedefined
+        ),
         num = event.num,
       }
     elseif addr == 0 then
@@ -97,10 +101,10 @@ local function fill_ev_type(events, symbols, event_type)
         name = "INTERNAL",
         num = event.num,
       }
-    elseif symbols[addr] then
+    elseif symbols.lfunc[addr] then
       ev_type.line[event.loc.line] = {
         name = string.format(
-          "%s:%d", symbols[addr].source, symbols[addr].linedefined
+          "%s:%d", symbols.lfunc[addr].source, symbols.lfunc[addr].linedefined
         ),
         num = event.num,
       }
@@ -116,7 +120,8 @@ end
 local function check_alloc_report(alloc, traceno, line, function_line, nevents)
   local expected_name, event
   if traceno ~= 0 then
-    expected_name = string.format("TRACE [%d]", traceno)
+    expected_name = string.format("TRACE [%d] ", traceno)..
+                    form_source_line(function_line)
     event = alloc.trace[traceno]
   else
     expected_name = form_source_line(function_line)
diff --git a/tools/memprof.lua b/tools/memprof.lua
index 18b44fdd..760122fc 100644
--- a/tools/memprof.lua
+++ b/tools/memprof.lua
@@ -104,8 +104,8 @@ local function dump(inputfile)
   if not leak_only then
     view.profile_info(events, symbols)
   end
-  local dheap = process.form_heap_delta(events, symbols)
-  view.leak_info(dheap)
+  local dheap = process.form_heap_delta(events)
+  view.leak_info(dheap, symbols)
   os.exit(0)
 end
 
diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
index 7771005d..7d30f976 100644
--- a/tools/memprof/humanize.lua
+++ b/tools/memprof/humanize.lua
@@ -7,6 +7,23 @@ local symtab = require "utils.symtab"
 
 local M = {}
 
+function M.describe_location(symbols, loc)
+  if loc.traceno == 0 then
+    return symtab.demangle(symbols, loc)
+  end
+
+  local trace = symbols.trace[loc.traceno]
+
+  -- If trace, which was remembered in the symtab, has not
+  -- been flushed, assotiate it with a proto, where trace
+  -- recording started.
+  if trace and trace.addr == loc.addr then
+    return symtab.demangle(symbols, loc).." started at "..
+           symtab.demangle(symbols, trace.start)
+  end
+  return symtab.demangle(symbols, loc)
+end
+
 function M.render(events, symbols)
   local ids = {}
 
@@ -21,7 +38,7 @@ function M.render(events, symbols)
   for i = 1, #ids do
     local event = events[ids[i]]
     print(string.format("%s: %d events\t+%d bytes\t-%d bytes",
-      symtab.demangle(symbols, event.loc),
+      M.describe_location(symbols, event.loc),
       event.num,
       event.alloc,
       event.free
@@ -29,7 +46,7 @@ function M.render(events, symbols)
 
     local prim_loc = {}
     for _, heap_chunk in pairs(event.primary) do
-      table.insert(prim_loc, symtab.demangle(symbols, heap_chunk.loc))
+      table.insert(prim_loc, M.describe_location(symbols, heap_chunk.loc))
     end
     if #prim_loc ~= 0 then
       table.sort(prim_loc)
@@ -56,13 +73,16 @@ function M.profile_info(events, symbols)
   print("")
 end
 
-function M.leak_info(dheap)
+function M.leak_info(dheap, symbols)
   local leaks = {}
-  for line, info in pairs(dheap) do
+  for _, info in pairs(dheap) do
     -- Report "INTERNAL" events inconsistencies for profiling
     -- with enabled jit.
     if info.dbytes > 0 then
-      table.insert(leaks, {line = line, dbytes = info.dbytes})
+      table.insert(leaks, {
+        line = M.describe_location(symbols, info.loc),
+        dbytes = info.dbytes
+      })
     end
   end
 
diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
index 0bcb965b..360f6cc4 100644
--- a/tools/memprof/process.lua
+++ b/tools/memprof/process.lua
@@ -4,7 +4,7 @@ local M = {}
 
 local symtab = require "utils.symtab"
 
-function M.form_heap_delta(events, symbols)
+function M.form_heap_delta(events)
   -- Auto resurrects source event lines for counting/reporting.
   local dheap = setmetatable({}, {__index = function(t, line)
     rawset(t, line, {
@@ -17,11 +17,12 @@ function M.form_heap_delta(events, symbols)
 
   for _, event in pairs(events.alloc) do
     if event.loc then
-      local ev_line = symtab.demangle(symbols, event.loc)
+      local ev_line = symtab.id(event.loc)
 
       if (event.alloc > 0) then
         dheap[ev_line].dbytes = dheap[ev_line].dbytes + event.alloc
         dheap[ev_line].nalloc = dheap[ev_line].nalloc + event.num
+        dheap[ev_line].loc = event.loc
       end
     end
   end
@@ -37,16 +38,18 @@ function M.form_heap_delta(events, symbols)
       -- that references the table with memory changed
       -- (may be empty).
       for _, heap_chunk in pairs(event.primary) do
-        local ev_line = symtab.demangle(symbols, heap_chunk.loc)
+        local ev_line = symtab.id(heap_chunk.loc)
 
         if (heap_chunk.alloced > 0) then
           dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.alloced
           dheap[ev_line].nalloc = dheap[ev_line].nalloc + heap_chunk.count
+          dheap[ev_line].loc = heap_chunk.loc
         end
 
         if (heap_chunk.freed > 0) then
           dheap[ev_line].dbytes = dheap[ev_line].dbytes - heap_chunk.freed
           dheap[ev_line].nfree = dheap[ev_line].nfree + heap_chunk.count
+          dheap[ev_line].loc = heap_chunk.loc
         end
       end
     end
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index 85945fb2..496d8480 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -10,11 +10,12 @@ local band = bit.band
 local string_format = string.format
 
 local LJS_MAGIC = "ljs"
-local LJS_CURRENT_VERSION = 1
+local LJS_CURRENT_VERSION = 0x2
 local LJS_EPILOGUE_HEADER = 0x80
 local LJS_SYMTYPE_MASK = 0x03
 
 local SYMTAB_LFUNC = 0
+local SYMTAB_TRACE = 1
 
 local M = {}
 
@@ -24,18 +25,38 @@ local function parse_sym_lfunc(reader, symtab)
   local sym_chunk = reader:read_string()
   local sym_line = reader:read_uleb128()
 
-  symtab[sym_addr] = {
+  symtab.lfunc[sym_addr] = {
     source = sym_chunk,
     linedefined = sym_line,
   }
 end
 
+local function parse_sym_trace(reader, symtab)
+  local traceno = reader:read_uleb128()
+  local trace_addr = reader:read_uleb128()
+  local sym_addr = reader:read_uleb128()
+  local sym_line = reader:read_uleb128()
+
+  symtab.trace[traceno] = {
+    addr = trace_addr,
+    start = {
+      addr = sym_addr,
+      line = sym_line,
+      traceno = 0,
+    },
+  }
+end
+
 local parsers = {
   [SYMTAB_LFUNC] = parse_sym_lfunc,
+  [SYMTAB_TRACE] = parse_sym_trace,
 }
 
 function M.parse(reader)
-  local symtab = {}
+  local symtab = {
+    lfunc = {},
+    trace = {},
+  }
   local magic = reader:read_octets(3)
   local version = reader:read_octets(1)
 
@@ -82,8 +103,8 @@ local function demangle_lfunc(symtab, loc)
 
   if addr == 0 then
     return "INTERNAL"
-  elseif symtab[addr] then
-    return string_format("%s:%d", symtab[addr].source, loc.line)
+  elseif symtab.lfunc[addr] then
+    return string_format("%s:%d", symtab.lfunc[addr].source, loc.line)
   end
   return string_format("CFUNC %#x", addr)
 end
-- 
2.33.0


  parent reply	other threads:[~2021-09-29 20:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20  7:05 [Tarantool-patches] [PATCH luajit v3 0/4] memprof: group allocations on traces by trace number Mikhail Shishatskiy via Tarantool-patches
2021-08-20  7:05 ` [Tarantool-patches] [PATCH luajit v3 1/5] core: add const to lj_debug_line proto parameter Mikhail Shishatskiy via Tarantool-patches
2021-09-16 15:29   ` Igor Munkin via Tarantool-patches
2021-08-20  7:05 ` [Tarantool-patches] [PATCH luajit v3 2/5] test: separate memprof Lua API tests into subtests Mikhail Shishatskiy via Tarantool-patches
2021-09-16 15:29   ` Igor Munkin via Tarantool-patches
2021-08-20  7:05 ` [Tarantool-patches] [PATCH luajit v3 3/5] memprof: dump traceno if allocate from trace Mikhail Shishatskiy via Tarantool-patches
2021-09-16 15:32   ` Igor Munkin via Tarantool-patches
2021-09-29 19:21     ` Mikhail Shishatskiy via Tarantool-patches
2021-08-20  7:05 ` [Tarantool-patches] [PATCH luajit v3 4/5] memprof: extend symtab with info about traces Mikhail Shishatskiy via Tarantool-patches
2021-09-16 15:32   ` Igor Munkin via Tarantool-patches
2021-09-29 19:21     ` Mikhail Shishatskiy via Tarantool-patches
2021-08-20  7:05 ` [Tarantool-patches] [PATCH luajit v3 5/5] luajit: change order of modules Mikhail Shishatskiy via Tarantool-patches
2021-09-16 15:32   ` Igor Munkin via Tarantool-patches
2021-09-29 20:07 ` [Tarantool-patches] [PATCH luajit v4 0/4] memprof: group allocations on traces by traceno Mikhail Shishatskiy via Tarantool-patches
2021-09-29 20:07   ` [Tarantool-patches] [PATCH luajit v4 1/4] test: separate memprof Lua API tests into subtests Mikhail Shishatskiy via Tarantool-patches
2021-10-27 13:56     ` Igor Munkin via Tarantool-patches
2021-10-27 15:07     ` Sergey Kaplun via Tarantool-patches
2021-09-29 20:07   ` [Tarantool-patches] [PATCH luajit v4 2/4] memprof: refactor location parsing Mikhail Shishatskiy via Tarantool-patches
2021-10-27 13:56     ` Igor Munkin via Tarantool-patches
     [not found]       ` <20211104130010.mcvnra6e4yl5moo2@surf.localdomain>
2021-11-10 15:38         ` Igor Munkin via Tarantool-patches
2021-09-29 20:07   ` [Tarantool-patches] [PATCH luajit v4 3/4] memprof: group allocations on traces by traceno Mikhail Shishatskiy via Tarantool-patches
2021-10-27 13:56     ` Igor Munkin via Tarantool-patches
     [not found]       ` <20211104130156.f2botlihlfhwd3yh@surf.localdomain>
2021-11-11 15:34         ` Igor Munkin via Tarantool-patches
2021-09-29 20:07   ` Mikhail Shishatskiy via Tarantool-patches [this message]
2021-11-01 16:31     ` [Tarantool-patches] [PATCH luajit v4 4/4] memprof: add info about trace start to symtab Igor Munkin via Tarantool-patches
     [not found]       ` <20211104130228.x6qcne5xeh544hm7@surf.localdomain>
2021-11-12 13:34         ` Igor Munkin via Tarantool-patches
2021-11-17  8:17           ` Sergey Kaplun via Tarantool-patches
2021-11-22 15:11           ` Mikhail Shishatskiy via Tarantool-patches
2021-11-24 12:42             ` Mikhail Shishatskiy via Tarantool-patches
2021-11-24 16:44             ` Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210929200758.149446-5-m.shishatskiy@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=m.shishatskiy@tarantool.org \
    --cc=skaplun@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git