[Tarantool-patches] [PATCH luajit v2 7/7] tools: introduce a memory profile parser
Sergey Kaplun
skaplun at tarantool.org
Sun Dec 27 10:16:56 MSK 2020
Igor,
Thanks for the review!
On 27.12.20, Igor Munkin wrote:
> 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?
Yes, fixed.
>
> > +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/.
Thanks! Fixed!
>
> > + 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.
Sorry, fixed!
>
> > +
>
> <snipped>
>
> > +local reader = bufread.new(inputfile)
> > +local symbols = symtab.parse(reader)
> > +local events = memprof.parse(reader, symbols)
>
> Typo: Still mess with whitespace.
My bad, fixed!
>
> > +
>
> <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>.
Yes, thanks, fixed!
>
> > +--
>
> <snipped>
>
> > --
> > 2.28.0
> >
>
> --
> Best regards,
> IM
Also fixed mess with quotes for strings.
See the iterative patch below. Branch is force-pushed.
===================================================================
diff --git a/Makefile b/Makefile
index ba4aa2f..61967df 100644
--- a/Makefile
+++ b/Makefile
@@ -37,9 +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
INSTALL_TOOLSLIB= $(INSTALL_LJLIBD)
+INSTALL_UTILSLIB= $(INSTALL_TOOLSLIB)/utils
+INSTALL_MEMPROFLIB= $(INSTALL_TOOLSLIB)/memprof
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
index e02c6fa..2366c00 100755
--- a/test/misclib-memprof-lapi.test.lua
+++ b/test/misclib-memprof-lapi.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
-local tap = require('tap')
+local tap = require("tap")
local test = tap.test("misc-memprof-lapi")
test:plan(9)
@@ -9,9 +9,9 @@ jit.off()
jit.flush()
-- FIXME: Launch tests with LUA_PATH enviroment variable.
-local path = arg[0]:gsub('/[^/]+%.test%.lua', '')
-local path_suffix = '../tools/?.lua;'
-package.path = ('%s/%s;'):format(path, path_suffix)..package.path
+local path = arg[0]:gsub("/[^/]+%.test%.lua", "")
+local path_suffix = "../tools/?.lua;"
+package.path = ("%s/%s;"):format(path, path_suffix)..package.path
local table_new = require "table.new"
@@ -19,8 +19,8 @@ local bufread = require "utils.bufread"
local memprof = require "memprof.parse"
local symtab = require "utils.symtab"
-local TMP_BINFILE = arg[0]:gsub('[^/]+%.test%.lua', '%.%1.memprofdata.tmp.bin')
-local BAD_PATH = arg[0]:gsub('[^/]+%.test%.lua', '%1/memprofdata.tmp.bin')
+local TMP_BINFILE = arg[0]:gsub("[^/]+%.test%.lua", "%.%1.memprofdata.tmp.bin")
+local BAD_PATH = arg[0]:gsub("[^/]+%.test%.lua", "%1/memprofdata.tmp.bin")
local function payload()
-- Preallocate table to avoid array part reallocations.
@@ -73,7 +73,7 @@ end
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(
+ assert(alloc[line].num == nevents, ("got=%d, expected=%d"):format(
alloc[line].num,
nevents
))
diff --git a/tools/memprof.lua b/tools/memprof.lua
index 7476757..92d192e 100644
--- a/tools/memprof.lua
+++ b/tools/memprof.lua
@@ -12,8 +12,8 @@
local bufread = require "utils.bufread"
local memprof = require "memprof.parse"
-local symtab = require "utils.symtab"
-local view = require "memprof.humanize"
+local symtab = require "utils.symtab"
+local view = require "memprof.humanize"
local stdout, stderr = io.stdout, io.stderr
local match, gmatch = string.match, string.gmatch
@@ -92,9 +92,9 @@ end
local inputfile = parseargs{...}
-local reader = bufread.new(inputfile)
+local reader = bufread.new(inputfile)
local symbols = symtab.parse(reader)
-local events = memprof.parse(reader, symbols)
+local events = memprof.parse(reader, symbols)
stdout:write("ALLOCATIONS", "\n")
view.render(events.alloc, symbols)
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index f3e5e31..03aadbd 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -1,5 +1,5 @@
-- Parser of LuaJIT's symtab binary stream.
--- The format spec can be found in <src/lj_symtab.h>.
+-- The format spec can be found in <src/lj_memprof.h>.
--
-- Major portions taken verbatim or adapted from the LuaVela.
-- Copyright (C) 2015-2019 IPONWEB Ltd.
===================================================================
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list