Re: [QUESTION] perf annotate: how do we fix the 'total period' feature ?

[Taeung Song]

Hi Namhyung :)

On 07/05/2017 03:07 PM, Namhyung Kim wrote:
> Hi Taeung,
>
> On Wed, Jul 5, 2017 at 2:47 PM, Taeung Song <treeze.taeung@xxxxxxxxx> wrote:
> > Hi,
> >
> > Two problems of perf-annotate were mentioned in recent PATCH reviews
> > by Milian and Namhyung.
> >
> > Currently perf-annotate has a '--show-total-period' option
> > and a 't' key "Toggle total period view" on TUI browser.
>
> Hmm... I didn't notice it has the option.  I think its name is
> incorrect and should be --show-nr-samples in accordance with perf
> report.

Okey, I got it.

> > However, they actually show the number of samples, not period(Raw number of
> > event count of sample).
> > So it's a different number to the perf report like below.
> >
> > For example,
> >
> >    $ perf report --stdio --show-nr-sample --show-total-period -S hex2u64
> > ...
> > # Overhead       Samples        Period  Command  Shared Object
> > # ........  ............  ............  .......  .............
> > #
> >       3.07%            36      26484668  perf     perf
> >
> >
> >    $ perf annotate  --stdio --show-total-period -s hex2u64
> >   Percent |      Source code & Disassembly of perf for cycles:ppp (36
> > samples)
> > -----------------------------------------------------------------------------
> >           :
> >           :
> >           :
> >           :      Disassembly of section .text:
> >           :
> >           :      000000000053ef9e <hex2u64>:
> >           :      hex2u64():
> >         0 :        53ef9e:       push   %rbp
> >         0 :        53ef9f:       mov    %rsp,%rbp
> >         0 :        53efa2:       sub    $0x30,%rsp
> >         1 :        53efa6:       callq  424810
> > <pthread_attr_setdetachstate@plt+0x10>
> >         0 :        53efab:       mov    %rdi,-0x28(%rbp)
> >         2 :        53efaf:       mov    %rsi,-0x30(%rbp)
> > ...
> >
> > Problems:
> >    1) the total period of perf-annotate is different from perf-report's
> >    2) perf-annotate only shows the first column as 'Percent'
> >       (even though the number of samples for each addr are actually printed)
> >
> > So we need to just rename it ? ('total period' -> 'samples')
> >
> > Or,
> > we should enable perf-annotate to support both features for 'periods' and
> > 'samples' ?
>
> I think we should support both.  Then you need to change the code to
> save periods when processing samples and show them in the annotate
> IMHO.
>
> Thanks,
> Namhyung

Currently perf-annotate only count the number of samples for each addr
in __symbol__inc_addr_samples() of util/annotate.c when processing samples.
But it seems to make new functions for 'periods'..

Okey, I got hints of how to fix this problems.
will send the PATCH for them ! :)

Thanks,
Taeung