[Tarantool-patches] [PATCH luajit 0/2] profilers: purge generations

[Tarantool-patches] [PATCH luajit 0/2] profilers: purge generations

[Maxim Kokryashkin via Tarantool-patches]

This series refactors memprof's binary data parser, so it
doesn't require generations now. Since both profilers don't
need generations now, that mechanism is purged to improve
memory footprint.

Branch: https://github.com/tarantool/luajit/tree/fckxorg/memprof-parser-refactoring
PR: https://github.com/tarantool/tarantool/pull/9051

Maxim Kokryashkin (2):
  memprof: refactor symbol resolution
  profilers: purge generation mechanism

 .../gh-5813-resolving-of-c-symbols.test.lua   |  7 +-
 .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
 tools/memprof/humanize.lua                    | 16 ++--
 tools/memprof/parse.lua                       |  4 +-
 tools/memprof/process.lua                     |  8 +-
 tools/sysprof/parse.lua                       |  9 +--
 tools/utils/avl.lua                           |  4 +-
 tools/utils/symtab.lua                        | 33 +++------
 8 files changed, 50 insertions(+), 104 deletions(-)

-- 
2.41.0

[Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Maxim Kokryashkin via Tarantool-patches]

This patch refactors symbol resolution in memprof, so
now they are resolved during the process of parsing.
This makes the generation mechanism excessive since
symtab updates are no longer affecting the previous events.

Follows up tarantool/tarantool#8700
---
 .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
 tools/memprof/humanize.lua                    | 16 ++--
 tools/memprof/parse.lua                       |  2 +-
 tools/memprof/process.lua                     |  8 +-
 4 files changed, 31 insertions(+), 68 deletions(-)

diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 3cb5c8be..0eee4e21 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -78,7 +78,7 @@ local function generate_parsed_output(payload)
   -- We don't need it any more.
   os.remove(TMP_BINFILE)
 
-  return symbols, events
+  return events
 end
 
 local function form_source_line(line, source)
@@ -86,44 +86,13 @@ local function form_source_line(line, source)
 end
 
 local function form_trace_line(traceno, line, source)
-  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
+  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)
 end
 
-local function fill_ev_type(events, symbols, event_type)
-  local ev_type = {
-    source = {},
-    trace = {},
-  }
+local function fill_ev_type(events, event_type)
+  local ev_type = {}
   for _, event in pairs(events[event_type]) do
-    local addr = event.loc.addr
-    local traceno = event.loc.traceno
-    local gen = event.loc.gen
-
-    if traceno ~= 0 and symbols.trace[traceno] then
-      local trace_loc = symbols.trace[traceno][gen].start
-      addr = trace_loc.addr
-      gen = trace_loc.gen
-      ev_type.trace[traceno] = {
-        name = form_trace_line(
-          traceno, trace_loc.line, symbols.lfunc[addr][gen].source
-        ),
-        num = event.num,
-      }
-    elseif addr == 0 then
-      ev_type.INTERNAL = {
-        name = "INTERNAL",
-        num = event.num,
-      }
-    elseif symbols.lfunc[addr] then
-      local source = symbols.lfunc[addr][gen].source
-
-      ev_type.source[source] = ev_type.source[source] or {}
-
-      ev_type.source[source][event.loc.line] = {
-        name = form_source_line(symbols.lfunc[addr][gen].linedefined, source),
-        num = event.num,
-      }
-    end
+    ev_type[event.loc] = event.num
   end
   return ev_type
 end
@@ -135,17 +104,15 @@ local function check_alloc_report(alloc, location, nevents)
   local source = location.source or SRC_PATH
   if traceno then
     expected_name = form_trace_line(traceno, location.line, source)
-    event = alloc.trace[traceno]
   else
-    expected_name = form_source_line(location.linedefined, source)
-    event = alloc.source[source][location.line]
+    expected_name = form_source_line(location.line, source)
   end
-  assert(expected_name == event.name, ("got='%s', expected='%s'"):format(
-    event.name,
+  event = alloc[expected_name]
+  assert(event, ("expected='%s', but no such event exists"):format(
     expected_name
   ))
-  assert(event.num == nevents, ("got=%d, expected=%d"):format(
-    event.num,
+  assert(event == nevents, ("got=%d, expected=%d"):format(
+    event,
     nevents
   ))
   return true
@@ -180,10 +147,10 @@ end)
 test:test("output", function(subtest)
   subtest:plan(7)
 
-  local symbols, events = generate_parsed_output(default_payload)
+  local events = generate_parsed_output(default_payload)
 
-  local alloc = fill_ev_type(events, symbols, "alloc")
-  local free = fill_ev_type(events, symbols, "free")
+  local alloc = fill_ev_type(events, "alloc")
+  local free = fill_ev_type(events, "free")
 
   -- Check allocation reports. The second argument is a line
   -- number of the allocation event itself. The third is a line
@@ -196,11 +163,11 @@ test:test("output", function(subtest)
   subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
 
   -- Collect all previous allocated objects.
-  subtest:ok(free.INTERNAL.num == 22)
+  subtest:ok(free.INTERNAL == 22)
 
   -- Tests for leak-only option.
   -- See also https://github.com/tarantool/tarantool/issues/5812.
-  local heap_delta = process.form_heap_delta(events, symbols)
+  local heap_delta = process.form_heap_delta(events)
   local tab_alloc_stats = heap_delta[form_source_line(37)]
   local str_alloc_stats = heap_delta[form_source_line(42)]
   subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
@@ -248,12 +215,12 @@ test:test("symtab-enrich-str", function(subtest)
     return M
   ]]
 
-  local symbols, events = generate_parsed_output(function()
+  local events = generate_parsed_output(function()
     local strchunk = assert(load(payloadstr, "strchunk"))()
     strchunk.payload()
   end)
 
-  local alloc = fill_ev_type(events, symbols, "alloc")
+  local alloc = fill_ev_type(events, "alloc")
 
   subtest:ok(check_alloc_report(
     alloc, { source = "strchunk", line = 2, linedefined = 0 }, 1)
@@ -275,13 +242,13 @@ test:test("jit-output", function(subtest)
 
   -- On this run traces are generated, JIT-related allocations
   -- will be recorded as well.
-  local symbols, events = generate_parsed_output(default_payload)
+  local events = generate_parsed_output(default_payload)
 
-  local alloc = fill_ev_type(events, symbols, "alloc")
+  local alloc = fill_ev_type(events, "alloc")
 
   -- Test for marking JIT-related allocations as internal.
   -- See also https://github.com/tarantool/tarantool/issues/5679.
-  subtest:is(alloc.source[form_source_line(0)], nil)
+  subtest:is(alloc[form_source_line(0)], nil)
 
   -- We expect, that loop will be compiled into a trace.
   -- 10 allocations in interpreter mode, 1 allocation for a trace
diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
index 267baa91..5b289ce3 100644
--- a/tools/memprof/humanize.lua
+++ b/tools/memprof/humanize.lua
@@ -3,11 +3,9 @@
 -- Major portions taken verbatim or adapted from the LuaVela.
 -- Copyright (C) 2015-2019 IPONWEB Ltd.
 
-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,7 +19,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),
+      event.loc,
       event.num,
       event.alloc,
       event.free
@@ -29,7 +27,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, heap_chunk.loc)
     end
     if #prim_loc ~= 0 then
       table.sort(prim_loc)
@@ -42,17 +40,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 d865267b..1b6eceb4 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -79,7 +79,7 @@ local function parse_location(reader, asource, symbols)
     error("Unknown asource "..asource)
   end
   local loc = symtab.loc(symbols, args)
-  return symtab.id(loc), loc
+  return symtab.id(loc), symtab.demangle(symbols, loc)
 end
 
 local function parse_alloc(reader, asource, events, heap, symbols)
diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
index 0bcb965b..faf4be4e 100644
--- a/tools/memprof/process.lua
+++ b/tools/memprof/process.lua
@@ -2,9 +2,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,7 +15,7 @@ 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 = event.loc
 
       if (event.alloc > 0) then
         dheap[ev_line].dbytes = dheap[ev_line].dbytes + event.alloc
@@ -37,7 +35,7 @@ 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 = heap_chunk.loc
 
         if (heap_chunk.alloced > 0) then
           dheap[ev_line].dbytes = dheap[ev_line].dbytes + heap_chunk.alloced
-- 
2.41.0

[Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Maxim Kokryashkin via Tarantool-patches]

Since both of the profiler parsers are now processing
the events in a stream-like fashion, the generation
mechanism is excessive and can be purged. This results
in a significant memory consumption drop, especially
for the AVL-tree part.

Follows up tarantool/tarantool#8700
---
 .../gh-5813-resolving-of-c-symbols.test.lua   |  7 ++--
 tools/memprof/parse.lua                       |  2 +-
 tools/sysprof/parse.lua                       |  9 ++---
 tools/utils/avl.lua                           |  4 +--
 tools/utils/symtab.lua                        | 33 +++++++------------
 5 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
index 30b8a3ca..1581ee4b 100644
--- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
+++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
@@ -24,12 +24,9 @@ local TMP_BINFILE = profilename("memprofdata.tmp.bin")
 local function tree_contains(node, name)
   if node == nil then
     return false
+  elseif node.value.name == name then
+    return true
   else
-    for i = 1, #node.value do
-      if node.value[i].name == name then
-        return true
-      end
-    end
     return tree_contains(node.left, name) or tree_contains(node.right, name)
   end
 end
diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index 1b6eceb4..6fd7451b 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -78,7 +78,7 @@ local function parse_location(reader, asource, symbols)
   else
     error("Unknown asource "..asource)
   end
-  local loc = symtab.loc(symbols, args)
+  local loc = symtab.loc(args)
   return symtab.id(loc), symtab.demangle(symbols, loc)
 end
 
diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
index 451edecb..bb1cb02c 100755
--- a/tools/sysprof/parse.lua
+++ b/tools/sysprof/parse.lua
@@ -59,7 +59,7 @@ end
 local function parse_lfunc(reader, symbols)
   local addr = reader:read_uleb128()
   local line = reader:read_uleb128()
-  local loc = symtab.loc(symbols, { addr = addr, line = line })
+  local loc = symtab.loc{ addr = addr, line = line }
   loc.type = FRAME.LFUNC
   return symtab.demangle(symbols, loc)
 end
@@ -71,7 +71,7 @@ end
 
 local function parse_cfunc(reader, symbols)
   local addr = reader:read_uleb128()
-  local loc = symtab.loc(symbols, { addr = addr })
+  local loc = symtab.loc{ addr = addr }
   loc.type = FRAME.CFUNC
   return symtab.demangle(symbols, loc)
 end
@@ -99,7 +99,7 @@ local function parse_host_callchain(reader, event, symbols)
   local addr = reader:read_uleb128()
 
   while addr ~= 0 do
-    local loc = symtab.loc(symbols, { addr = addr })
+    local loc = symtab.loc{ addr = addr }
     table.insert(event.host.callchain, 1, symtab.demangle(symbols, loc))
     addr = reader:read_uleb128()
   end
@@ -113,14 +113,11 @@ local function parse_trace_callchain(reader, event, symbols)
   loc.addr = reader:read_uleb128()
   loc.line = reader:read_uleb128()
 
-  local gen = symtab.loc(symbols, loc).gen
   local name_lua = symtab.demangle(symbols, {
     addr = loc.addr,
     traceno = loc.traceno,
-    gen = gen
   })
   event.lua.trace = loc
-  event.lua.trace.gen = gen
   event.lua.trace.name = name_lua
 end
 
diff --git a/tools/utils/avl.lua b/tools/utils/avl.lua
index d5baa534..81ef9265 100644
--- a/tools/utils/avl.lua
+++ b/tools/utils/avl.lua
@@ -78,7 +78,7 @@ end
 function M.insert(node, key, value)
   assert(key, "Key can't be nil")
   if node == nil then
-    return create_node(key, { value })
+    return create_node(key, value)
   end
 
   if key < node.key then
@@ -86,7 +86,7 @@ function M.insert(node, key, value)
   elseif key > node.key then
     node.right = M.insert(node.right, key, value)
   else
-    table.insert(node.value, value)
+    node.value = value
   end
 
   update_height(node)
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index c26a9e8c..2afd2fd1 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -22,22 +22,13 @@ local SYMTAB_TRACE = 2
 
 local M = {}
 
-function M.loc(symtab, args)
+function M.loc(args)
   local loc = {
     addr = args.addr or 0,
     line = args.line or 0,
     traceno = args.traceno or 0,
   }
 
-  if loc.traceno ~= 0 and symtab.trace[loc.traceno] then
-    loc.gen = #symtab.trace[loc.traceno]
-  elseif symtab.lfunc[loc.addr] then
-    loc.gen = #symtab.lfunc[loc.addr]
-  else
-    local _, csym = avl.floor(symtab.cfunc, loc.addr)
-    loc.gen = csym and #csym or 1
-  end
-
   return loc
 end
 
@@ -56,10 +47,10 @@ function M.parse_sym_lfunc(reader, symtab)
     )
   end
 
-  table.insert(symtab.lfunc[sym_addr], {
+  symtab.lfunc[sym_addr] = {
     source = sym_chunk,
     linedefined = sym_line,
-  })
+  }
 end
 
 function M.parse_sym_trace(reader, symtab)
@@ -69,9 +60,9 @@ function M.parse_sym_trace(reader, symtab)
 
   symtab.trace[traceno] = symtab.trace[traceno] or {}
 
-  table.insert(symtab.trace[traceno], {
-    start = M.loc(symtab, { addr = sym_addr, line = sym_line })
-  })
+  symtab.trace[traceno] = {
+    start = M.loc{ addr = sym_addr, line = sym_line }
+  }
 end
 
 -- Parse a single entry in a symtab: .so library
@@ -136,7 +127,7 @@ end
 
 function M.id(loc)
   return string_format(
-    "f%#xl%dt%dg%d", loc.addr, loc.line, loc.traceno, loc.gen
+    "f%#xl%dt%dg", loc.addr, loc.line, loc.traceno
   )
 end
 
@@ -146,8 +137,7 @@ local function demangle_trace(symtab, loc)
   assert(traceno ~= 0, "Location is a trace")
 
   local trace_str = string_format("TRACE [%d]", traceno)
-  local gens = symtab.trace[traceno]
-  local trace = gens and gens[loc.gen]
+  local trace = symtab.trace[traceno]
 
   if trace then
     assert(trace.start.traceno == 0, "Trace start is not a trace")
@@ -162,21 +152,20 @@ function M.demangle(symtab, loc)
   end
 
   local addr = loc.addr
-  local gen = loc.gen
 
   if addr == 0 then
     return "INTERNAL"
   end
 
-  if symtab.lfunc[addr] and symtab.lfunc[addr][gen] then
-    local source = symtab.lfunc[addr][gen].source
+  if symtab.lfunc[addr] then
+    local source = symtab.lfunc[addr].source
     return string_format("%s:%d", symtab.alias[source] or source, loc.line)
   end
 
   local key, value = avl.floor(symtab.cfunc, addr)
 
   if key then
-    return string_format("%s:%#x", value[gen].name, key)
+    return string_format("%s:%#x", value.name, key)
   end
 
   return string_format("CFUNC %#x", addr)
-- 
2.41.0

Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
LGTM, with a minor nit below.

On 28.08.23, Maxim Kokryashkin wrote:
> This patch refactors symbol resolution in memprof, so
> now they are resolved during the process of parsing.
> This makes the generation mechanism excessive since
> symtab updates are no longer affecting the previous events.
> 
> Follows up tarantool/tarantool#8700
> ---
>  .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
>  tools/memprof/humanize.lua                    | 16 ++--
>  tools/memprof/parse.lua                       |  2 +-
>  tools/memprof/process.lua                     |  8 +-
>  4 files changed, 31 insertions(+), 68 deletions(-)
> 
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index 3cb5c8be..0eee4e21 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua

<snipped>

> @@ -86,44 +86,13 @@ local function form_source_line(line, source)
>  end
>  
>  local function form_trace_line(traceno, line, source)
> -  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
> +  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)

Nit: Line width is more than 80 symbols.

>  end
>  
> -local function fill_ev_type(events, symbols, event_type)

<snipped>

> diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
> index 267baa91..5b289ce3 100644
> --- a/tools/memprof/humanize.lua
> +++ b/tools/memprof/humanize.lua

<snipped>

> diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> index d865267b..1b6eceb4 100644
> --- a/tools/memprof/parse.lua
> +++ b/tools/memprof/parse.lua

<snipped>

> diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
> index 0bcb965b..faf4be4e 100644
> --- a/tools/memprof/process.lua
> +++ b/tools/memprof/process.lua

<snipped>

> -- 
> 2.41.0
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
I see improvements in time of the parsing even for small payloads!
Nice work! LGTM, with minor nits below.

On 28.08.23, Maxim Kokryashkin wrote:
> Since both of the profiler parsers are now processing
> the events in a stream-like fashion, the generation
> mechanism is excessive and can be purged. This results
> in a significant memory consumption drop, especially
> for the AVL-tree part.
> 
> Follows up tarantool/tarantool#8700
> ---
>  .../gh-5813-resolving-of-c-symbols.test.lua   |  7 ++--
>  tools/memprof/parse.lua                       |  2 +-
>  tools/sysprof/parse.lua                       |  9 ++---
>  tools/utils/avl.lua                           |  4 +--
>  tools/utils/symtab.lua                        | 33 +++++++------------
>  5 files changed, 19 insertions(+), 36 deletions(-)
> 
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> index 30b8a3ca..1581ee4b 100644
> --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> @@ -24,12 +24,9 @@ local TMP_BINFILE = profilename("memprofdata.tmp.bin")

<snipped>

> diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> index 1b6eceb4..6fd7451b 100644
> --- a/tools/memprof/parse.lua
> +++ b/tools/memprof/parse.lua

<snipped>

> diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
> index 451edecb..bb1cb02c 100755
> --- a/tools/sysprof/parse.lua
> +++ b/tools/sysprof/parse.lua
> @@ -59,7 +59,7 @@ end
>  local function parse_lfunc(reader, symbols)
>    local addr = reader:read_uleb128()
>    local line = reader:read_uleb128()
> -  local loc = symtab.loc(symbols, { addr = addr, line = line })
> +  local loc = symtab.loc{ addr = addr, line = line }

Minor: Please, use `()` for function calls. Here and below.

>    loc.type = FRAME.LFUNC
>    return symtab.demangle(symbols, loc)
>  end
> @@ -71,7 +71,7 @@ end
>  
>  local function parse_cfunc(reader, symbols)
>    local addr = reader:read_uleb128()
> -  local loc = symtab.loc(symbols, { addr = addr })
> +  local loc = symtab.loc{ addr = addr }
>    loc.type = FRAME.CFUNC
>    return symtab.demangle(symbols, loc)
>  end
> @@ -99,7 +99,7 @@ local function parse_host_callchain(reader, event, symbols)
>    local addr = reader:read_uleb128()
>  
>    while addr ~= 0 do
> -    local loc = symtab.loc(symbols, { addr = addr })
> +    local loc = symtab.loc{ addr = addr }
>      table.insert(event.host.callchain, 1, symtab.demangle(symbols, loc))
>      addr = reader:read_uleb128()
>    end
> @@ -113,14 +113,11 @@ local function parse_trace_callchain(reader, event, symbols)

<snipped>

> diff --git a/tools/utils/avl.lua b/tools/utils/avl.lua
> index d5baa534..81ef9265 100644
> --- a/tools/utils/avl.lua
> +++ b/tools/utils/avl.lua

<snipped>

> diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
> index c26a9e8c..2afd2fd1 100644
> --- a/tools/utils/symtab.lua
> +++ b/tools/utils/symtab.lua

<snipped>

> @@ -69,9 +60,9 @@ function M.parse_sym_trace(reader, symtab)
>  
>    symtab.trace[traceno] = symtab.trace[traceno] or {}
>  
> -  table.insert(symtab.trace[traceno], {
> -    start = M.loc(symtab, { addr = sym_addr, line = sym_line })
> -  })
> +  symtab.trace[traceno] = {
> +    start = M.loc{ addr = sym_addr, line = sym_line }
> +  }
>  end
>  
>  -- Parse a single entry in a symtab: .so library
> @@ -136,7 +127,7 @@ end

<snipped>

> -- 
> 2.41.0
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Fixed your comment, the branch is force pushed.

On Sun, Sep 03, 2023 at 01:14:37PM +0300, Sergey Kaplun wrote:
> Hi, Maxim!
> Thanks for the patch!
> LGTM, with a minor nit below.
> 
> On 28.08.23, Maxim Kokryashkin wrote:
> > This patch refactors symbol resolution in memprof, so
> > now they are resolved during the process of parsing.
> > This makes the generation mechanism excessive since
> > symtab updates are no longer affecting the previous events.
> > 
> > Follows up tarantool/tarantool#8700
> > ---
> >  .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
> >  tools/memprof/humanize.lua                    | 16 ++--
> >  tools/memprof/parse.lua                       |  2 +-
> >  tools/memprof/process.lua                     |  8 +-
> >  4 files changed, 31 insertions(+), 68 deletions(-)
> > 
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index 3cb5c8be..0eee4e21 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> 
> <snipped>
> 
> > @@ -86,44 +86,13 @@ local function form_source_line(line, source)
> >  end
> >  
> >  local function form_trace_line(traceno, line, source)
> > -  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
> > +  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)
> 
> Nit: Line width is more than 80 symbols.

Here is the diff:
==========================================================================
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 0eee4e21..b97462a5 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -86,7 +86,11 @@ local function form_source_line(line, source)
 end
 
 local function form_trace_line(traceno, line, source)
-  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)
+  return ("TRACE [%d] started at %s:%d"):format(
+    traceno,
+    source or SRC_PATH,
+    line
+  )
 end
 
 local function fill_ev_type(events, event_type)
==========================================================================
> 
> >  end
> >  
> > -local function fill_ev_type(events, symbols, event_type)
> 
> <snipped>
> 
> > diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
> > index 267baa91..5b289ce3 100644
> > --- a/tools/memprof/humanize.lua
> > +++ b/tools/memprof/humanize.lua
> 
> <snipped>
> 
> > diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> > index d865267b..1b6eceb4 100644
> > --- a/tools/memprof/parse.lua
> > +++ b/tools/memprof/parse.lua
> 
> <snipped>
> 
> > diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
> > index 0bcb965b..faf4be4e 100644
> > --- a/tools/memprof/process.lua
> > +++ b/tools/memprof/process.lua
> 
> <snipped>
> 
> > -- 
> > 2.41.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Fixed your comments, the branch is force-pushed.
On Sun, Sep 03, 2023 at 01:21:35PM +0300, Sergey Kaplun wrote:
> Hi, Maxim!
> Thanks for the patch!
> I see improvements in time of the parsing even for small payloads!
> Nice work! LGTM, with minor nits below.
> 
> On 28.08.23, Maxim Kokryashkin wrote:
> > Since both of the profiler parsers are now processing
> > the events in a stream-like fashion, the generation
> > mechanism is excessive and can be purged. This results
> > in a significant memory consumption drop, especially
> > for the AVL-tree part.
> > 
> > Follows up tarantool/tarantool#8700
> > ---
> >  .../gh-5813-resolving-of-c-symbols.test.lua   |  7 ++--
> >  tools/memprof/parse.lua                       |  2 +-
> >  tools/sysprof/parse.lua                       |  9 ++---
> >  tools/utils/avl.lua                           |  4 +--
> >  tools/utils/symtab.lua                        | 33 +++++++------------
> >  5 files changed, 19 insertions(+), 36 deletions(-)
> > 
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > index 30b8a3ca..1581ee4b 100644
> > --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > @@ -24,12 +24,9 @@ local TMP_BINFILE = profilename("memprofdata.tmp.bin")
> 
> <snipped>
> 
> > diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> > index 1b6eceb4..6fd7451b 100644
> > --- a/tools/memprof/parse.lua
> > +++ b/tools/memprof/parse.lua
> 
> <snipped>
> 
> > diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
> > index 451edecb..bb1cb02c 100755
> > --- a/tools/sysprof/parse.lua
> > +++ b/tools/sysprof/parse.lua
> > @@ -59,7 +59,7 @@ end
> >  local function parse_lfunc(reader, symbols)
> >    local addr = reader:read_uleb128()
> >    local line = reader:read_uleb128()
> > -  local loc = symtab.loc(symbols, { addr = addr, line = line })
> > +  local loc = symtab.loc{ addr = addr, line = line }
> 
> Minor: Please, use `()` for function calls. Here and below.
Fixed. Here is the diff:

==========================================================================
diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
index bb1cb02c..64c4a455 100755
--- a/tools/sysprof/parse.lua
+++ b/tools/sysprof/parse.lua
@@ -59,7 +59,7 @@ end
 local function parse_lfunc(reader, symbols)
   local addr = reader:read_uleb128()
   local line = reader:read_uleb128()
-  local loc = symtab.loc{ addr = addr, line = line }
+  local loc = symtab.loc({ addr = addr, line = line })
   loc.type = FRAME.LFUNC
   return symtab.demangle(symbols, loc)
 end
@@ -71,7 +71,7 @@ end
 
 local function parse_cfunc(reader, symbols)
   local addr = reader:read_uleb128()
-  local loc = symtab.loc{ addr = addr }
+  local loc = symtab.loc({ addr = addr })
   loc.type = FRAME.CFUNC
   return symtab.demangle(symbols, loc)
 end
@@ -99,7 +99,7 @@ local function parse_host_callchain(reader, event, symbols)
   local addr = reader:read_uleb128()
 
   while addr ~= 0 do
-    local loc = symtab.loc{ addr = addr }
+    local loc = symtab.loc({ addr = addr })
     table.insert(event.host.callchain, 1, symtab.demangle(symbols, loc))
     addr = reader:read_uleb128()
   end
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index 2afd2fd1..0c878e7a 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -61,7 +61,7 @@ function M.parse_sym_trace(reader, symtab)
   symtab.trace[traceno] = symtab.trace[traceno] or {}
 
   symtab.trace[traceno] = {
-    start = M.loc{ addr = sym_addr, line = sym_line }
+    start = M.loc({ addr = sym_addr, line = sym_line })
   }
 end

==========================================================================
> 
> >    loc.type = FRAME.LFUNC
> >    return symtab.demangle(symbols, loc)
> >  end
> > @@ -71,7 +71,7 @@ end
> >  
> >  local function parse_cfunc(reader, symbols)
> >    local addr = reader:read_uleb128()
> > -  local loc = symtab.loc(symbols, { addr = addr })
> > +  local loc = symtab.loc{ addr = addr }
> >    loc.type = FRAME.CFUNC
> >    return symtab.demangle(symbols, loc)
> >  end
> > @@ -99,7 +99,7 @@ local function parse_host_callchain(reader, event, symbols)
> >    local addr = reader:read_uleb128()
> >  
> >    while addr ~= 0 do
> > -    local loc = symtab.loc(symbols, { addr = addr })
> > +    local loc = symtab.loc{ addr = addr }
> >      table.insert(event.host.callchain, 1, symtab.demangle(symbols, loc))
> >      addr = reader:read_uleb128()
> >    end
> > @@ -113,14 +113,11 @@ local function parse_trace_callchain(reader, event, symbols)
> 
> <snipped>
> 
> > diff --git a/tools/utils/avl.lua b/tools/utils/avl.lua
> > index d5baa534..81ef9265 100644
> > --- a/tools/utils/avl.lua
> > +++ b/tools/utils/avl.lua
> 
> <snipped>
> 
> > diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
> > index c26a9e8c..2afd2fd1 100644
> > --- a/tools/utils/symtab.lua
> > +++ b/tools/utils/symtab.lua
> 
> <snipped>
> 
> > @@ -69,9 +60,9 @@ function M.parse_sym_trace(reader, symtab)
> >  
> >    symtab.trace[traceno] = symtab.trace[traceno] or {}
> >  
> > -  table.insert(symtab.trace[traceno], {
> > -    start = M.loc(symtab, { addr = sym_addr, line = sym_line })
> > -  })
> > +  symtab.trace[traceno] = {
> > +    start = M.loc{ addr = sym_addr, line = sym_line }
> > +  }
> >  end
> >  
> >  -- Parse a single entry in a symtab: .so library
> > @@ -136,7 +127,7 @@ end
> 
> <snipped>
> 
> > -- 
> > 2.41.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Sergey Bronnikov via Tarantool-patches]

Hi, Max


thanks for the patch! LGTM with a two minor comments.


It is not related to submitted patches, but anyway - there is a typo:

--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -36,7 +36,7 @@ local function default_payload()
    -- Preallocate table to avoid table array part reallocations.
    local _ = table_new(20, 0)

-  -- Want too see 20 objects here.
+  -- Want to see 20 objects here.
    for i = 1, 20 do
      -- Try to avoid crossing with "test" module objects.
      _[i] = "memprof-str-"..i


On 8/28/23 18:23, Maxim Kokryashkin wrote:
> This patch refactors symbol resolution in memprof, so
> now they are resolved during the process of parsing.
> This makes the generation mechanism excessive since
> symtab updates are no longer affecting the previous events.
>
> Follows up tarantool/tarantool#8700
> ---
>   .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
>   tools/memprof/humanize.lua                    | 16 ++--
>   tools/memprof/parse.lua                       |  2 +-
>   tools/memprof/process.lua                     |  8 +-
>   4 files changed, 31 insertions(+), 68 deletions(-)
>
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index 3cb5c8be..0eee4e21 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -78,7 +78,7 @@ local function generate_parsed_output(payload)
>     -- We don't need it any more.
>     os.remove(TMP_BINFILE)
>   
> -  return symbols, events
> +  return events
>   end
>   
>   local function form_source_line(line, source)
> @@ -86,44 +86,13 @@ local function form_source_line(line, source)
>   end
>   
>   local function form_trace_line(traceno, line, source)
> -  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
> +  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)
>   end

Nit: this change is not relevant. In the branch hunk is different:

@@ -86,44 +86,17 @@ local function form_source_line(line, source)
  end

  local function form_trace_line(traceno, line, source)
-  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
+  return ("TRACE [%d] started at %s:%d"):format(
+    traceno,
+    source or SRC_PATH,
+    line
+  )
  end

>   
<snipped>

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Bronnikov via Tarantool-patches]

Hi, Max

thanks for the patch. See my comment below.


On 8/28/23 18:23, Maxim Kokryashkin wrote:
> Since both of the profiler parsers are now processing
> the events in a stream-like fashion, the generation
> mechanism is excessive and can be purged. This results
> in a significant memory consumption drop, especially
> for the AVL-tree part.

Would be interesting to see a numbers with memory consumption

before and after the patch.

Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
On Tue, Sep 05, 2023 at 02:47:15PM +0300, Sergey Bronnikov wrote:
> Hi, Max
> 
> 
> thanks for the patch! LGTM with a two minor comments.
> 
> 
> It is not related to submitted patches, but anyway - there is a typo:
> 
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -36,7 +36,7 @@ local function default_payload()
>    -- Preallocate table to avoid table array part reallocations.
>    local _ = table_new(20, 0)
> 
> -  -- Want too see 20 objects here.
> +  -- Want to see 20 objects here.
>    for i = 1, 20 do
>      -- Try to avoid crossing with "test" module objects.
>      _[i] = "memprof-str-"..i
I don't really want to change it in scope of this patch set,
so ignoring.
 
> Nit: this change is not relevant. In the branch hunk is different:
> 
> @@ -86,44 +86,17 @@ local function form_source_line(line, source)
>  end
> 
>  local function form_trace_line(traceno, line, source)
> -  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
> +  return ("TRACE [%d] started at %s:%d"):format(
> +    traceno,
> +    source or SRC_PATH,
> +    line
> +  )
>  end
It is not irrelevant. This patchset required tests to be rewritten,
which resulted in addition of `started at` to this format string
and it went over the 80-character limit.
> 
> <snipped>

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]


Hi, Sergey!
Sure, consider this script:
| jit.off()
| misc.sysprof.start{mode = 'C', interval=10}
| for i = 1, 1e7 do tostring(i) end
| misc.sysprof.stop()
 
After running it with luajit you can parse the output
like this:
| $ time -v luajit-parse-sysprof sysprof.bin
 
So, before the patch:
| Maximum resident set size (kbytes): 224928
 
And after the patch:
| Maximum resident set size (kbytes): 32780
 
Which is 85% reduction in memory consumption.
--
Best regards,
Maxim Kokryashkin
 
  
>Вторник, 5 сентября 2023, 14:53 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Hi, Max
>
>thanks for the patch. See my comment below.
>
>
>On  8/28/23 18 :23, Maxim Kokryashkin wrote:
>> Since both of the profiler parsers are now processing
>> the events in a stream-like fashion, the generation
>> mechanism is excessive and can be purged. This results
>> in a significant memory consumption drop, especially
>> for the AVL-tree part.
>Would be interesting to see a numbers with memory consumption
>
>before and after the patch.
>
> 
 

[-- Attachment #2: Type: text/html, Size: 1869 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 3881 bytes --]

Hi, Max


On 9/12/23 13:26, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Sure, consider this script:
> | jit.off()
> | misc.sysprof.start{mode = 'C', interval=10}
> | for i = 1, 1e7 do tostring(i) end
> | misc.sysprof.stop()

I failed with step:

$ cat prof.lua
jit.off()
misc.sysprof.start{mode = 'C', interval=10}
for i = 1, 1e7 do tostring(i) end
misc.sysprof.stop()

$ ./build/src/luajit prof.lua
Segmentation fault (core dumped)

119       lj_wbuf_addu64(buf, (uint64_t)pt->firstline);
(gdb) bt
#0  0x0000563092387308 in stream_lfunc (buf=0x563092449458 <sysprof+24>, 
func=0x7fdec2bda0a0)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:119
#1  0x00005630923874a9 in stream_frame_lua (buf=0x563092449458 
<sysprof+24>, frame=0x7fdec48a2cd8)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:141
#2  0x00005630923876b1 in stream_backtrace_lua (sp=0x563092449440 <sysprof>)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:169
#3  0x0000563092387ab1 in stream_guest (sp=0x563092449440 <sysprof>, 
vmstate=2)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:237
#4  0x0000563092387c3f in stream_event (sp=0x563092449440 <sysprof>, 
vmstate=2)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:274
#5  0x0000563092387d26 in sysprof_record_sample (sp=0x563092449440 
<sysprof>, info=0x7ffe5b59b730)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:296
#6  0x0000563092387ddc in sysprof_signal_handler (sig=27, 
info=0x7ffe5b59b730, ctx=0x7ffe5b59b600)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:312
#7  <signal handler called>
#8  0x00005630923ca3c5 in lj_fff_res1 () at buildvm_x86.dasc:1890
#9  0x000056309232c8e2 in lua_pcall (L=0x7fdec48a1378, nargs=0, 
nresults=-1, errfunc=2)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_api.c:1172
#10 0x000056309231ec29 in docall (L=0x7fdec48a1378, narg=0, clear=0) at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:133
#11 0x000056309231f63d in handle_script (L=0x7fdec48a1378, 
argx=0x7ffe5b59c0b0)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:303
#12 0x000056309232059b in pmain (L=0x7fdec48a1378) at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:603
#13 0x00005630923c9029 in lj_BC_FUNCC () at buildvm_x86.dasc:852
#14 0x000056309232cf4a in lua_cpcall (L=0x7fdec48a1378, 
func=0x5630923203a3 <pmain>, ud=0x0)
     at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_api.c:1207
#15 0x00005630923206b7 in main (argc=2, argv=0x7ffe5b59c0a8) at 
/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:632
(gdb)


> After running it with luajit you can parse the output
> like this:
> | $ time -v luajit-parse-sysprof sysprof.bin
> So, before the patch:
> | Maximum resident set size (kbytes): 224928
> And after the patch:
> | Maximum resident set size (kbytes): 32780
> Which is 85% reduction in memory consumption.


I propose to add these numbers to commit message.


> --
> Best regards,
> Maxim Kokryashkin
>
>     Вторник, 5 сентября 2023, 14:53 +03:00 от Sergey Bronnikov
>     <sergeyb@tarantool.org>:
>     Hi, Max
>
>     thanks for the patch. See my comment below.
>
>
>     On 8/28/23 18:23, Maxim Kokryashkin wrote:
>     > Since both of the profiler parsers are now processing
>     > the events in a stream-like fashion, the generation
>     > mechanism is excessive and can be purged. This results
>     > in a significant memory consumption drop, especially
>     > for the AVL-tree part.
>
>     Would be interesting to see a numbers with memory consumption
>
>     before and after the patch.
>

[-- Attachment #2: Type: text/html, Size: 6612 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 4978 bytes --]


Hi, Sergey!
Thanks for the comments!
Here is the new message:
=======
profilers: purge generation mechanism
 
Since both of the profiler parsers are now processing
the events in a stream-like fashion, the generation
mechanism is excessive and can be purged. This results
in a significant memory consumption drop, especially
for the AVL-tree part.
 
Consider this script:
| jit.off()
| misc.sysprof.start{mode = 'C', interval=10}
| for i = 1, 1e7 do tostring(i) end
| misc.sysprof.stop()
 
After executing it with LuaJIT, you can parse it like this:
| $ time -v luajit-parse-sysprof sysprof.bin
 
So, before the patch:
| Maximum resident set size (kbytes): 224928
 
And after the patch:
| Maximum resident set size (kbytes): 32780
 
That is the 85% reduction in memory consumption.
 
Follows up tarantool/tarantool#8700
=======
 
> 
>>Hi, Max
>> 
>>On 9/12/23 13:26, Maxim Kokryashkin wrote:
>>>Hi, Sergey!
>>>Sure, consider this script:
>>>| jit.off()
>>>| misc.sysprof.start{mode = 'C', interval=10}
>>>| for i = 1, 1e7 do tostring(i) end
>>>| misc.sysprof.stop()
>>I failed with step:
>>$ cat prof.lua
>>jit.off()
>>misc.sysprof.start{mode = 'C', interval=10}
>>for i = 1, 1e7 do tostring(i) end
>>misc.sysprof.stop()
>> 
>>$ ./build/src/luajit prof.lua
>>Segmentation fault (core dumped)
>That may happen sometimes, since we still haven’t merged my patch for ffuncs.
>> 
>>119       lj_wbuf_addu64(buf, (uint64_t)pt->firstline);
>>(gdb) bt
>>#0  0x0000563092387308 in stream_lfunc (buf=0x563092449458 <sysprof+24>, func=0x7fdec2bda0a0)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:119
>>#1  0x00005630923874a9 in stream_frame_lua (buf=0x563092449458 <sysprof+24>, frame=0x7fdec48a2cd8)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:141
>>#2  0x00005630923876b1 in stream_backtrace_lua (sp=0x563092449440 <sysprof>)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:169
>>#3  0x0000563092387ab1 in stream_guest (sp=0x563092449440 <sysprof>, vmstate=2)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:237
>>#4  0x0000563092387c3f in stream_event (sp=0x563092449440 <sysprof>, vmstate=2)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:274
>>#5  0x0000563092387d26 in sysprof_record_sample (sp=0x563092449440 <sysprof>, info=0x7ffe5b59b730)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:296
>>#6  0x0000563092387ddc in sysprof_signal_handler (sig=27, info=0x7ffe5b59b730, ctx=0x7ffe5b59b600)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_sysprof.c:312
>>#7  <signal handler called>
>>#8  0x00005630923ca3c5 in lj_fff_res1 () at buildvm_x86.dasc:1890
>>#9  0x000056309232c8e2 in lua_pcall (L=0x7fdec48a1378, nargs=0, nresults=-1, errfunc=2)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_api.c:1172
>>#10 0x000056309231ec29 in docall (L=0x7fdec48a1378, narg=0, clear=0) at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:133
>>#11 0x000056309231f63d in handle_script (L=0x7fdec48a1378, argx=0x7ffe5b59c0b0)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:303
>>#12 0x000056309232059b in pmain (L=0x7fdec48a1378) at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:603
>>#13 0x00005630923c9029 in lj_BC_FUNCC () at buildvm_x86.dasc:852
>>#14 0x000056309232cf4a in lua_cpcall (L=0x7fdec48a1378, func=0x5630923203a3 <pmain>, ud=0x0)
>>    at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_api.c:1207
>>#15 0x00005630923206b7 in main (argc=2, argv=0x7ffe5b59c0a8) at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/luajit.c:632
>>(gdb)
>> 
>> 
>>> 
>>>After running it with luajit you can parse the output
>>>like this:
>>>| $ time -v luajit-parse-sysprof sysprof.bin
>>> 
>>>So, before the patch:
>>>| Maximum resident set size (kbytes): 224928
>>> 
>>>And after the patch:
>>>| Maximum resident set size (kbytes): 32780
>>> 
>>>Which is 85% reduction in memory consumption.
>> 
>>I propose to add these numbers to commit message.
>> 
>>>--
>>>Best regards,
>>>Maxim Kokryashkin
>>> 
>>>  
>>>>Вторник, 5 сентября 2023, 14:53 +03:00 от Sergey Bronnikov  <sergeyb@tarantool.org> :
>>>> 
>>>>Hi, Max
>>>>
>>>>thanks for the patch. See my comment below.
>>>>
>>>>
>>>>On  8/28/23 18 :23, Maxim Kokryashkin wrote:
>>>>> Since both of the profiler parsers are now processing
>>>>> the events in a stream-like fashion, the generation
>>>>> mechanism is excessive and can be purged. This results
>>>>> in a significant memory consumption drop, especially
>>>>> for the AVL-tree part.
>>>>Would be interesting to see a numbers with memory consumption
>>>>
>>>>before and after the patch.
>>>>
>>>> 
>>> 
>--
>Best regards,
>Maxim Kokryashkin
> 

[-- Attachment #2: Type: text/html, Size: 7404 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Bronnikov via Tarantool-patches]

Max,

On 9/12/23 13:26, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Sure, consider this script:
> | jit.off()
> | misc.sysprof.start{mode = 'C', interval=10}
> | for i = 1, 1e7 do tostring(i) end
> | misc.sysprof.stop()
> After running it with luajit you can parse the output
> like this:
> | $ time -v luajit-parse-sysprof sysprof.bin
> So, before the patch:
> | Maximum resident set size (kbytes): 224928
> And after the patch:
> | Maximum resident set size (kbytes): 32780
> Which is 85% reduction in memory consumption.
>
I would put these number to commit message.

Feel free to ignore.

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Bronnikov via Tarantool-patches]

Hi, Max

On 9/18/23 12:18, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the comments!
> Here is the new message:
> =======
> profilers: purge generation mechanism
> Since both of the profiler parsers are now processing
> the events in a stream-like fashion, the generation
> mechanism is excessive and can be purged. This results
> in a significant memory consumption drop, especially
> for the AVL-tree part.
> Consider this script:
> | jit.off()
> | misc.sysprof.start{mode = 'C', interval=10}
> | for i = 1, 1e7 do tostring(i) end
> | misc.sysprof.stop()
> After executing it with LuaJIT, you can parse it like this:
> | $ time -v luajit-parse-sysprof sysprof.bin
> So, before the patch:
> | Maximum resident set size (kbytes): 224928
> And after the patch:
> | Maximum resident set size (kbytes): 32780
> That is the 85% reduction in memory consumption.
> Follows up tarantool/tarantool#8700


Seems you forgot force-push to the branch.

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Bronnikov via Tarantool-patches]

On 10/6/23 17:43, Sergey Bronnikov via Tarantool-patches wrote:
> Hi, Max
>
> On 9/18/23 12:18, Maxim Kokryashkin wrote:
>> Hi, Sergey!
>> Thanks for the comments!
>> Here is the new message:
>> =======
>> profilers: purge generation mechanism
>> Since both of the profiler parsers are now processing
>> the events in a stream-like fashion, the generation
>> mechanism is excessive and can be purged. This results
>> in a significant memory consumption drop, especially
>> for the AVL-tree part.
>> Consider this script:
>> | jit.off()
>> | misc.sysprof.start{mode = 'C', interval=10}
>> | for i = 1, 1e7 do tostring(i) end
>> | misc.sysprof.stop()
>> After executing it with LuaJIT, you can parse it like this:
>> | $ time -v luajit-parse-sysprof sysprof.bin
>> So, before the patch:
>> | Maximum resident set size (kbytes): 224928
>> And after the patch:
>> | Maximum resident set size (kbytes): 32780
>> That is the 85% reduction in memory consumption.
>> Follows up tarantool/tarantool#8700
>
>
> Seems you forgot force-push to the branch.
>
Please disregard.

Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Sergey Bronnikov via Tarantool-patches]

Max,

On 9/8/23 14:04, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the review!
> On Tue, Sep 05, 2023 at 02:47:15PM +0300, Sergey Bronnikov wrote:
>
LGTM


<snipped>

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Sergey Bronnikov via Tarantool-patches]

Max,

On 9/12/23 13:26, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Sure, consider this script:
> | jit.off()
> | misc.sysprof.start{mode = 'C', interval=10}
> | for i = 1, 1e7 do tostring(i) end
> | misc.sysprof.stop()
> After running it with luajit you can parse the output
> like this:
> | $ time -v luajit-parse-sysprof sysprof.bin
> So, before the patch:
> | Maximum resident set size (kbytes): 224928
> And after the patch:
> | Maximum resident set size (kbytes): 32780
> Which is 85% reduction in memory consumption.
> --
> Best regards,
> Maxim Kokryashkin

LGTM

<snipped>

Re: [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism

[Igor Munkin via Tarantool-patches]

Max,

On 12.09.23, Maxim Kokryashkin via Tarantool-patches wrote:
> 
> Hi, Sergey!
> Sure, consider this script:
> | jit.off()
> | misc.sysprof.start{mode = 'C', interval=10}
> | for i = 1, 1e7 do tostring(i) end
> | misc.sysprof.stop()
>  
> After running it with luajit you can parse the output
> like this:
> | $ time -v luajit-parse-sysprof sysprof.bin
>  
> So, before the patch:
> | Maximum resident set size (kbytes): 224928
>  
> And after the patch:
> | Maximum resident set size (kbytes): 32780
>  
> Which is 85% reduction in memory consumption.

Neat, thanks for the patch!

> --
> Best regards,
> Maxim Kokryashkin
>  

<snipped>

>  

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 0/2] profilers: purge generations

[Igor Munkin via Tarantool-patches]

Max,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 28.08.23, Maxim Kokryashkin via Tarantool-patches wrote:
> This series refactors memprof's binary data parser, so it
> doesn't require generations now. Since both profilers don't
> need generations now, that mechanism is purged to improve
> memory footprint.
> 
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/memprof-parser-refactoring
> PR: https://github.com/tarantool/tarantool/pull/9051
> 
> Maxim Kokryashkin (2):
>   memprof: refactor symbol resolution
>   profilers: purge generation mechanism
> 
>  .../gh-5813-resolving-of-c-symbols.test.lua   |  7 +-
>  .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
>  tools/memprof/humanize.lua                    | 16 ++--
>  tools/memprof/parse.lua                       |  4 +-
>  tools/memprof/process.lua                     |  8 +-
>  tools/sysprof/parse.lua                       |  9 +--
>  tools/utils/avl.lua                           |  4 +-
>  tools/utils/symtab.lua                        | 33 +++------
>  8 files changed, 50 insertions(+), 104 deletions(-)
> 
> -- 
> 2.41.0
> 

-- 
Best regards,
IM