mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Taeung Song <treeze.taeung@gmail.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-kernel@vger.kernel.org, Namhyung Kim <namhyung@kernel.org>,
	Milian Wolff <milian.wolff@kdab.com>,
	Jiri Olsa <jolsa@redhat.com>
Subject: Re: [PATCH 2/4] perf anntoate browser: Fix the toggle total period view to show period, not number of samples
Date: Thu, 13 Jul 2017 16:25:42 +0900	[thread overview]
Message-ID: <9b73b50a-126e-fd1c-7937-63bb60da96a1@gmail.com> (raw)
In-Reply-To: <20170712201057.GR27350@kernel.org>



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

      reply	other threads:[~2017-07-13  7:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-11 22:14 Taeung Song
2017-07-12 20:10 ` Arnaldo Carvalho de Melo
2017-07-13  7:25   ` Taeung Song [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9b73b50a-126e-fd1c-7937-63bb60da96a1@gmail.com \
    --to=treeze.taeung@gmail.com \
    --cc=acme@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=milian.wolff@kdab.com \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome