[Tarantool-patches] [PATCH luajit v2 7/7] tools: introduce a memory profile parser
Igor Munkin
imun at tarantool.org
Sun Dec 27 01:56:51 MSK 2020
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 <luajit-parse-memprof> that displays
> human readable parsed data to a stdout. It calls <memprof.lua> 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
<snipped>
> 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 @@
<snipped>
> +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
<snipped>
> 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 @@
<snipped>
> +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.
> +
<snipped>
> +local reader = bufread.new(inputfile)
> +local symbols = symtab.parse(reader)
> +local events = memprof.parse(reader, symbols)
Typo: Still mess with whitespace.
> +
<snipped>
> 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 <src/lj_symtab.h>.
I see no such file. I guess you mean <src/lj_memprof.h>.
> +--
<snipped>
> --
> 2.28.0
>
--
Best regards,
IM
More information about the Tarantool-patches
mailing list