2017-07-11 22:14:23

by Taeung Song

Subject: [PATCH 2/4] perf anntoate browser: Fix the toggle total period view to show period, not number of samples

Currently the toggle total period view on the annotate TUI
shows the number of samples, not period like below.

$ perf annotate --show-total-period

Percent│
│
│
│ Disassembly of section .text:
│
│ 0000000000109a90 <_mcount@@GLIBC_2.2.5>:
│ sub $0x38,%rsp
3 │ mov %rax,(%rsp)
3 │ mov %rcx,0x8(%rsp)

This output has two problem:

1) Wrong column i.e. 'Percent'
2) Show number of samples, not period

So fix the toggle total period view on the annotate TUI like below.

Event count│
│
│
│ Disassembly of section .text:
│
│ 0000000000109a90 <_mcount@@GLIBC_2.2.5>:
│ sub $0x38,%rsp
2204022 │ mov %rax,(%rsp)
2207405 │ mov %rcx,0x8(%rsp)

Cc: Namhyung Kim <[email protected]>
Cc: Milian Wolff <[email protected]>
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/ui/browsers/annotate.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 73e5ae2..0ddc3b2 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -18,6 +18,7 @@
struct disasm_line_samples {
double percent;
u64 nr;
+ u64 period;
};

#define IPC_WIDTH 6
@@ -113,6 +114,10 @@ static int annotate_browser__pcnt_width(struct annotate_browser *ab)

if (ab->have_cycles)
w += IPC_WIDTH + CYCLES_WIDTH;
+
+ if (annotate_browser__opts.show_total_period)
+ w += 4 * ab->nr_events;
+
return w;
}

@@ -150,8 +155,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
bdl->samples[i].percent,
current_entry);
if (annotate_browser__opts.show_total_period) {
- ui_browser__printf(browser, "%6" PRIu64 " ",
- bdl->samples[i].nr);
+ ui_browser__printf(browser, "%10" PRIu64 " ",
+ bdl->samples[i].period);
} else {
ui_browser__printf(browser, "%6.2f ",
bdl->samples[i].percent);
@@ -161,9 +166,13 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
ui_browser__set_percent_color(browser, 0, current_entry);

if (!show_title)
- ui_browser__write_nstring(browser, " ", 7 * ab->nr_events);
- else
- ui_browser__printf(browser, "%*s", 7, "Percent");
+ ui_browser__write_nstring(browser, " ", pcnt_width);
+ else {
+ if (annotate_browser__opts.show_total_period)
+ ui_browser__printf(browser, "%*s", 11, "Event count");
+ else
+ ui_browser__printf(browser, "%*s", 7, "Percent");
+ }
}
if (ab->have_cycles) {
if (dl->ipc)
@@ -457,6 +466,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
next ? next->offset : len,
&path, &nr_samples, &period);
bpos->samples[i].nr = nr_samples;
+ bpos->samples[i].period = period;

if (max_percent < bpos->samples[i].percent)
max_percent = bpos->samples[i].percent;
--
2.7.4

2017-07-12 20:11:02

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH 2/4] perf anntoate browser: Fix the toggle total period view to show period, not number of samples

Em Wed, Jul 12, 2017 at 07:14:10AM +0900, Taeung Song escreveu:
> Currently the toggle total period view on the annotate TUI
> shows the number of samples, not period like below.
>
> $ perf annotate --show-total-period
>
> Percent│
> │
> │
> │ Disassembly of section .text:
> │
> │ 0000000000109a90 <_mcount@@GLIBC_2.2.5>:
> │ sub $0x38,%rsp
> 3 │ mov %rax,(%rsp)
> 3 │ mov %rcx,0x8(%rsp)
>
> This output has two problem:
>
> 1) Wrong column i.e. 'Percent'
> 2) Show number of samples, not period
>
> So fix the toggle total period view on the annotate TUI like below.
>
> Event count│
> │
> │
> │ Disassembly of section .text:
> │
> │ 0000000000109a90 <_mcount@@GLIBC_2.2.5>:
> │ sub $0x38,%rsp
> 2204022 │ mov %rax,(%rsp)
> 2207405 │ mov %rcx,0x8(%rsp)
>
> Cc: Namhyung Kim <[email protected]>
> Cc: Milian Wolff <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/ui/browsers/annotate.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 73e5ae2..0ddc3b2 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -18,6 +18,7 @@
> struct disasm_line_samples {
> double percent;
> u64 nr;
> + u64 period;

So now this becomes a sym_hist_entry? (nr, period)

> };
>
> #define IPC_WIDTH 6
> @@ -113,6 +114,10 @@ static int annotate_browser__pcnt_width(struct annotate_browser *ab)
>
> if (ab->have_cycles)
> w += IPC_WIDTH + CYCLES_WIDTH;
> +
> + if (annotate_browser__opts.show_total_period)
> + w += 4 * ab->nr_events;
> +
> return w;
> }
>
> @@ -150,8 +155,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
> bdl->samples[i].percent,
> current_entry);
> if (annotate_browser__opts.show_total_period) {
> - ui_browser__printf(browser, "%6" PRIu64 " ",
> - bdl->samples[i].nr);
> + ui_browser__printf(browser, "%10" PRIu64 " ",
> + bdl->samples[i].period);
> } else {
> ui_browser__printf(browser, "%6.2f ",
> bdl->samples[i].percent);
> @@ -161,9 +166,13 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
> ui_browser__set_percent_color(browser, 0, current_entry);
>
> if (!show_title)
> - ui_browser__write_nstring(browser, " ", 7 * ab->nr_events);
> - else
> - ui_browser__printf(browser, "%*s", 7, "Percent");
> + ui_browser__write_nstring(browser, " ", pcnt_width);
> + else {
> + if (annotate_browser__opts.show_total_period)
> + ui_browser__printf(browser, "%*s", 11, "Event count");
> + else
> + ui_browser__printf(browser, "%*s", 7, "Percent");
> + }
> }
> if (ab->have_cycles) {
> if (dl->ipc)
> @@ -457,6 +466,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> next ? next->offset : len,
> &path, &nr_samples, &period);
> bpos->samples[i].nr = nr_samples;
> + bpos->samples[i].period = period;
>
> if (max_percent < bpos->samples[i].percent)
> max_percent = bpos->samples[i].percent;
> --
> 2.7.4

2017-07-13 07:25:47

by Taeung Song

Subject: Re: [PATCH 2/4] perf anntoate browser: Fix the toggle total period view to show period, not number of samples

On 07/13/2017 05:10 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 12, 2017 at 07:14:10AM +0900, Taeung Song escreveu:
>> Currently the toggle total period view on the annotate TUI
>> shows the number of samples, not period like below.
>>
>> $ perf annotate --show-total-period
>>
>> Percent│
>> │
>> │
>> │ Disassembly of section .text:
>> │
>> │ 0000000000109a90 <_mcount@@GLIBC_2.2.5>:
>> │ sub $0x38,%rsp
>> 3 │ mov %rax,(%rsp)
>> 3 │ mov %rcx,0x8(%rsp)
>>
>> This output has two problem:
>>
>> 1) Wrong column i.e. 'Percent'
>> 2) Show number of samples, not period
>>
>> So fix the toggle total period view on the annotate TUI like below.
>>
>> Event count│
>> │
>> │
>> │ Disassembly of section .text:
>> │
>> │ 0000000000109a90 <_mcount@@GLIBC_2.2.5>:
>> │ sub $0x38,%rsp
>> 2204022 │ mov %rax,(%rsp)
>> 2207405 │ mov %rcx,0x8(%rsp)
>>
>> Cc: Namhyung Kim <[email protected]>
>> Cc: Milian Wolff <[email protected]>
>> Cc: Jiri Olsa <[email protected]>
>> Signed-off-by: Taeung Song <[email protected]>
>> ---
>> tools/perf/ui/browsers/annotate.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
>> index 73e5ae2..0ddc3b2 100644
>> --- a/tools/perf/ui/browsers/annotate.c
>> +++ b/tools/perf/ui/browsers/annotate.c
>> @@ -18,6 +18,7 @@
>> struct disasm_line_samples {
>> double percent;
>> u64 nr;
>> + u64 period;
>
> So now this becomes a sym_hist_entry? (nr, period)
>

Yep!

>> };
>>
>> #define IPC_WIDTH 6
>> @@ -113,6 +114,10 @@ static int annotate_browser__pcnt_width(struct annotate_browser *ab)
>>
>> if (ab->have_cycles)
>> w += IPC_WIDTH + CYCLES_WIDTH;
>> +
>> + if (annotate_browser__opts.show_total_period)
>> + w += 4 * ab->nr_events;
>> +
>> return w;
>> }
>>
>> @@ -150,8 +155,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>> bdl->samples[i].percent,
>> current_entry);
>> if (annotate_browser__opts.show_total_period) {
>> - ui_browser__printf(browser, "%6" PRIu64 " ",
>> - bdl->samples[i].nr);
>> + ui_browser__printf(browser, "%10" PRIu64 " ",
>> + bdl->samples[i].period);
>> } else {
>> ui_browser__printf(browser, "%6.2f ",
>> bdl->samples[i].percent);
>> @@ -161,9 +166,13 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>> ui_browser__set_percent_color(browser, 0, current_entry);
>>
>> if (!show_title)
>> - ui_browser__write_nstring(browser, " ", 7 * ab->nr_events);
>> - else
>> - ui_browser__printf(browser, "%*s", 7, "Percent");
>> + ui_browser__write_nstring(browser, " ", pcnt_width);
>> + else {
>> + if (annotate_browser__opts.show_total_period)
>> + ui_browser__printf(browser, "%*s", 11, "Event count");
>> + else
>> + ui_browser__printf(browser, "%*s", 7, "Percent");
>> + }
>> }
>> if (ab->have_cycles) {
>> if (dl->ipc)
>> @@ -457,6 +466,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
>> next ? next->offset : len,
>> &path, &nr_samples, &period);
>> bpos->samples[i].nr = nr_samples;
>> + bpos->samples[i].period = period;
>>
>> if (max_percent < bpos->samples[i].percent)
>> max_percent = bpos->samples[i].percent;
>> --
>> 2.7.4