Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mikhail Shishatskiy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, imun@tarantool.org,
	skaplun@tarantool.org
Subject: [Tarantool-patches] [PATCH luajit v1] memprof: report all JIT-related allocations as internal
Date: Thu, 22 Jul 2021 18:46:17 +0700	[thread overview]
Message-ID: <20210722114617.194747-1-m.shishatskiy@tarantool.org> (raw)

There are cases when the memory profiler attempts to attribute
allocations triggered by the JIT engine recording phase
with a Lua function to be recorded. In this case,
lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).

Previously, these situations were ignored and the profiler
reported, that the source line was equal to zero.

This patch adjusts profiler behavior to treat allocations
described above as internal by dumping ASOURCE_INT if
lj_debug_frameline() returns a negative value.

Resolves tarantool/tarantool#5679
---

Issue: https://github.com/tarantool/tarantool/issues/5679
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5679-report-jit-allocations-as-internal

 src/lj_memprof.c                              | 28 +++++++----
 .../misclib-memprof-lapi.test.lua             | 50 ++++++++++++-------
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 2c1ef3b8..b4985b8e 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -89,19 +89,27 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
 				GCfunc *fn, struct lua_State *L,
 				cTValue *nextframe)
 {
-  const BCLine line = lj_debug_frameline(L, fn, nextframe);
-  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
-  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
   /*
-  ** Line is >= 0 if we are inside a Lua function.
-  ** There are cases when the memory profiler attempts
-  ** to attribute allocations triggered by JIT engine recording
-  ** phase with a Lua function to be recorded. At this case
-  ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).
-  ** Equals to zero when LuaJIT is built with the
+  ** Line equals to zero when LuaJIT is built with the
   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
   */
-  lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0);
+  const BCLine line = lj_debug_frameline(L, fn, nextframe);
+
+  if (line < 0) {
+    /*
+    ** Line is >= 0 if we are inside a Lua function.
+    ** There are cases when the memory profiler attempts
+    ** to attribute allocations triggered by JIT engine recording
+    ** phase with a Lua function to be recorded. It this case,
+    ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).
+    ** We report such allocations as internal in order not to confuse users.
+    */
+    lj_wbuf_addbyte(out, aevent | ASOURCE_INT);
+  } else {
+    lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
+    lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
+    lj_wbuf_addu64(out, (uint64_t)line);
+  }
 }
 
 static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 06d96b3b..e35d8c29 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -7,7 +7,7 @@ require("utils").skipcond(
 local tap = require("tap")
 
 local test = tap.test("misc-memprof-lapi")
-test:plan(13)
+test:plan(14)
 
 jit.off()
 jit.flush()
@@ -22,7 +22,7 @@ local symtab = require "utils.symtab"
 local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
 local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/memprofdata.tmp.bin")
 
-local function payload()
+local function default_payload()
   -- Preallocate table to avoid table array part reallocations.
   local _ = table_new(100, 0)
 
@@ -37,7 +37,7 @@ local function payload()
   collectgarbage()
 end
 
-local function generate_output(filename)
+local function generate_output(filename, payload)
   -- Clean up all garbage to avoid pollution of free.
   collectgarbage()
 
@@ -52,6 +52,25 @@ local function generate_output(filename)
   assert(res, err)
 end
 
+local function generate_parsed_output(filename, payload)
+  local res, err = pcall(generate_output, filename, payload)
+
+  -- Want to cleanup carefully if something went wrong.
+  if not res then
+    os.remove(filename)
+    error(err)
+  end
+
+  local reader = bufread.new(filename)
+  local symbols = symtab.parse(reader)
+  local events = memprof.parse(reader)
+
+  -- We don't need it any more.
+  os.remove(filename)
+
+  return symbols, events
+end
+
 local function fill_ev_type(events, symbols, event_type)
   local ev_type = {}
   for _, event in pairs(events[event_type]) do
@@ -107,20 +126,7 @@ test:ok(res == nil and err:match("profiler is not running"))
 test:ok(type(errno) == "number")
 
 -- Test profiler output and parse.
-res, err = pcall(generate_output, TMP_BINFILE)
-
--- Want to cleanup carefully if something went wrong.
-if not res then
-  os.remove(TMP_BINFILE)
-  error(err)
-end
-
-local reader = bufread.new(TMP_BINFILE)
-local symbols = symtab.parse(reader)
-local events = memprof.parse(reader, symbols)
-
--- We don't need it any more.
-os.remove(TMP_BINFILE)
+local symbols, events = generate_parsed_output(TMP_BINFILE, default_payload)
 
 local alloc = fill_ev_type(events, symbols, "alloc")
 local free = fill_ev_type(events, symbols, "free")
@@ -166,5 +172,15 @@ local co = coroutine.create(f)
 coroutine.resume(co)
 misc.memprof.stop()
 
+-- Test for marking jit-related allocations as internal.
+-- See also https://github.com/tarantool/tarantool/issues/5679.
 jit.on()
+symbols, events = generate_parsed_output(TMP_BINFILE, function()
+  for _ = 1, 100 do
+    local _ = {_, _}
+  end
+end)
+alloc = fill_ev_type(events, symbols, "alloc")
+test:ok(alloc[0] == nil)
+
 os.exit(test:check() and 0 or 1)
-- 
2.32.0


             reply	other threads:[~2021-07-22 11:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 11:46 Mikhail Shishatskiy via Tarantool-patches [this message]
2021-07-27 13:59 ` Sergey Kaplun via Tarantool-patches
2021-07-29  6:24   ` Mikhail Shishatskiy via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210722114617.194747-1-m.shishatskiy@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=m.shishatskiy@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit v1] memprof: report all JIT-related allocations as internal' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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