Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples

[Taeung Song]

Arnaldo,

I think we had a little communication problem.
So would you mind if I finally sum up this situation ?

Currently there are the two calculation setting for 'number of samples'.
(At util/annotate.c, not TUI code ui/browsers/annotate.c)

  A) disasm__calc_percent() -> util/annotate.c:881~
  B) symbol__get_source_line() -> util/annotate.c:1634~

If testing them as the three cases,

  1) $ perf annotate --stdio -l --show-total-period
  2) $ perf annotate --stdio --show-total-period
  3) $ perf annotate # using 'k' on TUI

The result is,

  1) NG
  2) OK
  3) OK

The problem of '1)' is to just show '0' zero values for number of samples.
Because 'B)' has a bug missing 'nr' of source_line_samples.
So I fix it.

  the case '1)' uses 'B)' and 'A)'
  the case '2)' uses only 'A)'
  the case '3)' uses only 'A)'

I read and analyzed all about 'number of samples' of perf-anntoate.
with the cset 0c4a5bcea460 you gave me.
This is my conclusion. What do you think about it ?


Thanks,
Taeung


NOTE: 'A)'(i.e. disasm__calc_percent()) has two alternative logic
when using '-l' option or not as below.

 888         if (src_line) {
                     ...
 903         } else {
                     ...
 914         }


On 03/29/2017 01:15 AM, Taeung Song wrote:
> On 03/29/2017 12:28 AM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
> > > On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
> > > > > The option 'show-total-period' works fine without a option '-l'.
> > > > > But if running 'perf annotate --stdio -l --show-total-period',
> > > > > you can see a problem showing only zero '0' for number of samples.
> > > >
> > > > If you do that at that point, don't you have to change the TUI not
> > > > to do
> > > > it again?
> > > >
> > > > - Arnaldo
> > >
> > > Yes, there isn't the same problem when using 't' on TUI of 'perf
> > > annotate'.
> > > (As you know, 't' is show-total-period option)
> >
> > Yeah, there is no problem on the TUI, but since you do it in a function
> > taht the TUI uses, it may not be needed to do this in
>
> No, TUI don't use the function I modified in this patch at all.
> (i.e. symbol__get_source_line())
>
> Only symbol__tty_annotate() use symbol__get_source_line().
>
> (As you know, annotate TUI use symbol__tui_annotate() )
>
> > tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
> > right?
> >
> > - Arnaldo
>
>
> Are there something I missed or misunderstand ?
>
> Thanks,
> Taeung

SNIP