Re: [Tarantool-patches] [PATCH luajit v3 0/4] sysprof: parser refactoring

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

Thanks for the fixes! Please consider my comments per patches.

On 31.07.23, Maxim Kokryashkin wrote:
> This patchset is targeted at reducing the memory footprint
> of the sysprof parser. It is mainly achieved by fixing a
> nasty bug in the AVL tree implementation, which caused most
> of the memory consumption, and by essentially purging the collapse.lua
> module, which means that there is no need to store the complete
> information about every parsed event and it is sufficient to merge
> stacks inplace.
> 
> Alongside with described changes two minor improvements were done:
> - `luajit-parse-sysprof` now has permissions to be executed
> - `split_by_vmstate` option is purged, because no one uses it, so there
>   is no actual reason to maintain it.
> 
> ## Note for Igor:
> There is no way to split the 'memory footprint reduction'-part and the
> 'collapse.lua purge'-part, since the main reason why memory consumption
> is so much lower now, other the AVL-tree bug, is that we don't need to
> pass a complete profile information between the `parse.lua` and
> `collapse.lua`. That, basically, almost turns the profile parsing
> process into some kind of stream-based procedure.

Got it, thanks for explanation!

> 
> PR: https://github.com/tarantool/tarantool/pull/8703
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8700-sysprof-parser-refactoring
> 
> Maxim Kokryashkin (4):
>   utils: remove unnecessary insertion in AVL-tree
>   sysprof: remove `split by vmstate` option
>   tools: add execution permission to sysprof parser
>   sysprof: improve parser's memory footprint
> 
>  .../gh-5813-resolving-of-c-symbols.test.lua   |   6 +-
>  tools/CMakeLists.txt                          |   4 +
>  tools/luajit-parse-sysprof.in                 |   0
>  tools/sysprof.lua                             |  28 +---
>  tools/sysprof/collapse.lua                    | 127 +-----------------
>  tools/sysprof/parse.lua                       | 126 ++++++++++++-----
>  tools/utils/avl.lua                           |   4 +-
>  tools/utils/symtab.lua                        |   2 +-
>  8 files changed, 106 insertions(+), 191 deletions(-)
>  mode change 100644 => 100755 tools/luajit-parse-sysprof.in
> 
> -- 
> 2.41.0
> 

-- 
Best regards,
IM