Re: [Tarantool-patches] [PATCH luajit v4 1/3] sysprof: remove `split by vmstate` option

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Igor!
Thanks for the review!
See my answers below.

On Mon, Aug 21, 2023 at 01:05:54PM +0000, Igor Munkin wrote:
> Max,
> 
> Thanks for the fixes! LGTM, but still one nit regarding the wording.
> 
> On 21.08.23, Maksim Kokryashkin wrote:
> > From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
> > 
> > The option was introduced early into the sysprof's
> > development to provide capabilities for per-VM-state
> > callchain visualization. However, during the sysprof's
> > recent active in-house usage period, it became clear
> > that performance issues associated with specific VM
> > states (such as GC, for example) have a distinct look
> > on a flamegraph itself, thanks to their name prefixes
> > and their surrounding context.
> 
> Minor: Besides, one can still use counters returned by <sysprof.report>,
> right? If so, it would be nice to mention this in reasoning too.
Right, added it.
Branch is force-pushed.
Here is the new commit message:
| sysprof: remove `split by vmstate` option
|
| The option was introduced early into the sysprof's
| development to provide capabilities for per-VM-state
| callchain visualization. However, during the sysprof's
| recent active in-house usage period, it became clear
| that performance issues associated with specific VM
| states (such as GC, for example) have a distinct look
| on a flamegraph itself, thanks to their name prefixes
| and their surrounding context. Furthermore, one can
| still use counters returned by `sysprof.report` to get
| insight into per-VM-state performance.
|
| This option is unneeded and was never used
| by anybody. There is no reason to maintain it
| in the scope of refactoring.
|
| Needed for tarantool/tarantool#8700
> 
> > 
> > This option is unneeded and was never used
> > by anybody. There is no reason to maintain it
> > in the scope of refactoring.
> > 
> > Needed for tarantool/tarantool#8700
> > ---
> >  tools/sysprof.lua          |  9 +--------
> >  tools/sysprof/collapse.lua | 10 +++-------
> >  2 files changed, 4 insertions(+), 15 deletions(-)
> > 
> 
> <snipped>
> 
> > -- 
> > 2.39.2 (Apple Git-143)
> > 
> 
> -- 
> Best regards,
> IM