From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Subject: [Tarantool-patches] [PATCH luajit 1/2 v8] tools: add cli flag to run profile dump parsers Date: Fri, 25 Aug 2023 12:14:45 +0300 [thread overview] Message-ID: <ba310224cde35b7e0c38a741fbd1fe2400a28428.1692954427.git.m.kokryashkin@tarantool.org> (raw) In-Reply-To: <cover.1692954427.git.m.kokryashkin@tarantool.org> It is really inconvenient to use a standalone shell script to parse binary dumps from profilers. That is why this commit introduces a CLI flag for tools to the LuaJIT, so now it is possible to parse a memprof dump as simple as: ``` luajit -tm memprof.bin luajit -tm --leak-only memprof.bin ``` And the sysprof too: ``` luajit -ts sysprof.bin luajit -ts --split sysprof.bin ``` The `luajit-parse-memprof` and `luajit-parse-sysprof` are purged as a result of this changes. Closes tarantool/tarantool#5688 --- This patch requires additional changes for tarantool proxying, that are done in the PR. Makefile.original | 20 --- src/luajit.c | 41 +++++- .../gh-5688-tool-cli-flag.test.lua | 130 ++++++++++++++++++ tools/CMakeLists.txt | 73 ---------- tools/luajit-parse-memprof.in | 6 - tools/luajit-parse-sysprof.in | 6 - tools/memprof.lua | 4 +- tools/sysprof.lua | 16 ++- 8 files changed, 186 insertions(+), 110 deletions(-) create mode 100644 test/tarantool-tests/gh-5688-tool-cli-flag.test.lua delete mode 100755 tools/luajit-parse-memprof.in delete mode 100644 tools/luajit-parse-sysprof.in diff --git a/Makefile.original b/Makefile.original index 0c92df9e..10836f7d 100644 --- a/Makefile.original +++ b/Makefile.original @@ -58,7 +58,6 @@ INSTALL_DYLIBSHORT2= libluajit-$(ABIVER).$(MAJVER).dylib INSTALL_DYLIBNAME= libluajit-$(ABIVER).$(MAJVER).$(MINVER).$(RELVER).dylib INSTALL_PCNAME= luajit.pc INSTALL_TMEMPROFNAME= luajit-$(VERSION)-parse-memprof -INSTALL_TMEMPROFSYMNAME= luajit-parse-memprof INSTALL_STATIC= $(INSTALL_LIB)/$(INSTALL_ANAME) INSTALL_DYN= $(INSTALL_LIB)/$(INSTALL_SONAME) @@ -68,7 +67,6 @@ INSTALL_T= $(INSTALL_BIN)/$(INSTALL_TNAME) INSTALL_TSYM= $(INSTALL_BIN)/$(INSTALL_TSYMNAME) INSTALL_PC= $(INSTALL_PKGCONFIG)/$(INSTALL_PCNAME) INSTALL_TMEMPROF= $(INSTALL_BIN)/$(INSTALL_TMEMPROFNAME) -INSTALL_TMEMPROFSYM= $(INSTALL_BIN)/$(INSTALL_TMEMPROFSYMNAME) INSTALL_DIRS= $(INSTALL_BIN) $(INSTALL_LIB) $(INSTALL_INC) $(INSTALL_MAN) \ $(INSTALL_PKGCONFIG) $(INSTALL_JITLIB) $(INSTALL_LMOD) $(INSTALL_CMOD) \ @@ -87,8 +85,6 @@ UNINSTALL= $(RM) LDCONFIG= ldconfig -n SED_PC= sed -e "s|@LUAJIT_PC_PREFIX@|$(PREFIX)|" \ -e "s|@LUAJIT_PC_MULTILIB@|$(MULTILIB)|" -SED_TMEMPROF= sed -e "s|@LUAJIT_TOOLS_DIR@|$(INSTALL_TOOLSLIB)|" \ - -e "s|@LUAJIT_TOOLS_BIN@|$(INSTALL_T)|" FILE_T= luajit FILE_A= libluajit.a @@ -103,7 +99,6 @@ FILES_JITLIB= bc.lua bcsave.lua dump.lua p.lua v.lua zone.lua \ FILES_UTILSLIB= avl.lua bufread.lua symtab.lua FILES_MEMPROFLIB= parse.lua humanize.lua FILES_TOOLSLIB= memprof.lua -FILE_TMEMPROF= luajit-parse-memprof ifeq (,$(findstring Windows,$(OS))) HOST_SYS:= $(shell uname -s) @@ -148,16 +143,12 @@ install: $(INSTALL_DEP) cd tools/utils && $(INSTALL_F) $(FILES_UTILSLIB) $(INSTALL_UTILSLIB) cd tools/memprof && $(INSTALL_F) $(FILES_MEMPROFLIB) $(INSTALL_MEMPROFLIB) cd tools && $(INSTALL_F) $(FILES_TOOLSLIB) $(INSTALL_TOOLSLIB) - cd tools && $(SED_TMEMPROF) $(FILE_TMEMPROF).in > $(FILE_TMEMPROF) && \ - $(INSTALL_X) $(FILE_TMEMPROF) $(INSTALL_TMEMPROF) && \ - $(RM) $(FILE_TMEMPROF) @echo "==== Successfully installed LuaJIT $(VERSION) to $(PREFIX) ====" @echo "" @echo "Note: the development releases deliberately do NOT install a symlink for luajit" @echo "You can do this now by running these commands (with sudo):" @echo "" @echo " $(SYMLINK) $(INSTALL_TNAME) $(INSTALL_TSYM)" - @echo " $(SYMLINK) $(INSTALL_TMEMPROFNAME) $(INSTALL_TMEMPROFSYM)" @echo "" @@ -190,19 +181,8 @@ amalg: tools $(MAKE) -C src -f Makefile.original amalg clean: - $(RM) tools/$(FILE_TMEMPROF) $(MAKE) -C src -f Makefile.original clean -tools: tools/$(FILE_TMEMPROF) - -# FIXME: This is an ugly hack to manually configure an auxiliary -# tools/luajit-parse-memprof. This file should go away in scope of -# https://github.com/tarantool/tarantool/issues/5688. -tools/$(FILE_TMEMPROF): src/luajit - @sed -e "s|@LUAJIT_TOOLS_DIR@|$(realpath tools)|" \ - -e "s|@LUAJIT_TOOLS_BIN@|$(realpath src/luajit)|" \ - $@.in > $@ - @chmod +x $@ .PHONY: all install amalg clean tools diff --git a/src/luajit.c b/src/luajit.c index 1ca24301..e9ed89bc 100644 --- a/src/luajit.c +++ b/src/luajit.c @@ -72,6 +72,7 @@ static void print_usage(void) " -O[opt] Control LuaJIT optimizations.\n" " -i Enter interactive mode after executing " LUA_QL("script") ".\n" " -v Show version information.\n" + " -t<cmd> Execute tool.\n" " -E Ignore environment variables.\n" " -- Stop handling options.\n" " - Execute stdin and stop handling options.\n", stderr); @@ -361,6 +362,34 @@ static int dojitcmd(lua_State *L, const char *cmd) return runcmdopt(L, opt ? opt+1 : opt); } +static int runtoolcmd(lua_State *L, const char *tool_name) +{ + lua_getglobal(L, "require"); + lua_pushstring(L, tool_name); + if (lua_pcall(L, 1, 1, 0)) { + const char *msg = lua_tostring(L, -1); + if (msg && !strncmp(msg, "module ", 7)) + l_message(progname, + "unknown luaJIT command or tools not installed"); + return 1; + } + lua_getglobal(L, "arg"); + return report(L, lua_pcall(L, 1, 1, 0)); +} + +static int dotoolcmd(lua_State *L, const char *cmd) +{ + switch (cmd[0]) { + case 'm': + return runtoolcmd(L, "memprof"); + case 's': + return runtoolcmd(L, "sysprof"); + default: + break; + } + return -1; +} + /* Optimization flags. */ static int dojitopt(lua_State *L, const char *opt) { @@ -398,6 +427,7 @@ static int dobytecode(lua_State *L, char **argv) #define FLAGS_EXEC 4 #define FLAGS_OPTION 8 #define FLAGS_NOENV 16 +#define FLAGS_TOOL 32 static int collectargs(char **argv, int *flags) { @@ -419,6 +449,11 @@ static int collectargs(char **argv, int *flags) notail(argv[i]); *flags |= FLAGS_VERSION; break; + case 't': + *flags |= FLAGS_TOOL; + if (argv[i][2] == '\0') return -1; + if (argv[i + 1] == NULL) return -1; + return i + 1; case 'e': *flags |= FLAGS_EXEC; case 'j': /* LuaJIT extension */ @@ -474,6 +509,10 @@ static int runargs(lua_State *L, char **argv, int argn) return 1; break; } + case 't': { /* Tarantool's fork extension. */ + const char *cmd = argv[i] + 2; + return dotoolcmd(L, cmd) != LUA_OK; + } case 'O': /* LuaJIT extension. */ if (dojitopt(L, argv[i] + 2)) return 1; @@ -535,7 +574,7 @@ static int pmain(lua_State *L) luaL_openlibs(L); lua_gc(L, LUA_GCRESTART, -1); - createargtable(L, argv, s->argc, argn); + createargtable(L, argv, s->argc, (flags & FLAGS_TOOL) ? argn - 1 : argn); if (!(flags & FLAGS_NOENV)) { s->status = handle_luainit(L); diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua new file mode 100644 index 00000000..b91902fd --- /dev/null +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua @@ -0,0 +1,130 @@ +local tap = require('tap') +local test = tap.test('gh-5688-tool-cli-flag'):skipcond({ + ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and + jit.arch ~= 'x64', + ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux', + -- XXX: Tarantool integration is required to run this test properly. + -- luacheck: no global + ['No profile tools CLI option integration'] = _TARANTOOL, +}) + +test:plan(3) + +jit.off() +jit.flush() + +local table_new = require('table.new') +local utils = require('utils') + +local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin') +local TMP_BINFILE_MEMPROF = utils.tools.profilename('memprofdata.tmp.bin') +local TMP_BINFILE_SYSPROF = utils.tools.profilename('sysprofdata.tmp.bin') + +local EXECUTABLE = utils.exec.luacmd(arg) +local MEMPROF_PARSER = EXECUTABLE .. ' -tm ' +local SYSPROF_PARSER = EXECUTABLE .. ' -ts ' + +local SUPPRESS_OUTPUT = ' > /dev/null 2>&1' + +local TABLE_SIZE = 20 + +local SMOKE_CMD_SET = { + { + cmd = EXECUTABLE .. ' -t ' .. BAD_PATH, + expected = false, + }, + { + cmd = EXECUTABLE .. ' -ta ' .. BAD_PATH, + expected = false, + }, +} + +local MEMPROF_CMD_SET = { + { + cmd = MEMPROF_PARSER .. BAD_PATH, + expected = false, + }, + { + cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF, + expected = true, + }, + { + cmd = MEMPROF_PARSER .. ' --wrong ' .. TMP_BINFILE_MEMPROF, + expected = false, + }, + { + cmd = MEMPROF_PARSER .. ' --leak-only ' .. TMP_BINFILE_MEMPROF, + expected = true, + }, +} + +local SYSPROF_CMD_SET = { + { + cmd = SYSPROF_PARSER .. BAD_PATH, + expected = false, + }, + { + cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF, + expected = true, + }, + { + cmd = SYSPROF_PARSER .. ' --wrong ' .. TMP_BINFILE_SYSPROF, + expected = false, + }, + { + cmd = SYSPROF_PARSER .. ' --split ' .. TMP_BINFILE_SYSPROF, + expected = true, + }, +} + +local function memprof_payload() + local _ = table_new(TABLE_SIZE, 0) + _ = nil + collectgarbage() +end + +local function sysprof_payload() + local function fib(n) + if n <= 1 then + return n + end + return fib(n - 1) + fib(n - 2) + end + return fib(32) +end + +local function generate_profiler_output(opts, payload, profiler) + local res, err = profiler.start(opts) + -- Should start succesfully. + assert(res, err) + + payload() + + res, err = profiler.stop() + -- Should stop succesfully. + assert(res, err) +end + +local function tool_test_case(case_name, cmd_set) + test:test(case_name, function(subtest) + subtest:plan(#cmd_set) + + for idx = 1, #cmd_set do + local result = os.execute(cmd_set[idx].cmd .. SUPPRESS_OUTPUT) == 0 + subtest:ok(result == cmd_set[idx].expected, cmd_set[idx].cmd) + end + end) +end + +tool_test_case('smoke', SMOKE_CMD_SET) + +generate_profiler_output(TMP_BINFILE_MEMPROF, memprof_payload, misc.memprof) +tool_test_case('memprof parsing tool', MEMPROF_CMD_SET) + +local sysprof_opts = { mode = 'C', path = TMP_BINFILE_SYSPROF } +generate_profiler_output(sysprof_opts, sysprof_payload, misc.sysprof) +tool_test_case('sysprof parsing tool', SYSPROF_CMD_SET) + +os.remove(TMP_BINFILE_MEMPROF) +os.remove(TMP_BINFILE_SYSPROF) +test:done(true) diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index dd7ec6bd..b7aeb472 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -11,22 +11,7 @@ set(LUAJIT_TOOLS_DEPS) if(LUAJIT_DISABLE_MEMPROF) message(STATUS "LuaJIT memory profiler support is disabled") else() - # XXX: Can use genex here since the value need to be evaluated - # at the configuration phase. Fortunately, we know the exact - # path where LuaJIT binary is located. - set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME}) - set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR}) - # XXX: Unfortunately, there is no convenient way to set - # particular permissions to the output file via CMake. - # Furthermore, I even failed to copy the given file to the same - # path to change its permissions. After looking at the docs, I - # realized that the valid solution would be too monstrous for - # such a simple task. As a result I've made the template itself - # executable, so the issue is resolved. - configure_file(luajit-parse-memprof.in luajit-parse-memprof @ONLY ESCAPE_QUOTES) - add_custom_target(tools-parse-memprof EXCLUDE_FROM_ALL DEPENDS - luajit-parse-memprof memprof/humanize.lua memprof/parse.lua memprof.lua @@ -66,27 +51,6 @@ else() WORLD_READ COMPONENT tools-parse-memprof ) - install(CODE - # XXX: The auxiliary script needs to be configured for to be - # used in repository directly. Furthermore, it needs to be - # reconfigured prior to its installation. The temporary - # <configure_file> output is stored to the project build - # directory and removed later after being installed. This - # script will have gone as a result of the issue: - # https://github.com/tarantool/tarantool/issues/5688. - " - set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME}) - set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR}) - configure_file(${CMAKE_CURRENT_SOURCE_DIR}/luajit-parse-memprof.in - ${PROJECT_BINARY_DIR}/luajit-parse-memprof @ONLY ESCAPE_QUOTES) - file(INSTALL ${PROJECT_BINARY_DIR}/luajit-parse-memprof - DESTINATION ${CMAKE_INSTALL_PREFIX}/bin - USE_SOURCE_PERMISSIONS - ) - file(REMOVE ${PROJECT_BINARY_DIR}/luajit-parse-memprof) - " - COMPONENT tools-parse-memprof - ) endif() @@ -94,23 +58,7 @@ endif() if(LUAJIT_DISABLE_SYSPROF) message(STATUS "LuaJIT system profiler support is disabled") else() - # XXX: Can use genex here since the value need to be evaluated - # at the configuration phase. Fortunately, we know the exact - # path where LuaJIT binary is located. - set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME}) - set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR}) - # XXX: Unfortunately, there is no convenient way to set - # particular permissions to the output file via CMake. - # Furthermore, I even failed to copy the given file to the same - # path to change its permissions. After looking at the docs, I - # realized that the valid solution would be too monstrous for - # such a simple task. As a result I've made the template itself - # executable, so the issue is resolved. - configure_file(luajit-parse-sysprof.in luajit-parse-sysprof @ONLY ESCAPE_QUOTES) - - add_custom_target(tools-parse-sysprof EXCLUDE_FROM_ALL DEPENDS - luajit-parse-sysprof sysprof/parse.lua sysprof/collapse.lua sysprof.lua @@ -148,27 +96,6 @@ else() WORLD_READ COMPONENT tools-parse-sysprof ) - install(CODE - # XXX: The auxiliary script needs to be configured for to be - # used in repository directly. Furthermore, it needs to be - # reconfigured prior to its installation. The temporary - # <configure_file> output is stored to the project build - # directory and removed later after being installed. This - # script will have gone as a result of the issue: - # https://github.com/tarantool/tarantool/issues/5688. - " - set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME}) - set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR}) - configure_file(${CMAKE_CURRENT_SOURCE_DIR}/luajit-parse-sysprof.in - ${PROJECT_BINARY_DIR}/luajit-parse-sysprof @ONLY ESCAPE_QUOTES) - file(INSTALL ${PROJECT_BINARY_DIR}/luajit-parse-sysprof - DESTINATION ${CMAKE_INSTALL_PREFIX}/bin - USE_SOURCE_PERMISSIONS - ) - file(REMOVE ${PROJECT_BINARY_DIR}/luajit-parse-sysprof) - " - COMPONENT tools-parse-sysprof - ) endif() add_custom_target(LuaJIT-tools DEPENDS ${LUAJIT_TOOLS_DEPS}) diff --git a/tools/luajit-parse-memprof.in b/tools/luajit-parse-memprof.in deleted file mode 100755 index 8867202a..00000000 --- a/tools/luajit-parse-memprof.in +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash -# -# Launcher for memprof parser. - -LUA_PATH="@LUAJIT_TOOLS_DIR@/?.lua;;" \ - @LUAJIT_TOOLS_BIN@ @LUAJIT_TOOLS_DIR@/memprof.lua $@ diff --git a/tools/luajit-parse-sysprof.in b/tools/luajit-parse-sysprof.in deleted file mode 100644 index 2be25eb3..00000000 --- a/tools/luajit-parse-sysprof.in +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash -# -# Launcher for sysprof parser. - -LUA_PATH="@LUAJIT_TOOLS_DIR@/?.lua;;" \ - @LUAJIT_TOOLS_BIN@ @LUAJIT_TOOLS_DIR@/sysprof.lua $@ diff --git a/tools/memprof.lua b/tools/memprof.lua index cf66dd9e..8df1630a 100644 --- a/tools/memprof.lua +++ b/tools/memprof.lua @@ -107,7 +107,9 @@ local function dump(inputfile) local dheap = process.form_heap_delta(events, symbols) view.leak_info(dheap) view.aliases(symbols) - os.exit(0) + -- XXX: The second argument is required to properly close Lua + -- universe (i.e. invoke <lua_close> before exiting). + os.exit(0, true) end -- XXX: When this script is used as a preloaded module by an diff --git a/tools/sysprof.lua b/tools/sysprof.lua index 1afab195..6ca7f2f2 100644 --- a/tools/sysprof.lua +++ b/tools/sysprof.lua @@ -107,13 +107,23 @@ local function dump(inputfile) traverse_calltree(calltree, '') - os.exit(0) + -- XXX: The second argument is required to properly close Lua + -- universe (i.e. invoke <lua_close> before exiting). + os.exit(0, true) +end + +-- XXX: When this script is used as a preloaded module by an +-- application, it should return one function for correct parsing +-- of command line flags like --leak-only and dumping profile +-- info. +local function dump_wrapped(...) + return dump(parseargs(...)) end -- FIXME: this script should be application-independent. local args = {...} if #args == 1 and args[1] == "sysprof" then - return dump + return dump_wrapped else - dump(parseargs(args)) + dump_wrapped(args) end -- 2.41.0
next prev parent reply other threads:[~2023-08-25 9:15 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-08-25 9:14 [Tarantool-patches] [PATCH luajit 0/2 v8] introduce cli for tools Maxim Kokryashkin via Tarantool-patches 2023-08-25 9:14 ` Maxim Kokryashkin via Tarantool-patches [this message] 2023-08-28 11:17 ` [Tarantool-patches] [PATCH luajit 1/2 v8] tools: add cli flag to run profile dump parsers Sergey Kaplun via Tarantool-patches 2023-08-25 9:14 ` [Tarantool-patches] [PATCH luajit 2/2 v8] test: don't skip tool CLI flag for tarantool Maxim Kokryashkin via Tarantool-patches 2023-08-28 11:21 ` Sergey Kaplun via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ba310224cde35b7e0c38a741fbd1fe2400a28428.1692954427.git.m.kokryashkin@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=max.kokryashkin@gmail.com \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit 1/2 v8] tools: add cli flag to run profile dump parsers' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox