Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit v2] memprof: report JIT-side allocations as internal
@ 2021-07-29  9:34 Mikhail Shishatskiy via Tarantool-patches
  2021-08-14 11:24 ` Sergey Kaplun via Tarantool-patches
  2022-02-02 17:29 ` Igor Munkin via Tarantool-patches
  0 siblings, 2 replies; 6+ messages in thread
From: Mikhail Shishatskiy via Tarantool-patches @ 2021-07-29  9:34 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
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


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-02 17:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29  9:34 [Tarantool-patches] [PATCH luajit v2] memprof: report JIT-side allocations as internal Mikhail Shishatskiy via Tarantool-patches
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
2022-02-02 17:26     ` Igor Munkin via Tarantool-patches
2022-02-02 17:29 ` Igor Munkin 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