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
next 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