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 v2] memprof: report JIT-side allocations as internal
Date: Thu, 29 Jul 2021 16:34:25 +0700	[thread overview]
Message-ID: <20210729093425.179158-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
CI: https://github.com/tarantool/tarantool/tree/shishqa/gh-5679-report-jit-allocations-as-internal

Changes in v2:
  - Fixed commit title to fit in width of 50 symbols;
  - Rebased to the branch [1];
  - Reused default_payload() as a payload to test patch behavior.

[1]: https://github.com/tarantool/luajit/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number

 src/lj_memprof.c                              | 28 ++++++++++++-------
 .../misclib-memprof-lapi.test.lua             | 16 ++++++++---
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 87512c3a..e4d819a0 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -108,19 +108,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 b7e456e1..96864c6a 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -12,7 +12,7 @@ utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
 local tap = require("tap")
 
 local test = tap.test("misc-memprof-lapi")
-test:plan(16)
+test:plan(17)
 
 jit.off()
 jit.flush()
@@ -81,7 +81,7 @@ local function fill_ev_type(events, symbols, event_type)
   for _, event in pairs(events[event_type]) do
     local addr = event.loc.addr
     local traceno = event.loc.traceno
-    if traceno ~= 0 then
+    if traceno ~= 0 and symbols.trace[traceno] then
       local trace_loc = symbols.trace[traceno]
       addr = trace_loc.addr
       ev_type[trace_loc.line] = {
@@ -214,9 +214,17 @@ misc.memprof.stop()
 -- Test profiler with enabled JIT.
 jit.on()
 
--- Pregenerate traces to get expected output:
-default_payload()
+-- On this run traces are generated, JIT-related allocations
+-- will be recorded as well.
+symbols, events = generate_parsed_output(default_payload)
+
+alloc = fill_ev_type(events, symbols, "alloc")
+
+-- Test for marking JIT-related allocations as internal.
+-- See also https://github.com/tarantool/tarantool/issues/5679.
+test:ok(alloc[0] == nil)
 
+-- Run already generated traces.
 symbols, events = generate_parsed_output(default_payload)
 
 alloc = fill_ev_type(events, symbols, "alloc")
-- 
2.32.0


             reply	other threads:[~2021-07-29  9:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29  9:34 Mikhail Shishatskiy via Tarantool-patches [this message]
2021-08-14 11:24 ` Sergey Kaplun via Tarantool-patches
2021-08-21  4:43   ` Mikhail Shishatskiy via Tarantool-patches
2021-11-24 13:01     ` 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=20210729093425.179158-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 v2] memprof: report JIT-side 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