[Tarantool-patches] [PATCH luajit v4 0/3] sysprof: parser refactoring

[Tarantool-patches] [PATCH luajit v4 0/3] sysprof: parser refactoring

[Maksim Kokryashkin via Tarantool-patches]

Changes in v4:
- Fixed comments as per review by Igor
- Dropped the AVL-tree changes, since the memprof still requires
  generations to function properly. The AVL-tree will be updated after
  the memprof's parser refactoring.

PR: https://github.com/tarantool/tarantool/pull/8703
Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8700-sysprof-parser-refactoring

Maxim Kokryashkin (3):
  sysprof: remove `split by vmstate` option
  tools: add execution permission to sysprof parser
  sysprof: improve parser's memory footprint

 tools/CMakeLists.txt          |   4 ++
 tools/luajit-parse-sysprof.in |   0
 tools/sysprof.lua             |  28 +-------
 tools/sysprof/collapse.lua    | 127 +---------------------------------
 tools/sysprof/parse.lua       | 125 +++++++++++++++++++++++----------
 5 files changed, 100 insertions(+), 184 deletions(-)
 mode change 100644 => 100755 tools/luajit-parse-sysprof.in

--
2.39.2 (Apple Git-143)

[Tarantool-patches] [PATCH luajit v4 1/3] sysprof: remove `split by vmstate` option

[Maksim Kokryashkin via Tarantool-patches]

From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

The option was introduced early into the sysprof's
development to provide capabilities for per-VM-state
callchain visualization. However, during the sysprof's
recent active in-house usage period, it became clear
that performance issues associated with specific VM
states (such as GC, for example) have a distinct look
on a flamegraph itself, thanks to their name prefixes
and their surrounding context.

This option is unneeded and was never used
by anybody. There is no reason to maintain it
in the scope of refactoring.

Needed for tarantool/tarantool#8700
---
 tools/sysprof.lua          |  9 +--------
 tools/sysprof/collapse.lua | 10 +++-------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/tools/sysprof.lua b/tools/sysprof.lua
index 1afab195..22c724e9 100644
--- a/tools/sysprof.lua
+++ b/tools/sysprof.lua
@@ -6,8 +6,6 @@ local misc = require "sysprof.collapse"
 local stdout, stderr = io.stdout, io.stderr
 local match, gmatch = string.match, string.gmatch
 
-local split_by_vmstate = false
-
 -- Program options.
 local opt_map = {}
 
@@ -23,15 +21,10 @@ luajit-parse-sysprof [options] sysprof.bin
 Supported options are:
 
   --help                            Show this help and exit
-  --split                           Split callchains by vmstate
 ]]
   os.exit(0)
 end
 
-function opt_map.split()
-  split_by_vmstate = true
-end
-
 -- Print error and exit with error status.
 local function opterror(...)
   stderr:write("luajit-parse-sysprof.lua: ERROR: ", ...)
@@ -103,7 +96,7 @@ local function dump(inputfile)
   local symbols = symtab.parse(reader)
 
   local events = sysprof.parse(reader, symbols)
-  local calltree = misc.collapse(events, symbols, split_by_vmstate)
+  local calltree = misc.collapse(events, symbols)
 
   traverse_calltree(calltree, '')
 
diff --git a/tools/sysprof/collapse.lua b/tools/sysprof/collapse.lua
index 3d83d5ea..ac5269ea 100755
--- a/tools/sysprof/collapse.lua
+++ b/tools/sysprof/collapse.lua
@@ -75,7 +75,7 @@ end
 
 -- merge lua and host callchains into one callchain representing
 -- transfer of control
-local function merge(event, symbols, sep_vmst)
+local function merge(event, symbols)
   local cc = {}
 
   for _,h_fr in pairs(event.host.callchain) do
@@ -98,19 +98,15 @@ local function merge(event, symbols, sep_vmst)
 
   end
 
-  if sep_vmst == true then
-    table.insert(cc, { name = VMST_NAMES[event.lua.vmstate] })
-  end
-
   return cc
 end
 
 -- Collapse all the events into call tree
-function M.collapse(events, symbols, sep_vmst)
+function M.collapse(events, symbols)
   local root = new_node('root', false)
 
   for _,ev in pairs(events) do
-    local callchain = merge(ev, symbols, sep_vmst)
+    local callchain = merge(ev, symbols)
     local curr_node = root
     for i=#callchain,1,-1 do
       curr_node = insert(callchain[i].name, curr_node, false)
-- 
2.39.2 (Apple Git-143)

[Tarantool-patches] [PATCH luajit v4 2/3] tools: add execution permission to sysprof parser

[Maksim Kokryashkin via Tarantool-patches]

From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

This patch adds execution permission to the
`luajit-parse-sysprof` helper script, so now
it is possible to call it without having to
change permissions beforehands.

Needed for tarantool/tarantool#8700
---
 tools/luajit-parse-sysprof.in | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tools/luajit-parse-sysprof.in

diff --git a/tools/luajit-parse-sysprof.in b/tools/luajit-parse-sysprof.in
old mode 100644
new mode 100755
-- 
2.39.2 (Apple Git-143)

[Tarantool-patches] [PATCH luajit v4 3/3] sysprof: improve parser's memory footprint

[Maksim Kokryashkin via Tarantool-patches]

From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

This patch reduces sysprof's parser memory footprint,
by avoiding reading all callchains before collapsing them.
Instead of it, parser merges stacks immediately after
reading them atnd stores counts in a lua table.

The `collapse.lua` module is purged as a result of the
patch, but it is left as a stub to keep the integrational
testing intact. This stub should be removed in the next
series.

Resolves tarantool/tarantool#8700
---
Note: the assertion, proposed by Igor wasn't added, since it breaks
integrational testing.

 tools/CMakeLists.txt       |   4 ++
 tools/sysprof.lua          |  21 +------
 tools/sysprof/collapse.lua | 123 +-----------------------------------
 tools/sysprof/parse.lua    | 125 ++++++++++++++++++++++++++-----------
 4 files changed, 100 insertions(+), 173 deletions(-)

diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index dd7ec6bd..51ee3877 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -112,6 +112,8 @@ else()
   add_custom_target(tools-parse-sysprof EXCLUDE_FROM_ALL DEPENDS
     luajit-parse-sysprof
     sysprof/parse.lua
+    # TODO: This line is not deleted only for the sake of integrational
+    # testing. It should be deleted in the next series.
     sysprof/collapse.lua
     sysprof.lua
     utils/bufread.lua
@@ -121,6 +123,8 @@ else()

   install(FILES
       ${CMAKE_CURRENT_SOURCE_DIR}/sysprof/parse.lua
+      # TODO: This line is not deleted only for the sake of integrational
+      # testing. It should be deleted in the next series.
       ${CMAKE_CURRENT_SOURCE_DIR}/sysprof/collapse.lua
     DESTINATION ${LUAJIT_DATAROOTDIR}/sysprof
     PERMISSIONS
diff --git a/tools/sysprof.lua b/tools/sysprof.lua
index 22c724e9..8e110a04 100644
--- a/tools/sysprof.lua
+++ b/tools/sysprof.lua
@@ -1,7 +1,6 @@
 local bufread = require "utils.bufread"
 local sysprof = require "sysprof.parse"
 local symtab = require "utils.symtab"
-local misc = require "sysprof.collapse"

 local stdout, stderr = io.stdout, io.stderr
 local match, gmatch = string.match, string.gmatch
@@ -78,28 +77,14 @@ local function parseargs(args)
   return args[args.argn]
 end

-local function traverse_calltree(node, prefix)
-  if node.is_leaf then
-    print(prefix..' '..node.count)
-  end
-
-  local sep_prefix = #prefix == 0 and prefix or prefix..';'
-
-  for name,child in pairs(node.children) do
-    traverse_calltree(child, sep_prefix..name)
-  end
-end
-
 local function dump(inputfile)
   local reader = bufread.new(inputfile)
-
   local symbols = symtab.parse(reader)
-
   local events = sysprof.parse(reader, symbols)
-  local calltree = misc.collapse(events, symbols)
-
-  traverse_calltree(calltree, '')

+  for stack, count in pairs(events) do
+    print(stack, count)
+  end
   os.exit(0)
 end

diff --git a/tools/sysprof/collapse.lua b/tools/sysprof/collapse.lua
index ac5269ea..cac92f1a 100755
--- a/tools/sysprof/collapse.lua
+++ b/tools/sysprof/collapse.lua
@@ -1,120 +1,3 @@
-local parse = require "sysprof.parse"
-local vmdef = require "jit.vmdef"
-local symtab = require "utils.symtab"
-
-local VMST_NAMES = {
-  [parse.VMST.INTERP] = "VMST_INTERP",
-  [parse.VMST.LFUNC]  = "VMST_LFUNC",
-  [parse.VMST.FFUNC]  = "VMST_FFUNC",
-  [parse.VMST.CFUNC]  = "VMST_CFUNC",
-  [parse.VMST.GC]     = "VMST_GC",
-  [parse.VMST.EXIT]   = "VMST_EXIT",
-  [parse.VMST.RECORD] = "VMST_RECORD",
-  [parse.VMST.OPT]    = "VMST_OPT",
-  [parse.VMST.ASM]    = "VMST_ASM",
-  [parse.VMST.TRACE]  = "VMST_TRACE",
-}
-
-local M = {}
-
-local function new_node(name, is_leaf)
-  return {
-    name = name,
-    count = 0,
-    is_leaf = is_leaf,
-    children = {}
-  }
-end
-
--- insert new child into a node (or increase counter in existing one)
-local function insert(name, node, is_leaf)
-  if node.children[name] == nil then
-    node.children[name] = new_node(name, is_leaf)
-  end
-
-  local child = node.children[name]
-  child.count = child.count + 1
-
-  return child
-end
-
-local function insert_lua_callchain(chain, lua, symbols)
-  local ins_cnt = 0
-  for _,fr in pairs(lua.callchain) do
-    local name_lua
-
-    ins_cnt = ins_cnt + 1
-    if fr.type == parse.FRAME.FFUNC then
-      name_lua = vmdef.ffnames[fr.ffid]
-    else
-      name_lua = symtab.demangle(symbols, {
-        addr = fr.addr,
-        line = fr.line,
-        gen = fr.gen
-      })
-      if lua.trace.traceno ~= nil and lua.trace.addr == fr.addr and
-          lua.trace.line == fr.line then
-        name_lua = symtab.demangle(symbols, {
-          addr = fr.addr,
-          traceno = lua.trace.traceno,
-          gen = fr.gen
-        })
-      end
-
-      if fr.type == parse.FRAME.CFUNC then
-        -- C function encountered, the next chunk
-        -- of frames is located on the C stack.
-        break
-      end
-    end
-
-    table.insert(chain, 1, { name = name_lua })
-  end
-  table.remove(lua.callchain, ins_cnt)
-end
-
--- merge lua and host callchains into one callchain representing
--- transfer of control
-local function merge(event, symbols)
-  local cc = {}
-
-  for _,h_fr in pairs(event.host.callchain) do
-    local name_host = symtab.demangle(symbols, {
-      addr = h_fr.addr,
-      gen = h_fr.gen
-    })
-    table.insert(cc, 1, { name = name_host })
-
-    if string.match(name_host, '^lua_cpcall') ~= nil then
-      -- Any C function is present on both the C and the Lua
-      -- stacks. It is more convenient to get its info from the
-      -- host stack, since it has information about child frames.
-      table.remove(event.lua.callchain, 1)
-    end
-
-    if string.match(name_host, '^lua_p?call') ~= nil then
-      insert_lua_callchain(cc, event.lua, symbols)
-    end
-
-  end
-
-  return cc
-end
-
--- Collapse all the events into call tree
-function M.collapse(events, symbols)
-  local root = new_node('root', false)
-
-  for _,ev in pairs(events) do
-    local callchain = merge(ev, symbols)
-    local curr_node = root
-    for i=#callchain,1,-1 do
-      curr_node = insert(callchain[i].name, curr_node, false)
-    end
-    insert('', curr_node, true)
-  end
-
-  return root
-end
-
-return M
+-- FIXME: This file is not deleted only for the sake of
+-- integrational testing. It should be deleted in the
+-- next series.
diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
index 5b52f104..92e475a6 100755
--- a/tools/sysprof/parse.lua
+++ b/tools/sysprof/parse.lua
@@ -2,6 +2,7 @@
 -- The format spec can be found in <src/lj_sysprof.h>.

 local symtab = require "utils.symtab"
+local vmdef = require "jit.vmdef"

 local string_format = string.format

@@ -10,7 +11,7 @@ local LJP_CURRENT_VERSION = 2

 local M = {}

-M.VMST = {
+local VMST = {
   INTERP = 0,
   LFUNC  = 1,
   FFUNC  = 2,
@@ -25,13 +26,14 @@ M.VMST = {
 }


-M.FRAME = {
+local FRAME = {
   LFUNC  = 1,
   CFUNC  = 2,
   FFUNC  = 3,
   BOTTOM = 0x80
 }

+
 local STREAM_END = 0x80
 local SYMTAB_LFUNC_EVENT = 10
 local SYMTAB_CFUNC_EVENT = 11
@@ -54,42 +56,40 @@ local function new_event()
   }
 end

-local function parse_lfunc(reader, event, symbols)
+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 })
-  loc.type = M.FRAME.LFUNC
-  table.insert(event.lua.callchain, 1, loc)
+  loc.type = FRAME.LFUNC
+  return symtab.demangle(symbols, loc)
 end

-local function parse_ffunc(reader, event, _)
+local function parse_ffunc(reader, _)
   local ffid = reader:read_uleb128()
-  table.insert(event.lua.callchain, 1, {
-    type = M.FRAME.FFUNC,
-    ffid = ffid,
-  })
+  return vmdef.ffnames[ffid]
 end

-local function parse_cfunc(reader, event, symbols)
+local function parse_cfunc(reader, symbols)
   local addr = reader:read_uleb128()
   local loc = symtab.loc(symbols, { addr = addr })
-  loc.type = M.FRAME.CFUNC
-  table.insert(event.lua.callchain, 1, loc)
+  loc.type = FRAME.CFUNC
+  return symtab.demangle(symbols, loc)
 end

 local frame_parsers = {
-  [M.FRAME.LFUNC] = parse_lfunc,
-  [M.FRAME.FFUNC] = parse_ffunc,
-  [M.FRAME.CFUNC] = parse_cfunc
+  [FRAME.LFUNC] = parse_lfunc,
+  [FRAME.FFUNC] = parse_ffunc,
+  [FRAME.CFUNC] = parse_cfunc
 }

 local function parse_lua_callchain(reader, event, symbols)
   while true do
     local frame_header = reader:read_octet()
-    if frame_header == M.FRAME.BOTTOM then
+    if frame_header == FRAME.BOTTOM then
       break
     end
-    frame_parsers[frame_header](reader, event, symbols)
+    local name = frame_parsers[frame_header](reader, symbols)
+    table.insert(event.lua.callchain, 1, {name = name, type = frame_header})
   end
 end

@@ -100,7 +100,7 @@ local function parse_host_callchain(reader, event, symbols)

   while addr ~= 0 do
     local loc = symtab.loc(symbols, { addr = addr })
-    table.insert(event.host.callchain, 1, loc)
+    table.insert(event.host.callchain, 1, symtab.demangle(symbols, loc))
     addr = reader:read_uleb128()
   end
 end
@@ -108,10 +108,20 @@ end
 --~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~--

 local function parse_trace_callchain(reader, event, symbols)
-  event.lua.trace.traceno  = reader:read_uleb128()
-  event.lua.trace.addr = reader:read_uleb128()
-  event.lua.trace.line = reader:read_uleb128()
-  event.lua.trace.gen = symtab.loc(symbols, event.lua.trace).gen
+  local loc = {}
+  loc.traceno = reader:read_uleb128()
+  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

 --~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~--
@@ -143,18 +153,63 @@ local function parse_symtab(reader, symbols, vmstate)
 end

 local event_parsers = {
-  [M.VMST.INTERP] = parse_host_only,
-  [M.VMST.LFUNC]  = parse_lua_host,
-  [M.VMST.FFUNC]  = parse_lua_host,
-  [M.VMST.CFUNC]  = parse_lua_host,
-  [M.VMST.GC]     = parse_host_only,
-  [M.VMST.EXIT]   = parse_host_only,
-  [M.VMST.RECORD] = parse_host_only,
-  [M.VMST.OPT]    = parse_host_only,
-  [M.VMST.ASM]    = parse_host_only,
-  [M.VMST.TRACE]  = parse_trace,
+  [VMST.INTERP] = parse_host_only,
+  [VMST.LFUNC]  = parse_lua_host,
+  [VMST.FFUNC]  = parse_lua_host,
+  [VMST.CFUNC]  = parse_lua_host,
+  [VMST.GC]     = parse_host_only,
+  [VMST.EXIT]   = parse_host_only,
+  [VMST.RECORD] = parse_host_only,
+  [VMST.OPT]    = parse_host_only,
+  [VMST.ASM]    = parse_host_only,
+  [VMST.TRACE]  = parse_trace,
 }

+local function insert_lua_callchain(chain, lua)
+  local ins_cnt = 0
+  local name_lua
+  for _, fr in ipairs(lua.callchain) do
+    ins_cnt = ins_cnt + 1
+    if fr.type == FRAME.CFUNC then
+      -- C function encountered, the next chunk
+      -- of frames is located on the C stack.
+      break
+    end
+    name_lua = fr.name
+
+    if fr.type == FRAME.LFUNC
+      and lua.trace.traceno ~= nil
+      and lua.trace.addr == fr.addr
+      and lua.trace.line == fr.line
+    then
+            name_lua = lua.trace.name
+    end
+
+    table.insert(chain, name_lua)
+  end
+  table.remove(lua.callchain, ins_cnt)
+end
+
+local function merge(event)
+  local callchain = {}
+
+  for _, name_host in ipairs(event.host.callchain) do
+    table.insert(callchain, name_host)
+    if string.match(name_host, '^lua_cpcall') ~= nil then
+      -- Any C function is present on both the C and the Lua
+      -- stacks. It is more convenient to get its info from the
+      -- host stack, since it has information about child frames.
+      table.remove(event.lua.callchain)
+    end
+
+    if string.match(name_host, '^lua_p?call') ~= nil then
+      insert_lua_callchain(callchain, event.lua)
+    end
+
+  end
+  return table.concat(callchain, ';') .. ';'
+end
+
 local function parse_event(reader, events, symbols)
   local event = new_event()

@@ -171,8 +226,8 @@ local function parse_event(reader, events, symbols)
   event.lua.vmstate = vmstate

   event_parsers[vmstate](reader, event, symbols)
-
-  table.insert(events, event)
+  local callchain = merge(event)
+  events[callchain] = (events[callchain] or 0) + 1
   return true
 end

--
2.39.2 (Apple Git-143)

Re: [Tarantool-patches] [PATCH luajit v4 3/3] sysprof: improve parser's memory footprint

[Igor Munkin via Tarantool-patches]

Max,

Thanks for the patch! LGTM, except several nits below.

On 21.08.23, Maksim Kokryashkin wrote:
> From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
> 
> This patch reduces sysprof's parser memory footprint,
> by avoiding reading all callchains before collapsing them.
> Instead of it, parser merges stacks immediately after
> reading them atnd stores counts in a lua table.

Typo: s/atnd/and/.
Typo: s/lua/Lua/.

> 
> The `collapse.lua` module is purged as a result of the
> patch, but it is left as a stub to keep the integrational
> testing intact. This stub should be removed in the next
> series.
> 
> Resolves tarantool/tarantool#8700
> ---
> Note: the assertion, proposed by Igor wasn't added, since it breaks
> integrational testing.

OK, anyway, thanks for trying.

> 
>  tools/CMakeLists.txt       |   4 ++
>  tools/sysprof.lua          |  21 +------
>  tools/sysprof/collapse.lua | 123 +-----------------------------------
>  tools/sysprof/parse.lua    | 125 ++++++++++++++++++++++++++-----------
>  4 files changed, 100 insertions(+), 173 deletions(-)

<snipped>

> diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
> index 5b52f104..92e475a6 100755
> --- a/tools/sysprof/parse.lua
> +++ b/tools/sysprof/parse.lua

<snipped>

> @@ -143,18 +153,63 @@ local function parse_symtab(reader, symbols, vmstate)

<snipped>

> +local function insert_lua_callchain(chain, lua)
> +  local ins_cnt = 0
> +  local name_lua
> +  for _, fr in ipairs(lua.callchain) do
> +    ins_cnt = ins_cnt + 1
> +    if fr.type == FRAME.CFUNC then
> +      -- C function encountered, the next chunk
> +      -- of frames is located on the C stack.
> +      break
> +    end
> +    name_lua = fr.name
> +
> +    if fr.type == FRAME.LFUNC
> +      and lua.trace.traceno ~= nil
> +      and lua.trace.addr == fr.addr
> +      and lua.trace.line == fr.line
> +    then
> +            name_lua = lua.trace.name

Still misindented operator.

> +    end
> +
> +    table.insert(chain, name_lua)
> +  end
> +  table.remove(lua.callchain, ins_cnt)
> +end
> +

<snipped>

> --
> 2.39.2 (Apple Git-143)
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit v4 1/3] sysprof: remove `split by vmstate` option

[Igor Munkin via Tarantool-patches]

Max,

Thanks for the fixes! LGTM, but still one nit regarding the wording.

On 21.08.23, Maksim Kokryashkin wrote:
> From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
> 
> The option was introduced early into the sysprof's
> development to provide capabilities for per-VM-state
> callchain visualization. However, during the sysprof's
> recent active in-house usage period, it became clear
> that performance issues associated with specific VM
> states (such as GC, for example) have a distinct look
> on a flamegraph itself, thanks to their name prefixes
> and their surrounding context.

Minor: Besides, one can still use counters returned by <sysprof.report>,
right? If so, it would be nice to mention this in reasoning too.

> 
> This option is unneeded and was never used
> by anybody. There is no reason to maintain it
> in the scope of refactoring.
> 
> Needed for tarantool/tarantool#8700
> ---
>  tools/sysprof.lua          |  9 +--------
>  tools/sysprof/collapse.lua | 10 +++-------
>  2 files changed, 4 insertions(+), 15 deletions(-)
> 

<snipped>

> -- 
> 2.39.2 (Apple Git-143)
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit v4 1/3] sysprof: remove `split by vmstate` option

[Maxim Kokryashkin via Tarantool-patches]

Hi, Igor!
Thanks for the review!
See my answers below.

On Mon, Aug 21, 2023 at 01:05:54PM +0000, Igor Munkin wrote:
> Max,
> 
> Thanks for the fixes! LGTM, but still one nit regarding the wording.
> 
> On 21.08.23, Maksim Kokryashkin wrote:
> > From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
> > 
> > The option was introduced early into the sysprof's
> > development to provide capabilities for per-VM-state
> > callchain visualization. However, during the sysprof's
> > recent active in-house usage period, it became clear
> > that performance issues associated with specific VM
> > states (such as GC, for example) have a distinct look
> > on a flamegraph itself, thanks to their name prefixes
> > and their surrounding context.
> 
> Minor: Besides, one can still use counters returned by <sysprof.report>,
> right? If so, it would be nice to mention this in reasoning too.
Right, added it.
Branch is force-pushed.
Here is the new commit message:
| sysprof: remove `split by vmstate` option
|
| The option was introduced early into the sysprof's
| development to provide capabilities for per-VM-state
| callchain visualization. However, during the sysprof's
| recent active in-house usage period, it became clear
| that performance issues associated with specific VM
| states (such as GC, for example) have a distinct look
| on a flamegraph itself, thanks to their name prefixes
| and their surrounding context. Furthermore, one can
| still use counters returned by `sysprof.report` to get
| insight into per-VM-state performance.
|
| This option is unneeded and was never used
| by anybody. There is no reason to maintain it
| in the scope of refactoring.
|
| Needed for tarantool/tarantool#8700
> 
> > 
> > This option is unneeded and was never used
> > by anybody. There is no reason to maintain it
> > in the scope of refactoring.
> > 
> > Needed for tarantool/tarantool#8700
> > ---
> >  tools/sysprof.lua          |  9 +--------
> >  tools/sysprof/collapse.lua | 10 +++-------
> >  2 files changed, 4 insertions(+), 15 deletions(-)
> > 
> 
> <snipped>
> 
> > -- 
> > 2.39.2 (Apple Git-143)
> > 
> 
> -- 
> Best regards,
> IM

Re: [Tarantool-patches] [PATCH luajit v4 0/3] sysprof: parser refactoring

[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 21.08.23, Maksim Kokryashkin wrote:
> Changes in v4:
> - Fixed comments as per review by Igor
> - Dropped the AVL-tree changes, since the memprof still requires
>   generations to function properly. The AVL-tree will be updated after
>   the memprof's parser refactoring.
> 
> PR: https://github.com/tarantool/tarantool/pull/8703
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8700-sysprof-parser-refactoring
> 
> Maxim Kokryashkin (3):
>   sysprof: remove `split by vmstate` option
>   tools: add execution permission to sysprof parser
>   sysprof: improve parser's memory footprint
> 
>  tools/CMakeLists.txt          |   4 ++
>  tools/luajit-parse-sysprof.in |   0
>  tools/sysprof.lua             |  28 +-------
>  tools/sysprof/collapse.lua    | 127 +---------------------------------
>  tools/sysprof/parse.lua       | 125 +++++++++++++++++++++++----------
>  5 files changed, 100 insertions(+), 184 deletions(-)
>  mode change 100644 => 100755 tools/luajit-parse-sysprof.in
> 
> --
> 2.39.2 (Apple Git-143)
> 

-- 
Best regards,
IM