Re: [Tarantool-patches] [PATCH luajit] tools: make memprof parser output user-friendly

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergos!

Thanks for the review!

On 29.03.21, Sergey Ostanevich wrote:
> 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:

Yep, I agree. We should count the length of each section:
* filename
* number of events
* allocated bytes
* freed bytes

I suppose, we should create a new one "good first issue" to someone to
onboard in Lua and LuaJIT in particular. Thoughts?

> 
> long/path/to/funct.lua:	3 events
> very/long/path/to/funct.lua:	3 events
> 
> 
> All of these are too much ‘vkusovschina’, so you have my LGTM.
> 
> Sergos
> 
> 
> 
> > On 29 Mar 2021, at 16:00, Sergey Kaplun <skaplun@tarantool.org <mailto: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
> 

-- 
Best regards,
Sergey Kaplun