From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Sergey Kaplun <skaplun@tarantool.org>, Maxim Kokryashkin <m.kokryashkin@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump` Date: Tue, 16 May 2023 13:55:47 +0300 [thread overview] Message-ID: <4959c9e3-e0e3-7d26-1a7f-836849cecf4c@tarantool.org> (raw) In-Reply-To: <44354d682d3cabf5a3ab1a268f900d7e6f981020.1683720396.git.skaplun@tarantool.org> Hello, Sergey Thanks for the patch! See my three comments inline. On 5/10/23 15:34, Sergey Kaplun wrote: > This commit adds simple parser for traces to be analyzed in the test. > For now nothing too fancy at all -- just start `jit.dump` to temporary > file and parse and remove this file when dump is stopped. The array > with resulting traces is returned. > > For now, just find a single IR by pattern and return ref number and IR, > if exists. More functionality may be added if it will be necessary for > tests. > --- > test/tarantool-tests/utils/jit_parse.lua | 156 +++++++++++++++++++++++ > 1 file changed, 156 insertions(+) > create mode 100644 test/tarantool-tests/utils/jit_parse.lua > > diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua > new file mode 100644 > index 00000000..fe8e0e08 > --- /dev/null > +++ b/test/tarantool-tests/utils/jit_parse.lua The main purpose of this script is using in tests. It could be dangerous to leave it without tests at all, breakage in it could require time for debugging someday. From other side I believe no one wants writing tests for test infrastructure. I suggest add at least a couple of unit tests that will parse trace in plain text format and check that results are equal to expected. > @@ -0,0 +1,156 @@ > +local jdump = require('jit.dump') > + > +local M = {} > + > +local trace_mt = {} > +trace_mt.__index = trace_mt > + > +-- Find and return the first IR ref and value matched the pattern > +-- for the trace. > +trace_mt.has_ir = function(trace, ir_pattern) > + for i = 1, #trace.ir do > + local ir = trace.ir[i] > + if ir:match(ir_pattern) then > + return i, ir > + end > + end > +end > + > +local function trace_new(n) > + return setmetatable({ > + number = n, > + parent = nil, > + parent_exitno = nil, > + start_loc = nil, > + bc = {}, > + ir = {}, > + mcode = {}, > + snaps = {}, > + }, trace_mt) > +end > + > +local function parse_bc(trace, line) > + trace.bc[#trace.bc + 1] = line > +end > + > +local function parse_snap(trace, n_snap, line) > + assert(trace.snaps[n_snap] == nil) > + trace[n_snap] = {ref = #trace.ir + 1, slots = line:match('%[(.*)%]$'),} > +end > + > +local function parse_ir(trace, line) > + local n_snap = line:match('SNAP%s+#(%d+)') > + if n_snap then > + parse_snap(trace, n_snap, line) > + return > + end > + > + local current_ins, ir = line:match('^(%d+)%s+(.*)$') > + current_ins = tonumber(current_ins) > + -- Insert NOP instruction hidden in IR dump. > + if current_ins ~= #trace.ir + 1 then > + trace.ir[#trace.ir + 1] = 'nil NOP' > + end > + assert(current_ins == #trace.ir + 1) > + trace.ir[current_ins] = ir > +end > + > +local function parse_mcode(trace, line) > + -- Skip loop label. > + if line == '-> LOOP:' then > + return > + end > + local addr, instruction = line:match('^(%w+)%s+(.*)$') > + trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,} > +end > + > +local function parse_trace_hdr(ctx, line, trace_num, status) > + local traces = ctx.traces > + if status == 'start' then > + local trace = trace_new(trace_num) > + trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)') > + -- XXX: Assume, that source line can't contain spaces. > + -- For example, it's not "(command line)". > + trace.start_loc = line:match(' ([^%s]+)$') > + traces[trace_num] = trace > + ctx.parsing_trace = trace_num > + ctx.parsing = 'bc' > + elseif status == 'stop' then > + assert(ctx.parsing_trace == trace_num) > + ctx.parsing_trace = nil > + ctx.parsing = nil > + elseif status == 'abort' then > + assert(ctx.parsing_trace == trace_num) > + ctx.parsing_trace = nil > + ctx.parsing = nil > + traces[trace_num] = nil > + elseif status == 'IR' then > + ctx.parsing = 'IR' > + elseif status == 'mcode' then > + ctx.parsing = 'mcode' > + else > + error('Unknown trace status: ' .. status) > + end > +end > + > +local function parse_line(ctx, line) > + if line == '' then > + return > + end > + > + if line:match('TRACE flush') then > + ctx.traces = {} > + return > + end > + > + local trace_num, status = line:match('TRACE (%d+) (%w+)') > + if trace_num then > + parse_trace_hdr(ctx, line, tonumber(trace_num), status) > + return > + end > + > + assert(ctx.parsing_trace) > + > + local trace = ctx.traces[ctx.parsing_trace] > + if ctx.parsing == 'bc' then > + parse_bc(trace, line) > + elseif ctx.parsing == 'IR' then > + parse_ir(trace, line) > + elseif ctx.parsing == 'mcode' then > + parse_mcode(trace, line) > + end > +end > + > +local JDUMP_FILE > + > +local function parse_jit_dump() > + local ctx = {traces = {}} > + for line in io.lines(JDUMP_FILE) do > + parse_line(ctx, line) > + end > + return ctx.traces > +end > + > +M.start = function(flags) Comment with description of function's arguments will be useful here and M.finish(). For internal functions too, but it is up to you. > + assert(JDUMP_FILE == nil, 'jit_parse is already running') > + -- Always use plain text output. > + flags = flags .. 'T' > + JDUMP_FILE = os.tmpname() > + jdump.start(flags, JDUMP_FILE) > +end > + > +M.finish = function() > + assert(JDUMP_FILE ~= nil, 'jit_parse is not running') > + jdump.off() > + local traces = parse_jit_dump() > + os.remove(JDUMP_FILE) > + JDUMP_FILE = nil > + return traces > +end > + > +-- Turn off compilation for these modules to avoid side effects. > +jit.off(true, true) > +jit.off(jdump.on, true) > +jit.off(jdump.off, true) I would put these three lines on top of file or even disable/enable compilation in M.start() and M.finish(). Without enabling compilation back you are mutating environment on using this script and this can be unexpected for those who will use scripts. > + > +return M
next prev parent reply other threads:[~2023-05-16 10:55 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-10 12:34 [Tarantool-patches] [PATCH luajit 0/2] Fix canonicalization of +-0.0 keys for IR_NEWREF Sergey Kaplun via Tarantool-patches 2023-05-10 12:34 ` [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump` Sergey Kaplun via Tarantool-patches 2023-05-15 11:11 ` Maxim Kokryashkin via Tarantool-patches 2023-05-15 12:00 ` Maxim Kokryashkin via Tarantool-patches 2023-05-21 7:47 ` Sergey Kaplun via Tarantool-patches 2023-05-21 7:39 ` Sergey Kaplun via Tarantool-patches 2023-05-22 7:04 ` Sergey Kaplun via Tarantool-patches 2023-05-29 13:55 ` Maxim Kokryashkin via Tarantool-patches 2023-05-16 10:55 ` Sergey Bronnikov via Tarantool-patches [this message] 2023-05-22 7:02 ` Sergey Kaplun via Tarantool-patches 2023-05-22 9:14 ` Sergey Kaplun via Tarantool-patches 2023-05-10 12:34 ` [Tarantool-patches] [PATCH luajit 2/2] Fix canonicalization of +-0.0 keys for IR_NEWREF Sergey Kaplun via Tarantool-patches 2023-05-15 12:05 ` Maxim Kokryashkin via Tarantool-patches 2023-05-20 15:03 ` Sergey Kaplun via Tarantool-patches 2023-05-16 12:17 ` Sergey Bronnikov via Tarantool-patches 2023-05-20 14:54 ` Sergey Kaplun via Tarantool-patches 2023-05-22 7:55 ` Sergey Bronnikov via Tarantool-patches 2023-06-27 13:28 ` [Tarantool-patches] [PATCH luajit 1/3] test: split utils.lua into several modules Igor Munkin via Tarantool-patches 2023-06-27 13:35 ` Igor Munkin via Tarantool-patches 2023-06-28 11:36 ` Sergey Kaplun via Tarantool-patches 2023-06-28 16:07 ` Igor Munkin via Tarantool-patches 2023-07-04 17:10 ` [Tarantool-patches] [PATCH luajit 0/2] Fix canonicalization of +-0.0 keys for IR_NEWREF Igor Munkin 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=4959c9e3-e0e3-7d26-1a7f-836849cecf4c@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=m.kokryashkin@tarantool.org \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`' \ /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