[Tarantool-patches] [PATCH luajit v9 0/2] introduce cli for tools

[Tarantool-patches] [PATCH luajit v9 0/2] introduce cli for tools

[Maxim Kokryashkin via Tarantool-patches]

This first patch requires additional changes for the Tarantool proxying.
These changes are done in the PR. The first patch has the test disabled for
Tarantool, so LuaJIT's integrational testing behaves properly. However,
the second patch in this series enables this test for Tarantool, and it
should be applied after the LuaJIT bump with the first patch.

To ensure the test results without the second patch, I've made and
additional branch just for the LuaJIT integrational testing, that
doesn't have the second patch. All of the required links are located
down below.

PR: https://github.com/tarantool/tarantool/pull/8002
Branch (whole series): https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse-tnt
Branch (the first patch only): https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse

Changes in v9:
- Fixed comments as per review by Sergey Kaplun

Maxim Kokryashkin (2):
  tools: add cli flag to run profile dump parsers
  test: don't skip tool CLI flag for tarantool

 Makefile.original                             |  24 +---
 src/luajit.c                                  |  44 ++++++-
 .../gh-5688-tool-cli-flag.test.lua            | 124 ++++++++++++++++++
 tools/CMakeLists.txt                          |  73 -----------
 tools/luajit-parse-memprof.in                 |   6 -
 tools/luajit-parse-sysprof.in                 |   6 -
 tools/memprof.lua                             |  12 +-
 tools/sysprof.lua                             |  18 ++-
 8 files changed, 183 insertions(+), 124 deletions(-)
 create mode 100644 test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
 delete mode 100755 tools/luajit-parse-memprof.in
 delete mode 100755 tools/luajit-parse-sysprof.in

--
2.41.0

[Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Maxim Kokryashkin via Tarantool-patches]

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 in 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 these changes.

Closes tarantool/tarantool#5688
---
 Makefile.original                             |  24 +---
 src/luajit.c                                  |  44 +++++-
 .../gh-5688-tool-cli-flag.test.lua            | 127 ++++++++++++++++++
 tools/CMakeLists.txt                          |  73 ----------
 tools/luajit-parse-memprof.in                 |   6 -
 tools/luajit-parse-sysprof.in                 |   6 -
 tools/memprof.lua                             |  12 +-
 tools/sysprof.lua                             |  18 ++-
 8 files changed, 186 insertions(+), 124 deletions(-)
 create mode 100644 test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
 delete mode 100755 tools/luajit-parse-memprof.in
 delete mode 100755 tools/luajit-parse-sysprof.in

diff --git a/Makefile.original b/Makefile.original
index 0c92df9e..e67b6aa8 100644
--- a/Makefile.original
+++ b/Makefile.original
@@ -57,8 +57,6 @@ INSTALL_DYLIBSHORT1= libluajit-$(ABIVER).dylib
 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)
@@ -67,8 +65,6 @@ INSTALL_SHORT2= $(INSTALL_LIB)/$(INSTALL_SOSHORT2)
 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 +83,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 +97,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,22 +141,18 @@ 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 ""
 
 
 uninstall:
 	@echo "==== Uninstalling LuaJIT $(VERSION) from $(PREFIX) ===="
-	$(UNINSTALL) $(INSTALL_T) $(INSTALL_STATIC) $(INSTALL_DYN) $(INSTALL_SHORT1) $(INSTALL_SHORT2) $(INSTALL_MAN)/$(FILE_MAN) $(INSTALL_PC) $(INSTALL_TMEMPROF)
+	$(UNINSTALL) $(INSTALL_T) $(INSTALL_STATIC) $(INSTALL_DYN) $(INSTALL_SHORT1) $(INSTALL_SHORT2) $(INSTALL_MAN)/$(FILE_MAN) $(INSTALL_PC)
 	for file in $(FILES_JITLIB); do \
 	  $(UNINSTALL) $(INSTALL_JITLIB)/$$file; \
 	  done
@@ -190,19 +179,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 3a3ec247..f57b7e95 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,37 @@ 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) {
+      if (!strncmp(msg, "module ", 7))
+        msg = "unknown luaJIT command or tools not installed";
+      l_message(progname, msg);
+    }
+    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:
+    l_message(progname, "unknown tool command");
+    break;
+  }
+  return -1;
+}
+
 /* Optimization flags. */
 static int dojitopt(lua_State *L, const char *opt)
 {
@@ -398,6 +430,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 +452,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;
       /* fallthrough */
@@ -475,6 +513,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;
@@ -536,7 +578,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..4dc4f6a1
--- /dev/null
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -0,0 +1,127 @@
+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 REDIRECT_OUTPUT = ' 2>&1'
+
+local TABLE_SIZE = 20
+
+local SMOKE_CMD_SET = {
+  {
+    cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
+    -- luacheck: no global
+    like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',
+  },
+  {
+    cmd = EXECUTABLE .. ' -ta ' .. BAD_PATH,
+    like = '.+unknown tool command',
+  },
+}
+
+local MEMPROF_CMD_SET = {
+  {
+    cmd = MEMPROF_PARSER .. BAD_PATH,
+    like = 'fopen, errno: 2',
+  },
+  {
+    cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF,
+    like = 'ALLOCATIONS.+',
+  },
+  {
+    cmd = MEMPROF_PARSER .. ' --wrong ' .. TMP_BINFILE_MEMPROF,
+    like = 'unrecognized option',
+  },
+  {
+    cmd = MEMPROF_PARSER .. ' --leak-only ' .. TMP_BINFILE_MEMPROF,
+    like = 'HEAP SUMMARY:.+',
+  },
+}
+
+local SYSPROF_CMD_SET = {
+  {
+    cmd = SYSPROF_PARSER .. BAD_PATH,
+    like = 'fopen, errno: 2',
+  },
+  {
+    cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF,
+    like = '[%w_@:;]+%s%d+\n*.*',
+  },
+  {
+    cmd = SYSPROF_PARSER .. ' --wrong ' .. TMP_BINFILE_SYSPROF,
+    like = 'unrecognized option',
+  },
+}
+
+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 output = io.popen(cmd_set[idx].cmd .. REDIRECT_OUTPUT):read('*all')
+      subtest:like(output, cmd_set[idx].like, 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 51ee3877..b951f767 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
     # TODO: This line is not deleted only for the sake of integrational
     # testing. It should be deleted in the next series.
@@ -152,27 +100,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 100755
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..e23bbf24 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
@@ -118,10 +120,4 @@ local function dump_wrapped(...)
   return dump(parseargs(...))
 end
 
--- FIXME: this script should be application-independent.
-local args = {...}
-if #args == 1 and args[1] == "memprof" then
-  return dump_wrapped
-else
-  dump_wrapped(args)
-end
+return dump_wrapped
diff --git a/tools/sysprof.lua b/tools/sysprof.lua
index 8e110a04..8449b23f 100644
--- a/tools/sysprof.lua
+++ b/tools/sysprof.lua
@@ -85,13 +85,17 @@ local function dump(inputfile)
   for stack, count in pairs(events) do
     print(stack, count)
   end
-  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
 
--- FIXME: this script should be application-independent.
-local args = {...}
-if #args == 1 and args[1] == "sysprof" then
-  return dump
-else
-  dump(parseargs(args))
+-- 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
+
+return dump_wrapped
-- 
2.41.0

[Tarantool-patches] [PATCH luajit v9 2/2] test: don't skip tool CLI flag for tarantool

[Maxim Kokryashkin via Tarantool-patches]

That skipcond was introduced to overcome the obstacles
of LuaJIT's integration testing in Tarantool. Since
the required patch is now in the Tarantool master, this
skipcond is now unnecessary.

Related to tarantool/tarantool#5688
---
 test/tarantool-tests/gh-5688-tool-cli-flag.test.lua | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
index 4dc4f6a1..77204014 100644
--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -3,9 +3,6 @@ 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)
-- 
2.41.0

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
LGTM, with a single nit below.


> +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) {
> +      if (!strncmp(msg, "module ", 7))
> +        msg = "unknown luaJIT command or tools not installed";

Nit: Use tab here for indentation.

> +      l_message(progname, msg);
> +    }
> +    return 1;
> +  }
> +  lua_getglobal(L, "arg");
> +  return report(L, lua_pcall(L, 1, 1, 0));
> +}
> +

<snipped>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit v9 2/2] test: don't skip tool CLI flag for tarantool

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
LGTM!

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Fixed your comment, branch is force-pushed.
On Sun, Sep 03, 2023 at 12:50:43PM +0300, Sergey Kaplun wrote:
> Hi, Maxim!
> Thanks for the patch!
> LGTM, with a single nit below.
> 
> 
> > +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) {
> > +      if (!strncmp(msg, "module ", 7))
> > +        msg = "unknown luaJIT command or tools not installed";
> 
> Nit: Use tab here for indentation.
Fixed. Here is the diff:
==============================================================================
diff --git a/src/luajit.c b/src/luajit.c
index f57b7e95..84472464 100644
--- a/src/luajit.c
+++ b/src/luajit.c
@@ -370,7 +370,7 @@ static int runtoolcmd(lua_State *L, const char *tool_name)
     const char *msg = lua_tostring(L, -1);
     if (msg) {
       if (!strncmp(msg, "module ", 7))
-        msg = "unknown luaJIT command or tools not installed";
+	msg = "unknown luaJIT command or tools not installed";
       l_message(progname, msg);
     }
     return 1;
==============================================================================
> 
> > +      l_message(progname, msg);
> > +    }
> > +    return 1;
> > +  }
> > +  lua_getglobal(L, "arg");
> > +  return report(L, lua_pcall(L, 1, 1, 0));
> > +}
> > +
> 
> <snipped>
> 
> -- 
> Best regards,
> Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Sergey Bronnikov via Tarantool-patches]

Hi, Max


On 8/31/23 14:35, Maxim Kokryashkin wrote:
> 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 in 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

$ ./build/src/luajit -ts --split sysprof.bin
luajit-parse-sysprof.lua: ERROR: unrecognized option `--split'. Try 
`--help'.


> ```
>
> The `luajit-parse-memprof` and `luajit-parse-sysprof` are purged
> as a result of these changes.
Typo: s/The/The scripts/
>
> Closes tarantool/tarantool#5688
> ---
<snipped>
> diff --git a/src/luajit.c b/src/luajit.c
> index 3a3ec247..f57b7e95 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"


When I run "luajit -b" without arguments LuaJIT shows me a usage for "-b":

[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -b
Save LuaJIT bytecode: luajit -b[options] input output
   -l        Only list bytecode.
   -s        Strip debug info (default).
   -g        Keep debug info.
   -n name   Set module name (default: auto-detect from input name).
   -t type   Set output file type (default: auto-detect from output name).
   -a arch   Override architecture for object files (default: native).
   -o os     Override OS for object files (default: native).
   -e chunk  Use chunk string as input.
   --        Stop handling options.
   -         Use stdin as input and/or stdout as output.

File types: c h obj o raw (default)


When I run "luajit -t" without arguments LuaJIT shows me the same usage, 
that is not helpful:


[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -t
usage: ./build/src/luajit [options]... [script [args]...].
Available options are:
   -e chunk  Execute string 'chunk'.
   -l name   Require library 'name'.
   -b ...    Save or list bytecode.
   -j cmd    Perform LuaJIT control command.
   -O[opt]   Control LuaJIT optimizations.
   -i        Enter interactive mode after executing 'script'.
   -v        Show version information.
   -t<cmd>   Execute tool.
   -E        Ignore environment variables.
   --        Stop handling options.
   -         Execute stdin and stop handling options.

Please show a usage with available arguments for "-t":


Parse profile dump: luajit -t[options] input
   -m       parse memprof dump.
   -s        parse sysprof dump (default).

   --leak-only ...

   --split ...



>     "  -E        Ignore environment variables.\n"
>     "  --        Stop handling options.\n"
>     "  -         Execute stdin and stop handling options.\n", stderr);
> @@ -361,6 +362,37 @@ static int dojitcmd(lua_State *L, const char *cmd)
>     return runcmdopt(L, opt ? opt+1 : opt);
>   }
>   

<snipped>


> 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..4dc4f6a1
> --- /dev/null
> +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> @@ -0,0 +1,127 @@
> +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',

indentation


> +  ['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 REDIRECT_OUTPUT = ' 2>&1'
> +
> +local TABLE_SIZE = 20
> +
> +local SMOKE_CMD_SET = {
> +  {
> +    cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
> +    -- luacheck: no global
> +    like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',

Let's define this global in .luacheckrc.

I think we will use _TARANTOOL more and this inline suppressions will be 
annoying.


<no obvious problems, snipped>

Re: [Tarantool-patches] [PATCH luajit v9 2/2] test: don't skip tool CLI flag for tarantool

[Sergey Bronnikov via Tarantool-patches]

Looks good to me.

On 8/31/23 14:35, Maxim Kokryashkin wrote:
> That skipcond was introduced to overcome the obstacles
> of LuaJIT's integration testing in Tarantool. Since
> the required patch is now in the Tarantool master, this
> skipcond is now unnecessary.
>
> Related to tarantool/tarantool#5688
> ---


<snipped>

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 7261 bytes --]


Hi, Sergey!
Thanks for the review!
Fixed all of your comments, except for the one about
`luacheckrc` and _TARANTOOL. I believe, it should be
done in a separate patch.
Here is the diff for other changes:
 
diff --git a/src/luajit.c b/src/luajit.c
index 84472464..b63c92d1 100644
--- a/src/luajit.c
+++ b/src/luajit.c
@@ -79,6 +79,17 @@ static void print_usage(void)
   fflush(stderr);
 }
 
+static void print_tools_usage(void)
+{
+  fputs("usage: ", stderr);
+  fputs(progname, stderr);
+  fputs(" -t<cmd>\n"
+  "Available tools are:\n"
+  "  -m [--leak-only] input  Memprof profile data parser.\n"
+  "  -s input                Sysprof profile data parser.\n", stderr);
+  fflush(stderr);
+}
+
 static void l_message(const char *pname, const char *msg)
 {
   if (pname) { fputs(pname, stderr); fputc(':', stderr); fputc(' ', stderr); }
@@ -387,7 +398,7 @@ static int dotoolcmd(lua_State *L, const char *cmd)
   case 's':
     return runtoolcmd(L, "sysprof");
   default:
-    l_message(progname, "unknown tool command");
+    print_tools_usage();
     break;
   }
   return -1;
@@ -454,8 +465,6 @@ static int collectargs(char **argv, int *flags)
       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;
diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
index ce99535a..8187c594 100644
--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -28,12 +28,11 @@ local TABLE_SIZE = 20
 local SMOKE_CMD_SET = {
   {
     cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
-    -- luacheck: no global
-    like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',
+    like = '.+Available tools.+',
   },
   {
     cmd = EXECUTABLE .. ' -ta ' .. BAD_PATH,
-    like = '.+unknown tool command',
+    like = '.+Available tools.+',
   },
 }
 
 
 
--
Best regards,
Maxim Kokryashkin
 
  
>Четверг, 7 сентября 2023, 12:41 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Hi, Max
>
>
>On  8/31/23 14 :35, Maxim Kokryashkin wrote:
>> 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 in 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
>
>$ ./build/src/luajit -ts --split sysprof.bin
>luajit-parse-sysprof.lua: ERROR: unrecognized option `--split'. Try
>`--help'.
>
>
>> ```
>>
>> The `luajit-parse-memprof` and `luajit-parse-sysprof` are purged
>> as a result of these changes.
>Typo: s/The/The scripts/
>>
>> Closes tarantool/tarantool#5688
>> ---
><snipped>
>> diff --git a/src/luajit.c b/src/luajit.c
>> index 3a3ec247..f57b7e95 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"
>
>
>When I run "luajit -b" without arguments LuaJIT shows me a usage for "-b":
>
>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -b
>Save LuaJIT bytecode: luajit -b[options] input output
>   -l        Only list bytecode.
>   -s        Strip debug info (default).
>   -g        Keep debug info.
>   -n name   Set module name (default: auto-detect from input name).
>   -t type   Set output file type (default: auto-detect from output name).
>   -a arch   Override architecture for object files (default: native).
>   -o os     Override OS for object files (default: native).
>   -e chunk  Use chunk string as input.
>   --        Stop handling options.
>   -         Use stdin as input and/or stdout as output.
>
>File types: c h obj o raw (default)
>
>
>When I run "luajit -t" without arguments LuaJIT shows me the same usage,
>that is not helpful:
>
>
>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -t
>usage: ./build/src/luajit [options]... [script [args]...].
>Available options are:
>   -e chunk  Execute string 'chunk'.
>   -l name   Require library 'name'.
>   -b ...    Save or list bytecode.
>   -j cmd    Perform LuaJIT control command.
>   -O[opt]   Control LuaJIT optimizations.
>   -i        Enter interactive mode after executing 'script'.
>   -v        Show version information.
>   -t<cmd>   Execute tool.
>   -E        Ignore environment variables.
>   --        Stop handling options.
>   -         Execute stdin and stop handling options.
>
>Please show a usage with available arguments for "-t":
>
>
>Parse profile dump: luajit -t[options] input
>   -m       parse memprof dump.
>   -s        parse sysprof dump (default).
>
>   --leak-only ...
>
>   --split ...
>
>
>
>> " -E Ignore environment variables.\n"
>> " -- Stop handling options.\n"
>> " - Execute stdin and stop handling options.\n", stderr);
>> @@ -361,6 +362,37 @@ static int dojitcmd(lua_State *L, const char *cmd)
>> return runcmdopt(L, opt ? opt+1 : opt);
>> }
>>
>
><snipped>
>
>
>> 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..4dc4f6a1
>> --- /dev/null
>> +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
>> @@ -0,0 +1,127 @@
>> +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',
>
>indentation
>
>
>> + ['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 REDIRECT_OUTPUT = ' 2>&1'
>> +
>> +local TABLE_SIZE = 20
>> +
>> +local SMOKE_CMD_SET = {
>> + {
>> + cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
>> + -- luacheck: no global
>> + like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',
>
>Let's define this global in .luacheckrc.
>
>I think we will use _TARANTOOL more and this inline suppressions will be
>annoying.
>
>
><no obvious problems, snipped>
 

[-- Attachment #2: Type: text/html, Size: 10029 bytes --]

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Sergey Bronnikov via Tarantool-patches]

Hi, Max

Thanks for the fixes!

Two new comments:

$  ./build/src/luajit -ts sysprof.bin
./build/src/luajit: ./tools/utils/symtab.lua:107: attempt to concatenate 
local 'magic' (a nil value)


Please fix.


$  ./build/src/luajit -ts mee
./build/src/luajit: ./tools/utils/bufread.lua:122: fopen, errno: 2


Seems the case is not handled in your code, let's fix it.


On 9/12/23 23:03, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed all of your comments, except for the one about
> `luacheckrc` and _TARANTOOL. I believe, it should be
> done in a separate patch.
>
<snipped>

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]


Hi!
  
>Четверг, 14 сентября 2023, 12:22 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Hi, Max
>
>Thanks for the fixes!
>
>Two new comments:
>
>$  ./build/src/luajit -ts sysprof.bin
>./build/src/luajit: ./tools/utils/symtab.lua:107: attempt to concatenate
>local 'magic' (a nil value)
Works just fine for me. Make sure you are using the right file.
>
>Please fix.
>
>
>$  ./build/src/luajit -ts mee
>./build/src/luajit: ./tools/utils/bufread.lua:122: fopen, errno: 2
>
>
>Seems the case is not handled in your code, let's fix it.
It is not relevant to flags and should be fixed for both the memprof
and the sysprof in a separate ticket with refactoring.
 
>
>On 9/12/23 23:03, Maxim Kokryashkin wrote:
>> Hi, Sergey!
>> Thanks for the review!
>> Fixed all of your comments, except for the one about
>> `luacheckrc` and _TARANTOOL. I believe, it should be
>> done in a separate patch.
>> <snipped>
 

[-- Attachment #2: Type: text/html, Size: 1722 bytes --]

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]


On 9/18/23 11:51, Maxim Kokryashkin wrote:
> Hi!
>
>     Четверг, 14 сентября 2023, 12:22 +03:00 от Sergey Bronnikov
>     <sergeyb@tarantool.org>:
>     Hi, Max
>
>     Thanks for the fixes!
>
>     Two new comments:
>
>     $  ./build/src/luajit -ts sysprof.bin
>     ./build/src/luajit: ./tools/utils/symtab.lua:107: attempt to
>     concatenate
>     local 'magic' (a nil value)
>
> Works just fine for me. Make sure you are using the right file.


$ ./build/src/luajit -ts sysprof.bin
$ file sysprof.bin
sysprof.bin: empty
$ ./build/src/luajit -ts sysprof.bin
./build/src/luajit: ./tools/utils/symtab.lua:107: attempt to concatenate 
local 'magic' (a nil value)
$

>
>     Please fix.
>
>
>     $  ./build/src/luajit -ts mee
>     ./build/src/luajit: ./tools/utils/bufread.lua:122: fopen, errno: 2
>
>
>     Seems the case is not handled in your code, let's fix it.
>
> It is not relevant to flags and should be fixed for both the memprof
> and the sysprof in a separate ticket with refactoring.

I have no objections, what is a number of the ticket?



>
>     On 9/12/23 23:03, Maxim Kokryashkin wrote:
>     > Hi, Sergey!
>     > Thanks for the review!
>     > Fixed all of your comments, except for the one about
>     > `luacheckrc` and _TARANTOOL. I believe, it should be
>     > done in a separate patch.
>     >
>     <snipped>
>

[-- Attachment #2: Type: text/html, Size: 3644 bytes --]

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]


Hi!
Here you go:  https://github.com/tarantool/tarantool/issues/9217
 
 
--
Best regards,
Maxim Kokryashkin
 
  
>Среда, 4 октября 2023, 11:58 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
> 
>On 9/18/23 11:51, Maxim Kokryashkin wrote:
>>Hi!
>>  
>>>Четверг, 14 сентября 2023, 12:22 +03:00 от Sergey Bronnikov  <sergeyb@tarantool.org> :
>>> 
>>>Hi, Max
>>>
>>>Thanks for the fixes!
>>>
>>>Two new comments:
>>>
>>>$  ./build/src/luajit -ts sysprof.bin
>>>./build/src/luajit: ./tools/utils/symtab.lua:107: attempt to concatenate
>>>local 'magic' (a nil value)
>>Works just fine for me. Make sure you are using the right file.
>
>$ ./build/src/luajit -ts sysprof.bin
>$ file sysprof.bin
>sysprof.bin: empty
>$ ./build/src/luajit -ts sysprof.bin
>./build/src/luajit: ./tools/utils/symtab.lua:107: attempt to concatenate local 'magic' (a nil value)
>$
> 
>>>
>>>Please fix.
>>>
>>>
>>>$  ./build/src/luajit -ts mee
>>>./build/src/luajit: ./tools/utils/bufread.lua:122: fopen, errno: 2
>>>
>>>
>>>Seems the case is not handled in your code, let's fix it.
>>It is not relevant to flags and should be fixed for both the memprof
>>and the sysprof in a separate ticket with refactoring.
>I have no objections, what is a number of the ticket?
>   
>> 
>>>
>>>On 9/12/23 23:03, Maxim Kokryashkin wrote:
>>>> Hi, Sergey!
>>>> Thanks for the review!
>>>> Fixed all of your comments, except for the one about
>>>> `luacheckrc` and _TARANTOOL. I believe, it should be
>>>> done in a separate patch.
>>>> <snipped>
>> 
 

[-- Attachment #2: Type: text/html, Size: 3200 bytes --]

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Sergey Bronnikov via Tarantool-patches]

On 10/4/23 14:49, Maxim Kokryashkin wrote:
> Hi!
> Here you go: https://github.com/tarantool/tarantool/issues/9217
Thanks! LGTM

Re: [Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers

[Igor Munkin via Tarantool-patches]

Max,

I've checked this patch into all long-term branches in tarantool/luajit
and bumped a new version in master, release/2.11 and release/2.10.

On 31.08.23, Maxim Kokryashkin via Tarantool-patches wrote:
> 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 in 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 these changes.
> 
> Closes tarantool/tarantool#5688
> ---
>  Makefile.original                             |  24 +---
>  src/luajit.c                                  |  44 +++++-
>  .../gh-5688-tool-cli-flag.test.lua            | 127 ++++++++++++++++++
>  tools/CMakeLists.txt                          |  73 ----------
>  tools/luajit-parse-memprof.in                 |   6 -
>  tools/luajit-parse-sysprof.in                 |   6 -
>  tools/memprof.lua                             |  12 +-
>  tools/sysprof.lua                             |  18 ++-
>  8 files changed, 186 insertions(+), 124 deletions(-)
>  create mode 100644 test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
>  delete mode 100755 tools/luajit-parse-memprof.in
>  delete mode 100755 tools/luajit-parse-sysprof.in
> 

<snipped>

> -- 
> 2.41.0
> 

-- 
Best regards,
IM