Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit v1] memprof: report all JIT-related allocations as internal
@ 2021-07-22 11:46 Mikhail Shishatskiy via Tarantool-patches
  2021-07-27 13:59 ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 3+ messages in thread
From: Mikhail Shishatskiy via Tarantool-patches @ 2021-07-22 11:46 UTC (permalink / raw)
  To: tarantool-patches, imun, skaplun

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


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

end of thread, other threads:[~2021-07-29  6:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 11:46 [Tarantool-patches] [PATCH luajit v1] memprof: report all JIT-related allocations as internal Mikhail Shishatskiy via Tarantool-patches
2021-07-27 13:59 ` Sergey Kaplun via Tarantool-patches
2021-07-29  6:24   ` Mikhail Shishatskiy via Tarantool-patches

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git