From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Andi Kleen <andi@firstfloor.org>
Cc: Jin Yao <yao.jin@linux.intel.com>,
jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com,
alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org,
ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH] perf report: Tag branch type/flag on "to" and tag cycles on "from"
Date: Mon, 24 Jul 2017 14:39:53 -0300 [thread overview]
Message-ID: <20170724173953.GU4134@kernel.org> (raw)
In-Reply-To: <1500894547-18411-1-git-send-email-yao.jin@linux.intel.com>
Em Mon, Jul 24, 2017 at 07:09:07PM +0800, Jin Yao escreveu:
> Current --branch-history LBR annotation displays confused
> data. For example, each cycles report is duplicated on both
> "from" and "to" entries.
Andi, can you take a look at this? An Acked-by you or Reviewed-by would
be great to have,
- Arnaldo
> For example:
> perf report --branch-history --no-children --stdio
>
> --2.32%--main div.c:39 (COND_BWD CROSS_2M predicted:49.7% cycles:1)
> main div.c:44 (predicted:49.7% cycles:1)
> main div.c:42 (RET CROSS_2M cycles:2)
> compute_flag div.c:28 (cycles:2)
> compute_flag div.c:27 (RET CROSS_2M cycles:1)
> rand rand.c:28 (cycles:1)
> rand rand.c:28 (RET CROSS_2M cycles:1)
> __random random.c:298 (cycles:1)
> __random random.c:297 (COND_BWD CROSS_2M cycles:1)
> __random random.c:295 (cycles:1)
> __random random.c:295 (COND_BWD CROSS_2M cycles:1)
> __random random.c:295 (cycles:1)
> __random random.c:295 (RET CROSS_2M cycles:9)
>
> The cycles should be tagged only on the "from". It's for
> the code block that ends with "from", not for "to".
>
> Another issue is the "predicted:49.7%" is duplicated too
> (tag on both "from" and "to").
>
> This patch tags the branch type/flag on "to" and tag the
> cycles on "from".
>
> For example:
>
> --2.32%--main div.c:39 (COND_BWD CROSS_2M predicted:49.7%)
> main div.c:44 (cycles:1)
> main div.c:42 (RET CROSS_2M)
> compute_flag div.c:28 (cycles:2)
> compute_flag div.c:27 (RET CROSS_2M)
> rand rand.c:28 (cycles:1)
> rand rand.c:28 (RET CROSS_2M)
> __random random.c:298 (cycles:1)
> __random random.c:297 (COND_BWD CROSS_2M)
> __random random.c:295 (cycles:1)
> __random random.c:295 (COND_BWD CROSS_2M)
> __random random.c:295 (cycles:1)
> __random random.c:295 (RET CROSS_2M)
> |
> --2.23%--__random_r random_r.c:392 (cycles:9)
>
> In this example, The "main div.c:39 (COND_BWD CROSS_2M predicted:49.7%)"
> is "to" of branch and "main div.c:44 (cycles:1)" is "from" of branch.
> It should be easier for understanding than before.
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/util/branch.h | 11 ++--
> tools/perf/util/callchain.c | 148 +++++++++++++++++++++++++++++++-------------
> 2 files changed, 111 insertions(+), 48 deletions(-)
>
> diff --git a/tools/perf/util/branch.h b/tools/perf/util/branch.h
> index 686f2b6..1e3c7c5 100644
> --- a/tools/perf/util/branch.h
> +++ b/tools/perf/util/branch.h
> @@ -5,11 +5,12 @@
> #include "../perf.h"
>
> struct branch_type_stat {
> - u64 counts[PERF_BR_MAX];
> - u64 cond_fwd;
> - u64 cond_bwd;
> - u64 cross_4k;
> - u64 cross_2m;
> + bool branch_to;
> + u64 counts[PERF_BR_MAX];
> + u64 cond_fwd;
> + u64 cond_bwd;
> + u64 cross_4k;
> + u64 cross_2m;
> };
>
> struct branch_flags;
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 22d413a..8c17ea6 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -563,20 +563,33 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
> if (cursor_node->branch) {
> call->branch_count = 1;
>
> - if (cursor_node->branch_flags.predicted)
> - call->predicted_count = 1;
> -
> - if (cursor_node->branch_flags.abort)
> - call->abort_count = 1;
> -
> - call->cycles_count = cursor_node->branch_flags.cycles;
> - call->iter_count = cursor_node->nr_loop_iter;
> - call->samples_count = cursor_node->samples;
> -
> - branch_type_count(&call->brtype_stat,
> - &cursor_node->branch_flags,
> - cursor_node->branch_from,
> - cursor_node->ip);
> + if (cursor_node->branch_from) {
> + /*
> + * branch_from is set with value somewhere else
> + * to imply it's "to" of a branch.
> + */
> + call->brtype_stat.branch_to = true;
> +
> + if (cursor_node->branch_flags.predicted)
> + call->predicted_count = 1;
> +
> + if (cursor_node->branch_flags.abort)
> + call->abort_count = 1;
> +
> + branch_type_count(&call->brtype_stat,
> + &cursor_node->branch_flags,
> + cursor_node->branch_from,
> + cursor_node->ip);
> + } else {
> + /*
> + * It's "from" of a branch
> + */
> + call->brtype_stat.branch_to = false;
> + call->cycles_count =
> + cursor_node->branch_flags.cycles;
> + call->iter_count = cursor_node->nr_loop_iter;
> + call->samples_count = cursor_node->samples;
> + }
> }
>
> list_add_tail(&call->list, &node->val);
> @@ -685,20 +698,32 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> if (node->branch) {
> cnode->branch_count++;
>
> - if (node->branch_flags.predicted)
> - cnode->predicted_count++;
> -
> - if (node->branch_flags.abort)
> - cnode->abort_count++;
> -
> - cnode->cycles_count += node->branch_flags.cycles;
> - cnode->iter_count += node->nr_loop_iter;
> - cnode->samples_count += node->samples;
> -
> - branch_type_count(&cnode->brtype_stat,
> - &node->branch_flags,
> - node->branch_from,
> - node->ip);
> + if (node->branch_from) {
> + /*
> + * It's "to" of a branch
> + */
> + cnode->brtype_stat.branch_to = true;
> +
> + if (node->branch_flags.predicted)
> + cnode->predicted_count++;
> +
> + if (node->branch_flags.abort)
> + cnode->abort_count++;
> +
> + branch_type_count(&cnode->brtype_stat,
> + &node->branch_flags,
> + node->branch_from,
> + node->ip);
> + } else {
> + /*
> + * It's "from" of a branch
> + */
> + cnode->brtype_stat.branch_to = false;
> + cnode->cycles_count +=
> + node->branch_flags.cycles;
> + cnode->iter_count += node->nr_loop_iter;
> + cnode->samples_count += node->samples;
> + }
> }
>
> return MATCH_EQ;
> @@ -1235,27 +1260,26 @@ static int count_pri64_printf(int idx, const char *str, u64 value, char *bf, int
> return printed;
> }
>
> -static int count_float_printf(int idx, const char *str, float value, char *bf, int bfsize)
> +static int count_float_printf(int idx, const char *str, float value,
> + char *bf, int bfsize, float threshold)
> {
> int printed;
>
> + if (threshold != 0.0 && value < threshold)
> + return 0;
> +
> printed = scnprintf(bf, bfsize, "%s%s:%.1f%%", (idx) ? " " : " (", str, value);
>
> return printed;
> }
>
> -static int counts_str_build(char *bf, int bfsize,
> - u64 branch_count, u64 predicted_count,
> - u64 abort_count, u64 cycles_count,
> - u64 iter_count, u64 samples_count,
> - struct branch_type_stat *brtype_stat)
> +static int branch_to_str(char *bf, int bfsize,
> + u64 branch_count, u64 predicted_count,
> + u64 abort_count,
> + struct branch_type_stat *brtype_stat)
> {
> - u64 cycles;
> int printed, i = 0;
>
> - if (branch_count == 0)
> - return scnprintf(bf, bfsize, " (calltrace)");
> -
> printed = branch_type_str(brtype_stat, bf, bfsize);
> if (printed)
> i++;
> @@ -1263,15 +1287,29 @@ static int counts_str_build(char *bf, int bfsize,
> if (predicted_count < branch_count) {
> printed += count_float_printf(i++, "predicted",
> predicted_count * 100.0 / branch_count,
> - bf + printed, bfsize - printed);
> + bf + printed, bfsize - printed, 0.0);
> }
>
> if (abort_count) {
> printed += count_float_printf(i++, "abort",
> abort_count * 100.0 / branch_count,
> - bf + printed, bfsize - printed);
> + bf + printed, bfsize - printed, 0.1);
> }
>
> + if (i)
> + printed += scnprintf(bf + printed, bfsize - printed, ")");
> +
> + return printed;
> +}
> +
> +static int branch_from_str(char *bf, int bfsize,
> + u64 branch_count,
> + u64 cycles_count, u64 iter_count,
> + u64 samples_count)
> +{
> + int printed = 0, i = 0;
> + u64 cycles;
> +
> cycles = cycles_count / branch_count;
> if (cycles) {
> printed += count_pri64_printf(i++, "cycles",
> @@ -1286,10 +1324,34 @@ static int counts_str_build(char *bf, int bfsize,
> }
>
> if (i)
> - return scnprintf(bf + printed, bfsize - printed, ")");
> + printed += scnprintf(bf + printed, bfsize - printed, ")");
>
> - bf[0] = 0;
> - return 0;
> + return printed;
> +}
> +
> +static int counts_str_build(char *bf, int bfsize,
> + u64 branch_count, u64 predicted_count,
> + u64 abort_count, u64 cycles_count,
> + u64 iter_count, u64 samples_count,
> + struct branch_type_stat *brtype_stat)
> +{
> + int printed;
> +
> + if (branch_count == 0)
> + return scnprintf(bf, bfsize, " (calltrace)");
> +
> + if (brtype_stat->branch_to) {
> + printed = branch_to_str(bf, bfsize, branch_count,
> + predicted_count, abort_count, brtype_stat);
> + } else {
> + printed = branch_from_str(bf, bfsize, branch_count,
> + cycles_count, iter_count, samples_count);
> + }
> +
> + if (!printed)
> + bf[0] = 0;
> +
> + return printed;
> }
>
> static int callchain_counts_printf(FILE *fp, char *bf, int bfsize,
> --
> 2.7.4
next prev parent reply other threads:[~2017-07-24 17:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-24 11:09 [PATCH] perf report: Tag branch type/flag on "to" and tag cycles on "from" Jin Yao
2017-07-24 17:39 ` Arnaldo Carvalho de Melo [this message]
2017-07-24 23:45 ` Andi Kleen
2017-07-25 14:18 ` Arnaldo Carvalho de Melo
2017-07-26 17:26 ` [tip:perf/core] " tip-bot for Jin Yao
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=20170724173953.GU4134@kernel.org \
--to=acme@kernel.org \
--cc=Linux-kernel@vger.kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=andi@firstfloor.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
--cc=yao.jin@linux.intel.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.