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 A594B716A75; Tue, 5 Dec 2023 12:23:41 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A594B716A75 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1701768221; bh=hqmGbKi2XFRsMfOrNRlvqByq8G8MnPo6dn86FdkWHLw=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=VrenZGK3lzS6N+bE1V4iyitg+T+adzJTmu5OINCFUcQ4XhrxZ4dieaVlX/mMoGn8a UTglBOin7vaEpqaazbpHZXkKhsaRLyUWphKRU6SFuhHEud5nypBEiUU4Sid27BVmca 5yRGBktF35Qqdma0mSlRyWNUQXqODrqfx+gAmHgw= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [95.163.41.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id ADEFC716A45 for ; Tue, 5 Dec 2023 12:23:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ADEFC716A45 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1rAReV-00AmBC-2c; Tue, 05 Dec 2023 12:23:40 +0300 Date: Tue, 5 Dec 2023 12:19:07 +0300 To: Maxim Kokryashkin Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD91486559A3ABF84E787E59C3E7C1E978B5A8C83476450FBF4182A05F538085040F7235A709CB83539DAAA3DF217326D8740CB6AD406C82187ECA43B69D8FED398 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE73C714006C69EB7BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AA32F0A5ADCF96E68638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D85A454A065BE7C41848C85A632B7CBCEE117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC60CDF180582EB8FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269176DF2183F8FC7C0406C186E56A1B26068655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7C440208D3F8D8ED8643847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5FDC86769C6B368B555863CD3B8609554A53FAAFAEFEB1E04F87CCE6106E1FC07E67D4AC08A07B9B04B3849D6E5CCBAFDBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF2ADC79C701552DEB5403F82F93E0EDFD484EEC536EEBB74D01A135E59E958CAE60B027A3EDF90BC19157FEB932BFB23D6320ECBFA0ADF3161E68A76D77195BF0A74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojJEmydpeiNq+Qq9kZW58zig== X-DA7885C5: C234DABCAFD006162652FB1CD43C7BCA3C9B158D59CFD97AC8455DA933263099262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F7393590D8C940224AE33A6F3872272C0B3863E3DEC595AFD82A10FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/4] memprof: introduce the `--human-readable` 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maxim! Thanks for the patch! Generally LGTM, after fixing my nits below. On 04.12.23, Maxim Kokryashkin wrote: > Prior to this patch, memprof could report only the raw > amount of bytes, which is hard to read. This patch adds the > `--human-readable` CLI option to the memprof parser, so the > memory is reported in KiB, MiB, or GiB, depending on what's > the biggest reasonable unit. > > This patch also refactors the options mechanism for parser, > so all of the options are passed into parser's modules as a > single config table instead of being handled individually. > > Part of tarantool/tarantool#5994 > --- > .../gh-5994-memprof-human-readable.test.lua | 72 +++++++++++++++++++ > tools/memprof.lua | 21 ++++-- > tools/memprof/humanize.lua | 46 +++++++++--- > 3 files changed, 122 insertions(+), 17 deletions(-) > create mode 100644 test/tarantool-tests/gh-5994-memprof-human-readable.test.lua > > diff --git a/test/tarantool-tests/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/gh-5994-memprof-human-readable.test.lua > new file mode 100644 > index 00000000..0a579cec > --- /dev/null > +++ b/test/tarantool-tests/gh-5994-memprof-human-readable.test.lua > @@ -0,0 +1,72 @@ > +local tap = require('tap') > +local test = tap.test('gh-5994-memprof-human-readable'):skipcond({ > + ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and > + jit.arch ~= 'x64', > + ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux', > + -- XXX: Tarantool integration is required to run this test properly. > + -- luacheck: no global > + ['No profile tools CLI option integration'] = _TARANTOOL, > +}) > +test:plan(5) We can use `#TEST_SET` here instead of `5`. > + diff --git a/tools/memprof.lua b/tools/memprof.lua > index e23bbf24..acadbc17 100644 > --- a/tools/memprof.lua > +++ b/tools/memprof.lua > -local leak_only = false > opt_map["leak-only"] = function() > - leak_only = true > + config.leak_only = true > +end > + > +opt_map["human-readable"] = function() > + config.human_readable = true > end It's good to add this option in the help ouput, too. > > -- Print error and exit with error status. > @@ -101,11 +110,11 @@ local function dump(inputfile) > local reader = bufread.new(inputfile) > local symbols = symtab.parse(reader) > local events = memprof.parse(reader, symbols) > - if not leak_only then > - view.profile_info(events, symbols) > + if not config.leak_only then > + view.profile_info(events, config) > end > - local dheap = process.form_heap_delta(events, symbols) I understand that this is minor change, but it's better to add this clenup via the separate commit. > - view.leak_info(dheap) > + local dheap = process.form_heap_delta(events) > + view.leak_info(dheap, config) > view.aliases(symbols) > -- XXX: The second argument is required to properly close Lua > -- universe (i.e. invoke before exiting). > diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua > index 5b289ce3..a5fdabf7 100644 > --- a/tools/memprof/humanize.lua > +++ b/tools/memprof/humanize.lua > @@ -5,7 +5,29 @@ > > local M = {} > > -function M.render(events) > +local function human_readable_bytes(bytes) > + local units = {"B", "KiB", "MiB", "GiB"} > + local i = 1 Minor: maybe it's better to use `magnitude` (or something like) instead of just `i`. > + > + while bytes >= 1024 and i < #units do > + bytes = bytes / 1024 > + i = i + 1 > + end > + local is_int = math.floor(bytes) == bytes > + local fmt = is_int and "%d%s" or "%.2f%s" > + > + return string.format(fmt, bytes, units[i]) > +end > + > +local function format_bytes(bytes, config) > + if config.human_readable then > + return human_readable_bytes(bytes) > + else > + return string.format('%dB', bytes) > + end > +end > + > +function M.render(events, config) Side note: Maybe it's better to give just flag (is_human_readable) here instead of the whole config, but I'm not sure... Feel free to ignore. > -- > 2.43.0 > -- Best regards, Sergey Kaplun