lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8076707.4PSiRuChLW@milian-kdab2>
Date:   Thu, 04 May 2017 11:01:48 +0200
From:   Milian Wolff <milian.wolff@...b.com>
To:     Jin Yao <yao.jin@...ux.intel.com>
Cc:     acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
        mingo@...hat.com, alexander.shishkin@...ux.intel.com,
        Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
        kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser

On Thursday, May 4, 2017 4:58:15 PM CEST Jin Yao wrote:
> The annotate browser is divided into 2 frames. Left frame
> contains 3 columns (some platforms only have one column).
> 
> For example:
> 
>                    │26  int compute_flag()
>                    │27  {
>  22.80  1.20       │      sub    $0x8,%rsp
>                    │25          int i;
>                    │
>                    │27          i = rand() % 2;
>  22.78  1.20     1 │    → callq  rand@plt
> 
> While it's hard for user to understand what the data is.
> 
> This patch adds the titles "Percent", "IPC" and "Cycle"
> on columns.
> 
> Percnt   IPC Cycle │
>                    │25  __attribute__((noinline))
>                    │26  int compute_flag()
>                    │27  {
>  22.80  1.20       │      sub    $0x8,%rsp
>                    │25          int i;
>                    │
>                    │27          i = rand() % 2;
>  22.78  1.20     1 │    → callq  rand@plt
> 
> The titles are displayed at row 0 of annotate browser if row 0
> doesn't have values of percent, ipc and cycle.

Functionality wise a really good improvement - thanks! But personally I find 
the abbreviation of one character (i.e. "Percnt" instead of "Percent") not so 
nice. If space really is an issue here, use "%"?

Also note though that it's unclear what this percentage actually is. I guess 
it's a sample percentage? Maybe a header should be added that explains these 
values to newbies. I bet many people won't even know what IPC is either.

Cheers

-- 
Milian Wolff | milian.wolff@...b.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ