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

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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