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

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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