Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752274AbdGEGIR (ORCPT <rfc822;w@1wt.eu>);
        Wed, 5 Jul 2017 02:08:17 -0400
Received: from mail-wm0-f48.google.com ([74.125.82.48]:36451 "EHLO
        mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750885AbdGEGIQ (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 5 Jul 2017 02:08:16 -0400
MIME-Version: 1.0
In-Reply-To: <a7a611a6-1544-ae69-9068-0c65ca6313cf@gmail.com>
References: <a7a611a6-1544-ae69-9068-0c65ca6313cf@gmail.com>
From: Namhyung Kim <namhyung@kernel.org>
Date: Wed, 5 Jul 2017 15:07:55 +0900
X-Google-Sender-Auth: ax6QEV1i2SajIL-XpnZdjtXN4ZA
Message-ID: <CAM9d7cjoBNK3bhchz9yKkE2OOE4OBO-J3a8jaGTN7PXHPFZM6Q@mail.gmail.com>
Subject: Re: [QUESTION] perf annotate: how do we fix the 'total period'
 feature ?
To: Taeung Song <treeze.taeung@gmail.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>, Jiri Olsa <jolsa@kernel.org>,
        Milian Wolff <milian.wolff@kdab.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2223
Lines: 68

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