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 03E9E6EC60; Mon, 29 Mar 2021 18:41:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 03E9E6EC60 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617032518; bh=hMB1jqPc2lPB55rOHABEeRhF1xG1IgxHpSTYdzdpCew=; h=Date:In-Reply-To:To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=WZNRiW8SFY7glLQmdkka5zumaD9DFeG3+hdJD/DEwVaXVxAcu5jzCa3C3Ldr+1WyZ xzk4wUWDk33GwK2yENyrlCDw9kutDeRxv1tYgUhABlBUG9QcQxng/CEbdx961zO41h kqHB4z8LfghLT6gq1VcH51z8+BCt4u0h1OJqeNuE= Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (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 E83026EC60 for ; Mon, 29 Mar 2021 18:41:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E83026EC60 Received: by smtp35.i.mail.ru with esmtpa (envelope-from ) id 1lQu1c-0005Fi-6Q; Mon, 29 Mar 2021 18:41:56 +0300 Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_FF60BA1C-4EF0-4DCF-AF14-E9B6C83154C6" Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.60.0.2.21\)) Date: Mon, 29 Mar 2021 18:41:54 +0300 In-Reply-To: To: Sergey Kaplun References: <20210326164855.30242-1-skaplun@tarantool.org> <20210326203356.GJ29703@tarantool.org> <20210329111613.GN29703@tarantool.org> X-Mailer: Apple Mail (2.3654.60.0.2.21) X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD9ED7173E37F4E32947427BE79D20CABD4F844F00D7AFD5C3C182A05F538085040A9C4A719DE4739177D5BE04612EE20DDD22DE1566CD397D74B0543FBEB3BB8A5 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70A10A23A3B64B805EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F907CB39E8CA2E228638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C7428A34725AB662DE7D0415649FA495B30EFE5552CD1EF70A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE7328B01A8D746D8839FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C317B107DEF921CE79117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CFF7ED4D6813B5CA0476E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8B25CF6EDB7BBA35B93AA81AA40904B5D9DBF02ECDB25306B2201CA6A4E26CD07C3BBE47FD9DD3FB595F5C1EE8F4F765FCA83251EDC214901ED5E8D9A59859A8B682BBBAF5DF00056E089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24A6D60772A99906F8E1CD14B953EB46D1EA52997C18619C5355D89D7DBCDD132 X-C1DE0DAB: 0D63561A33F958A5C2FE3151289BD192028E1293C85BC48C5C6D6904EA402F24D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34697E0FA301E282154B2D8087EE56F08E3559246C9D81CA0A9F03E496370101A7C5902E4B397CFEC71D7E09C32AA3244C495EB622E2798CDE41E066D71343FDB23FD9C8CA1B0515E0FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojljIiQOC84rQK0CLSnXCGtw== X-Mailru-Sender: 455D65AE3A139168626D8C76E86D3AC0455CD0091E6C40457B6FB8436D37D5F3DDAC019A3742A8B676D79013C85012CDC77752E0C033A69E4BBE7EBD99111A499D0AB74157175C036C18EFA0BB12DBB0 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] tools: make memprof parser output user-friendly 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" --Apple-Mail=_FF60BA1C-4EF0-4DCF-AF14-E9B6C83154C6 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi! Thank you for the patch and review! Let me propose some technical fix: long/path/to/function.lua: 3 events very/long/path/to/function.lua: 3 events I think dump as above is not-so-readable for humans. A single-symbol = fixup gives=20 long/path/to/function.lua: 3 events very/long/path/to/function.lua: 3 events + print(string.format("%s:\t%d events\t+%d bytes\t-%d bytes", The better solution would be to keep the longest name and append/prepend = all of them with spaces, so that the following will not happen: long/path/to/funct.lua: 3 events very/long/path/to/funct.lua: 3 events All of these are too much =E2=80=98vkusovschina=E2=80=99, so you have my = LGTM. Sergos > On 29 Mar 2021, at 16:00, Sergey Kaplun > wrote: >=20 > Igor, >=20 > On 29.03.21, Igor Munkin wrote: >> Sergey, >>=20 >> Thanks for the fixes! I'll push it to the trunk as soon as Sergos = gives >> his LGTM. >>=20 >> On 29.03.21, Sergey Kaplun wrote: >>> Igor, >>>=20 >>> Thanks for the review! >>>=20 >>=20 >> >>=20 >>>=20 >>> Missed ChangeLog entry, feel free to change it at your pleasure: >>=20 >> Great, thanks! I propose the following wording: >=20 > Thanks! I like it, exept some typos: >=20 >>=20 >> | ##feature/luajit >> | >> | * Make LuaJIT memory profiler parser output more user-friendly = (gh-5811). >> | Now the source line definition where the event occurs is much = clearer: >> | only source file name and allocation-related line are presented, >=20 > Typo? s/are presented/is presented/ >=20 >> | function definition line number is omitted. Moreover, = event-related >=20 > Typo: /function definition line number/the function definition line = number/. >=20 >> | statistics are indicated with units. >> | **Breaking change**: Line info of the line function definition is >> | saved in symbol info table by field `linedefined` now and field = `name` >> | is renamed to `source` with the respect to Lua Debug API. >=20 > Typo: s/to Lua Debug/to the Lua Debug/ >=20 >>=20 >=20 > >=20 >>>>> --=20 >>>>> 2.31.0 >>>>>=20 >>>>=20 >>>> --=20 >>>> Best regards, >>>> IM >>>=20 >>> --=20 >>> Best regards, >>> Sergey Kaplun >>=20 >> --=20 >> Best regards, >> IM >=20 > --=20 > Best regards, > Sergey Kaplun --Apple-Mail=_FF60BA1C-4EF0-4DCF-AF14-E9B6C83154C6 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
Hi!

Thank you for the patch and = review!

Let me = propose some technical fix:

long/path/to/function.lua: 3 = events
very/long/path/to/function.lua: 3 = events

I think dump as above is not-so-readable for humans. A = single-symbol fixup gives 

long/path/to/function.lua: 3 = events
very/long/path/to/function.lua: 3 = events

+ =    print(string.format("%s:\t%d events\t+%d bytes\t-%d = bytes",

The better solution would be to keep = the longest name and append/prepend all of them
with = spaces, so that the following will not happen:

long/path/to/funct.lua: 3 events
very/long/path/to/funct.lua: 3 events


All = of these are too much =E2=80=98vkusovschina=E2=80=99, so you have my = LGTM.

Sergos



On 29 Mar 2021, at 16:00, Sergey Kaplun <skaplun@tarantool.org> wrote:

Igor,

On 29.03.21, Igor Munkin wrote:
Sergey,

Thanks for the fixes! I'll push it to the trunk as soon as = Sergos gives
his LGTM.

On = 29.03.21, Sergey Kaplun wrote:
Igor,

Thanks for the review!


<snipped>


Missed ChangeLog entry, feel free to change it at your = pleasure:

Great, thanks! I = propose the following wording:

Thanks! I = like it, exept some typos:


| ##feature/luajit
|
| * Make LuaJIT memory profiler parser output = more user-friendly (gh-5811).
|   Now the source = line definition where the event occurs is much clearer:
| =   only source file name and allocation-related line are = presented,

Typo? s/are presented/is presented/

| =   function definition line number is omitted. Moreover, = event-related

Typo: /function definition line number/the function = definition line number/.

|   statistics are = indicated with units.
|   **Breaking change**: = Line info of the line function definition is
| =   saved in symbol info table by field `linedefined` now and = field `name`
|   is renamed to `source` with the = respect to Lua Debug API.

Typo: s/to = Lua Debug/to the Lua Debug/



<snipped>

-- 
2.31.0


-- 
Best regards,
IM

-- 
Best regards,
Sergey Kaplun

-- 
Best = regards,
IM

-- 
Best = regards,
Sergey = Kaplun

= --Apple-Mail=_FF60BA1C-4EF0-4DCF-AF14-E9B6C83154C6--