Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors
@ 2024-03-26 16:09 Maxim Kokryashkin via Tarantool-patches
  2024-04-04 11:19 ` Sergey Bronnikov via Tarantool-patches
  2024-04-11 17:06 ` Sergey Kaplun via Tarantool-patches
  0 siblings, 2 replies; 6+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2024-03-26 16:09 UTC (permalink / raw)
  To: tarantool-patches, skaplun, sergeyb

Prior to this patch, there was no error-checking in profilers,
which resulted in raw Lua errors being reported in cases of
non-existing paths or corrupt file structures. This patch
makes use of the event reader module introduced in commit
df5a07218dba8cefa370499a1a54ba75fb73b99b ("profilers: introduce
event reader module") to add error handling, so all parsing
errors are now reported in a user-friendly manner.

Tool CLI flag tests are adapted correspondingly to error message
changes.

Resolves tarantool/tarantool#9217
Part of tarantool/tarantool#5994
---
Branch: https://github.com/tarantool/luajit/tree/mkokryashkin/profile-parsers-refactoring-p2

 .../gh-5688-tool-cli-flag.test.lua            |  4 +-
 ...17-profile-parsers-error-handling.test.lua | 90 +++++++++++++++++++
 tools/memprof.lua                             | 11 +--
 tools/memprof/parse.lua                       |  5 +-
 tools/sysprof.lua                             | 10 +--
 tools/sysprof/parse.lua                       |  5 +-
 tools/utils/symtab.lua                        |  6 +-
 7 files changed, 112 insertions(+), 19 deletions(-)
 create mode 100644 test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua

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 75293f11..8ead83b5 100644
--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -42,7 +42,7 @@ local SMOKE_CMD_SET = {
 local MEMPROF_CMD_SET = {
   {
     cmd = MEMPROF_PARSER .. BAD_PATH,
-    like = 'fopen, errno: 2',
+    like = 'Failed to open.*fopen, errno: 2',
   },
   {
     cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF,
@@ -61,7 +61,7 @@ local MEMPROF_CMD_SET = {
 local SYSPROF_CMD_SET = {
   {
     cmd = SYSPROF_PARSER .. BAD_PATH,
-    like = 'fopen, errno: 2',
+    like = 'Failed to open.*fopen, errno: 2',
   },
   {
     cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF,
diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
new file mode 100644
index 00000000..c2e0f0a6
--- /dev/null
+++ b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
@@ -0,0 +1,90 @@
+local tap = require('tap')
+local test = tap.test('gh-9217-profile-parsers-error-handling'):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,
+})
+
+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 NON_PROFILE_DATA = utils.tools.profilename('not-profile-data.tmp.bin')
+local CORRUPT_PROFILE = utils.tools.profilename('profdata.tmp.bin')
+
+local EXECUTABLE = utils.exec.luacmd(arg)
+local PARSERS = {
+  memprof = EXECUTABLE .. ' -tm ',
+  sysprof = EXECUTABLE .. ' -ts ',
+}
+local REDIRECT_OUTPUT = ' 2>&1'
+
+local TABLE_SIZE = 20
+
+local TEST_CASES = {
+  {
+    path = BAD_PATH,
+    err_msg = 'Failed to open'
+  },
+  {
+    path = NON_PROFILE_DATA,
+    err_msg = 'Failed to parse symtab from'
+  },
+  {
+    path = CORRUPT_PROFILE,
+    err_msg = 'Failed to parse profile data from'
+  },
+}
+
+test:plan(2 * #TEST_CASES)
+
+local function generate_non_profile_data(path)
+  local file = io.open(path, 'w')
+  file:write('data')
+  file:close()
+end
+
+local function generate_corrupt_profile(path)
+  local res, err = misc.memprof.start(path)
+  -- Should start successfully.
+  assert(res, err)
+
+  local _ = table_new(TABLE_SIZE, 0)
+   _ = nil
+  collectgarbage()
+
+  res, err = misc.memprof.stop()
+  -- Should stop successfully.
+  assert(res, err)
+
+  local file = io.open(path, 'r')
+  local content = file:read('*all')
+  file:close()
+  local index = string.find(content, 'ljm')
+
+  file = io.open(path, 'w')
+  file:write(string.sub(content, 1, index - 1))
+  file:close()
+end
+
+generate_non_profile_data(NON_PROFILE_DATA)
+generate_corrupt_profile(CORRUPT_PROFILE)
+
+for _, case in ipairs(TEST_CASES) do
+  for profiler, parser in pairs(PARSERS) do
+    local path = case.path
+    local err_msg = case.err_msg
+    local output = io.popen(parser .. path .. REDIRECT_OUTPUT):read('*all')
+    test:like(output, err_msg, string.format('%s: %s', profiler, err_msg))
+  end
+end
+
+os.remove(NON_PROFILE_DATA)
+os.remove(CORRUPT_PROFILE)
+test:done(true)
diff --git a/tools/memprof.lua b/tools/memprof.lua
index 537cb869..7d7e8e05 100644
--- a/tools/memprof.lua
+++ b/tools/memprof.lua
@@ -10,10 +10,9 @@
 -- Major portions taken verbatim or adapted from the LuaVela.
 -- Copyright (C) 2015-2019 IPONWEB Ltd.

-local bufread = require "utils.bufread"
 local memprof = require "memprof.parse"
 local process = require "memprof.process"
-local symtab = require "utils.symtab"
+local evread = require "utils.evread"
 local view = require "memprof.humanize"

 local stdout, stderr = io.stdout, io.stderr
@@ -108,9 +107,11 @@ local function parseargs(args)
 end

 local function dump(inputfile)
-  local reader = bufread.new(inputfile)
-  local symbols = symtab.parse(reader)
-  local events = memprof.parse(reader, symbols)
+  -- XXX: This function exits with a non-zero exit code and
+  -- prints an error message if it encounters any failure during
+  -- the process of parsing.
+  local events, symbols = evread(memprof.parse, inputfile)
+
   if not config.leak_only then
     view.profile_info(events, config)
   end
diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index cc66a23e..11a79a1d 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -206,12 +206,13 @@ function M.parse(reader, symbols)
   local _ = reader:read_octets(3)

   if magic ~= LJM_MAGIC then
-    error("Bad LJM format prologue: "..magic)
+    error("Bad memprof event format prologue: "..magic)
   end

   if string.byte(version) ~= LJM_CURRENT_VERSION then
     error(string_format(
-      "LJM format version mismatch: the tool expects %d, but your data is %d",
+      "Memprof event format version mismatch:"..
+      " the tool expects %d, but your data is %d",
       LJM_CURRENT_VERSION,
       string.byte(version)
     ))
diff --git a/tools/sysprof.lua b/tools/sysprof.lua
index 8449b23f..420ca41c 100644
--- a/tools/sysprof.lua
+++ b/tools/sysprof.lua
@@ -1,6 +1,5 @@
-local bufread = require "utils.bufread"
+local evread = require "utils.evread"
 local sysprof = require "sysprof.parse"
-local symtab = require "utils.symtab"

 local stdout, stderr = io.stdout, io.stderr
 local match, gmatch = string.match, string.gmatch
@@ -78,9 +77,10 @@ local function parseargs(args)
 end

 local function dump(inputfile)
-  local reader = bufread.new(inputfile)
-  local symbols = symtab.parse(reader)
-  local events = sysprof.parse(reader, symbols)
+  -- XXX: This function exits with a non-zero exit code and
+  -- prints an error message if it encounters any failure during
+  -- the process of parsing.
+  local events = evread(sysprof.parse, inputfile)

   for stack, count in pairs(events) do
     print(stack, count)
diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
index 64c4a455..0e416d37 100755
--- a/tools/sysprof/parse.lua
+++ b/tools/sysprof/parse.lua
@@ -237,12 +237,13 @@ function M.parse(reader, symbols)
   local _ = reader:read_octets(3)

   if magic ~= LJP_MAGIC then
-    error("Bad LJP format prologue: "..magic)
+    error("Bad sysprof event format prologue: " .. tostring(magic))
   end

   if string.byte(version) ~= LJP_CURRENT_VERSION then
     error(string_format(
-      "LJP format version mismatch: the tool expects %d, but your data is %d",
+      "Sysprof event format version mismatch:"..
+      " the tool expects %d, but your data is %d",
       LJP_CURRENT_VERSION,
       string.byte(version)
     ))
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index 0c878e7a..e80b9f33 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -95,13 +95,13 @@ function M.parse(reader)
   local _ = reader:read_octets(3)

   if magic ~= LJS_MAGIC then
-    error("Bad LJS format prologue: "..magic)
+    error("Bad LuaJIT symbol table format prologue: " .. tostring(magic))
   end

   if string.byte(version) ~= LJS_CURRENT_VERSION then
     error(string_format(
-         "LJS format version mismatch:"..
-         "the tool expects %d, but your data is %d",
+         "LuaJIT symbol table format version mismatch:"..
+         " the tool expects %d, but your data is %d",
          LJS_CURRENT_VERSION,
          string.byte(version)
     ))
--
2.44.0


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

* Re: [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors
  2024-03-26 16:09 [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors Maxim Kokryashkin via Tarantool-patches
@ 2024-04-04 11:19 ` Sergey Bronnikov via Tarantool-patches
  2024-04-04 13:33   ` Sergey Kaplun via Tarantool-patches
  2024-04-08  9:33   ` Maxim Kokryashkin via Tarantool-patches
  2024-04-11 17:06 ` Sergey Kaplun via Tarantool-patches
  1 sibling, 2 replies; 6+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-04-04 11:19 UTC (permalink / raw)
  To: Maxim Kokryashkin, tarantool-patches, skaplun

Hi, Max


thanks for the patch! LGTM with minor comments.


On 3/26/24 19:09, Maxim Kokryashkin wrote:
> Prior to this patch, there was no error-checking in profilers,
> which resulted in raw Lua errors being reported in cases of
> non-existing paths or corrupt file structures. This patch
> makes use of the event reader module introduced in commit
> df5a07218dba8cefa370499a1a54ba75fb73b99b ("profilers: introduce
> event reader module") to add error handling, so all parsing
> errors are now reported in a user-friendly manner.
>
> Tool CLI flag tests are adapted correspondingly to error message
> changes.
>
> Resolves tarantool/tarantool#9217
> Part of tarantool/tarantool#5994
> ---
> Branch: https://github.com/tarantool/luajit/tree/mkokryashkin/profile-parsers-refactoring-p2
>
>   .../gh-5688-tool-cli-flag.test.lua            |  4 +-
>   ...17-profile-parsers-error-handling.test.lua | 90 +++++++++++++++++++
>   tools/memprof.lua                             | 11 +--
>   tools/memprof/parse.lua                       |  5 +-
>   tools/sysprof.lua                             | 10 +--
>   tools/sysprof/parse.lua                       |  5 +-
>   tools/utils/symtab.lua                        |  6 +-
>   7 files changed, 112 insertions(+), 19 deletions(-)
>   create mode 100644 test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
>
> 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 75293f11..8ead83b5 100644
> --- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> @@ -42,7 +42,7 @@ local SMOKE_CMD_SET = {
>   local MEMPROF_CMD_SET = {
>     {
>       cmd = MEMPROF_PARSER .. BAD_PATH,
> -    like = 'fopen, errno: 2',
> +    like = 'Failed to open.*fopen, errno: 2',
>     },
>     {
>       cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF,
> @@ -61,7 +61,7 @@ local MEMPROF_CMD_SET = {
>   local SYSPROF_CMD_SET = {
>     {
>       cmd = SYSPROF_PARSER .. BAD_PATH,
> -    like = 'fopen, errno: 2',
> +    like = 'Failed to open.*fopen, errno: 2',
>     },
>     {
>       cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF,
> diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
> new file mode 100644
> index 00000000..c2e0f0a6
> --- /dev/null
> +++ b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
> @@ -0,0 +1,90 @@
> +local tap = require('tap')
> +local test = tap.test('gh-9217-profile-parsers-error-handling'):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,
> +})
> +
> +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 NON_PROFILE_DATA = utils.tools.profilename('not-profile-data.tmp.bin')
> +local CORRUPT_PROFILE = utils.tools.profilename('profdata.tmp.bin')
> +
> +local EXECUTABLE = utils.exec.luacmd(arg)
> +local PARSERS = {
> +  memprof = EXECUTABLE .. ' -tm ',
> +  sysprof = EXECUTABLE .. ' -ts ',
> +}
> +local REDIRECT_OUTPUT = ' 2>&1'
> +
> +local TABLE_SIZE = 20
> +
> +local TEST_CASES = {
> +  {
> +    path = BAD_PATH,
> +    err_msg = 'Failed to open'
> +  },
> +  {
> +    path = NON_PROFILE_DATA,
> +    err_msg = 'Failed to parse symtab from'
> +  },
> +  {
> +    path = CORRUPT_PROFILE,
> +    err_msg = 'Failed to parse profile data from'
> +  },

Nit: I would rename CORRUPT_PROFILE to CORRUPT_PROFILE_DATA for consitency

with NON_PROFILE_DATA.

> +}
> +
> +test:plan(2 * #TEST_CASES)
> +
> +local function generate_non_profile_data(path)
> +  local file = io.open(path, 'w')
> +  file:write('data')
> +  file:close()
> +end
> +
> +local function generate_corrupt_profile(path)
Nit: generate_corrupt_profile -> generate_corrupt_profile_data
> +  local res, err = misc.memprof.start(path)
> +  -- Should start successfully.
> +  assert(res, err)
> +
> +  local _ = table_new(TABLE_SIZE, 0)
> +   _ = nil
> +  collectgarbage()
> +
> +  res, err = misc.memprof.stop()
> +  -- Should stop successfully.
> +  assert(res, err)
> +
> +  local file = io.open(path, 'r')
> +  local content = file:read('*all')
> +  file:close()
> +  local index = string.find(content, 'ljm')
> +
> +  file = io.open(path, 'w')
> +  file:write(string.sub(content, 1, index - 1))
> +  file:close()
> +end
> +
> +generate_non_profile_data(NON_PROFILE_DATA)
> +generate_corrupt_profile(CORRUPT_PROFILE)
> +
> +for _, case in ipairs(TEST_CASES) do
> +  for profiler, parser in pairs(PARSERS) do
> +    local path = case.path
> +    local err_msg = case.err_msg
> +    local output = io.popen(parser .. path .. REDIRECT_OUTPUT):read('*all')
> +    test:like(output, err_msg, string.format('%s: %s', profiler, err_msg))
> +  end
> +end
> +
> +os.remove(NON_PROFILE_DATA)
> +os.remove(CORRUPT_PROFILE)
> +test:done(true)
> diff --git a/tools/memprof.lua b/tools/memprof.lua
> index 537cb869..7d7e8e05 100644
> --- a/tools/memprof.lua
> +++ b/tools/memprof.lua
> @@ -10,10 +10,9 @@
>   -- Major portions taken verbatim or adapted from the LuaVela.
>   -- Copyright (C) 2015-2019 IPONWEB Ltd.
>
> -local bufread = require "utils.bufread"
>   local memprof = require "memprof.parse"
>   local process = require "memprof.process"
> -local symtab = require "utils.symtab"
> +local evread = require "utils.evread"
>   local view = require "memprof.humanize"
>
>   local stdout, stderr = io.stdout, io.stderr
> @@ -108,9 +107,11 @@ local function parseargs(args)
>   end
>
>   local function dump(inputfile)
> -  local reader = bufread.new(inputfile)
> -  local symbols = symtab.parse(reader)
> -  local events = memprof.parse(reader, symbols)
> +  -- XXX: This function exits with a non-zero exit code and
> +  -- prints an error message if it encounters any failure during
> +  -- the process of parsing.
> +  local events, symbols = evread(memprof.parse, inputfile)
> +
>     if not config.leak_only then
>       view.profile_info(events, config)
>     end
> diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> index cc66a23e..11a79a1d 100644
> --- a/tools/memprof/parse.lua
> +++ b/tools/memprof/parse.lua
> @@ -206,12 +206,13 @@ function M.parse(reader, symbols)
>     local _ = reader:read_octets(3)
>
>     if magic ~= LJM_MAGIC then
> -    error("Bad LJM format prologue: "..magic)
> +    error("Bad memprof event format prologue: "..magic)
>     end
>
>     if string.byte(version) ~= LJM_CURRENT_VERSION then
>       error(string_format(
> -      "LJM format version mismatch: the tool expects %d, but your data is %d",
> +      "Memprof event format version mismatch:"..
> +      " the tool expects %d, but your data is %d",
>         LJM_CURRENT_VERSION,
>         string.byte(version)
>       ))
> diff --git a/tools/sysprof.lua b/tools/sysprof.lua
> index 8449b23f..420ca41c 100644
> --- a/tools/sysprof.lua
> +++ b/tools/sysprof.lua
> @@ -1,6 +1,5 @@
> -local bufread = require "utils.bufread"
> +local evread = require "utils.evread"
>   local sysprof = require "sysprof.parse"
> -local symtab = require "utils.symtab"
>
>   local stdout, stderr = io.stdout, io.stderr
>   local match, gmatch = string.match, string.gmatch
> @@ -78,9 +77,10 @@ local function parseargs(args)
>   end
>
>   local function dump(inputfile)
> -  local reader = bufread.new(inputfile)
> -  local symbols = symtab.parse(reader)
> -  local events = sysprof.parse(reader, symbols)
> +  -- XXX: This function exits with a non-zero exit code and
> +  -- prints an error message if it encounters any failure during
> +  -- the process of parsing.
> +  local events = evread(sysprof.parse, inputfile)
>
>     for stack, count in pairs(events) do
>       print(stack, count)
> diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
> index 64c4a455..0e416d37 100755
> --- a/tools/sysprof/parse.lua
> +++ b/tools/sysprof/parse.lua
> @@ -237,12 +237,13 @@ function M.parse(reader, symbols)
>     local _ = reader:read_octets(3)
>
>     if magic ~= LJP_MAGIC then
> -    error("Bad LJP format prologue: "..magic)
> +    error("Bad sysprof event format prologue: " .. tostring(magic))
>     end
>
>     if string.byte(version) ~= LJP_CURRENT_VERSION then
>       error(string_format(
> -      "LJP format version mismatch: the tool expects %d, but your data is %d",
> +      "Sysprof event format version mismatch:"..
> +      " the tool expects %d, but your data is %d",
>         LJP_CURRENT_VERSION,
>         string.byte(version)
>       ))
> diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
> index 0c878e7a..e80b9f33 100644
> --- a/tools/utils/symtab.lua
> +++ b/tools/utils/symtab.lua
> @@ -95,13 +95,13 @@ function M.parse(reader)
>     local _ = reader:read_octets(3)
>
>     if magic ~= LJS_MAGIC then
> -    error("Bad LJS format prologue: "..magic)
> +    error("Bad LuaJIT symbol table format prologue: " .. tostring(magic))
>     end
>
>     if string.byte(version) ~= LJS_CURRENT_VERSION then
>       error(string_format(
> -         "LJS format version mismatch:"..
> -         "the tool expects %d, but your data is %d",
> +         "LuaJIT symbol table format version mismatch:"..
> +         " the tool expects %d, but your data is %d",

Nit: could be replaced with a single line patch after removing 
whitespace on a single line:


    if string.byte(version) ~= LJS_CURRENT_VERSION then
      error(string_format(
-         "LJS format version mismatch:"..
+         "LuaJIT symbol table format version mismatch: "..
           "the tool expects %d, but your data is %d",
           LJS_CURRENT_VERSION,
           string.byte(version)

>            LJS_CURRENT_VERSION,
>            string.byte(version)
>       ))
> --
> 2.44.0
>

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

* Re: [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors
  2024-04-04 11:19 ` Sergey Bronnikov via Tarantool-patches
@ 2024-04-04 13:33   ` Sergey Kaplun via Tarantool-patches
  2024-04-08  9:33   ` Maxim Kokryashkin via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-04-04 13:33 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: Maxim Kokryashkin, tarantool-patches

Hi, Max!
Thanks for the patch!
LGTM, after fixing Sergey's comment and rebasing the branch on the current
tarantool/master.

-- 
Best regards,
Sergey Kaplun

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

* Re: [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors
  2024-04-04 11:19 ` Sergey Bronnikov via Tarantool-patches
  2024-04-04 13:33   ` Sergey Kaplun via Tarantool-patches
@ 2024-04-08  9:33   ` Maxim Kokryashkin via Tarantool-patches
  2024-04-08 15:42     ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 6+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2024-04-08  9:33 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: Maxim Kokryashkin, tarantool-patches

Hi, Sergey!
Thanks for the review!
Fixed your comments, branch is rebased and force-pushed.
Here is the diff with fixes:
===
diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
index c2e0f0a6..0b7ee2dd 100644
--- a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
+++ b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
@@ -16,7 +16,7 @@ local utils = require('utils')

 local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin')
 local NON_PROFILE_DATA = utils.tools.profilename('not-profile-data.tmp.bin')
-local CORRUPT_PROFILE = utils.tools.profilename('profdata.tmp.bin')
+local CORRUPT_PROFILE_DATA = utils.tools.profilename('profdata.tmp.bin')

 local EXECUTABLE = utils.exec.luacmd(arg)
 local PARSERS = {
@@ -37,7 +37,7 @@ local TEST_CASES = {
     err_msg = 'Failed to parse symtab from'
   },
   {
-    path = CORRUPT_PROFILE,
+    path = CORRUPT_PROFILE_DATA,
     err_msg = 'Failed to parse profile data from'
   },
 }
@@ -50,7 +50,7 @@ local function generate_non_profile_data(path)
   file:close()
 end

-local function generate_corrupt_profile(path)
+local function generate_corrupt_profile_data(path)
   local res, err = misc.memprof.start(path)
   -- Should start successfully.
   assert(res, err)
@@ -74,7 +74,7 @@ local function generate_corrupt_profile(path)
 end

 generate_non_profile_data(NON_PROFILE_DATA)
-generate_corrupt_profile(CORRUPT_PROFILE)
+generate_corrupt_profile_data(CORRUPT_PROFILE_DATA)

 for _, case in ipairs(TEST_CASES) do
   for profiler, parser in pairs(PARSERS) do
@@ -86,5 +86,5 @@ for _, case in ipairs(TEST_CASES) do
 end

 os.remove(NON_PROFILE_DATA)
-os.remove(CORRUPT_PROFILE)
+os.remove(CORRUPT_PROFILE_DATA)
 test:done(true)
diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
index e80b9f33..f00d6bc0 100644
--- a/tools/utils/symtab.lua
+++ b/tools/utils/symtab.lua
@@ -100,8 +100,8 @@ function M.parse(reader)

   if string.byte(version) ~= LJS_CURRENT_VERSION then
     error(string_format(
-         "LuaJIT symbol table format version mismatch:"..
-         " the tool expects %d, but your data is %d",
+         "LuaJIT symbol table format version mismatch: "..
+         "the tool expects %d, but your data is %d",
          LJS_CURRENT_VERSION,
          string.byte(version)
     ))
===

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

* Re: [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors
  2024-04-08  9:33   ` Maxim Kokryashkin via Tarantool-patches
@ 2024-04-08 15:42     ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-04-08 15:42 UTC (permalink / raw)
  To: Maxim Kokryashkin; +Cc: Maxim Kokryashkin, tarantool-patches

Hi, Max

thanks for the fixes! LGTM


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

* Re: [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors
  2024-03-26 16:09 [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors Maxim Kokryashkin via Tarantool-patches
  2024-04-04 11:19 ` Sergey Bronnikov via Tarantool-patches
@ 2024-04-11 17:06 ` Sergey Kaplun via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-04-11 17:06 UTC (permalink / raw)
  To: Maxim Kokryashkin; +Cc: tarantool-patches

Maxim,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master [1], release/3.0 [2]
and release/2.11 [3].

[1]: https://github.com/tarantool/tarantool/pull/9938
[2]: https://github.com/tarantool/tarantool/pull/9939
[3]: https://github.com/tarantool/tarantool/pull/9940

-- 
Best regards,
Sergey Kaplun

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

end of thread, other threads:[~2024-04-11 17:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-26 16:09 [Tarantool-patches] [PATCH luajit v1] profilers: print user-friendly errors Maxim Kokryashkin via Tarantool-patches
2024-04-04 11:19 ` Sergey Bronnikov via Tarantool-patches
2024-04-04 13:33   ` Sergey Kaplun via Tarantool-patches
2024-04-08  9:33   ` Maxim Kokryashkin via Tarantool-patches
2024-04-08 15:42     ` Sergey Bronnikov via Tarantool-patches
2024-04-11 17:06 ` Sergey Kaplun 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