[Tarantool-patches] [PATCH luajit] tools: make memprof parser output user-friendly
Sergey Kaplun
skaplun at tarantool.org
Mon Mar 29 18:56:01 MSK 2021
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 at tarantool.org <mailto:skaplun at 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
More information about the Tarantool-patches
mailing list