Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1681291 > unrolled thread

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

Started byTaeung Song <treeze.taeung@gmail.com>
First post2017-07-05 07:50 +0200
Last post2017-07-05 09:00 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [QUESTION] perf annotate: how do we fix the 'total period' feature ? Taeung Song <treeze.taeung@gmail.com> - 2017-07-05 07:50 +0200
    Re: [QUESTION] perf annotate: how do we fix the 'total period'  feature ? Namhyung Kim <namhyung@kernel.org> - 2017-07-05 08:10 +0200
      Re: [QUESTION] perf annotate: how do we fix the 'total period'  feature ? Taeung Song <treeze.taeung@gmail.com> - 2017-07-05 09:00 +0200

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

FromTaeung Song <treeze.taeung@gmail.com>
Date2017-07-05 07:50 +0200
Subject[QUESTION] perf annotate: how do we fix the 'total period' feature ?
Message-ID<tZLmi-27Q-11@gated-at.bofh.it>
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.

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' ?


Thanks,
Taeung

[toc] | [next] | [standalone]


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

FromNamhyung Kim <namhyung@kernel.org>
Date2017-07-05 08:10 +0200
SubjectRe: [QUESTION] perf annotate: how do we fix the 'total period' feature ?
Message-ID<tZLFE-2ti-13@gated-at.bofh.it>
In reply to#1681291
Hi Taeung,

On Wed, Jul 5, 2017 at 2:47 PM, Taeung Song <treeze.taeung@gmail.com> 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.


>
> 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

[toc] | [prev] | [next] | [standalone]


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

FromTaeung Song <treeze.taeung@gmail.com>
Date2017-07-05 09:00 +0200
SubjectRe: [QUESTION] perf annotate: how do we fix the 'total period' feature ?
Message-ID<tZMs2-2Ms-9@gated-at.bofh.it>
In reply to#1681298
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@gmail.com> 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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web