All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Taeung Song <treeze.taeung@gmail.com>
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: Wed, 12 Jul 2017 17:10:57 -0300	[thread overview]
Message-ID: <20170712201057.GR27350@kernel.org> (raw)
In-Reply-To: <1499811250-9310-1-git-send-email-treeze.taeung@gmail.com>

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)

>  };
>  
>  #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-12 20:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-11 22:14 [PATCH 2/4] perf anntoate browser: Fix the toggle total period view to show period, not number of samples Taeung Song
2017-07-12 20:10 ` Arnaldo Carvalho de Melo [this message]
2017-07-13  7:25   ` Taeung Song

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=20170712201057.GR27350@kernel.org \
    --to=acme@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=milian.wolff@kdab.com \
    --cc=namhyung@kernel.org \
    --cc=treeze.taeung@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.