From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3E51D4765E0 for ; Sun, 27 Dec 2020 01:56:57 +0300 (MSK) Date: Sun, 27 Dec 2020 01:56:51 +0300 From: Igor Munkin Message-ID: <20201226225651.GI5396@tarantool.org> References: <03ac70e3bfb9bf7061ad71c1bac50ed3f8e853fc.1608907726.git.skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <03ac70e3bfb9bf7061ad71c1bac50ed3f8e853fc.1608907726.git.skaplun@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH luajit v2 7/7] tools: introduce a memory profile parser List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Sergey, Thanks for the patch! LGTM, except the several nits below. On 25.12.20, Sergey Kaplun wrote: > This patch adds a parser for binary data dumped via the memory profiler. It is > a set of the following Lua modules: > * utils/bufread.lua: read binary data from a file. > * utils/symtab.lua: symbol table decode functions > * memprof/parse.lua: decode the memory profiler event stream > * memprof/humanize.lua: display decoded data in human readable format > * memprof.lua: Lua script to display data > > There is also a stand-alone bash script that displays > human readable parsed data to a stdout. It calls with a > corresponding LUA_PATH. > > Part of tarantool/tarantool#5442 > Part of tarantool/tarantool#5490 > --- > > Changes in v2: > - Add (un)?install sections in Makefile > - Modify bash script correspondingly. > - Change Lua modules layout. > - Adjusted test. Check that errno returns in case of error is added. > - Code clean up. > > Makefile | 39 +++++- > test/misclib-memprof-lapi.test.lua | 135 +++++++++++++++++++++ > tools/luajit-parse-memprof | 9 ++ > tools/memprof.lua | 109 +++++++++++++++++ > tools/memprof/humanize.lua | 45 +++++++ > tools/memprof/parse.lua | 188 +++++++++++++++++++++++++++++ > tools/utils/bufread.lua | 147 ++++++++++++++++++++++ > tools/utils/symtab.lua | 89 ++++++++++++++ > 8 files changed, 757 insertions(+), 4 deletions(-) > create mode 100755 test/misclib-memprof-lapi.test.lua > create mode 100755 tools/luajit-parse-memprof > create mode 100644 tools/memprof.lua > create mode 100644 tools/memprof/humanize.lua > create mode 100644 tools/memprof/parse.lua > create mode 100644 tools/utils/bufread.lua > create mode 100644 tools/utils/symtab.lua > > diff --git a/Makefile b/Makefile > index 4a56917..ba4aa2f 100644 > --- a/Makefile > +++ b/Makefile > @@ -37,6 +37,9 @@ INSTALL_INC= $(DPREFIX)/include/luajit-$(MAJVER).$(MINVER) > > INSTALL_LJLIBD= $(INSTALL_SHARE)/luajit-$(VERSION) > INSTALL_JITLIB= $(INSTALL_LJLIBD)/jit > +INSTALL_UTILSLIB= $(INSTALL_LJLIBD)/utils > +INSTALL_MEMPROFLIB= $(INSTALL_LJLIBD)/memprof Minor: It's better to use $(INSALL_TOOLSLIB) here, isn't it? > +INSTALL_TOOLSLIB= $(INSTALL_LJLIBD) > INSTALL_LMODD= $(INSTALL_SHARE)/lua > INSTALL_LMOD= $(INSTALL_LMODD)/$(ABIVER) > INSTALL_CMODD= $(INSTALL_LIB)/lua > diff --git a/test/misclib-memprof-lapi.test.lua b/test/misclib-memprof-lapi.test.lua > new file mode 100755 > index 0000000..e02c6fa > --- /dev/null > +++ b/test/misclib-memprof-lapi.test.lua > @@ -0,0 +1,135 @@ > +local function check_alloc_report(alloc, line, function_line, nevents) > + assert(string.format("@%s:%d", arg[0], function_line) == alloc[line].name) > + assert(alloc[line].num == nevents, ("got=%d, ecpected=%d"):format( Typo: s/ecpected/expected/. > + alloc[line].num, > + nevents > + )) > + return true > +end > diff --git a/tools/luajit-parse-memprof b/tools/luajit-parse-memprof > new file mode 100755 > index 0000000..c814301 > --- /dev/null > +++ b/tools/luajit-parse-memprof > @@ -0,0 +1,9 @@ > +#!/bin/bash > +# > +# Launcher for memprof parser. > + > +# This two variables are replaced on installing. > +TOOL_DIR=$(dirname `readlink -f $0`) > +LUAJIT_BIN=$TOOL_DIR/../src/luajit > + > +LUA_PATH="$TOOL_DIR/?.lua;;" $LUAJIT_BIN $TOOL_DIR/memprof.lua $@ > diff --git a/tools/memprof.lua b/tools/memprof.lua > new file mode 100644 > index 0000000..7476757 > --- /dev/null > +++ b/tools/memprof.lua > @@ -0,0 +1,109 @@ > +local bufread = require "utils.bufread" > +local memprof = require "memprof.parse" > +local symtab = require "utils.symtab" > +local view = require "memprof.humanize" Typo: Still mess with whitespace. > + > +local reader = bufread.new(inputfile) > +local symbols = symtab.parse(reader) > +local events = memprof.parse(reader, symbols) Typo: Still mess with whitespace. > + > diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua > new file mode 100644 > index 0000000..f3e5e31 > --- /dev/null > +++ b/tools/utils/symtab.lua > @@ -0,0 +1,89 @@ > +-- Parser of LuaJIT's symtab binary stream. > +-- The format spec can be found in . I see no such file. I guess you mean . > +-- > -- > 2.28.0 > -- Best regards, IM