Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] memprof: introduce a CLI flag to run dump parser
@ 2021-07-29 11:51 Maxim Kokryashkin via Tarantool-patches
  2021-07-29 11:53 ` Максим Корякшин via Tarantool-patches
  2021-08-20 15:49 ` Sergey Kaplun via Tarantool-patches
  0 siblings, 2 replies; 6+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2021-07-29 11:51 UTC (permalink / raw)
  To: tarantool-patches, imun, skaplun

It is inconvenient to use a standalone shell script to parse a memprof
dump. That is why this commit adds a CLI flag to call the parser into
the LuaJIT, so now it is possible to parse a dump as simple as:
```
luajit -m memprof.bin
```

Closes tarantool/tarantool#5688
---
 CMakeLists.txt         |  8 +++++---
 src/CMakeLists.txt     |  5 +++++
 src/lj_tools_conf.h.in |  6 ++++++
 src/luajit.c           | 36 ++++++++++++++++++++++++++++--------
 tools/CMakeLists.txt   |  2 ++
 5 files changed, 46 insertions(+), 11 deletions(-)
 create mode 100644 src/lj_tools_conf.h.in

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5348e043..619e9441 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -250,6 +250,11 @@ endif()
 # related compiler and linker flags passed. This should be done
 # the right way later.
 
+# --- Tools --------------------------------------------------------------------
+
+add_subdirectory(tools)
+set(LUAJIT_TOOLS_DIR "${LUAJIT_TOOLS_DIR}")
+
 # --- Main source tree ---------------------------------------------------------
 
 add_subdirectory(src)
@@ -258,9 +263,6 @@ add_subdirectory(src)
 
 add_subdirectory(etc)
 
-# --- Tools --------------------------------------------------------------------
-
-add_subdirectory(tools)
 
 # --- Testing source tree ------------------------------------------------------
 
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 809aac68..d9debccf 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -142,6 +142,8 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
     ${SOURCES_UTILS}
 )
 
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
+
 set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
 
 # Build JIT sources if JIT support is enabled.
@@ -248,6 +250,9 @@ add_custom_target(
           jit/vmdef.lua
 )
 
+# --- Generate luajit tools config header -------------------------------------
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
+
 # --- Generate core and VM object files ---------------------------------------
 
 # Virtual machine.
diff --git a/src/lj_tools_conf.h.in b/src/lj_tools_conf.h.in
new file mode 100644
index 00000000..366c3ec4
--- /dev/null
+++ b/src/lj_tools_conf.h.in
@@ -0,0 +1,6 @@
+#ifndef LJ_TOOLS_CONF_H
+#define LJ_TOOLS_CONF_H
+
+#define PARSER_PATH "@LUAJIT_TOOLS_DIR@/memprof.lua"
+
+#endif
diff --git a/src/luajit.c b/src/luajit.c
index 1ca24301..6697d00f 100644
--- a/src/luajit.c
+++ b/src/luajit.c
@@ -19,6 +19,8 @@
 
 #include "lj_arch.h"
 
+#include "lj_tools_conf.h"
+
 #if LJ_TARGET_POSIX
 #include <unistd.h>
 #define lua_stdin_is_tty()	isatty(0)
@@ -72,6 +74,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"
+  "  -m        Parse memprof profile data.\n"
   "  -E        Ignore environment variables.\n"
   "  --        Stop handling options.\n"
   "  -         Execute stdin and stop handling options.\n", stderr);
@@ -266,21 +269,17 @@ static void dotty(lua_State *L)
   progname = oldprogname;
 }
 
-static int handle_script(lua_State *L, char **argx)
+static int call_script(lua_State *L, const char *fname)
 {
-  int status;
-  const char *fname = argx[0];
-  if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
-    fname = NULL;  /* stdin */
-  status = luaL_loadfile(L, fname);
+  int status = luaL_loadfile(L, fname);
   if (status == LUA_OK) {
     /* Fetch args from arg table. LUA_INIT or -e might have changed them. */
     int narg = 0;
     lua_getglobal(L, "arg");
     if (lua_istable(L, -1)) {
       do {
-	narg++;
-	lua_rawgeti(L, -narg, narg);
+	      narg++;
+	      lua_rawgeti(L, -narg, narg);
       } while (!lua_isnil(L, -1));
       lua_pop(L, 1);
       lua_remove(L, -narg);
@@ -290,6 +289,16 @@ static int handle_script(lua_State *L, char **argx)
     }
     status = docall(L, narg, 0);
   }
+  return status;
+}
+
+static int handle_script(lua_State *L, char **argx)
+{
+  int status;
+  const char *fname = argx[0];
+  if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
+    fname = NULL;  /* stdin */
+  call_script(L, fname);
   return report(L, status);
 }
 
@@ -398,6 +407,7 @@ static int dobytecode(lua_State *L, char **argv)
 #define FLAGS_EXEC		4
 #define FLAGS_OPTION		8
 #define FLAGS_NOENV		16
+#define FLAGS_MEMPROF_PARSE 32
 
 static int collectargs(char **argv, int *flags)
 {
@@ -411,6 +421,10 @@ static int collectargs(char **argv, int *flags)
       return i+1;
     case '\0':
       return i;
+    case 'm':
+      notail(argv[i]);
+      *flags |= FLAGS_MEMPROF_PARSE;
+      return i;
     case 'i':
       notail(argv[i]);
       *flags |= FLAGS_INTERACTIVE;
@@ -547,6 +561,12 @@ static int pmain(lua_State *L)
   s->status = runargs(L, argv, argn);
   if (s->status != LUA_OK) return 0;
 
+  if((flags & FLAGS_MEMPROF_PARSE)) {
+    //char* parser_path = "/usr/local/share/luajit-2.1.0-beta3/memprof.lua";
+    s->status = report(L, call_script(L, PARSER_PATH));
+    return s->status == LUA_OK;
+  }
+
   if (s->argc > argn) {
     s->status = handle_script(L, argv + argn);
     if (s->status != LUA_OK) return 0;
diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index 61830e44..84129153 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -16,6 +16,7 @@ else()
   # path where LuaJIT binary is located.
   set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME})
   set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+  set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
   # 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
@@ -75,6 +76,7 @@ else()
     "
       set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME})
       set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR})
+      set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
       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
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches]  [PATCH] memprof: introduce a CLI flag to run dump parser
  2021-07-29 11:51 [Tarantool-patches] [PATCH] memprof: introduce a CLI flag to run dump parser Maxim Kokryashkin via Tarantool-patches
@ 2021-07-29 11:53 ` Максим Корякшин via Tarantool-patches
  2021-07-29 11:55   ` Максим Корякшин via Tarantool-patches
  2021-08-20 15:49 ` Sergey Kaplun via Tarantool-patches
  1 sibling, 1 reply; 6+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-07-29 11:53 UTC (permalink / raw)
  To: Maxim Kokryashkin; +Cc: tarantool-patches

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


Sorry, I forgot to add the ‘LuaJIT’ tag to the subject
 
   
>It is inconvenient to use a standalone shell script to parse a memprof
>dump. That is why this commit adds a CLI flag to call the parser into
>the LuaJIT, so now it is possible to parse a dump as simple as:
>```
>luajit -m memprof.bin
>```
>
>Closes tarantool/tarantool#5688
>---
> CMakeLists.txt | 8 +++++---
> src/CMakeLists.txt | 5 +++++
> src/lj_tools_conf.h.in | 6 ++++++
> src/luajit.c | 36 ++++++++++++++++++++++++++++--------
> tools/CMakeLists.txt | 2 ++
> 5 files changed, 46 insertions(+), 11 deletions(-)
> create mode 100644 src/lj_tools_conf.h.in
>
>diff --git a/CMakeLists.txt b/CMakeLists.txt
>index 5348e043..619e9441 100644
>--- a/CMakeLists.txt
>+++ b/CMakeLists.txt
>@@ -250,6 +250,11 @@ endif()
> # related compiler and linker flags passed. This should be done
> # the right way later.
> 
>+# --- Tools --------------------------------------------------------------------
>+
>+add_subdirectory(tools)
>+set(LUAJIT_TOOLS_DIR "${LUAJIT_TOOLS_DIR}")
>+
> # --- Main source tree ---------------------------------------------------------
> 
> add_subdirectory(src)
>@@ -258,9 +263,6 @@ add_subdirectory(src)
> 
> add_subdirectory(etc)
> 
>-# --- Tools --------------------------------------------------------------------
>-
>-add_subdirectory(tools)
> 
> # --- Testing source tree ------------------------------------------------------
> 
>diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>index 809aac68..d9debccf 100644
>--- a/src/CMakeLists.txt
>+++ b/src/CMakeLists.txt
>@@ -142,6 +142,8 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
>     ${SOURCES_UTILS}
> )
> 
>+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>+
> set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
> 
> # Build JIT sources if JIT support is enabled.
>@@ -248,6 +250,9 @@ add_custom_target(
>           jit/vmdef.lua
> )
> 
>+# --- Generate luajit tools config header -------------------------------------
>+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>+
> # --- Generate core and VM object files ---------------------------------------
> 
> # Virtual machine.
>diff --git a/src/lj_tools_conf.h.in b/src/lj_tools_conf.h.in
>new file mode 100644
>index 00000000..366c3ec4
>--- /dev/null
>+++ b/src/lj_tools_conf.h.in
>@@ -0,0 +1,6 @@
>+#ifndef LJ_TOOLS_CONF_H
>+#define LJ_TOOLS_CONF_H
>+
>+#define PARSER_PATH "@LUAJIT_TOOLS_DIR@/memprof.lua"
>+
>+#endif
>diff --git a/src/luajit.c b/src/luajit.c
>index 1ca24301..6697d00f 100644
>--- a/src/luajit.c
>+++ b/src/luajit.c
>@@ -19,6 +19,8 @@
> 
> #include "lj_arch.h"
> 
>+#include "lj_tools_conf.h"
>+
> #if LJ_TARGET_POSIX
> #include <unistd.h>
> #define lua_stdin_is_tty() isatty(0)
>@@ -72,6 +74,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"
>+ " -m Parse memprof profile data.\n"
>   " -E Ignore environment variables.\n"
>   " -- Stop handling options.\n"
>   " - Execute stdin and stop handling options.\n", stderr);
>@@ -266,21 +269,17 @@ static void dotty(lua_State *L)
>   progname = oldprogname;
> }
> 
>-static int handle_script(lua_State *L, char **argx)
>+static int call_script(lua_State *L, const char *fname)
> {
>- int status;
>- const char *fname = argx[0];
>- if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
>- fname = NULL; /* stdin */
>- status = luaL_loadfile(L, fname);
>+ int status = luaL_loadfile(L, fname);
>   if (status == LUA_OK) {
>     /* Fetch args from arg table. LUA_INIT or -e might have changed them. */
>     int narg = 0;
>     lua_getglobal(L, "arg");
>     if (lua_istable(L, -1)) {
>       do {
>- narg++;
>- lua_rawgeti(L, -narg, narg);
>+ narg++;
>+ lua_rawgeti(L, -narg, narg);
>       } while (!lua_isnil(L, -1));
>       lua_pop(L, 1);
>       lua_remove(L, -narg);
>@@ -290,6 +289,16 @@ static int handle_script(lua_State *L, char **argx)
>     }
>     status = docall(L, narg, 0);
>   }
>+ return status;
>+}
>+
>+static int handle_script(lua_State *L, char **argx)
>+{
>+ int status;
>+ const char *fname = argx[0];
>+ if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
>+ fname = NULL; /* stdin */
>+ call_script(L, fname);
>   return report(L, status);
> }
> 
>@@ -398,6 +407,7 @@ static int dobytecode(lua_State *L, char **argv)
> #define FLAGS_EXEC 4
> #define FLAGS_OPTION 8
> #define FLAGS_NOENV 16
>+#define FLAGS_MEMPROF_PARSE 32
> 
> static int collectargs(char **argv, int *flags)
> {
>@@ -411,6 +421,10 @@ static int collectargs(char **argv, int *flags)
>       return i+1;
>     case '\0':
>       return i;
>+ case 'm':
>+ notail(argv[i]);
>+ *flags |= FLAGS_MEMPROF_PARSE;
>+ return i;
>     case 'i':
>       notail(argv[i]);
>       *flags |= FLAGS_INTERACTIVE;
>@@ -547,6 +561,12 @@ static int pmain(lua_State *L)
>   s->status = runargs(L, argv, argn);
>   if (s->status != LUA_OK) return 0;
> 
>+ if((flags & FLAGS_MEMPROF_PARSE)) {
>+ //char* parser_path = "/usr/local/share/luajit-2.1.0-beta3/memprof.lua";
>+ s->status = report(L, call_script(L, PARSER_PATH));
>+ return s->status == LUA_OK;
>+ }
>+
>   if (s->argc > argn) {
>     s->status = handle_script(L, argv + argn);
>     if (s->status != LUA_OK) return 0;
>diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
>index 61830e44..84129153 100644
>--- a/tools/CMakeLists.txt
>+++ b/tools/CMakeLists.txt
>@@ -16,6 +16,7 @@ else()
>   # path where LuaJIT binary is located.
>   set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME})
>   set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
>+ set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
>   # 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
>@@ -75,6 +76,7 @@ else()
>     "
>       set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME})
>       set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR})
>+ set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
>       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
>--
>2.32.0
 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches]  [PATCH] memprof: introduce a CLI flag to run dump parser
  2021-07-29 11:53 ` Максим Корякшин via Tarantool-patches
@ 2021-07-29 11:55   ` Максим Корякшин via Tarantool-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-07-29 11:55 UTC (permalink / raw)
  To: Максим
	Корякшин
  Cc: tarantool-patches, Maxim Kokryashkin

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


Yikes! And the GitHub branch too:
https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse
 
> 
>>Sorry, I forgot to add the ‘LuaJIT’ tag to the subject
>> 
>>   
>>>It is inconvenient to use a standalone shell script to parse a memprof
>>>dump. That is why this commit adds a CLI flag to call the parser into
>>>the LuaJIT, so now it is possible to parse a dump as simple as:
>>>```
>>>luajit -m memprof.bin
>>>```
>>>
>>>Closes tarantool/tarantool#5688
>>>---
>>> CMakeLists.txt | 8 +++++---
>>> src/CMakeLists.txt | 5 +++++
>>> src/lj_tools_conf.h.in | 6 ++++++
>>> src/luajit.c | 36 ++++++++++++++++++++++++++++--------
>>> tools/CMakeLists.txt | 2 ++
>>> 5 files changed, 46 insertions(+), 11 deletions(-)
>>> create mode 100644 src/lj_tools_conf.h.in
>>>
>>>diff --git a/CMakeLists.txt b/CMakeLists.txt
>>>index 5348e043..619e9441 100644
>>>--- a/CMakeLists.txt
>>>+++ b/CMakeLists.txt
>>>@@ -250,6 +250,11 @@ endif()
>>> # related compiler and linker flags passed. This should be done
>>> # the right way later.
>>> 
>>>+# --- Tools --------------------------------------------------------------------
>>>+
>>>+add_subdirectory(tools)
>>>+set(LUAJIT_TOOLS_DIR "${LUAJIT_TOOLS_DIR}")
>>>+
>>> # --- Main source tree ---------------------------------------------------------
>>> 
>>> add_subdirectory(src)
>>>@@ -258,9 +263,6 @@ add_subdirectory(src)
>>> 
>>> add_subdirectory(etc)
>>> 
>>>-# --- Tools --------------------------------------------------------------------
>>>-
>>>-add_subdirectory(tools)
>>> 
>>> # --- Testing source tree ------------------------------------------------------
>>> 
>>>diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>>>index 809aac68..d9debccf 100644
>>>--- a/src/CMakeLists.txt
>>>+++ b/src/CMakeLists.txt
>>>@@ -142,6 +142,8 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
>>>     ${SOURCES_UTILS}
>>> )
>>> 
>>>+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>>>+
>>> set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
>>> 
>>> # Build JIT sources if JIT support is enabled.
>>>@@ -248,6 +250,9 @@ add_custom_target(
>>>           jit/vmdef.lua
>>> )
>>> 
>>>+# --- Generate luajit tools config header -------------------------------------
>>>+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>>>+
>>> # --- Generate core and VM object files ---------------------------------------
>>> 
>>> # Virtual machine.
>>>diff --git a/src/lj_tools_conf.h.in b/src/lj_tools_conf.h.in
>>>new file mode 100644
>>>index 00000000..366c3ec4
>>>--- /dev/null
>>>+++ b/src/lj_tools_conf.h.in
>>>@@ -0,0 +1,6 @@
>>>+#ifndef LJ_TOOLS_CONF_H
>>>+#define LJ_TOOLS_CONF_H
>>>+
>>>+#define PARSER_PATH "@LUAJIT_TOOLS_DIR@/memprof.lua"
>>>+
>>>+#endif
>>>diff --git a/src/luajit.c b/src/luajit.c
>>>index 1ca24301..6697d00f 100644
>>>--- a/src/luajit.c
>>>+++ b/src/luajit.c
>>>@@ -19,6 +19,8 @@
>>> 
>>> #include "lj_arch.h"
>>> 
>>>+#include "lj_tools_conf.h"
>>>+
>>> #if LJ_TARGET_POSIX
>>> #include <unistd.h>
>>> #define lua_stdin_is_tty() isatty(0)
>>>@@ -72,6 +74,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"
>>>+ " -m Parse memprof profile data.\n"
>>>   " -E Ignore environment variables.\n"
>>>   " -- Stop handling options.\n"
>>>   " - Execute stdin and stop handling options.\n", stderr);
>>>@@ -266,21 +269,17 @@ static void dotty(lua_State *L)
>>>   progname = oldprogname;
>>> }
>>> 
>>>-static int handle_script(lua_State *L, char **argx)
>>>+static int call_script(lua_State *L, const char *fname)
>>> {
>>>- int status;
>>>- const char *fname = argx[0];
>>>- if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
>>>- fname = NULL; /* stdin */
>>>- status = luaL_loadfile(L, fname);
>>>+ int status = luaL_loadfile(L, fname);
>>>   if (status == LUA_OK) {
>>>     /* Fetch args from arg table. LUA_INIT or -e might have changed them. */
>>>     int narg = 0;
>>>     lua_getglobal(L, "arg");
>>>     if (lua_istable(L, -1)) {
>>>       do {
>>>- narg++;
>>>- lua_rawgeti(L, -narg, narg);
>>>+ narg++;
>>>+ lua_rawgeti(L, -narg, narg);
>>>       } while (!lua_isnil(L, -1));
>>>       lua_pop(L, 1);
>>>       lua_remove(L, -narg);
>>>@@ -290,6 +289,16 @@ static int handle_script(lua_State *L, char **argx)
>>>     }
>>>     status = docall(L, narg, 0);
>>>   }
>>>+ return status;
>>>+}
>>>+
>>>+static int handle_script(lua_State *L, char **argx)
>>>+{
>>>+ int status;
>>>+ const char *fname = argx[0];
>>>+ if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
>>>+ fname = NULL; /* stdin */
>>>+ call_script(L, fname);
>>>   return report(L, status);
>>> }
>>> 
>>>@@ -398,6 +407,7 @@ static int dobytecode(lua_State *L, char **argv)
>>> #define FLAGS_EXEC 4
>>> #define FLAGS_OPTION 8
>>> #define FLAGS_NOENV 16
>>>+#define FLAGS_MEMPROF_PARSE 32
>>> 
>>> static int collectargs(char **argv, int *flags)
>>> {
>>>@@ -411,6 +421,10 @@ static int collectargs(char **argv, int *flags)
>>>       return i+1;
>>>     case '\0':
>>>       return i;
>>>+ case 'm':
>>>+ notail(argv[i]);
>>>+ *flags |= FLAGS_MEMPROF_PARSE;
>>>+ return i;
>>>     case 'i':
>>>       notail(argv[i]);
>>>       *flags |= FLAGS_INTERACTIVE;
>>>@@ -547,6 +561,12 @@ static int pmain(lua_State *L)
>>>   s->status = runargs(L, argv, argn);
>>>   if (s->status != LUA_OK) return 0;
>>> 
>>>+ if((flags & FLAGS_MEMPROF_PARSE)) {
>>>+ //char* parser_path = "/usr/local/share/luajit-2.1.0-beta3/memprof.lua";
>>>+ s->status = report(L, call_script(L, PARSER_PATH));
>>>+ return s->status == LUA_OK;
>>>+ }
>>>+
>>>   if (s->argc > argn) {
>>>     s->status = handle_script(L, argv + argn);
>>>     if (s->status != LUA_OK) return 0;
>>>diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
>>>index 61830e44..84129153 100644
>>>--- a/tools/CMakeLists.txt
>>>+++ b/tools/CMakeLists.txt
>>>@@ -16,6 +16,7 @@ else()
>>>   # path where LuaJIT binary is located.
>>>   set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME})
>>>   set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
>>>+ set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
>>>   # 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
>>>@@ -75,6 +76,7 @@ else()
>>>     "
>>>       set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME})
>>>       set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR})
>>>+ set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
>>>       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
>>>--
>>>2.32.0
>> 
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH] memprof: introduce a CLI flag to run dump parser
  2021-07-29 11:51 [Tarantool-patches] [PATCH] memprof: introduce a CLI flag to run dump parser Maxim Kokryashkin via Tarantool-patches
  2021-07-29 11:53 ` Максим Корякшин via Tarantool-patches
@ 2021-08-20 15:49 ` Sergey Kaplun via Tarantool-patches
  2021-09-05 13:59   ` Максим Корякшин via Tarantool-patches
  1 sibling, 1 reply; 6+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-08-20 15:49 UTC (permalink / raw)
  To: Maxim Kokryashkin; +Cc: tarantool-patches

Hi, Maxim!

Thanks for the patch!

Please, consider my comments below.

On 29.07.21, Maxim Kokryashkin wrote:
> It is inconvenient to use a standalone shell script to parse a memprof
> dump. That is why this commit adds a CLI flag to call the parser into
> the LuaJIT, so now it is possible to parse a dump as simple as:
> ```
> luajit -m memprof.bin
> ```

And what about profile enabling/disabling?
How it scales for system profiler for example?

Yes, we should add new flag -m (stands for misc. builtin), but it should
work more similar like -j flag. So we can run memprof enabled (like
`-jp`). Also, we can use an **another** flag for tools (`-t`?) i.e. for
memprof data parsing.

The tricky part here is to come up with simple and user-friendly naming
for each (if we really need this) tool utility.

Thoughts?

> 
> Closes tarantool/tarantool#5688
> ---
>  CMakeLists.txt         |  8 +++++---
>  src/CMakeLists.txt     |  5 +++++
>  src/lj_tools_conf.h.in |  6 ++++++
>  src/luajit.c           | 36 ++++++++++++++++++++++++++++--------
>  tools/CMakeLists.txt   |  2 ++
>  5 files changed, 46 insertions(+), 11 deletions(-)
>  create mode 100644 src/lj_tools_conf.h.in
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 5348e043..619e9441 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -250,6 +250,11 @@ endif()
>  # related compiler and linker flags passed. This should be done
>  # the right way later.
>  
> +# --- Tools --------------------------------------------------------------------
> +
> +add_subdirectory(tools)
> +set(LUAJIT_TOOLS_DIR "${LUAJIT_TOOLS_DIR}")
> +
>  # --- Main source tree ---------------------------------------------------------
>  
>  add_subdirectory(src)
> @@ -258,9 +263,6 @@ add_subdirectory(src)
>  
>  add_subdirectory(etc)
>  
> -# --- Tools --------------------------------------------------------------------
> -
> -add_subdirectory(tools)

Why do we need this code movement?

>  
>  # --- Testing source tree ------------------------------------------------------
>  
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 809aac68..d9debccf 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -142,6 +142,8 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
>      ${SOURCES_UTILS}
>  )
>  
> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
> +
>  set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
>  
>  # Build JIT sources if JIT support is enabled.
> @@ -248,6 +250,9 @@ add_custom_target(
>            jit/vmdef.lua
>  )
>  
> +# --- Generate luajit tools config header -------------------------------------
> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
> +
>  # --- Generate core and VM object files ---------------------------------------
>  
>  # Virtual machine.
> diff --git a/src/lj_tools_conf.h.in b/src/lj_tools_conf.h.in
> new file mode 100644
> index 00000000..366c3ec4
> --- /dev/null
> +++ b/src/lj_tools_conf.h.in
> @@ -0,0 +1,6 @@
> +#ifndef LJ_TOOLS_CONF_H
> +#define LJ_TOOLS_CONF_H
> +
> +#define PARSER_PATH "@LUAJIT_TOOLS_DIR@/memprof.lua"
> +
> +#endif

For what do we need this file?

> diff --git a/src/luajit.c b/src/luajit.c
> index 1ca24301..6697d00f 100644
> --- a/src/luajit.c
> +++ b/src/luajit.c
> @@ -19,6 +19,8 @@
>  
>  #include "lj_arch.h"
>  
> +#include "lj_tools_conf.h"
> +
>  #if LJ_TARGET_POSIX
>  #include <unistd.h>
>  #define lua_stdin_is_tty()	isatty(0)
> @@ -72,6 +74,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"
> +  "  -m        Parse memprof profile data.\n"
>    "  -E        Ignore environment variables.\n"
>    "  --        Stop handling options.\n"
>    "  -         Execute stdin and stop handling options.\n", stderr);
> @@ -266,21 +269,17 @@ static void dotty(lua_State *L)
>    progname = oldprogname;
>  }
>  
> -static int handle_script(lua_State *L, char **argx)
> +static int call_script(lua_State *L, const char *fname)
>  {
> -  int status;
> -  const char *fname = argx[0];
> -  if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
> -    fname = NULL;  /* stdin */
> -  status = luaL_loadfile(L, fname);
> +  int status = luaL_loadfile(L, fname);
>    if (status == LUA_OK) {
>      /* Fetch args from arg table. LUA_INIT or -e might have changed them. */
>      int narg = 0;
>      lua_getglobal(L, "arg");
>      if (lua_istable(L, -1)) {
>        do {
> -	narg++;
> -	lua_rawgeti(L, -narg, narg);
> +	      narg++;
> +	      lua_rawgeti(L, -narg, narg);
>        } while (!lua_isnil(L, -1));
>        lua_pop(L, 1);
>        lua_remove(L, -narg);
> @@ -290,6 +289,16 @@ static int handle_script(lua_State *L, char **argx)
>      }
>      status = docall(L, narg, 0);
>    }
> +  return status;
> +}
> +
> +static int handle_script(lua_State *L, char **argx)
> +{
> +  int status;
> +  const char *fname = argx[0];
> +  if (strcmp(fname, "-") == 0 && strcmp(argx[-1], "--") != 0)
> +    fname = NULL;  /* stdin */
> +  call_script(L, fname);
>    return report(L, status);
>  }
>  
> @@ -398,6 +407,7 @@ static int dobytecode(lua_State *L, char **argv)
>  #define FLAGS_EXEC		4
>  #define FLAGS_OPTION		8
>  #define FLAGS_NOENV		16
> +#define FLAGS_MEMPROF_PARSE 32

I believe, that we don't need a new flag only for memprof parsing.
It can be handled by FLAGS_OPTION as -j flag.
We can introduce the new function `domisccmd()` like the `dojitcmd()`.

>  
>  static int collectargs(char **argv, int *flags)
>  {
> @@ -411,6 +421,10 @@ static int collectargs(char **argv, int *flags)
>        return i+1;
>      case '\0':
>        return i;
> +    case 'm':
> +      notail(argv[i]);
> +      *flags |= FLAGS_MEMPROF_PARSE;
> +      return i;
>      case 'i':
>        notail(argv[i]);
>        *flags |= FLAGS_INTERACTIVE;
> @@ -547,6 +561,12 @@ static int pmain(lua_State *L)
>    s->status = runargs(L, argv, argn);
>    if (s->status != LUA_OK) return 0;
>  
> +  if((flags & FLAGS_MEMPROF_PARSE)) {
> +    //char* parser_path = "/usr/local/share/luajit-2.1.0-beta3/memprof.lua";
> +    s->status = report(L, call_script(L, PARSER_PATH));
> +    return s->status == LUA_OK;
> +  }
> +
>    if (s->argc > argn) {
>      s->status = handle_script(L, argv + argn);
>      if (s->status != LUA_OK) return 0;
> diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
> index 61830e44..84129153 100644
> --- a/tools/CMakeLists.txt
> +++ b/tools/CMakeLists.txt
> @@ -16,6 +16,7 @@ else()
>    # path where LuaJIT binary is located.
>    set(LUAJIT_TOOLS_BIN ${LUAJIT_BINARY_DIR}/${LUAJIT_CLI_NAME})
>    set(LUAJIT_TOOLS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
> +  set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
>    # 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
> @@ -75,6 +76,7 @@ else()
>      "
>        set(LUAJIT_TOOLS_BIN ${CMAKE_INSTALL_PREFIX}/bin/${LUAJIT_CLI_NAME})
>        set(LUAJIT_TOOLS_DIR ${CMAKE_INSTALL_PREFIX}/${LUAJIT_DATAROOTDIR})
> +      set(LUAJIT_TOOLS_DIR ${LUAJIT_TOOLS_DIR} PARENT_SCOPE)
>        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
> -- 
> 2.32.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches]  [PATCH] memprof: introduce a CLI flag to run dump parser
  2021-08-20 15:49 ` Sergey Kaplun via Tarantool-patches
@ 2021-09-05 13:59   ` Максим Корякшин via Tarantool-patches
  2021-09-12  9:34     ` Максим Корякшин via Tarantool-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-09-05 13:59 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: Maxim Kokryashkin, tarantool-patches

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


 
 
<snipped>
>
>>
>> Closes tarantool/tarantool#5688
>> ---
>> CMakeLists.txt | 8 +++++---
>> src/CMakeLists.txt | 5 +++++
>> src/lj_tools_conf.h.in | 6 ++++++
>> src/luajit.c | 36 ++++++++++++++++++++++++++++--------
>> tools/CMakeLists.txt | 2 ++
>> 5 files changed, 46 insertions(+), 11 deletions(-)
>> create mode 100644 src/lj_tools_conf.h.in
>>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index 5348e043..619e9441 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -250,6 +250,11 @@ endif()
>> # related compiler and linker flags passed. This should be done
>> # the right way later.
>>
>> +# --- Tools --------------------------------------------------------------------
>> +
>> +add_subdirectory(tools)
>> +set(LUAJIT_TOOLS_DIR "${LUAJIT_TOOLS_DIR}")
>> +
>> # --- Main source tree ---------------------------------------------------------
>>
>> add_subdirectory(src)
>> @@ -258,9 +263,6 @@ add_subdirectory(src)
>>
>> add_subdirectory(etc)
>>
>> -# --- Tools --------------------------------------------------------------------
>> -
>> -add_subdirectory(tools)
>Why do we need this code movement?
We need this because there is a template file, which CMake generates during the
build process. 
>>
>> # --- Testing source tree ------------------------------------------------------
>>
>> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>> index 809aac68..d9debccf 100644
>> --- a/src/CMakeLists.txt
>> +++ b/src/CMakeLists.txt
>> @@ -142,6 +142,8 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
>> ${SOURCES_UTILS}
>> )
>>
>> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>> +
>> set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
>>
>> # Build JIT sources if JIT support is enabled.
>> @@ -248,6 +250,9 @@ add_custom_target(
>> jit/vmdef.lua
>> )
>>
>> +# --- Generate luajit tools config header -------------------------------------
>> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>> +
>> # --- Generate core and VM object files ---------------------------------------
>>
>> # Virtual machine.
>> diff --git a/src/lj_tools_conf.h.in b/src/lj_tools_conf.h.in
>> new file mode 100644
>> index 00000000..366c3ec4
>> --- /dev/null
>> +++ b/src/lj_tools_conf.h.in
>> @@ -0,0 +1,6 @@
>> +#ifndef LJ_TOOLS_CONF_H
>> +#define LJ_TOOLS_CONF_H
>> +
>> +#define PARSER_PATH "@LUAJIT_TOOLS_DIR@/memprof.lua"
>> +
>> +#endif
As I already said, this is a template file, which holds a path to
memprof.lua script after build completion. Thanks to this template,
the flag will properly work regardless of whether the luajit was
installed or not. 
>For what do we need this file?
<snipped>
 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches]  [PATCH] memprof: introduce a CLI flag to run dump parser
  2021-09-05 13:59   ` Максим Корякшин via Tarantool-patches
@ 2021-09-12  9:34     ` Максим Корякшин via Tarantool-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-09-12  9:34 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: Maxim Kokryashkin, tarantool-patches

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


Hello again, Sergey!
Sorry, I just realized I forgot to squash commit with CMake modifications into this patch,
so it is incomplete. Anyway, I’ll fix relevant issues and send v2.
 
Best regards,
Maxim Kokryashkin 
>Воскресенье, 5 сентября 2021, 16:59 +03:00 от Максим Корякшин <m.kokryashkin@tarantool.org>:
> 
><snipped>
>>
>>>
>>> Closes tarantool/tarantool#5688
>>> ---
>>> CMakeLists.txt | 8 +++++---
>>> src/CMakeLists.txt | 5 +++++
>>> src/lj_tools_conf.h.in | 6 ++++++
>>> src/luajit.c | 36 ++++++++++++++++++++++++++++--------
>>> tools/CMakeLists.txt | 2 ++
>>> 5 files changed, 46 insertions(+), 11 deletions(-)
>>> create mode 100644 src/lj_tools_conf.h.in
>>>
>>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>>> index 5348e043..619e9441 100644
>>> --- a/CMakeLists.txt
>>> +++ b/CMakeLists.txt
>>> @@ -250,6 +250,11 @@ endif()
>>> # related compiler and linker flags passed. This should be done
>>> # the right way later.
>>>
>>> +# --- Tools --------------------------------------------------------------------
>>> +
>>> +add_subdirectory(tools)
>>> +set(LUAJIT_TOOLS_DIR "${LUAJIT_TOOLS_DIR}")
>>> +
>>> # --- Main source tree ---------------------------------------------------------
>>>
>>> add_subdirectory(src)
>>> @@ -258,9 +263,6 @@ add_subdirectory(src)
>>>
>>> add_subdirectory(etc)
>>>
>>> -# --- Tools --------------------------------------------------------------------
>>> -
>>> -add_subdirectory(tools)
>>Why do we need this code movement?
>We need this because there is a template file, which CMake generates during the
>build process. 
>>>
>>> # --- Testing source tree ------------------------------------------------------
>>>
>>> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>>> index 809aac68..d9debccf 100644
>>> --- a/src/CMakeLists.txt
>>> +++ b/src/CMakeLists.txt
>>> @@ -142,6 +142,8 @@ make_source_list(SOURCES_CORE_NO_JIT_FFI
>>> ${SOURCES_UTILS}
>>> )
>>>
>>> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>>> +
>>> set(SOURCES_CORE ${SOURCES_CORE_NO_JIT_FFI})
>>>
>>> # Build JIT sources if JIT support is enabled.
>>> @@ -248,6 +250,9 @@ add_custom_target(
>>> jit/vmdef.lua
>>> )
>>>
>>> +# --- Generate luajit tools config header -------------------------------------
>>> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h.in ${CMAKE_CURRENT_SOURCE_DIR}/lj_tools_conf.h)
>>> +
>>> # --- Generate core and VM object files ---------------------------------------
>>>
>>> # Virtual machine.
>>> diff --git a/src/lj_tools_conf.h.in b/src/lj_tools_conf.h.in
>>> new file mode 100644
>>> index 00000000..366c3ec4
>>> --- /dev/null
>>> +++ b/src/lj_tools_conf.h.in
>>> @@ -0,0 +1,6 @@
>>> +#ifndef LJ_TOOLS_CONF_H
>>> +#define LJ_TOOLS_CONF_H
>>> +
>>> +#define PARSER_PATH "@LUAJIT_TOOLS_DIR@/memprof.lua"
>>> +
>>> +#endif
>As I already said, this is a template file, which holds a path to
>memprof.lua script after build completion. Thanks to this template,
>the flag will properly work regardless of whether the luajit was
>installed or not. 
>>For what do we need this file?
><snipped>
> 
 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-12  9:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 11:51 [Tarantool-patches] [PATCH] memprof: introduce a CLI flag to run dump parser Maxim Kokryashkin via Tarantool-patches
2021-07-29 11:53 ` Максим Корякшин via Tarantool-patches
2021-07-29 11:55   ` Максим Корякшин via Tarantool-patches
2021-08-20 15:49 ` Sergey Kaplun via Tarantool-patches
2021-09-05 13:59   ` Максим Корякшин via Tarantool-patches
2021-09-12  9:34     ` Максим Корякшин via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox