Re: [PATCH v4 3/9] perf anntoate browser: Fix the toggle total period view showing number of samples

[Taeung Song <treeze.taeung@gmail.com>]

On 07/28/2017 01:57 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jul 28, 2017 at 01:16:02AM +0900, Taeung Song escreveu:
>> Currently the toggle total period view on the annotate TUI
>> shows the number of samples, not period like below.
>> So fix the toggle total period view on the annotate TUI like below.
>>
>>    $ perf annotate --show-total-period
>>
>> Before:
>>
>>         │    Disassembly of section .text:
>>         │
>>         │    0000000000109a90 <_mcount@@GLIBC_2.2.5>:
>>         │      sub    $0x38,%rsp
>>       3 │      mov    %rax,(%rsp)
>>       3 │      mov    %rcx,0x8(%rsp)
>>
>> After:
>>
>>             │    Disassembly of section .text:
>>             │
>>             │    0000000000109a90 <_mcount@@GLIBC_2.2.5>:
>>             │      sub    $0x38,%rsp
>>     2204022 │      mov    %rax,(%rsp)
>>     2207405 │      mov    %rcx,0x8(%rsp)
>>
>> Reported-by: Namhyung Kim <namhyung@kernel.org>
>> Cc: Milian Wolff <milian.wolff@kdab.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>   tools/perf/ui/browsers/annotate.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
>> index dbe4e63..87b0395 100644
>> --- a/tools/perf/ui/browsers/annotate.c
>> +++ b/tools/perf/ui/browsers/annotate.c
>> @@ -18,7 +18,7 @@
>>   
>>   struct disasm_line_samples {
>>   	double		percent;
>> -	u64		nr;
>> +	struct sym_hist_entry sample;
>>   };
>>   
>>   #define IPC_WIDTH 6
>> @@ -110,7 +110,9 @@ static int annotate_browser__set_jumps_percent_color(struct annotate_browser *br
>>   
>>   static int annotate_browser__pcnt_width(struct annotate_browser *ab)
>>   {
>> -	int w = 7 * ab->nr_events;
>> +	int w = annotate_browser__opts.show_total_period ? 11 : 7;
>> +
>> +	w *= ab->nr_events;
>>   
>>   	if (ab->have_cycles)
>>   		w += IPC_WIDTH + CYCLES_WIDTH;
>> @@ -151,8 +153,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].sample.period);
>>   			} else {
>>   				ui_browser__printf(browser, "%6.2f ",
>>   						   bdl->samples[i].percent);
>> @@ -162,7 +164,7 @@ 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);
>> +			ui_browser__write_nstring(browser, " ", pcnt_width);
> 
> This one had to be on a separate patch to clarify pcnt_width usage, that
> can't be used here because we may have cycles_width in it:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=ef03136d8df02b4aad314ad7c66c275f4df94225
> 
>>   		else
>>   			ui_browser__printf(browser, "%*s", 7, "Percent");
>>   	}
>> @@ -457,7 +459,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
>>   						pos->offset,
>>   						next ? next->offset : len,
>>   						&path, &sample);
>> -			bpos->samples[i].nr = sample.nr_samples;
>> +			bpos->samples[i].sample = sample;
> 
> this one is at:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=ac57d540fbdd7429300eafe5878cb06047904db0
> 
> Take a look at current acme/perf/core.
>   
> I'll take care of the rest, thanks.
> 
> - Arnaldo

I got it, I checked current acme/perf/core !

Thanks,
Taeung

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