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 v1 2/4] memprof: demangle symbols on the spot
Date: Sat, 21 Aug 2021 19:50:00 +0700	[thread overview]
Message-ID: <20210821125002.408132-3-m.shishatskiy@tarantool.org> (raw)
In-Reply-To: <20210821125002.408132-1-m.shishatskiy@tarantool.org>

There may be situations, when new prototype 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:

| Dump symtab on the start:
|   addr  name
| * 0x001 proto_1
| * 0x002 proto_2
| * 0x003 proto_3
|
| Memprof running:
| ...
| * alloc: 0x001 proto_1
| // 0x001 proto_1 collected by the GC
| // new proto loaded on the 0x001 address
| * alloc: 0x001 proto_NEW
|   + symtab enriching -> [0x001] = proto_NEW

If we demangle symbols in the end, we will get 2 allocations
by proto_NEW because of the collision. But since symtab enriching
is implemented, we have enough information to avoid collision
while parsing events, demangling now happens on the spot, while
symtab entries are not shadowed by new ones.

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
Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5815-enrich-symtab-when-prototype-is-allocated

 tools/memprof.lua          |  4 ++--
 tools/memprof/humanize.lua | 16 ++++++++--------
 tools/memprof/parse.lua    | 26 +++++++++++++-------------
 tools/memprof/process.lua  | 12 +++++-------
 4 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/tools/memprof.lua b/tools/memprof.lua
index 18b44fdd..496a0d78 100644
--- a/tools/memprof.lua
+++ b/tools/memprof.lua
@@ -102,9 +102,9 @@ local function dump(inputfile)
   local symbols = symtab.parse(reader)
   local events = memprof.parse(reader, symbols)
   if not leak_only then
-    view.profile_info(events, symbols)
+    view.profile_info(events)
   end
-  local dheap = process.form_heap_delta(events, symbols)
+  local dheap = process.form_heap_delta(events)
   view.leak_info(dheap)
   os.exit(0)
 end
diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
index 800a465e..0360362d 100644
--- a/tools/memprof/humanize.lua
+++ b/tools/memprof/humanize.lua
@@ -7,7 +7,7 @@ local symtab = require "utils.symtab"
 
 local M = {}
 
-function M.render(events, symbols)
+function M.render(events)
   local ids = {}
 
   for id, _ in pairs(events) do
@@ -21,15 +21,15 @@ 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",
-      M.describe_location(symbols, event.loc),
+      ids[i],
       event.num,
       event.alloc,
       event.free
     ))
 
     local prim_loc = {}
-    for _, heap_chunk in pairs(event.primary) do
-      table.insert(prim_loc, M.describe_location(symbols, heap_chunk.loc))
+    for id, _ in pairs(event.primary) do
+      table.insert(prim_loc, id)
     end
     if #prim_loc ~= 0 then
       table.sort(prim_loc)
@@ -42,17 +42,17 @@ function M.render(events, symbols)
   end
 end
 
-function M.profile_info(events, symbols)
+function M.profile_info(events)
   print("ALLOCATIONS")
-  M.render(events.alloc, symbols)
+  M.render(events.alloc)
   print("")
 
   print("REALLOCATIONS")
-  M.render(events.realloc, symbols)
+  M.render(events.realloc)
   print("")
 
   print("DEALLOCATIONS")
-  M.render(events.free, symbols)
+  M.render(events.free)
   print("")
 end
 
diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index 888c6636..a540360b 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -9,6 +9,7 @@ local band = bit.band
 local lshift = bit.lshift
 
 local symtab = require "utils.symtab"
+local humanize = require "memprof.humanize"
 
 local string_format = string.format
 
@@ -66,12 +67,13 @@ local function link_to_previous(heap_chunk, e, nsize)
   end
 end
 
-local function id_location(addr, line, traceno)
-  return string_format("f%#xl%dt%d", addr, line, traceno), {
+local function id_location(addr, line, traceno, symbols)
+  local loc =  {
     addr = addr,
     line = line,
     traceno = traceno,
   }
+  return humanize.describe_location(symbols, loc), loc
 end
 
 local function parse_location_symbols(reader, asource, symbols)
@@ -79,21 +81,22 @@ local function parse_location_symbols(reader, asource, symbols)
     return id_location(
       symtab.parse_sym_lfunc(reader, symbols),
       reader:read_uleb128(),
-      0
+      0,
+      symbols
     )
   end
   error("Unknown asource "..asource)
 end
 
-local function parse_location_common(reader, asource)
+local function parse_location_common(reader, asource, symbols)
   if asource == ASOURCE_INT then
-    return id_location(0, 0, 0)
+    return id_location(0, 0, 0, symbols)
   elseif asource == ASOURCE_CFUNC then
-    return id_location(reader:read_uleb128(), 0, 0)
+    return id_location(reader:read_uleb128(), 0, 0, symbols)
   elseif asource == ASOURCE_LFUNC then
-    return id_location(reader:read_uleb128(), reader:read_uleb128(), 0)
+    return id_location(reader:read_uleb128(), reader:read_uleb128(), 0, symbols)
   elseif asource == ASOURCE_TRACE then
-    return id_location(reader:read_uleb128(), 0, reader:read_uleb128())
+    return id_location(reader:read_uleb128(), 0, reader:read_uleb128(), symbols)
   end
   error("Unknown asource "..asource)
 end
@@ -101,13 +104,10 @@ end
 local function parse_location(reader, asource, symbols)
   local has_symbols  = band(asource, LJM_SYMTAB)
   local asource_type = band(asource, ASOURCE_TYPE_MASK)
-  local id, loc
   if has_symbols == LJM_SYMTAB then
-    id, loc = parse_location_symbols(reader, asource_type, symbols)
-  else
-    id, loc = parse_location_common(reader, asource_type)
+    return parse_location_symbols(reader, asource_type, symbols)
   end
-  return id, loc
+  return parse_location_common(reader, asource_type, symbols)
 end
 
 local function parse_alloc(reader, asource, events, heap, symbols)
diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
index f277ed84..0a39d333 100644
--- a/tools/memprof/process.lua
+++ b/tools/memprof/process.lua
@@ -2,9 +2,7 @@
 
 local M = {}
 
-local humanize = require "memprof.humanize"
-
-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, {
@@ -15,9 +13,9 @@ function M.form_heap_delta(events, symbols)
     return t[line]
   end})
 
-  for _, event in pairs(events.alloc) do
+  for id, event in pairs(events.alloc) do
     if event.loc then
-      local ev_line = humanize.describe_location(symbols, event.loc)
+      local ev_line = id
 
       if (event.alloc > 0) then
         dheap[ev_line].dbytes = dheap[ev_line].dbytes + event.alloc
@@ -36,8 +34,8 @@ function M.form_heap_delta(events, symbols)
       -- Realloc and free events always have key named "primary"
       -- that references the table with memory changed
       -- (may be empty).
-      for _, heap_chunk in pairs(event.primary) do
-        local ev_line = humanize.describe_location(symbols, heap_chunk.loc)
+      for id, heap_chunk in pairs(event.primary) do
+        local ev_line = id
 
         if (heap_chunk.alloced > 0) then
           dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.alloced
-- 
2.32.0


  parent reply	other threads:[~2021-08-21 12:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-21 12:49 [Tarantool-patches] [PATCH luajit v1 0/4] memprof: enrich symtab when meeting new prototype Mikhail Shishatskiy via Tarantool-patches
2021-08-21 12:49 ` [Tarantool-patches] [PATCH luajit v1 1/4] " Mikhail Shishatskiy via Tarantool-patches
2021-08-24 14:16   ` Mikhail Shishatskiy via Tarantool-patches
2021-08-21 12:50 ` Mikhail Shishatskiy via Tarantool-patches [this message]
2021-08-21 12:50 ` [Tarantool-patches] [PATCH luajit v1 3/4] memprof: substitute long proto names with aliases Mikhail Shishatskiy via Tarantool-patches
2021-08-21 12:50 ` [Tarantool-patches] [PATCH luajit v1 4/4] luajit: change order of modules Mikhail Shishatskiy 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=20210821125002.408132-3-m.shishatskiy@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=m.shishatskiy@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit v1 2/4] memprof: demangle symbols on the spot' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox