From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 251A36EC5C; Fri, 29 Jan 2021 18:17:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 251A36EC5C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1611933469; bh=rwMzdffpw7fjjeVNrQzasVxpUzLxJCBzKLnzrDwPUGs=; h=In-Reply-To:Date:References:To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=cPDkyCp75z2pkTp9iCOYT3PJPHmRswlXNIQjRG8eGOwom5TWh+MP3YqIWLX0e0m5J gE0AnVfRs+EGq6UVkYwjLkYHTgoGoLmfBE3OGDi/FrgjObdfk7IvXofixfZ1J3Y8cy TQpTk3Cgy0Hi/GPB9oIJxj0F+R7xjJLwYE7UEG0Q= Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BE05C6EC5C for ; Fri, 29 Jan 2021 18:17:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BE05C6EC5C Received: by smtp17.mail.ru with esmtpa (envelope-from ) id 1l5VWs-0001Ei-Op; Fri, 29 Jan 2021 18:17:47 +0300 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.40.0.2.32\)) In-Reply-To: <20210125193544.18050-1-skaplun@tarantool.org> Date: Fri, 29 Jan 2021 18:17:45 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20210125193544.18050-1-skaplun@tarantool.org> To: Sergey Kaplun X-Mailer: Apple Mail (2.3654.40.0.2.32) X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD953AC099BC0052A9C67057FEA5FC82C6F3EDA0BC0B7FA0DC2182A05F53808504059736B415873B7B2E57AF7CB68F16CAE3DA947617788301C08341C2D18D814D7 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE776377A057133B646EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637328FC23D015AFE6E8638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FCC9502DD27318FC34317801E7C7F1C0B1624DCA21123FF7DE389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6D082881546D93491CC7F00164DA146DA6F5DAA56C3B73B23C77107234E2CFBA567F23339F89546C55F5C1EE8F4F765FCB9CEE4F2B4A90F8475ECD9A6C639B01BBD4B6F7A4D31EC0BC0CAF46E325F83A522CA9DD8327EE4930A3850AC1BE2E735262FEC7FBD7D1F5BB5C8C57E37DE458B4C7702A67D5C3316FA3894348FB808DBCF17F1EDFBC1FB573B503F486389A921A5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5BEAE776CA7CB1C00F6FA22C7AE8F9B3E17E70A5EA4BB4A38D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75F04B387B5D7535DE410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D340A4C04F5DECA7EE9E0DC704DAB02F691DF3CA62B8F90DBE01768CBAF09A10881C12809A44F88F8FE1D7E09C32AA3244CFC5969F881E9EB40528194587C437B098894E9C85370243EFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj0axADxPFAF+UFseOsFU9LA== X-Mailru-Sender: 3B9A0136629DC912F4AABCEFC589C81E28BF5AC3D85717199AE63EFBC2696603A40AD7AC6ECEA9DFAD07DD1419AC565FA614486B47F28B67C5E079CCF3B0523AED31B7EB2E253A9E112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v1] tools: introduce --leak-only memprof parser option X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Ostanevich via Tarantool-patches Reply-To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi!=20 Thanks for the patch, some comments below. Regards, Sergos > On 25 Jan 2021, at 22:35, Sergey Kaplun wrote: >=20 > This patch adds a new --leak-only memory profiler parser option. > When the parser runs with that option, it will report only lines > (or "INTERNAL") that allocate and do not free some amount of bytes. > The parser also reports the number of allocation and deallocation > events related to each line. > --- >=20 > Branch: = https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-memprof-memle= aks-option > Testing branch: = https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-memprof-me= mleaks-option >=20 > Side note: I only update commit message thats why CI from the "old" > commit. > CI: https://gitlab.com/tarantool/tarantool/-/pipelines/246541599 >=20 > This patch is a result of offline discussion of memory profiler > and its usage and feedback from Mons. >=20 > I don't know - is it reasonable to reference [1] issue here or create = a > new one? I think it=E2=80=99s ok to mention [1] >=20 > The example of output. Assuming we have file : > | 1 jit.off() -- more verbose reports > | 2 > | 3 local function concat(a) > | 4 local nstr =3D a.."a" > | 5 return nstr > | 6 end > | 7 > | 8 local function format(a) > | 9 local nstr =3D string.format("%sa", a) > | 10 return nstr > | 11 end > | 12 > | 13 collectgarbage() -- cleanup > | 14 > | 15 local binfile =3D = "/tmp/memprof_"..(arg[0]):match("/([^/]*).lua")..".bin" > | 16 > | 17 local st, err =3D misc.memprof.start(binfile) > | 18 assert(st, err) > | 19 > | 20 for i =3D 1, 10000 do > | 21 local f =3D format(i) > | 22 local c =3D concat(i) > | 23 end > | 24 > | 25 local st, err =3D misc.memprof.stop() > | 26 assert(st, err) > | 27 > | 28 os.exit() >=20 > Parser's output without option: > | ALLOCATIONS > | @/home/burii/reports/demo_memprof/format_concat.lua:8, line 9: 19998 = 624322 0 > |=20 > | REALLOCATIONS > |=20 > | DEALLOCATIONS > | INTERNAL: 283 0 5602 > | Overrides: > | @/home/burii/reports/demo_memprof/format_concat.lua:8, = line 9 > |=20 > | @/home/burii/reports/demo_memprof/format_concat.lua:8, line 9: 2 = 0 98304 > | Overrides: > | @/home/burii/reports/demo_memprof/format_concat.lua:8, = line 9 >=20 > And with: >=20 > | HEAP SUMMARY: > | @/home/burii/reports/demo_memprof/format_concat.lua:8, line 9 holds = 553214 bytes 19998 allocs, 283 frees >=20 > Side note: The attentive reader will notice that we have 283+2 frees = in > the first case and 283 in the second. > This is because --leak-only considers deallocation/reallocation events > for which we know the source address. The default report aggregates > these all deallocations into one. See `link_to_previous()` in > for details. >=20 > tools/memprof.lua | 23 +++++----- > tools/memprof/humanize.lua | 91 +++++++++++++++++++++++++++++++++++++- > tools/memprof/parse.lua | 20 +++++++-- > 3 files changed, 115 insertions(+), 19 deletions(-) >=20 > diff --git a/tools/memprof.lua b/tools/memprof.lua > index 9f96208..5d09fbd 100644 > --- a/tools/memprof.lua > +++ b/tools/memprof.lua > @@ -37,6 +37,11 @@ Supported options are: > os.exit(0) > end >=20 > +local leak_only =3D false > +opt_map["leak-only"] =3D function() Should you add this into the SYNOPSIS part? > + leak_only =3D true > +end > + > -- Print error and exit with error status. > local function opterror(...) > stderr:write("luajit-parse-memprof.lua: ERROR: ", ...) > @@ -94,19 +99,11 @@ local function dump(inputfile) > local reader =3D bufread.new(inputfile) > local symbols =3D symtab.parse(reader) > local events =3D memprof.parse(reader, symbols) > - > - stdout:write("ALLOCATIONS", "\n") > - view.render(events.alloc, symbols) > - stdout:write("\n") > - > - stdout:write("REALLOCATIONS", "\n") > - view.render(events.realloc, symbols) > - stdout:write("\n") > - > - stdout:write("DEALLOCATIONS", "\n") > - view.render(events.free, symbols) > - stdout:write("\n") > - Should the HEAP SUMMARY appear in the end of the =E2=80=98all=E2=80=99 = report? Otherwise the =E2=80=98_only=E2=80=99 in the option name is mesleading: = show part of=20 the report, while it shows completely different one. Same for the code below. > + if leak_only then > + view.leak_only(events, symbols) > + else > + view.all(events, symbols) > + end > os.exit(0) > end >=20 > diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua > index 109a39d..bad0597 100644 > --- a/tools/memprof/humanize.lua > +++ b/tools/memprof/humanize.lua > @@ -28,8 +28,8 @@ function M.render(events, symbols) > )) >=20 > local prim_loc =3D {} > - for _, loc in pairs(event.primary) do > - table.insert(prim_loc, symtab.demangle(symbols, loc)) > + for _, heap_chunk in pairs(event.primary) do > + table.insert(prim_loc, symtab.demangle(symbols, = heap_chunk.loc)) The order of allocations is changed, was it incorrect in some way? > end > if #prim_loc ~=3D 0 then > table.sort(prim_loc) > @@ -42,4 +42,91 @@ function M.render(events, symbols) > end > end >=20 > +function M.all(events, symbols) > + print("ALLOCATIONS") > + M.render(events.alloc, symbols) > + print("") > + > + print("REALLOCATIONS") > + M.render(events.realloc, symbols) > + print("") > + > + print("DEALLOCATIONS") > + M.render(events.free, symbols) > + print("") > +end > + > +function M.leak_only(events, symbols) > + -- Auto resurrects source event lines for counting/reporting. > + local heap =3D setmetatable({}, {__index =3D function(t, line) > + t[line] =3D { > + size =3D 0, > + cnt_alloc =3D 0, > + cnt_free =3D 0, > + } > + return t[line] > + end}) > + > + for _, event in pairs(events.alloc) do > + if event.loc then > + local ev_line =3D symtab.demangle(symbols, event.loc) > + > + heap[ev_line].size =3D heap[ev_line].size + event.alloc > + if (event.alloc > 0) then > + heap[ev_line].cnt_alloc =3D heap[ev_line].cnt_alloc + = event.num > + end > + end > + end > + > + -- Realloc and free events are pretty the same. > + -- We don't interesting in aggregated alloc/free sizes for aren=E2=80=99t interested > + -- the event, but only for new and old size values inside > + -- alloc-realloc-free chain. Assuming that we have > + -- no collisions between different object addresses. I believe the address is source of the chunk {_|de|re}allocation, is = there any different one? > + local function process_non_alloc_events(events_by_type) > + for _, event in pairs(events_by_type) do > + -- Realloc and free events always have "primary" key > + -- that references table with rewrited memory > + -- (may be empty). > + for _, heap_chunk in pairs(event.primary) do > + local ev_line =3D symtab.demangle(symbols, heap_chunk.loc) > + > + heap[ev_line].size =3D heap[ev_line].size + = heap_chunk.alloced > + if (heap_chunk.alloced > 0) then Can it be negative? If not - the above line can be put under this if = also. > + heap[ev_line].cnt_alloc =3D heap[ev_line].cnt_alloc + = heap_chunk.cnt > + end > + > + heap[ev_line].size =3D heap[ev_line].size - heap_chunk.freed > + if (heap_chunk.freed > 0) then > + heap[ev_line].cnt_free =3D heap[ev_line].cnt_free + = heap_chunk.cnt > + end > + end > + end > + end > + process_non_alloc_events(events.realloc) > + process_non_alloc_events(events.free) > + > + local rest_heap =3D {} > + for line, info in pairs(heap) do > + -- Report "INTERNAL" events inconsistencies for profiling > + -- with enabled jit. > + if info.size > 0 then > + table.insert(rest_heap, {line =3D line, hold_bytes =3D = info.size}) > + end > + end > + > + table.sort(rest_heap, function(h1, h2) > + return h1.hold_bytes > h2.hold_bytes > + end) > + > + print("HEAP SUMMARY:") > + for _, h in pairs(rest_heap) do > + print(string.format( > + "%s holds %d bytes %d allocs, %d frees", > + h.line, h.hold_bytes, heap[h.line].cnt_alloc, = heap[h.line].cnt_free > + )) > + end > + print("") > +end > + > return M > diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua > index 6dae22d..374686c 100644 > --- a/tools/memprof/parse.lua > +++ b/tools/memprof/parse.lua > @@ -39,11 +39,23 @@ local function new_event(loc) > } > end >=20 > -local function link_to_previous(heap_chunk, e) > +local function link_to_previous(heap_chunk, e, nsize) > -- Memory at this chunk was allocated before we start tracking. > if heap_chunk then > -- Save Lua code location (line) by address (id). > - e.primary[heap_chunk[2]] =3D heap_chunk[3] > + if e.primary[heap_chunk[2]] =3D=3D nil then > + e.primary[heap_chunk[2]] =3D { > + loc =3D heap_chunk[3], > + alloced =3D 0, > + freed =3D 0, > + cnt =3D 0, > + } > + end > + -- Save memory diff heap information. > + local location_data =3D e.primary[heap_chunk[2]] > + location_data.alloced =3D location_data.alloced + nsize > + location_data.freed =3D location_data.freed + heap_chunk[1] > + location_data.cnt =3D location_data.cnt + 1 > end > end >=20 > @@ -97,7 +109,7 @@ local function parse_realloc(reader, asource, = events, heap) > e.free =3D e.free + osize > e.alloc =3D e.alloc + nsize >=20 > - link_to_previous(heap[oaddr], e) > + link_to_previous(heap[oaddr], e, nsize) >=20 > heap[oaddr] =3D nil > heap[naddr] =3D {nsize, id, loc} > @@ -116,7 +128,7 @@ local function parse_free(reader, asource, events, = heap) > e.num =3D e.num + 1 > e.free =3D e.free + osize >=20 > - link_to_previous(heap[oaddr], e) > + link_to_previous(heap[oaddr], e, 0) >=20 > heap[oaddr] =3D nil > end > --=20 > 2.28.0 >=20 > [1]: https://github.com/tarantool/tarantool/issues/5657