linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/9] perf diff: diff cycles at basic block level
@ 2019-05-20 13:27 Jin Yao
  2019-05-20 13:27 ` [PATCH v1 1/9] perf util: Create block_info structure Jin Yao
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

In some cases small changes in hot loops can show big differences.
But it's difficult to identify these differences.

perf diff currently can only diff symbols (functions). We can also expand
it to diff cycles of individual programs blocks as reported by timed LBR.
This would allow to identify changes in specific code accurately.

With this patch set, for example,

    perf record -b ./div
    perf record -b ./div
    perf diff --basic-block

     # Cycles diff  Basic block (start:end)
     # ...........  .......................
     #
              -20   native_write_msr (7fff9a069900:7fff9a06990b)
               -3   __indirect_thunk_start (7fff9ac02ca0:7fff9ac02ca0)
                1   __indirect_thunk_start (7fff9ac02cac:7fff9ac02cb0)
                0   rand@plt (490:490)
                0   rand (3af60:3af64)
                0   rand (3af69:3af6d)
                0   main (4e8:4ea)
                0   main (4ef:500)
                0   main (4ef:535)
                0   compute_flag (640:644)
                0   compute_flag (649:659)
                0   __random_r (3ac40:3ac76)
                0   __random_r (3ac40:3ac88)
                0   __random_r (3ac90:3ac9c)
                0   __random (3aac0:3aad2)
                0   __random (3aae0:3aae7)
                0   __random (3ab03:3ab0f)
                0   __random (3ab14:3ab1b)
                0   __random (3ab28:3ab2e)
                0   __random (3ab4a:3ab53)

The "basic-block" is a new perf-diff option, which enables the displaying
of cycles difference of same program basic block amongst two or more
perf.data. The program basic block is the code block between two branches
in a function.

Jin Yao (9):
  perf util: Create block_info structure
  perf util: Add block_info in hist_entry
  perf diff: Check if all data files with branch stacks
  perf diff: Get a list of symbols(functions)
  perf diff: Use hists to manage basic blocks
  perf diff: Link same basic blocks among different data files
  perf diff: Compute cycles diff of basic blocks
  perf diff: Print the basic block cycles diff
  perf diff: Documentation --basic-block option

 tools/perf/Documentation/perf-diff.txt |   5 +
 tools/perf/builtin-diff.c              | 521 ++++++++++++++++++++++++++++++++-
 tools/perf/util/hist.c                 |  24 +-
 tools/perf/util/hist.h                 |   6 +
 tools/perf/util/sort.h                 |   4 +
 tools/perf/util/symbol.c               |  22 ++
 tools/perf/util/symbol.h               |  23 ++
 7 files changed, 594 insertions(+), 11 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v1 1/9] perf util: Create block_info structure
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 2/9] perf util: Add block_info in hist_entry Jin Yao
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

perf diff currently can only diff symbols(functions). We should expand it
to diff cycles of individual programs blocks as reported by timed LBR.
This would allow to identify changes in specific code accurately.

We need a new structure to maintain the basic block information, such as,
symbol(function), start/end addrress of this block, cycles. This patch
creates this structure and with some ops.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/util/symbol.c | 22 ++++++++++++++++++++++
 tools/perf/util/symbol.h | 23 +++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5cbad55..3a90e72 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2262,3 +2262,25 @@ struct mem_info *mem_info__new(void)
 		refcount_set(&mi->refcnt, 1);
 	return mi;
 }
+
+struct block_info *block_info__get(struct block_info *bi)
+{
+	if (bi)
+		refcount_inc(&bi->refcnt);
+	return bi;
+}
+
+void block_info__put(struct block_info *bi)
+{
+	if (bi && refcount_dec_and_test(&bi->refcnt))
+		free(bi);
+}
+
+struct block_info *block_info__new(void)
+{
+	struct block_info *bi = zalloc(sizeof(*bi));
+
+	if (bi)
+		refcount_set(&bi->refcnt, 1);
+	return bi;
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 9a8fe01..12755b4 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -131,6 +131,17 @@ struct mem_info {
 	refcount_t		refcnt;
 };
 
+struct block_info {
+	struct symbol		*sym;
+	u64			start;
+	u64			end;
+	u64			cycles;
+	u64			cycles_aggr;
+	int			num;
+	int			num_aggr;
+	refcount_t		refcnt;
+};
+
 struct addr_location {
 	struct machine *machine;
 	struct thread *thread;
@@ -332,4 +343,16 @@ static inline void __mem_info__zput(struct mem_info **mi)
 
 #define mem_info__zput(mi) __mem_info__zput(&mi)
 
+struct block_info *block_info__new(void);
+struct block_info *block_info__get(struct block_info *bi);
+void   block_info__put(struct block_info *bi);
+
+static inline void __block_info__zput(struct block_info **bi)
+{
+	block_info__put(*bi);
+	*bi = NULL;
+}
+
+#define block_info__zput(bi) __block_info__zput(&bi)
+
 #endif /* __PERF_SYMBOL */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 2/9] perf util: Add block_info in hist_entry
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
  2019-05-20 13:27 ` [PATCH v1 1/9] perf util: Create block_info structure Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 3/9] perf diff: Check if all data files with branch stacks Jin Yao
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

The block_info contains the program basic block information, i.e,
contains the start address and the end address of this basic block and
how much cycles it takes. We need to compare, sort and even print out
the basic block by some orders, i.e. sort by cycles.

For this purpose, we add block_info field to hist_entry. In order not to
impact current interface, we creates a new function hists__add_entry_block.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/util/hist.c | 22 ++++++++++++++++++++--
 tools/perf/util/hist.h |  6 ++++++
 tools/perf/util/sort.h |  1 +
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 7ace7a10..3810460 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -574,6 +574,8 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 			 */
 			mem_info__zput(entry->mem_info);
 
+			block_info__zput(entry->block_info);
+
 			/* If the map of an existing hist_entry has
 			 * become out-of-date due to an exec() or
 			 * similar, update it.  Otherwise we will
@@ -645,6 +647,7 @@ __hists__add_entry(struct hists *hists,
 		   struct symbol *sym_parent,
 		   struct branch_info *bi,
 		   struct mem_info *mi,
+		   struct block_info *block_info,
 		   struct perf_sample *sample,
 		   bool sample_self,
 		   struct hist_entry_ops *ops)
@@ -677,6 +680,7 @@ __hists__add_entry(struct hists *hists,
 		.hists	= hists,
 		.branch_info = bi,
 		.mem_info = mi,
+		.block_info = block_info,
 		.transaction = sample->transaction,
 		.raw_data = sample->raw_data,
 		.raw_size = sample->raw_size,
@@ -699,7 +703,7 @@ struct hist_entry *hists__add_entry(struct hists *hists,
 				    struct perf_sample *sample,
 				    bool sample_self)
 {
-	return __hists__add_entry(hists, al, sym_parent, bi, mi,
+	return __hists__add_entry(hists, al, sym_parent, bi, mi, NULL,
 				  sample, sample_self, NULL);
 }
 
@@ -712,10 +716,24 @@ struct hist_entry *hists__add_entry_ops(struct hists *hists,
 					struct perf_sample *sample,
 					bool sample_self)
 {
-	return __hists__add_entry(hists, al, sym_parent, bi, mi,
+	return __hists__add_entry(hists, al, sym_parent, bi, mi, NULL,
 				  sample, sample_self, ops);
 }
 
+struct hist_entry *hists__add_entry_block(struct hists *hists,
+					  struct hist_entry_ops *ops,
+					  struct addr_location *al,
+					  struct block_info *block_info)
+{
+	struct hist_entry entry = {
+		.ops = ops,
+		.block_info = block_info,
+		.hists = hists,
+	}, *he = hists__findnew_entry(hists, &entry, al, false);
+
+	return he;
+}
+
 static int
 iter_next_nop_entry(struct hist_entry_iter *iter __maybe_unused,
 		    struct addr_location *al __maybe_unused)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 76ff6c6..c8f7d66 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -16,6 +16,7 @@ struct addr_location;
 struct map_symbol;
 struct mem_info;
 struct branch_info;
+struct block_info;
 struct symbol;
 
 enum hist_filter {
@@ -149,6 +150,11 @@ struct hist_entry *hists__add_entry_ops(struct hists *hists,
 					struct perf_sample *sample,
 					bool sample_self);
 
+struct hist_entry *hists__add_entry_block(struct hists *hists,
+					  struct hist_entry_ops *ops,
+					  struct addr_location *al,
+					  struct block_info *bi);
+
 int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
 			 int max_stack_depth, void *arg);
 
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index ce376a7..43623fa 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -144,6 +144,7 @@ struct hist_entry {
 	long			time;
 	struct hists		*hists;
 	struct mem_info		*mem_info;
+	struct block_info	*block_info;
 	void			*raw_data;
 	u32			raw_size;
 	int			num_res;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 3/9] perf diff: Check if all data files with branch stacks
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
  2019-05-20 13:27 ` [PATCH v1 1/9] perf util: Create block_info structure Jin Yao
  2019-05-20 13:27 ` [PATCH v1 2/9] perf util: Add block_info in hist_entry Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 4/9] perf diff: Get a list of symbols(functions) Jin Yao
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

We will expand perf diff to support diff cycles of individual programs
blocks, so it requires all data files having branch stacks.

This patch checks HEADER_BRANCH_STACK in header, and only set the flag
has_br_stack when HEADER_BRANCH_STACK are set in all data files.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-diff.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 6e79207..6023f8c 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -32,6 +32,7 @@ struct perf_diff {
 	struct perf_time_interval	*ptime_range;
 	int				 range_size;
 	int				 range_num;
+	bool				 has_br_stack;
 };
 
 /* Diff command specific HPP columns. */
@@ -873,12 +874,41 @@ static int parse_time_str(struct data__file *d, char *abstime_ostr,
 	return ret;
 }
 
+static int check_file_brstack(void)
+{
+	struct data__file *d;
+	bool has_br_stack;
+	int i;
+
+	data__for_each_file(i, d) {
+		d->session = perf_session__new(&d->data, false, &pdiff.tool);
+		if (!d->session) {
+			pr_err("Failed to open %s\n", d->data.path);
+			return -1;
+		}
+
+		has_br_stack = perf_header__has_feat(&d->session->header,
+						     HEADER_BRANCH_STACK);
+		perf_session__delete(d->session);
+		if (!has_br_stack)
+			return 0;
+	}
+
+	/* Set only all files having branch stacks */
+	pdiff.has_br_stack = true;
+	return 0;
+}
+
 static int __cmd_diff(void)
 {
 	struct data__file *d;
 	int ret, i;
 	char *abstime_ostr, *abstime_tmp;
 
+	ret = check_file_brstack();
+	if (ret)
+		return ret;
+
 	ret = abstime_str_dup(&abstime_ostr);
 	if (ret)
 		return ret;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 4/9] perf diff: Get a list of symbols(functions)
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
                   ` (2 preceding siblings ...)
  2019-05-20 13:27 ` [PATCH v1 3/9] perf diff: Check if all data files with branch stacks Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 5/9] perf diff: Use hists to manage basic blocks Jin Yao
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

We already have a function hist__account_cycles() which can be used
to account cycles per basic block in symbol/function. But we also
need to know what the symbols are, since we need to get basic blocks
of all symbols(functions) before diff.

This patch records the sorted symbols in sym_hists, which will be
used in next patch for accounting cycles per basic block per
function.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-diff.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 73 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 6023f8c..e067ac6 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -26,6 +26,12 @@
 #include <stdlib.h>
 #include <math.h>
 
+struct block_hists {
+	struct hists		sym_hists;
+	struct perf_hpp_list	sym_list;
+	struct perf_hpp_fmt	sym_fmt;
+};
+
 struct perf_diff {
 	struct perf_tool		 tool;
 	const char			*time_str;
@@ -33,6 +39,8 @@ struct perf_diff {
 	int				 range_size;
 	int				 range_num;
 	bool				 has_br_stack;
+	bool				 basic_block;
+	struct block_hists		*block_hists;
 };
 
 /* Diff command specific HPP columns. */
@@ -62,6 +70,7 @@ struct data__file {
 	int			 idx;
 	struct hists		*hists;
 	struct diff_hpp_fmt	 fmt[PERF_HPP_DIFF__MAX_INDEX];
+	struct block_hists       block_hists;
 };
 
 static struct data__file *data__files;
@@ -363,9 +372,18 @@ static int diff__process_sample_event(struct perf_tool *tool,
 		goto out_put;
 	}
 
-	if (!hists__add_entry(hists, &al, NULL, NULL, NULL, sample, true)) {
-		pr_warning("problem incrementing symbol period, skipping event\n");
-		goto out_put;
+	if (pdiff->has_br_stack && pdiff->basic_block) {
+		if (!hists__add_entry(&pdiff->block_hists->sym_hists, &al,
+			NULL, NULL, NULL, sample, false)) {
+			pr_warning("problem counting symbol for basic block\n");
+			goto out_put;
+		}
+	} else {
+		if (!hists__add_entry(hists, &al, NULL, NULL, NULL,
+				      sample, true)) {
+			pr_warning("problem incrementing symbol period, skipping event\n");
+			goto out_put;
+		}
 	}
 
 	/*
@@ -899,6 +917,37 @@ static int check_file_brstack(void)
 	return 0;
 }
 
+static int64_t symbol_se_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
+			     struct hist_entry *a, struct hist_entry *b)
+{
+	return sort_sym.se_cmp(a, b);
+}
+
+static int block_sym_hists_init(struct block_hists *hists)
+{
+	struct perf_hpp_fmt *fmt;
+
+	__hists__init(&hists->sym_hists, &hists->sym_list);
+	perf_hpp_list__init(&hists->sym_list);
+	fmt = &hists->sym_fmt;
+	INIT_LIST_HEAD(&fmt->list);
+	INIT_LIST_HEAD(&fmt->sort_list);
+
+	fmt->cmp = symbol_se_cmp;
+	perf_hpp_list__register_sort_field(&hists->sym_list, fmt);
+	return 0;
+}
+
+static void basic_block_process(void)
+{
+	struct data__file *d;
+	int i;
+
+	data__for_each_file(i, d) {
+		hists__delete_entries(&d->block_hists.sym_hists);
+	}
+}
+
 static int __cmd_diff(void)
 {
 	struct data__file *d;
@@ -937,6 +986,16 @@ static int __cmd_diff(void)
 				goto out_delete;
 		}
 
+		if (pdiff.has_br_stack && pdiff.basic_block) {
+			ret = block_sym_hists_init(&d->block_hists);
+			if (ret) {
+				pr_err("Failed to initialize basic block hists\n");
+				goto out_delete;
+			}
+
+			pdiff.block_hists = &d->block_hists;
+		}
+
 		ret = perf_session__process_events(d->session);
 		if (ret) {
 			pr_err("Failed to process %s\n", d->data.path);
@@ -949,7 +1008,10 @@ static int __cmd_diff(void)
 			zfree(&pdiff.ptime_range);
 	}
 
-	data_process();
+	if (pdiff.has_br_stack && pdiff.basic_block)
+		basic_block_process();
+	else
+		data_process();
 
  out_delete:
 	data__for_each_file(i, d) {
@@ -1019,6 +1081,8 @@ static const struct option options[] = {
 		   "only consider symbols in these pids"),
 	OPT_STRING(0, "tid", &symbol_conf.tid_list_str, "tid[,tid...]",
 		   "only consider symbols in these tids"),
+	OPT_BOOLEAN(0, "basic-block", &pdiff.basic_block,
+		    "display the differential program basic block"),
 	OPT_END()
 };
 
@@ -1517,10 +1581,11 @@ int cmd_diff(int argc, const char **argv)
 	if (data_init(argc, argv) < 0)
 		return -1;
 
-	if (ui_init() < 0)
-		return -1;
-
-	sort__mode = SORT_MODE__DIFF;
+	if (!pdiff.basic_block) {
+		if (ui_init() < 0)
+			return -1;
+		sort__mode = SORT_MODE__DIFF;
+	}
 
 	if (setup_sorting(NULL) < 0)
 		usage_with_options(diff_usage, options);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 5/9] perf diff: Use hists to manage basic blocks
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
                   ` (3 preceding siblings ...)
  2019-05-20 13:27 ` [PATCH v1 4/9] perf diff: Get a list of symbols(functions) Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 6/9] perf diff: Link same basic blocks among different data files Jin Yao
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

The function hist__account_cycles() can account cycles per basic
block. The basic block information are saved in a per-symbol
cycles_hist structure.

This patch processes each symbol, get basic blocks from cycles_hist
and add the basic block entry to a hists. Using a hists is because
we need to compare, sort and print the basic blocks.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-diff.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index e067ac6..09551fe 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -20,6 +20,7 @@
 #include "util/data.h"
 #include "util/config.h"
 #include "util/time-utils.h"
+#include "util/annotate.h"
 
 #include <errno.h>
 #include <inttypes.h>
@@ -30,6 +31,9 @@ struct block_hists {
 	struct hists		sym_hists;
 	struct perf_hpp_list	sym_list;
 	struct perf_hpp_fmt	sym_fmt;
+	struct hists		hists;
+	struct perf_hpp_list	list;
+	struct perf_hpp_fmt	fmt;
 };
 
 struct perf_diff {
@@ -73,6 +77,8 @@ struct data__file {
 	struct block_hists       block_hists;
 };
 
+static struct addr_location dummy_al;
+
 static struct data__file *data__files;
 static int data__files_cnt;
 
@@ -378,6 +384,7 @@ static int diff__process_sample_event(struct perf_tool *tool,
 			pr_warning("problem counting symbol for basic block\n");
 			goto out_put;
 		}
+		hist__account_cycles(sample->branch_stack, &al, sample, false);
 	} else {
 		if (!hists__add_entry(hists, &al, NULL, NULL, NULL,
 				      sample, true)) {
@@ -938,13 +945,149 @@ static int block_sym_hists_init(struct block_hists *hists)
 	return 0;
 }
 
+static int64_t block_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
+			 struct hist_entry *left, struct hist_entry *right)
+{
+	struct block_info *bi_l = left->block_info;
+	struct block_info *bi_r = right->block_info;
+	int cmp;
+
+	if (!bi_l->sym || !bi_r->sym) {
+		if (!bi_l->sym && !bi_r->sym)
+			return 0;
+		else if (!bi_l->sym)
+			return -1;
+		else
+			return 1;
+	}
+
+	if (bi_l->sym == bi_r->sym) {
+		if (bi_l->start == bi_r->start) {
+			if (bi_l->end == bi_r->end)
+				return 0;
+			else
+				return (int64_t)(bi_r->end - bi_l->end);
+		} else
+			return (int64_t)(bi_r->start - bi_l->start);
+	} else {
+		cmp = strcmp(bi_l->sym->name, bi_r->sym->name);
+		return cmp;
+	}
+
+	if (bi_l->sym->start != bi_r->sym->start)
+		return (int64_t)(bi_r->sym->start - bi_l->sym->start);
+
+	return (int64_t)(bi_r->sym->end - bi_l->sym->end);
+}
+
+static int block_hists_init(struct block_hists *hists)
+{
+	struct perf_hpp_fmt *fmt;
+
+	__hists__init(&hists->hists, &hists->list);
+	perf_hpp_list__init(&hists->list);
+	fmt = &hists->fmt;
+	INIT_LIST_HEAD(&fmt->list);
+	INIT_LIST_HEAD(&fmt->sort_list);
+
+	fmt->cmp = block_cmp;
+
+	perf_hpp_list__register_sort_field(&hists->list, fmt);
+	return 0;
+}
+
+static void *block_he_zalloc(size_t size)
+{
+	return zalloc(size + sizeof(struct hist_entry));
+}
+
+static void block_he_free(void *he)
+{
+	struct block_info *bi = ((struct hist_entry *)he)->block_info;
+
+	block_info__put(bi);
+	free(he);
+}
+
+struct hist_entry_ops block_he_ops = {
+	.new	= block_he_zalloc,
+	.free	= block_he_free,
+};
+
+static void init_block_info(struct block_info *bi, struct symbol *sym,
+			    struct cyc_hist *ch, int offset)
+{
+	bi->sym = sym;
+	bi->start = ch->start;
+	bi->end = offset;
+	bi->cycles = ch->cycles;
+	bi->cycles_aggr = ch->cycles_aggr;
+	bi->num = ch->num;
+	bi->num_aggr = ch->num_aggr;
+}
+
+static int process_block_per_sym(struct data__file *d)
+{
+	struct hists *hists = &d->block_hists.sym_hists;
+	struct rb_root_cached *root = hists->entries_in;
+	struct rb_node *next = rb_first_cached(root);
+	struct annotation *notes;
+	struct cyc_hist *ch;
+	unsigned int i;
+	struct block_info *bi;
+	struct hist_entry *he, *he_block;
+
+	while (next != NULL) {
+		he = rb_entry(next, struct hist_entry, rb_node_in);
+		next = rb_next(&he->rb_node_in);
+
+		if (!he->ms.map || !he->ms.sym)
+			continue;
+
+		notes = symbol__annotation(he->ms.sym);
+		if (!notes || !notes->src || !notes->src->cycles_hist)
+			continue;
+
+		ch = notes->src->cycles_hist;
+
+		for (i = 0; i < symbol__size(he->ms.sym); i++) {
+			if (ch[i].num_aggr) {
+				bi = block_info__new();
+				if (!bi)
+					return -ENOMEM;
+
+				init_block_info(bi, he->ms.sym, &ch[i], i);
+
+				he_block = hists__add_entry_block(
+							&d->block_hists.hists,
+							&block_he_ops,
+							&dummy_al, bi);
+				if (!he_block) {
+					block_info__put(bi);
+					return -ENOMEM;
+				}
+			}
+		}
+	}
+
+	return 0;
+}
+
 static void basic_block_process(void)
 {
 	struct data__file *d;
 	int i;
 
+	memset(&dummy_al, 0, sizeof(dummy_al));
+
+	data__for_each_file(i, d) {
+		block_hists_init(&d->block_hists);
+		process_block_per_sym(d);
+	}
+
 	data__for_each_file(i, d) {
 		hists__delete_entries(&d->block_hists.sym_hists);
+		hists__delete_entries(&d->block_hists.hists);
 	}
 }
 
@@ -1575,6 +1718,8 @@ int cmd_diff(int argc, const char **argv)
 	if (quiet)
 		perf_quiet_option();
 
+	symbol__annotation_init();
+
 	if (symbol__init(NULL) < 0)
 		return -1;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 6/9] perf diff: Link same basic blocks among different data files
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
                   ` (4 preceding siblings ...)
  2019-05-20 13:27 ` [PATCH v1 5/9] perf diff: Use hists to manage basic blocks Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 7/9] perf diff: Compute cycles diff of basic blocks Jin Yao
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

The target is to compare the performance difference (cycles
diff) for the same basic blocks in different data files.

The same basic block means same function, same start address
and same end address. This patch finds the same basic blocks
from different data files and link them together.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-diff.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 09551fe..72c33ab 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1073,8 +1073,69 @@ static int process_block_per_sym(struct data__file *d)
 	return 0;
 }
 
+static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b)
+{
+	struct block_info *bi_a = a->block_info;
+	struct block_info *bi_b = b->block_info;
+	int cmp;
+
+	if (!bi_a->sym || !bi_b->sym)
+		return -1;
+
+	if (bi_a->sym->name && bi_b->sym->name) {
+		cmp = strcmp(bi_a->sym->name, bi_b->sym->name);
+		if ((!cmp) && (bi_a->start == bi_b->start) &&
+		    (bi_a->end == bi_b->end)) {
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
+static struct hist_entry *get_block_pair(struct hist_entry *he,
+					 struct hists *hists_pair)
+{
+	struct rb_root_cached *root = hists_pair->entries_in;
+	struct rb_node *next = rb_first_cached(root);
+	int cmp;
+
+	while (next != NULL) {
+		struct hist_entry *he_pair = rb_entry(next, struct hist_entry,
+						      rb_node_in);
+
+		next = rb_next(&he_pair->rb_node_in);
+
+		cmp = block_pair_cmp(he_pair, he);
+		if (!cmp)
+			return he_pair;
+	}
+
+	return NULL;
+}
+
+static void block_hists_match(struct hists *hists_base,
+			      struct hists *hists_pair)
+{
+	struct rb_root_cached *root = hists_base->entries_in;
+	struct rb_node *next = rb_first_cached(root);
+
+	while (next != NULL) {
+		struct hist_entry *he = rb_entry(next, struct hist_entry,
+						 rb_node_in);
+		struct hist_entry *pair = get_block_pair(he, hists_pair);
+
+		next = rb_next(&he->rb_node_in);
+
+		if (pair)
+			hist_entry__add_pair(pair, he);
+	}
+}
+
 static void basic_block_process(void)
 {
+	struct hists *hists_base = &data__files[0].block_hists.hists;
+	struct hists *hists;
 	struct data__file *d;
 	int i;
 
@@ -1085,6 +1146,11 @@ static void basic_block_process(void)
 		process_block_per_sym(d);
 	}
 
+	data__for_each_file_new(i, d) {
+		hists = &d->block_hists.hists;
+		block_hists_match(hists_base, hists);
+	}
+
 	data__for_each_file(i, d) {
 		hists__delete_entries(&d->block_hists.sym_hists);
 		hists__delete_entries(&d->block_hists.hists);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 7/9] perf diff: Compute cycles diff of basic blocks
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
                   ` (5 preceding siblings ...)
  2019-05-20 13:27 ` [PATCH v1 6/9] perf diff: Link same basic blocks among different data files Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-20 13:27 ` [PATCH v1 8/9] perf diff: Print the basic block cycles diff Jin Yao
  2019-05-20 13:27 ` [PATCH v1 9/9] perf diff: Documentation --basic-block option Jin Yao
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

In previous patch, we have already linked up the same basic blocks.
Now we compute the cycles diff value of basic blocks, in order to
sort by diff cycles later.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-diff.c | 31 +++++++++++++++++++++++++++++++
 tools/perf/util/sort.h    |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 72c33ab..47e34a3 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1132,6 +1132,31 @@ static void block_hists_match(struct hists *hists_base,
 	}
 }
 
+static void compute_block_hists_diff(struct block_hists *block_hists,
+				     struct data__file *d)
+{
+	struct hists *hists = &block_hists->hists;
+	struct rb_root_cached *root = hists->entries_in;
+	struct rb_node *next = rb_first_cached(root);
+
+	while (next != NULL) {
+		struct hist_entry *he = rb_entry(next, struct hist_entry,
+						 rb_node_in);
+		struct hist_entry *pair = get_pair_data(he, d);
+
+		next = rb_next(&he->rb_node_in);
+
+		if (pair) {
+			pair->diff.computed = true;
+			if (pair->block_info->num && he->block_info->num) {
+				pair->diff.cycles_diff =
+					pair->block_info->cycles_aggr / pair->block_info->num_aggr -
+					he->block_info->cycles_aggr / he->block_info->num_aggr;
+			}
+		}
+	}
+}
+
 static void basic_block_process(void)
 {
 	struct hists *hists_base = &data__files[0].block_hists.hists;
@@ -1151,6 +1176,12 @@ static void basic_block_process(void)
 		block_hists_match(hists_base, hists);
 	}
 
+	data__for_each_file_new(i, d) {
+		hists = &d->block_hists.hists;
+		d->hists = hists;
+		compute_block_hists_diff(&data__files[0].block_hists, d);
+	}
+
 	data__for_each_file(i, d) {
 		hists__delete_entries(&d->block_hists.sym_hists);
 		hists__delete_entries(&d->block_hists.hists);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 43623fa..de9e61a 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -79,6 +79,8 @@ struct hist_entry_diff {
 
 		/* HISTC_WEIGHTED_DIFF */
 		s64	wdiff;
+
+		s64     cycles_diff;
 	};
 };
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 8/9] perf diff: Print the basic block cycles diff
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
                   ` (6 preceding siblings ...)
  2019-05-20 13:27 ` [PATCH v1 7/9] perf diff: Compute cycles diff of basic blocks Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  2019-05-22 14:04   ` Jiri Olsa
  2019-05-20 13:27 ` [PATCH v1 9/9] perf diff: Documentation --basic-block option Jin Yao
  8 siblings, 1 reply; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

Currently we only support sorting by diff cycles.

For example,

perf record -b ./div
perf record -b ./div
perf diff --basic-block

 # Cycles diff  Basic block (start:end)
 # ...........  .......................
 #
          -20   native_write_msr (7fff9a069900:7fff9a06990b)
           -3   __indirect_thunk_start (7fff9ac02ca0:7fff9ac02ca0)
            1   __indirect_thunk_start (7fff9ac02cac:7fff9ac02cb0)
            0   rand@plt (490:490)
            0   rand (3af60:3af64)
            0   rand (3af69:3af6d)
            0   main (4e8:4ea)
            0   main (4ef:500)
            0   main (4ef:535)
            0   compute_flag (640:644)
            0   compute_flag (649:659)
            0   __random_r (3ac40:3ac76)
            0   __random_r (3ac40:3ac88)
            0   __random_r (3ac90:3ac9c)
            0   __random (3aac0:3aad2)
            0   __random (3aae0:3aae7)
            0   __random (3ab03:3ab0f)
            0   __random (3ab14:3ab1b)
            0   __random (3ab28:3ab2e)
            0   __random (3ab4a:3ab53)

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-diff.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.c    |   2 +-
 tools/perf/util/sort.h    |   1 +
 3 files changed, 170 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 47e34a3..dbf242d 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -27,6 +27,12 @@
 #include <stdlib.h>
 #include <math.h>
 
+struct block_hpp_fmt {
+	struct perf_hpp_fmt	fmt;
+	struct data__file	*file;
+	int			width;
+};
+
 struct block_hists {
 	struct hists		sym_hists;
 	struct perf_hpp_list	sym_list;
@@ -34,6 +40,8 @@ struct block_hists {
 	struct hists		hists;
 	struct perf_hpp_list	list;
 	struct perf_hpp_fmt	fmt;
+	struct block_hpp_fmt	block_fmt;
+	struct perf_hpp_fmt	desc_fmt;
 };
 
 struct perf_diff {
@@ -1157,6 +1165,162 @@ static void compute_block_hists_diff(struct block_hists *block_hists,
 	}
 }
 
+static int64_t block_cycles_diff_cmp(struct perf_hpp_fmt *fmt,
+				     struct hist_entry *left,
+				     struct hist_entry *right)
+{
+	struct block_hpp_fmt *block_fmt = container_of(fmt,
+						       struct block_hpp_fmt,
+						       fmt);
+	struct data__file *d = block_fmt->file;
+	bool pairs_left  = hist_entry__has_pairs(left);
+	bool pairs_right = hist_entry__has_pairs(right);
+	struct hist_entry *p_right, *p_left;
+	s64 l, r;
+
+	if (!pairs_left && !pairs_right)
+		return 0;
+
+	if (!pairs_left || !pairs_right)
+		return pairs_left ? -1 : 1;
+
+	p_left  = get_pair_data(left, d);
+	p_right  = get_pair_data(right, d);
+
+	if (!p_left && !p_right)
+		return 0;
+
+	if (!p_left || !p_right)
+		return p_left ?  -1 : 1;
+
+	l = abs(p_left->diff.cycles_diff);
+	r = abs(p_right->diff.cycles_diff);
+
+	return r - l;
+}
+
+static int64_t block_diff_sort(struct perf_hpp_fmt *fmt,
+			       struct hist_entry *left, struct hist_entry *right)
+{
+	return block_cycles_diff_cmp(fmt, right, left);
+}
+
+static int block_diff_header(struct perf_hpp_fmt *fmt __maybe_unused,
+			     struct perf_hpp *hpp,
+			     struct hists *hists __maybe_unused,
+			     int line __maybe_unused,
+			     int *span __maybe_unused)
+{
+	return scnprintf(hpp->buf, hpp->size, "Cycles diff");
+}
+
+static int block_diff_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+			    struct hist_entry *he)
+{
+	struct block_hpp_fmt *block_fmt = container_of(fmt,
+						       struct block_hpp_fmt,
+						       fmt);
+	struct data__file *d = block_fmt->file;
+	struct hist_entry *pair = get_pair_data(he, d);
+
+	if (pair && pair->diff.computed) {
+		return scnprintf(hpp->buf, hpp->size, "%*ld", block_fmt->width,
+				 pair->diff.cycles_diff);
+	}
+
+	return scnprintf(hpp->buf, hpp->size, "%*s", block_fmt->width, " ");
+}
+
+static int block_diff_width(struct perf_hpp_fmt *fmt,
+			    struct perf_hpp *hpp __maybe_unused,
+			    struct hists *hists __maybe_unused)
+{
+	struct block_hpp_fmt *block_fmt =
+		container_of(fmt, struct block_hpp_fmt, fmt);
+
+	return block_fmt->width;
+}
+
+static int block_sym_width(struct perf_hpp_fmt *fmt __maybe_unused,
+			    struct perf_hpp *hpp __maybe_unused,
+			    struct hists *hists __maybe_unused)
+{
+	return 23;
+}
+
+static int block_sym_entry(struct perf_hpp_fmt *fmt __maybe_unused,
+			   struct perf_hpp *hpp, struct hist_entry *he)
+{
+	struct block_info *bi = he->block_info;
+
+	return scnprintf(hpp->buf, hpp->size, "%s (%lx:%lx)",
+			 bi->sym->name, bi->sym->start + bi->start,
+			 bi->sym->start + bi->end);
+}
+
+static int block_sym_header(struct perf_hpp_fmt *fmt __maybe_unused,
+			    struct perf_hpp *hpp,
+			    struct hists *hists __maybe_unused,
+			    int line __maybe_unused,
+			    int *span __maybe_unused)
+{
+	return scnprintf(hpp->buf, hpp->size, "Basic block (start:end)");
+}
+
+static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
+{
+	he->not_collen = true;
+
+	return 0;
+}
+
+static void hists_block_printf_sorted(struct hists *hists)
+{
+	hists__fprintf(hists, true, 0, 0, 0, stdout, true);
+}
+
+static void init_block_hists_fmt(void)
+{
+	struct block_hists *hists_base = &data__files[0].block_hists;
+	struct data__file *d;
+	int i;
+	struct perf_hpp_fmt *fmt;
+
+	perf_hpp__reset_output_field(&hists_base->list);
+
+	hists_base->list.nr_header_lines = 1;
+
+	data__for_each_file_new(i, d) {
+		struct block_hpp_fmt *block_fmt = &d->block_hists.block_fmt;
+
+		fmt = &block_fmt->fmt;
+		block_fmt->file = d;
+		block_fmt->width = 11;
+
+		INIT_LIST_HEAD(&fmt->list);
+		INIT_LIST_HEAD(&fmt->sort_list);
+
+		fmt->sort = block_diff_sort;
+		fmt->header = block_diff_header;
+		fmt->entry = block_diff_entry;
+		fmt->width = block_diff_width;
+
+		perf_hpp_list__column_register(&hists_base->list, fmt);
+		perf_hpp_list__register_sort_field(&hists_base->list, fmt);
+	}
+
+	/* fmt for description */
+	fmt = &hists_base->desc_fmt;
+
+	fmt->width = block_sym_width;
+	fmt->entry = block_sym_entry;
+	fmt->header = block_sym_header;
+	fmt->sort = hist_entry__cmp_nop;
+
+	perf_hpp_list__column_register(&hists_base->list, fmt);
+	perf_hpp_list__register_sort_field(&hists_base->list, fmt);
+}
+
 static void basic_block_process(void)
 {
 	struct hists *hists_base = &data__files[0].block_hists.hists;
@@ -1182,6 +1346,10 @@ static void basic_block_process(void)
 		compute_block_hists_diff(&data__files[0].block_hists, d);
 	}
 
+	init_block_hists_fmt();
+	hists__output_resort_cb(hists_base, NULL, filter_cb);
+	hists_block_printf_sorted(hists_base);
+
 	data__for_each_file(i, d) {
 		hists__delete_entries(&d->block_hists.sym_hists);
 		hists__delete_entries(&d->block_hists.hists);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 3810460..ae95191 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1846,7 +1846,7 @@ static void output_resort(struct hists *hists, struct ui_progress *prog,
 		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits, use_callchain);
 		hists__inc_stats(hists, n);
 
-		if (!n->filtered)
+		if ((!n->filtered) && (!n->not_collen))
 			hists__calc_col_len(hists, n);
 
 		if (prog)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index de9e61a..1e921fe 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -121,6 +121,7 @@ struct hist_entry {
 
 	char			level;
 	u8			filtered;
+	bool			not_collen;
 
 	u16			callchain_size;
 	union {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 9/9] perf diff: Documentation --basic-block option
  2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
                   ` (7 preceding siblings ...)
  2019-05-20 13:27 ` [PATCH v1 8/9] perf diff: Print the basic block cycles diff Jin Yao
@ 2019-05-20 13:27 ` Jin Yao
  8 siblings, 0 replies; 12+ messages in thread
From: Jin Yao @ 2019-05-20 13:27 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

Documentation the new option '--basic-block'.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/Documentation/perf-diff.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index da7809b..b242af8 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -174,6 +174,11 @@ OPTIONS
 --tid=::
 	Only diff samples for given thread ID (comma separated list).
 
+--basic-block::
+	Display the cycles difference of same program basic block amongst
+	two or more perf.data. The program basic block is the code block
+	between two branches in a function.
+
 COMPARISON
 ----------
 The comparison is governed by the baseline file. The baseline perf.data
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 8/9] perf diff: Print the basic block cycles diff
  2019-05-20 13:27 ` [PATCH v1 8/9] perf diff: Print the basic block cycles diff Jin Yao
@ 2019-05-22 14:04   ` Jiri Olsa
  2019-05-24  0:39     ` Jin, Yao
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2019-05-22 14:04 UTC (permalink / raw)
  To: Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Mon, May 20, 2019 at 09:27:55PM +0800, Jin Yao wrote:
> Currently we only support sorting by diff cycles.
> 
> For example,
> 
> perf record -b ./div
> perf record -b ./div
> perf diff --basic-block
> 
>  # Cycles diff  Basic block (start:end)
>  # ...........  .......................
>  #
>           -20   native_write_msr (7fff9a069900:7fff9a06990b)
>            -3   __indirect_thunk_start (7fff9ac02ca0:7fff9ac02ca0)
>             1   __indirect_thunk_start (7fff9ac02cac:7fff9ac02cb0)
>             0   rand@plt (490:490)
>             0   rand (3af60:3af64)
>             0   rand (3af69:3af6d)
>             0   main (4e8:4ea)
>             0   main (4ef:500)
>             0   main (4ef:535)
>             0   compute_flag (640:644)
>             0   compute_flag (649:659)
>             0   __random_r (3ac40:3ac76)
>             0   __random_r (3ac40:3ac88)
>             0   __random_r (3ac90:3ac9c)
>             0   __random (3aac0:3aad2)
>             0   __random (3aae0:3aae7)
>             0   __random (3ab03:3ab0f)
>             0   __random (3ab14:3ab1b)
>             0   __random (3ab28:3ab2e)
>             0   __random (3ab4a:3ab53)

really nice, could you keep the standard diff format
and display the 'Baseline' and Shared Object columns?

jirka

> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  tools/perf/builtin-diff.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/hist.c    |   2 +-
>  tools/perf/util/sort.h    |   1 +
>  3 files changed, 170 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 47e34a3..dbf242d 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -27,6 +27,12 @@
>  #include <stdlib.h>
>  #include <math.h>
>  
> +struct block_hpp_fmt {
> +	struct perf_hpp_fmt	fmt;
> +	struct data__file	*file;
> +	int			width;
> +};
> +
>  struct block_hists {
>  	struct hists		sym_hists;
>  	struct perf_hpp_list	sym_list;
> @@ -34,6 +40,8 @@ struct block_hists {
>  	struct hists		hists;
>  	struct perf_hpp_list	list;
>  	struct perf_hpp_fmt	fmt;
> +	struct block_hpp_fmt	block_fmt;
> +	struct perf_hpp_fmt	desc_fmt;
>  };
>  
>  struct perf_diff {
> @@ -1157,6 +1165,162 @@ static void compute_block_hists_diff(struct block_hists *block_hists,
>  	}
>  }
>  
> +static int64_t block_cycles_diff_cmp(struct perf_hpp_fmt *fmt,
> +				     struct hist_entry *left,
> +				     struct hist_entry *right)
> +{
> +	struct block_hpp_fmt *block_fmt = container_of(fmt,
> +						       struct block_hpp_fmt,
> +						       fmt);
> +	struct data__file *d = block_fmt->file;
> +	bool pairs_left  = hist_entry__has_pairs(left);
> +	bool pairs_right = hist_entry__has_pairs(right);
> +	struct hist_entry *p_right, *p_left;
> +	s64 l, r;
> +
> +	if (!pairs_left && !pairs_right)
> +		return 0;
> +
> +	if (!pairs_left || !pairs_right)
> +		return pairs_left ? -1 : 1;
> +
> +	p_left  = get_pair_data(left, d);
> +	p_right  = get_pair_data(right, d);
> +
> +	if (!p_left && !p_right)
> +		return 0;
> +
> +	if (!p_left || !p_right)
> +		return p_left ?  -1 : 1;
> +
> +	l = abs(p_left->diff.cycles_diff);
> +	r = abs(p_right->diff.cycles_diff);
> +
> +	return r - l;
> +}
> +
> +static int64_t block_diff_sort(struct perf_hpp_fmt *fmt,
> +			       struct hist_entry *left, struct hist_entry *right)
> +{
> +	return block_cycles_diff_cmp(fmt, right, left);
> +}
> +
> +static int block_diff_header(struct perf_hpp_fmt *fmt __maybe_unused,
> +			     struct perf_hpp *hpp,
> +			     struct hists *hists __maybe_unused,
> +			     int line __maybe_unused,
> +			     int *span __maybe_unused)
> +{
> +	return scnprintf(hpp->buf, hpp->size, "Cycles diff");
> +}
> +
> +static int block_diff_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> +			    struct hist_entry *he)
> +{
> +	struct block_hpp_fmt *block_fmt = container_of(fmt,
> +						       struct block_hpp_fmt,
> +						       fmt);
> +	struct data__file *d = block_fmt->file;
> +	struct hist_entry *pair = get_pair_data(he, d);
> +
> +	if (pair && pair->diff.computed) {
> +		return scnprintf(hpp->buf, hpp->size, "%*ld", block_fmt->width,
> +				 pair->diff.cycles_diff);
> +	}
> +
> +	return scnprintf(hpp->buf, hpp->size, "%*s", block_fmt->width, " ");
> +}
> +
> +static int block_diff_width(struct perf_hpp_fmt *fmt,
> +			    struct perf_hpp *hpp __maybe_unused,
> +			    struct hists *hists __maybe_unused)
> +{
> +	struct block_hpp_fmt *block_fmt =
> +		container_of(fmt, struct block_hpp_fmt, fmt);
> +
> +	return block_fmt->width;
> +}
> +
> +static int block_sym_width(struct perf_hpp_fmt *fmt __maybe_unused,
> +			    struct perf_hpp *hpp __maybe_unused,
> +			    struct hists *hists __maybe_unused)
> +{
> +	return 23;
> +}
> +
> +static int block_sym_entry(struct perf_hpp_fmt *fmt __maybe_unused,
> +			   struct perf_hpp *hpp, struct hist_entry *he)
> +{
> +	struct block_info *bi = he->block_info;
> +
> +	return scnprintf(hpp->buf, hpp->size, "%s (%lx:%lx)",
> +			 bi->sym->name, bi->sym->start + bi->start,
> +			 bi->sym->start + bi->end);
> +}
> +
> +static int block_sym_header(struct perf_hpp_fmt *fmt __maybe_unused,
> +			    struct perf_hpp *hpp,
> +			    struct hists *hists __maybe_unused,
> +			    int line __maybe_unused,
> +			    int *span __maybe_unused)
> +{
> +	return scnprintf(hpp->buf, hpp->size, "Basic block (start:end)");
> +}
> +
> +static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
> +{
> +	he->not_collen = true;
> +
> +	return 0;
> +}
> +
> +static void hists_block_printf_sorted(struct hists *hists)
> +{
> +	hists__fprintf(hists, true, 0, 0, 0, stdout, true);
> +}
> +
> +static void init_block_hists_fmt(void)
> +{
> +	struct block_hists *hists_base = &data__files[0].block_hists;
> +	struct data__file *d;
> +	int i;
> +	struct perf_hpp_fmt *fmt;
> +
> +	perf_hpp__reset_output_field(&hists_base->list);
> +
> +	hists_base->list.nr_header_lines = 1;
> +
> +	data__for_each_file_new(i, d) {
> +		struct block_hpp_fmt *block_fmt = &d->block_hists.block_fmt;
> +
> +		fmt = &block_fmt->fmt;
> +		block_fmt->file = d;
> +		block_fmt->width = 11;
> +
> +		INIT_LIST_HEAD(&fmt->list);
> +		INIT_LIST_HEAD(&fmt->sort_list);
> +
> +		fmt->sort = block_diff_sort;
> +		fmt->header = block_diff_header;
> +		fmt->entry = block_diff_entry;
> +		fmt->width = block_diff_width;
> +
> +		perf_hpp_list__column_register(&hists_base->list, fmt);
> +		perf_hpp_list__register_sort_field(&hists_base->list, fmt);
> +	}
> +
> +	/* fmt for description */
> +	fmt = &hists_base->desc_fmt;
> +
> +	fmt->width = block_sym_width;
> +	fmt->entry = block_sym_entry;
> +	fmt->header = block_sym_header;
> +	fmt->sort = hist_entry__cmp_nop;
> +
> +	perf_hpp_list__column_register(&hists_base->list, fmt);
> +	perf_hpp_list__register_sort_field(&hists_base->list, fmt);
> +}
> +
>  static void basic_block_process(void)
>  {
>  	struct hists *hists_base = &data__files[0].block_hists.hists;
> @@ -1182,6 +1346,10 @@ static void basic_block_process(void)
>  		compute_block_hists_diff(&data__files[0].block_hists, d);
>  	}
>  
> +	init_block_hists_fmt();
> +	hists__output_resort_cb(hists_base, NULL, filter_cb);
> +	hists_block_printf_sorted(hists_base);
> +
>  	data__for_each_file(i, d) {
>  		hists__delete_entries(&d->block_hists.sym_hists);
>  		hists__delete_entries(&d->block_hists.hists);
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 3810460..ae95191 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1846,7 +1846,7 @@ static void output_resort(struct hists *hists, struct ui_progress *prog,
>  		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits, use_callchain);
>  		hists__inc_stats(hists, n);
>  
> -		if (!n->filtered)
> +		if ((!n->filtered) && (!n->not_collen))
>  			hists__calc_col_len(hists, n);
>  
>  		if (prog)
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index de9e61a..1e921fe 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -121,6 +121,7 @@ struct hist_entry {
>  
>  	char			level;
>  	u8			filtered;
> +	bool			not_collen;
>  
>  	u16			callchain_size;
>  	union {
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 8/9] perf diff: Print the basic block cycles diff
  2019-05-22 14:04   ` Jiri Olsa
@ 2019-05-24  0:39     ` Jin, Yao
  0 siblings, 0 replies; 12+ messages in thread
From: Jin, Yao @ 2019-05-24  0:39 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 5/22/2019 10:04 PM, Jiri Olsa wrote:
> On Mon, May 20, 2019 at 09:27:55PM +0800, Jin Yao wrote:
>> Currently we only support sorting by diff cycles.
>>
>> For example,
>>
>> perf record -b ./div
>> perf record -b ./div
>> perf diff --basic-block
>>
>>   # Cycles diff  Basic block (start:end)
>>   # ...........  .......................
>>   #
>>            -20   native_write_msr (7fff9a069900:7fff9a06990b)
>>             -3   __indirect_thunk_start (7fff9ac02ca0:7fff9ac02ca0)
>>              1   __indirect_thunk_start (7fff9ac02cac:7fff9ac02cb0)
>>              0   rand@plt (490:490)
>>              0   rand (3af60:3af64)
>>              0   rand (3af69:3af6d)
>>              0   main (4e8:4ea)
>>              0   main (4ef:500)
>>              0   main (4ef:535)
>>              0   compute_flag (640:644)
>>              0   compute_flag (649:659)
>>              0   __random_r (3ac40:3ac76)
>>              0   __random_r (3ac40:3ac88)
>>              0   __random_r (3ac90:3ac9c)
>>              0   __random (3aac0:3aad2)
>>              0   __random (3aae0:3aae7)
>>              0   __random (3ab03:3ab0f)
>>              0   __random (3ab14:3ab1b)
>>              0   __random (3ab28:3ab2e)
>>              0   __random (3ab4a:3ab53)
> 
> really nice, could you keep the standard diff format
> and display the 'Baseline' and Shared Object columns?
> 
> jirka
> 

Thanks Jiri!

Let me check how to do that.

Thanks
Jin Yao

>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>>   tools/perf/builtin-diff.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tools/perf/util/hist.c    |   2 +-
>>   tools/perf/util/sort.h    |   1 +
>>   3 files changed, 170 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
>> index 47e34a3..dbf242d 100644
>> --- a/tools/perf/builtin-diff.c
>> +++ b/tools/perf/builtin-diff.c
>> @@ -27,6 +27,12 @@
>>   #include <stdlib.h>
>>   #include <math.h>
>>   
>> +struct block_hpp_fmt {
>> +	struct perf_hpp_fmt	fmt;
>> +	struct data__file	*file;
>> +	int			width;
>> +};
>> +
>>   struct block_hists {
>>   	struct hists		sym_hists;
>>   	struct perf_hpp_list	sym_list;
>> @@ -34,6 +40,8 @@ struct block_hists {
>>   	struct hists		hists;
>>   	struct perf_hpp_list	list;
>>   	struct perf_hpp_fmt	fmt;
>> +	struct block_hpp_fmt	block_fmt;
>> +	struct perf_hpp_fmt	desc_fmt;
>>   };
>>   
>>   struct perf_diff {
>> @@ -1157,6 +1165,162 @@ static void compute_block_hists_diff(struct block_hists *block_hists,
>>   	}
>>   }
>>   
>> +static int64_t block_cycles_diff_cmp(struct perf_hpp_fmt *fmt,
>> +				     struct hist_entry *left,
>> +				     struct hist_entry *right)
>> +{
>> +	struct block_hpp_fmt *block_fmt = container_of(fmt,
>> +						       struct block_hpp_fmt,
>> +						       fmt);
>> +	struct data__file *d = block_fmt->file;
>> +	bool pairs_left  = hist_entry__has_pairs(left);
>> +	bool pairs_right = hist_entry__has_pairs(right);
>> +	struct hist_entry *p_right, *p_left;
>> +	s64 l, r;
>> +
>> +	if (!pairs_left && !pairs_right)
>> +		return 0;
>> +
>> +	if (!pairs_left || !pairs_right)
>> +		return pairs_left ? -1 : 1;
>> +
>> +	p_left  = get_pair_data(left, d);
>> +	p_right  = get_pair_data(right, d);
>> +
>> +	if (!p_left && !p_right)
>> +		return 0;
>> +
>> +	if (!p_left || !p_right)
>> +		return p_left ?  -1 : 1;
>> +
>> +	l = abs(p_left->diff.cycles_diff);
>> +	r = abs(p_right->diff.cycles_diff);
>> +
>> +	return r - l;
>> +}
>> +
>> +static int64_t block_diff_sort(struct perf_hpp_fmt *fmt,
>> +			       struct hist_entry *left, struct hist_entry *right)
>> +{
>> +	return block_cycles_diff_cmp(fmt, right, left);
>> +}
>> +
>> +static int block_diff_header(struct perf_hpp_fmt *fmt __maybe_unused,
>> +			     struct perf_hpp *hpp,
>> +			     struct hists *hists __maybe_unused,
>> +			     int line __maybe_unused,
>> +			     int *span __maybe_unused)
>> +{
>> +	return scnprintf(hpp->buf, hpp->size, "Cycles diff");
>> +}
>> +
>> +static int block_diff_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>> +			    struct hist_entry *he)
>> +{
>> +	struct block_hpp_fmt *block_fmt = container_of(fmt,
>> +						       struct block_hpp_fmt,
>> +						       fmt);
>> +	struct data__file *d = block_fmt->file;
>> +	struct hist_entry *pair = get_pair_data(he, d);
>> +
>> +	if (pair && pair->diff.computed) {
>> +		return scnprintf(hpp->buf, hpp->size, "%*ld", block_fmt->width,
>> +				 pair->diff.cycles_diff);
>> +	}
>> +
>> +	return scnprintf(hpp->buf, hpp->size, "%*s", block_fmt->width, " ");
>> +}
>> +
>> +static int block_diff_width(struct perf_hpp_fmt *fmt,
>> +			    struct perf_hpp *hpp __maybe_unused,
>> +			    struct hists *hists __maybe_unused)
>> +{
>> +	struct block_hpp_fmt *block_fmt =
>> +		container_of(fmt, struct block_hpp_fmt, fmt);
>> +
>> +	return block_fmt->width;
>> +}
>> +
>> +static int block_sym_width(struct perf_hpp_fmt *fmt __maybe_unused,
>> +			    struct perf_hpp *hpp __maybe_unused,
>> +			    struct hists *hists __maybe_unused)
>> +{
>> +	return 23;
>> +}
>> +
>> +static int block_sym_entry(struct perf_hpp_fmt *fmt __maybe_unused,
>> +			   struct perf_hpp *hpp, struct hist_entry *he)
>> +{
>> +	struct block_info *bi = he->block_info;
>> +
>> +	return scnprintf(hpp->buf, hpp->size, "%s (%lx:%lx)",
>> +			 bi->sym->name, bi->sym->start + bi->start,
>> +			 bi->sym->start + bi->end);
>> +}
>> +
>> +static int block_sym_header(struct perf_hpp_fmt *fmt __maybe_unused,
>> +			    struct perf_hpp *hpp,
>> +			    struct hists *hists __maybe_unused,
>> +			    int line __maybe_unused,
>> +			    int *span __maybe_unused)
>> +{
>> +	return scnprintf(hpp->buf, hpp->size, "Basic block (start:end)");
>> +}
>> +
>> +static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
>> +{
>> +	he->not_collen = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static void hists_block_printf_sorted(struct hists *hists)
>> +{
>> +	hists__fprintf(hists, true, 0, 0, 0, stdout, true);
>> +}
>> +
>> +static void init_block_hists_fmt(void)
>> +{
>> +	struct block_hists *hists_base = &data__files[0].block_hists;
>> +	struct data__file *d;
>> +	int i;
>> +	struct perf_hpp_fmt *fmt;
>> +
>> +	perf_hpp__reset_output_field(&hists_base->list);
>> +
>> +	hists_base->list.nr_header_lines = 1;
>> +
>> +	data__for_each_file_new(i, d) {
>> +		struct block_hpp_fmt *block_fmt = &d->block_hists.block_fmt;
>> +
>> +		fmt = &block_fmt->fmt;
>> +		block_fmt->file = d;
>> +		block_fmt->width = 11;
>> +
>> +		INIT_LIST_HEAD(&fmt->list);
>> +		INIT_LIST_HEAD(&fmt->sort_list);
>> +
>> +		fmt->sort = block_diff_sort;
>> +		fmt->header = block_diff_header;
>> +		fmt->entry = block_diff_entry;
>> +		fmt->width = block_diff_width;
>> +
>> +		perf_hpp_list__column_register(&hists_base->list, fmt);
>> +		perf_hpp_list__register_sort_field(&hists_base->list, fmt);
>> +	}
>> +
>> +	/* fmt for description */
>> +	fmt = &hists_base->desc_fmt;
>> +
>> +	fmt->width = block_sym_width;
>> +	fmt->entry = block_sym_entry;
>> +	fmt->header = block_sym_header;
>> +	fmt->sort = hist_entry__cmp_nop;
>> +
>> +	perf_hpp_list__column_register(&hists_base->list, fmt);
>> +	perf_hpp_list__register_sort_field(&hists_base->list, fmt);
>> +}
>> +
>>   static void basic_block_process(void)
>>   {
>>   	struct hists *hists_base = &data__files[0].block_hists.hists;
>> @@ -1182,6 +1346,10 @@ static void basic_block_process(void)
>>   		compute_block_hists_diff(&data__files[0].block_hists, d);
>>   	}
>>   
>> +	init_block_hists_fmt();
>> +	hists__output_resort_cb(hists_base, NULL, filter_cb);
>> +	hists_block_printf_sorted(hists_base);
>> +
>>   	data__for_each_file(i, d) {
>>   		hists__delete_entries(&d->block_hists.sym_hists);
>>   		hists__delete_entries(&d->block_hists.hists);
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index 3810460..ae95191 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -1846,7 +1846,7 @@ static void output_resort(struct hists *hists, struct ui_progress *prog,
>>   		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits, use_callchain);
>>   		hists__inc_stats(hists, n);
>>   
>> -		if (!n->filtered)
>> +		if ((!n->filtered) && (!n->not_collen))
>>   			hists__calc_col_len(hists, n);
>>   
>>   		if (prog)
>> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
>> index de9e61a..1e921fe 100644
>> --- a/tools/perf/util/sort.h
>> +++ b/tools/perf/util/sort.h
>> @@ -121,6 +121,7 @@ struct hist_entry {
>>   
>>   	char			level;
>>   	u8			filtered;
>> +	bool			not_collen;
>>   
>>   	u16			callchain_size;
>>   	union {
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-05-24  0:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20 13:27 [PATCH v1 0/9] perf diff: diff cycles at basic block level Jin Yao
2019-05-20 13:27 ` [PATCH v1 1/9] perf util: Create block_info structure Jin Yao
2019-05-20 13:27 ` [PATCH v1 2/9] perf util: Add block_info in hist_entry Jin Yao
2019-05-20 13:27 ` [PATCH v1 3/9] perf diff: Check if all data files with branch stacks Jin Yao
2019-05-20 13:27 ` [PATCH v1 4/9] perf diff: Get a list of symbols(functions) Jin Yao
2019-05-20 13:27 ` [PATCH v1 5/9] perf diff: Use hists to manage basic blocks Jin Yao
2019-05-20 13:27 ` [PATCH v1 6/9] perf diff: Link same basic blocks among different data files Jin Yao
2019-05-20 13:27 ` [PATCH v1 7/9] perf diff: Compute cycles diff of basic blocks Jin Yao
2019-05-20 13:27 ` [PATCH v1 8/9] perf diff: Print the basic block cycles diff Jin Yao
2019-05-22 14:04   ` Jiri Olsa
2019-05-24  0:39     ` Jin, Yao
2019-05-20 13:27 ` [PATCH v1 9/9] perf diff: Documentation --basic-block option Jin Yao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).