Re: [Tarantool-patches] [PATCH luajit v2] memprof: report JIT-side allocations as internal

[Mikhail Shishatskiy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Rebased to the updated gh-5814 [1] branch.

@ChangeLog
======================================================================
##feature/luajit

* Now memory profiler reports JIT-related allocations, which are not
   connected to a particular trace, as INTERNAL.
======================================================================

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

On 21.08.2021 11:43, Mikhail Shishatskiy via Tarantool-patches wrote:
>Hi, Sergey!
>Thanks for the review!
>
>On 14.08.2021 14:24, Sergey Kaplun wrote:
>>Hi, thanks for the fixes!
>>
>>Please, move test adjustment mentioned below to the patch for the symtab
>>with group allocations by trace's number.
>>Otherwise, LGTM.
>>
>>On 29.07.21, Mikhail Shishatskiy wrote:
>>>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
>>
>><snipped>
>>
>>>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
>>
>>This should be moved to the commit, that changes symtab, this change
>>looks unrelated to this patch, but relates to previous one [1].
>
>Fixed in the patchset [1] and rebased to the branch [2]
>
>>
>>>local trace_loc = symbols.trace[traceno]
>>>addr = trace_loc.addr
>>>ev_type[trace_loc.line] = {
>>>@@ -214,9 +214,17 @@ misc.memprof.stop()
>>
>><snipped>
>>
>>>--
>>>2.32.0
>>>
>>
>>[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2021-July/024944.html
>>
>>--
>>Best regards,
>>Sergey Kaplun
>
>Also, I slightly changed the test by deleting the check for deallocations
>number because now trace generation happens for memprof payload, too, and
>we can meet untrivial JIT-related deallocations in the second run.
>
>---
>diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>index f84b6df0..17bcb2f1 100644
>--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
>+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>@@ -231,19 +231,25 @@ test:test("jit-output", function(subtest)
>    jit.opt.start(3, "hotloop=10")
>    jit.flush()
>- -- Pregenerate traces to fill symtab entries in the next run.
>- default_payload()
>-
>+ -- On this run traces are generated, JIT-related allocations
>+ -- will be recorded as well.
>    local symbols, events = generate_parsed_output(default_payload)
>    local alloc = fill_ev_type(events, symbols, "alloc")
>- local free = fill_ev_type(events, symbols, "free")
>+
>+ -- Test for marking JIT-related allocations as internal.
>+ -- See also https://github.com/tarantool/tarantool/issues/5679.
>+ subtest:ok(alloc[0] == nil)
>+
>+ -- Run already generated traces.
>+ symbols, events = generate_parsed_output(default_payload)
>+
>+ alloc = fill_ev_type(events, symbols, "alloc")
>    -- We expect, that loop will be compiled into a trace.
>    subtest:ok(check_alloc_report(alloc, 1, 37, 32, 20))
>    -- See same checks with jit.off().
>    subtest:ok(check_alloc_report(alloc, 0, 34, 32, 2))
>- subtest:ok(free.INTERNAL.num == 22)
>    -- Restore default JIT settings.
>    jit.opt.start(unpack(jit_opt_default))
>--
>2.32.0
>
>[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2021-August/025680.html
>[2]: https://github.com/tarantool/luajit/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number
>
>Issue: https://github.com/tarantool/tarantool/issues/5679
>Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5679-report-jit-allocations-as-internal
>Tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5679-report-jit-allocations-as-internal
>
>Best regards,
>Mikhail Shishatskiy