linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/14] perf, diff: multiple perf.data file support
@ 2012-12-13 13:08 Jiri Olsa
  2012-12-13 13:08 ` [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute Jiri Olsa
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

hi,
adding support to display diff for more than 2 perf.data files.

The basic idea is describe in the initial RFC in here:
https://lkml.org/lkml/2012/11/28/200

It's rebased on current Arnaldo's perf core with Namhyung's
changes.

Attached patches:
  01/14 perf diff: Use internal rb tree for hists__precompute
  02/14 perf hists: Rename hist_entry__add_pair arguments
  03/14 perf tools: Add struct perf_hpp_fmt into hpp callbacks
  04/14 perf tools: Centralize default columns init in perf_hpp__init
  05/14 perf diff: Introducing diff_data object to hold files
  06/14 perf diff: Switching the base hists to be pairs head
  07/14 perf hists: Marking dummy hists entries
  08/14 perf diff: Display data file info ahead of the diff output
  09/14 perf diff: Move diff related columns into diff command
  10/14 perf diff: Move columns into struct data__file
  11/14 perf diff: Change diff command to work over multiple data files
  12/14 perf diff: Update perf diff documentation for multiple data comparison
  13/14 perf diff: Making compute functions static
  14/14 perf diff: Add generic order option for compute sorting

thanks,
jirka


Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-diff.txt |  79 +++++++++--
 tools/perf/builtin-diff.c              | 673 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
 tools/perf/builtin-report.c            |   1 -
 tools/perf/ui/browsers/hists.c         |  12 +-
 tools/perf/ui/gtk/browser.c            |  12 +-
 tools/perf/ui/hist.c                   | 298 ++++++++++-------------------------------
 tools/perf/ui/setup.c                  |   1 -
 tools/perf/ui/stdio/hist.c             |   4 +-
 tools/perf/util/hist.c                 |   1 +
 tools/perf/util/hist.h                 |  24 +---
 tools/perf/util/sort.h                 |   9 +-
 11 files changed, 717 insertions(+), 397 deletions(-)

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

* [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
@ 2012-12-13 13:08 ` Jiri Olsa
  2013-05-23  9:17   ` Arnaldo Carvalho de Melo
  2013-05-31 11:44   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2012-12-13 13:09 ` [PATCH 02/14] perf hists: Rename hist_entry__add_pair arguments Jiri Olsa
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

There's missing change for hists__precompute to iterate either
entries_collapsed or entries_in tree. The change was initiated
for hists_compute_resort function in commit:

  2a2d3ce perf diff: Use internal rb tree for compute resort

but was missing for hists__precompute function changes.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4af0b58..1f0a22d 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -322,13 +322,22 @@ static void hists__baseline_only(struct hists *hists)
 
 static void hists__precompute(struct hists *hists)
 {
-	struct rb_node *next = rb_first(&hists->entries);
+	struct rb_root *root;
+	struct rb_node *next;
+
+	if (sort__need_collapse)
+		root = &hists->entries_collapsed;
+	else
+		root = hists->entries_in;
 
+	next = rb_first(root);
 	while (next != NULL) {
-		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
-		struct hist_entry *pair = hist_entry__next_pair(he);
+		struct hist_entry *he, *pair;
 
-		next = rb_next(&he->rb_node);
+		he   = rb_entry(next, struct hist_entry, rb_node_in);
+		pair = hist_entry__next_pair(he);
+
+		next = rb_next(&he->rb_node_in);
 		if (!pair)
 			continue;
 
-- 
1.7.11.7


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

* [PATCH 02/14] perf hists: Rename hist_entry__add_pair arguments
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
  2012-12-13 13:08 ` [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2013-05-31 11:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2012-12-13 13:09 ` [PATCH 03/14] perf tools: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

The current logic is to attach pair to the leader hist_entry.
Arguments of hist_entry__add_pair function were placed
the other way round.. driving me crazy.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index a1c0d56..7425f8d 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -115,10 +115,10 @@ static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
 	return NULL;
 }
 
-static inline void hist_entry__add_pair(struct hist_entry *he,
-					struct hist_entry *pair)
+static inline void hist_entry__add_pair(struct hist_entry *pair,
+					struct hist_entry *he)
 {
-	list_add_tail(&he->pairs.head, &pair->pairs.node);
+	list_add_tail(&pair->pairs.node, &he->pairs.head);
 }
 
 enum sort_type {
-- 
1.7.11.7


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

* [PATCH 03/14] perf tools: Add struct perf_hpp_fmt into hpp callbacks
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
  2012-12-13 13:08 ` [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute Jiri Olsa
  2012-12-13 13:09 ` [PATCH 02/14] perf hists: Rename hist_entry__add_pair arguments Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2013-05-23  9:56   ` Arnaldo Carvalho de Melo
  2012-12-13 13:09 ` [PATCH 04/14] perf tools: Centralize default columns init in perf_hpp__init Jiri Olsa
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Adding 'struct perf_hpp_fmt' into hpp callbacks, so commands
can access their private data.

It'll be handy for diff command in future to be able to access
file related data for each column.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c |  10 +--
 tools/perf/ui/gtk/browser.c    |  10 +--
 tools/perf/ui/hist.c           | 159 +++++++++++++++++++++++++++--------------
 tools/perf/ui/stdio/hist.c     |   4 +-
 tools/perf/util/hist.h         |  10 +--
 5 files changed, 127 insertions(+), 66 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 57b82c2..b560abd 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -568,8 +568,10 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 }
 
 #define HPP__COLOR_FN(_name, _field)					\
-static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp,	\
-					     struct hist_entry *he)	\
+static int hist_browser__hpp_color_ ## _name(				\
+			struct perf_hpp_fmt *fmt __maybe_unused,	\
+			struct perf_hpp *hpp,				\
+			struct hist_entry *he)				\
 {									\
 	struct hists *hists = he->hists;				\
 	double percent = 100.0 * he->stat._field / hists->stats.total_period; \
@@ -647,7 +649,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			if (fmt->color) {
 				hpp.ptr = &percent;
 				/* It will set percent for us. See HPP__COLOR_FN above. */
-				width -= fmt->color(&hpp, entry);
+				width -= fmt->color(fmt, &hpp, entry);
 
 				ui_browser__set_percent_color(&browser->b, percent, current_entry);
 
@@ -661,7 +663,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 				if (!current_entry || !browser->b.navkeypressed)
 					ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
 			} else {
-				width -= fmt->entry(&hpp, entry);
+				width -= fmt->entry(fmt, &hpp, entry);
 				slsmg_printf("%s", s);
 			}
 
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index e59ba33..e3ae11a 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -46,7 +46,9 @@ static const char *perf_gtk__get_percent_color(double percent)
 }
 
 #define HPP__COLOR_FN(_name, _field)						\
-static int perf_gtk__hpp_color_ ## _name(struct perf_hpp *hpp,			\
+static int									\
+perf_gtk__hpp_color_ ## _name(struct perf_hpp_fmt *fmt __maybe_unused,		\
+					 struct perf_hpp *hpp,			\
 					 struct hist_entry *he)			\
 {										\
 	struct hists *hists = he->hists;					\
@@ -129,7 +131,7 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	col_idx = 0;
 
 	perf_hpp__for_each_format(fmt) {
-		fmt->header(&hpp);
+		fmt->header(fmt, &hpp);
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
 							    -1, s,
 							    renderer, "markup",
@@ -163,9 +165,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 		perf_hpp__for_each_format(fmt) {
 			if (fmt->color)
-				fmt->color(&hpp, h);
+				fmt->color(fmt, &hpp, h);
 			else
-				fmt->entry(&hpp, h);
+				fmt->entry(fmt, &hpp, h);
 
 			gtk_list_store_set(store, &iter, col_idx++, s, -1);
 		}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 1889c12..093ccf3 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -6,17 +6,20 @@
 
 
 /* hist period print (hpp) functions */
-static int hpp__header_overhead(struct perf_hpp *hpp)
+static int hpp__header_overhead(struct perf_hpp_fmt *fmt __maybe_unused,
+				struct perf_hpp *hpp)
 {
 	return scnprintf(hpp->buf, hpp->size, "Overhead");
 }
 
-static int hpp__width_overhead(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_overhead(struct perf_hpp_fmt *fmt __maybe_unused,
+			       struct perf_hpp *hpp __maybe_unused)
 {
 	return 8;
 }
 
-static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_overhead(struct perf_hpp_fmt *fmt __maybe_unused,
+			       struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period / hists->stats.total_period;
@@ -24,7 +27,8 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 	return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
 }
 
-static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_overhead(struct perf_hpp_fmt *_fmt __maybe_unused,
+			       struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period / hists->stats.total_period;
@@ -33,19 +37,22 @@ static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
-static int hpp__header_overhead_sys(struct perf_hpp *hpp)
+static int hpp__header_overhead_sys(struct perf_hpp_fmt *_fmt __maybe_unused,
+				    struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "sys");
 }
 
-static int hpp__width_overhead_sys(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_overhead_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+				   struct perf_hpp *hpp __maybe_unused)
 {
 	return 7;
 }
 
-static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_overhead_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+				   struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_sys / hists->stats.total_period;
@@ -53,7 +60,8 @@ static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 	return percent_color_snprintf(hpp->buf, hpp->size, "%6.2f%%", percent);
 }
 
-static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_overhead_sys(struct perf_hpp_fmt *_fmt __maybe_unused,
+				   struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_sys / hists->stats.total_period;
@@ -62,19 +70,22 @@ static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
-static int hpp__header_overhead_us(struct perf_hpp *hpp)
+static int hpp__header_overhead_us(struct perf_hpp_fmt *_fmt __maybe_unused,
+				   struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "user");
 }
 
-static int hpp__width_overhead_us(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_overhead_us(struct perf_hpp_fmt *fmt __maybe_unused,
+				  struct perf_hpp *hpp __maybe_unused)
 {
 	return 7;
 }
 
-static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_overhead_us(struct perf_hpp_fmt *fmt __maybe_unused,
+				  struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_us / hists->stats.total_period;
@@ -82,7 +93,8 @@ static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 	return percent_color_snprintf(hpp->buf, hpp->size, "%6.2f%%", percent);
 }
 
-static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_overhead_us(struct perf_hpp_fmt *_fmt __maybe_unused,
+				  struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_us / hists->stats.total_period;
@@ -91,18 +103,24 @@ static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
-static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
+static int
+hpp__header_overhead_guest_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+			       struct perf_hpp *hpp)
 {
 	return scnprintf(hpp->buf, hpp->size, "guest sys");
 }
 
-static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __maybe_unused)
+static int
+hpp__width_overhead_guest_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+			      struct perf_hpp *hpp __maybe_unused)
 {
 	return 9;
 }
 
-static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
-					 struct hist_entry *he)
+static int
+hpp__color_overhead_guest_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+			      struct perf_hpp *hpp,
+			      struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_guest_sys / hists->stats.total_period;
@@ -110,8 +128,10 @@ static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
 	return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%% ", percent);
 }
 
-static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
-					 struct hist_entry *he)
+static int
+hpp__entry_overhead_guest_sys(struct perf_hpp_fmt *_fmt __maybe_unused,
+			      struct perf_hpp *hpp,
+			      struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_guest_sys / hists->stats.total_period;
@@ -120,18 +140,24 @@ static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
 	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
-static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
+static int
+hpp__header_overhead_guest_us(struct perf_hpp_fmt *fmt __maybe_unused,
+			      struct perf_hpp *hpp)
 {
 	return scnprintf(hpp->buf, hpp->size, "guest usr");
 }
 
-static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __maybe_unused)
+static int
+hpp__width_overhead_guest_us(struct perf_hpp_fmt *fmt __maybe_unused,
+			     struct perf_hpp *hpp __maybe_unused)
 {
 	return 9;
 }
 
-static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
-					struct hist_entry *he)
+static int
+hpp__color_overhead_guest_us(struct perf_hpp_fmt *fmt __maybe_unused,
+			     struct perf_hpp *hpp,
+			     struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_guest_us / hists->stats.total_period;
@@ -139,8 +165,10 @@ static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
 	return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%% ", percent);
 }
 
-static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
-					struct hist_entry *he)
+static int
+hpp__entry_overhead_guest_us(struct perf_hpp_fmt *_fmt __maybe_unused,
+			     struct perf_hpp *hpp,
+			     struct hist_entry *he)
 {
 	struct hists *hists = he->hists;
 	double percent = 100.0 * he->stat.period_guest_us / hists->stats.total_period;
@@ -149,12 +177,14 @@ static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
 	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
-static int hpp__header_baseline(struct perf_hpp *hpp)
+static int hpp__header_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+				struct perf_hpp *hpp)
 {
 	return scnprintf(hpp->buf, hpp->size, "Baseline");
 }
 
-static int hpp__width_baseline(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+			       struct perf_hpp *hpp __maybe_unused)
 {
 	return 8;
 }
@@ -175,7 +205,8 @@ static double baseline_percent(struct hist_entry *he)
 	return percent;
 }
 
-static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+			       struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = baseline_percent(he);
 
@@ -185,7 +216,8 @@ static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
 		return scnprintf(hpp->buf, hpp->size, "        ");
 }
 
-static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
+			       struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = baseline_percent(he);
 	const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
@@ -196,57 +228,67 @@ static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
 		return scnprintf(hpp->buf, hpp->size, "            ");
 }
 
-static int hpp__header_samples(struct perf_hpp *hpp)
+static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
+			       struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Samples");
 }
 
-static int hpp__width_samples(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_samples(struct perf_hpp_fmt *fmt __maybe_unused,
+			      struct perf_hpp *hpp __maybe_unused)
 {
 	return 11;
 }
 
-static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
+			      struct perf_hpp *hpp, struct hist_entry *he)
 {
 	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%11" PRIu64;
 
 	return scnprintf(hpp->buf, hpp->size, fmt, he->stat.nr_events);
 }
 
-static int hpp__header_period(struct perf_hpp *hpp)
+static int hpp__header_period(struct perf_hpp_fmt *_fmt __maybe_unused,
+			      struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Period");
 }
 
-static int hpp__width_period(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_period(struct perf_hpp_fmt *fmt __maybe_unused,
+			     struct perf_hpp *hpp __maybe_unused)
 {
 	return 12;
 }
 
-static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_period(struct perf_hpp_fmt *_fmt __maybe_unused,
+			     struct perf_hpp *hpp, struct hist_entry *he)
 {
 	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
 
 	return scnprintf(hpp->buf, hpp->size, fmt, he->stat.period);
 }
 
-static int hpp__header_period_baseline(struct perf_hpp *hpp)
+static int hpp__header_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
+				       struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Period Base");
 }
 
-static int hpp__width_period_baseline(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_period_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+				      struct perf_hpp *hpp __maybe_unused)
 {
 	return 12;
 }
 
-static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
+				      struct perf_hpp *hpp,
+				      struct hist_entry *he)
 {
 	struct hist_entry *pair = hist_entry__next_pair(he);
 	u64 period = pair ? pair->stat.period : 0;
@@ -254,19 +296,23 @@ static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *h
 
 	return scnprintf(hpp->buf, hpp->size, fmt, period);
 }
-static int hpp__header_delta(struct perf_hpp *hpp)
+
+static int hpp__header_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
+			     struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
 }
 
-static int hpp__width_delta(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_delta(struct perf_hpp_fmt *fmt __maybe_unused,
+			    struct perf_hpp *hpp __maybe_unused)
 {
 	return 7;
 }
 
-static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
+			    struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hist_entry *pair = hist_entry__next_pair(he);
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
@@ -287,19 +333,22 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
-static int hpp__header_ratio(struct perf_hpp *hpp)
+static int hpp__header_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
+			     struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Ratio");
 }
 
-static int hpp__width_ratio(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
+			    struct perf_hpp *hpp __maybe_unused)
 {
 	return 14;
 }
 
-static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
+			    struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hist_entry *pair = hist_entry__next_pair(he);
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
@@ -319,19 +368,22 @@ static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he)
 	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
-static int hpp__header_wdiff(struct perf_hpp *hpp)
+static int hpp__header_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
+			     struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Weighted diff");
 }
 
-static int hpp__width_wdiff(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
+			    struct perf_hpp *hpp __maybe_unused)
 {
 	return 14;
 }
 
-static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
+			    struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hist_entry *pair = hist_entry__next_pair(he);
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
@@ -351,19 +403,22 @@ static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he)
 	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
-static int hpp__header_formula(struct perf_hpp *hpp)
+static int hpp__header_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
+			       struct perf_hpp *hpp)
 {
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%70s";
 
 	return scnprintf(hpp->buf, hpp->size, fmt, "Formula");
 }
 
-static int hpp__width_formula(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_formula(struct perf_hpp_fmt *fmt __maybe_unused,
+			      struct perf_hpp *hpp __maybe_unused)
 {
 	return 70;
 }
 
-static int hpp__entry_formula(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
+			      struct perf_hpp *hpp, struct hist_entry *he)
 {
 	struct hist_entry *pair = hist_entry__next_pair(he);
 	const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
@@ -471,9 +526,9 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
 			first = false;
 
 		if (color && fmt->color)
-			ret = fmt->color(hpp, he);
+			ret = fmt->color(fmt, hpp, he);
 		else
-			ret = fmt->entry(hpp, he);
+			ret = fmt->entry(fmt, hpp, he);
 
 		advance_hpp(hpp, ret);
 	}
@@ -513,7 +568,7 @@ unsigned int hists__sort_list_width(struct hists *hists)
 		if (i)
 			ret += 2;
 
-		ret += fmt->width(NULL);
+		ret += fmt->width(fmt, NULL);
 	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 0eae3b2..531bc7a 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -363,7 +363,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		else
 			first = false;
 
-		fmt->header(&dummy_hpp);
+		fmt->header(fmt, &dummy_hpp);
 		fprintf(fp, "%s", bf);
 	}
 
@@ -408,7 +408,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		else
 			first = false;
 
-		width = fmt->width(&dummy_hpp);
+		width = fmt->width(fmt, &dummy_hpp);
 		for (i = 0; i < width; i++)
 			fprintf(fp, ".");
 	}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index d3664ab..ae6754d 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -127,10 +127,12 @@ struct perf_hpp {
 };
 
 struct perf_hpp_fmt {
-	int (*header)(struct perf_hpp *hpp);
-	int (*width)(struct perf_hpp *hpp);
-	int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
-	int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+	int (*header)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp);
+	int (*width)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp);
+	int (*color)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		     struct hist_entry *he);
+	int (*entry)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		     struct hist_entry *he);
 
 	struct list_head list;
 };
-- 
1.7.11.7


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

* [PATCH 04/14] perf tools: Centralize default columns init in perf_hpp__init
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (2 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 03/14] perf tools: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2013-05-23 10:01   ` Arnaldo Carvalho de Melo
  2012-12-13 13:09 ` [PATCH 05/14] perf diff: Introducing diff_data object to hold files Jiri Olsa
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Now when diff command is separated from other standard outputs,
we can use perf_hpp__init to initialize all standard columns.

Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
and removing extra enable calls.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c    | 1 -
 tools/perf/ui/browsers/hists.c | 2 --
 tools/perf/ui/gtk/browser.c    | 2 --
 tools/perf/ui/hist.c           | 2 ++
 tools/perf/ui/setup.c          | 1 -
 5 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 5134acf..fc25100 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -692,7 +692,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		setup_browser(true);
 	else {
 		use_browser = 0;
-		perf_hpp__column_enable(PERF_HPP__OVERHEAD);
 		perf_hpp__init();
 	}
 
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b560abd..d774efc 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -589,8 +589,6 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
 
 void hist_browser__init_hpp(void)
 {
-	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
-
 	perf_hpp__init();
 
 	perf_hpp__format[PERF_HPP__OVERHEAD].color =
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index e3ae11a..0d94b3ba0 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -76,8 +76,6 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
 
 void perf_gtk__init_hpp(void)
 {
-	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
-
 	perf_hpp__init();
 
 	perf_hpp__format[PERF_HPP__OVERHEAD].color =
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 093ccf3..19e3a78 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -468,6 +468,8 @@ LIST_HEAD(perf_hpp__list);
 
 void perf_hpp__init(void)
 {
+	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
+
 	if (symbol_conf.show_cpu_utilization) {
 		perf_hpp__column_enable(PERF_HPP__OVERHEAD_SYS);
 		perf_hpp__column_enable(PERF_HPP__OVERHEAD_US);
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 166f13d..ebb4cc1 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -30,7 +30,6 @@ void setup_browser(bool fallback_to_pager)
 		if (fallback_to_pager)
 			setup_pager();
 
-		perf_hpp__column_enable(PERF_HPP__OVERHEAD);
 		perf_hpp__init();
 		break;
 	}
-- 
1.7.11.7


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

* [PATCH 05/14] perf diff: Introducing diff_data object to hold files
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (3 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 04/14] perf tools: Centralize default columns init in perf_hpp__init Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2012-12-13 13:09 ` [PATCH 06/14] perf diff: Switching the base hists to be pairs head Jiri Olsa
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Introducing struct diff_data to hold data file specifics.
It will be handy when dealing with more than 2 data files.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 143 ++++++++++++++++++++++++++++++----------------
 1 file changed, 95 insertions(+), 48 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 1f0a22d..d488ee9 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -19,10 +19,24 @@
 
 #include <stdlib.h>
 
-static char const *input_old = "perf.data.old",
-		  *input_new = "perf.data";
-static char	  diff__default_sort_order[] = "dso,symbol";
-static bool  force;
+struct data__file {
+	struct perf_session	*session;
+	const char		*file;
+	int			 idx;
+};
+
+static struct data__file *data__files;
+static int data__files_cnt;
+
+#define data__for_each_file_start(i, d, s)	\
+	for (i = s, d = &data__files[s];	\
+	     i < data__files_cnt;		\
+	     i++, d = &data__files[i])
+
+#define data__for_each_file(i, d) data__for_each_file_start(i, d, 0)
+
+static char diff__default_sort_order[] = "dso,symbol";
+static bool force;
 static bool show_period;
 static bool show_formula;
 static bool show_baseline_only;
@@ -468,56 +482,62 @@ static void hists__process(struct hists *old, struct hists *new)
 	hists__fprintf(new, true, 0, 0, stdout);
 }
 
-static int __cmd_diff(void)
+static void data_process(void)
 {
-	int ret, i;
-#define older (session[0])
-#define newer (session[1])
-	struct perf_session *session[2];
-	struct perf_evlist *evlist_new, *evlist_old;
-	struct perf_evsel *evsel;
+	struct perf_evlist *evlist_old = data__files[0].session->evlist;
+	struct perf_evlist *evlist_new = data__files[1].session->evlist;
+	struct perf_evsel *evsel_old;
 	bool first = true;
 
-	older = perf_session__new(input_old, O_RDONLY, force, false,
-				  &tool);
-	newer = perf_session__new(input_new, O_RDONLY, force, false,
-				  &tool);
-	if (session[0] == NULL || session[1] == NULL)
-		return -ENOMEM;
+	list_for_each_entry(evsel_old, &evlist_old->entries, node) {
+		struct perf_evsel *evsel_new;
 
-	for (i = 0; i < 2; ++i) {
-		ret = perf_session__process_events(session[i], &tool);
-		if (ret)
-			goto out_delete;
-	}
+		evsel_new = evsel_match(evsel_old, evlist_new);
+		if (!evsel_new)
+			continue;
 
-	evlist_old = older->evlist;
-	evlist_new = newer->evlist;
+		fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
+			perf_evsel__name(evsel_old));
 
-	perf_evlist__collapse_resort(evlist_old);
-	perf_evlist__collapse_resort(evlist_new);
+		first = false;
 
-	list_for_each_entry(evsel, &evlist_new->entries, node) {
-		struct perf_evsel *evsel_old;
+		hists__process(&evsel_old->hists, &evsel_new->hists);
+	}
+}
 
-		evsel_old = evsel_match(evsel, evlist_old);
-		if (!evsel_old)
-			continue;
+static int __cmd_diff(void)
+{
+	struct data__file *d;
+	int ret = -EINVAL, i;
+
+	data__for_each_file(i, d) {
+		d->session = perf_session__new(d->file, O_RDONLY, force,
+					       false, &tool);
+		if (!d->session) {
+			pr_err("Failed to open %s\n", d->file);
+			ret = -ENOMEM;
+			goto out_delete;
+		}
 
-		fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
-			perf_evsel__name(evsel));
+		ret = perf_session__process_events(d->session, &tool);
+		if (ret) {
+			pr_err("Failed to process %s\n", d->file);
+			goto out_delete;
+		}
 
-		first = false;
+		perf_evlist__collapse_resort(d->session->evlist);
+	}
+
+	data_process();
 
-		hists__process(&evsel_old->hists, &evsel->hists);
+ out_delete:
+	data__for_each_file(i, d) {
+		if (d->session)
+			perf_session__delete(d->session);
 	}
 
-out_delete:
-	for (i = 0; i < 2; ++i)
-		perf_session__delete(session[i]);
+	free(data__files);
 	return ret;
-#undef older
-#undef newer
 }
 
 static const char * const diff_usage[] = {
@@ -590,28 +610,55 @@ static void ui_init(void)
 	}
 }
 
-int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
+static int data_init(int argc, const char **argv)
 {
-	sort_order = diff__default_sort_order;
-	argc = parse_options(argc, argv, options, diff_usage, 0);
+	struct data__file *d;
+	static const char *defaults[] = {
+		"perf.data.old",
+		"perf.data",
+	};
+	int i;
+
+	data__files_cnt = 2;
+
 	if (argc) {
 		if (argc > 2)
 			usage_with_options(diff_usage, options);
 		if (argc == 2) {
-			input_old = argv[0];
-			input_new = argv[1];
+			defaults[0] = argv[0];
+			defaults[1] = argv[1];
 		} else
-			input_new = argv[0];
+			defaults[1] = argv[0];
 	} else if (symbol_conf.default_guest_vmlinux_name ||
 		   symbol_conf.default_guest_kallsyms) {
-		input_old = "perf.data.host";
-		input_new = "perf.data.guest";
+		defaults[0] = "perf.data.host";
+		defaults[1] = "perf.data.guest";
 	}
 
+	data__files = zalloc(sizeof(*data__files) * data__files_cnt);
+	if (!data__files)
+		return -ENOMEM;
+
+	data__for_each_file(i, d) {
+		d->file = defaults[i];
+		d->idx  = i;
+	}
+
+	return 0;
+}
+
+int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
+{
+	sort_order = diff__default_sort_order;
+	argc = parse_options(argc, argv, options, diff_usage, 0);
+
 	symbol_conf.exclude_other = false;
 	if (symbol__init() < 0)
 		return -1;
 
+	if (data_init(argc, argv) < 0)
+		return -1;
+
 	ui_init();
 
 	setup_sorting(diff_usage, options);
-- 
1.7.11.7


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

* [PATCH 06/14] perf diff: Switching the base hists to be pairs head
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (4 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 05/14] perf diff: Introducing diff_data object to hold files Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2012-12-13 13:09 ` [PATCH 07/14] perf hists: Marking dummy hists entries Jiri Olsa
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Making the baseline hists to act as a pairs head.

So far we don't care which hists act as a pairs head, because
we have only 2 files to deal with and any of them is suitable
to do the job.

But if we want to process more files, we need to pick up one
hists to act as pairs head, and the baseline hists is the most
suitable.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 60 +++++++++++++++++++++++------------------------
 tools/perf/ui/hist.c      | 25 ++++----------------
 2 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index d488ee9..a3e3f7a 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -167,34 +167,34 @@ double perf_diff__period_percent(struct hist_entry *he, u64 period)
 
 double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
 {
-	double new_percent = perf_diff__period_percent(he, he->stat.period);
-	double old_percent = perf_diff__period_percent(pair, pair->stat.period);
+	double old_percent = perf_diff__period_percent(he, he->stat.period);
+	double new_percent = perf_diff__period_percent(pair, pair->stat.period);
 
-	he->diff.period_ratio_delta = new_percent - old_percent;
-	he->diff.computed = true;
-	return he->diff.period_ratio_delta;
+	pair->diff.period_ratio_delta = new_percent - old_percent;
+	pair->diff.computed = true;
+	return pair->diff.period_ratio_delta;
 }
 
 double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
 {
-	double new_period = he->stat.period;
-	double old_period = pair->stat.period;
+	double old_period = he->stat.period ?: 1;
+	double new_period = pair->stat.period;
 
-	he->diff.computed = true;
-	he->diff.period_ratio = new_period / old_period;
-	return he->diff.period_ratio;
+	pair->diff.computed = true;
+	pair->diff.period_ratio = new_period / old_period;
+	return pair->diff.period_ratio;
 }
 
 s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
 {
-	u64 new_period = he->stat.period;
-	u64 old_period = pair->stat.period;
+	u64 old_period = he->stat.period;
+	u64 new_period = pair->stat.period;
 
-	he->diff.computed = true;
-	he->diff.wdiff = new_period * compute_wdiff_w2 -
-			 old_period * compute_wdiff_w1;
+	pair->diff.computed = true;
+	pair->diff.wdiff = new_period * compute_wdiff_w2 -
+			   old_period * compute_wdiff_w1;
 
-	return he->diff.wdiff;
+	return pair->diff.wdiff;
 }
 
 static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
@@ -203,15 +203,15 @@ static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
 	return scnprintf(buf, size,
 			 "(%" PRIu64 " * 100 / %" PRIu64 ") - "
 			 "(%" PRIu64 " * 100 / %" PRIu64 ")",
-			  he->stat.period, he->hists->stats.total_period,
-			  pair->stat.period, pair->hists->stats.total_period);
+			  pair->stat.period, pair->hists->stats.total_period,
+			  he->stat.period, he->hists->stats.total_period);
 }
 
 static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
 			 char *buf, size_t size)
 {
-	double new_period = he->stat.period;
-	double old_period = pair->stat.period;
+	double old_period = he->stat.period;
+	double new_period = pair->stat.period;
 
 	return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period);
 }
@@ -219,8 +219,8 @@ static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
 static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair,
 			 char *buf, size_t size)
 {
-	u64 new_period = he->stat.period;
-	u64 old_period = pair->stat.period;
+	u64 old_period = he->stat.period;
+	u64 new_period = pair->stat.period;
 
 	return scnprintf(buf, size,
 		  "(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
@@ -463,23 +463,23 @@ static void hists__compute_resort(struct hists *hists)
 	}
 }
 
-static void hists__process(struct hists *old, struct hists *new)
+static void hists__process(struct hists *base, struct hists *new)
 {
-	hists__match(new, old);
+	hists__match(base, new);
 
 	if (show_baseline_only)
-		hists__baseline_only(new);
+		hists__baseline_only(base);
 	else
-		hists__link(new, old);
+		hists__link(base, new);
 
 	if (sort_compute) {
-		hists__precompute(new);
-		hists__compute_resort(new);
+		hists__precompute(base);
+		hists__compute_resort(base);
 	} else {
-		hists__output_resort(new);
+		hists__output_resort(base);
 	}
 
-	hists__fprintf(new, true, 0, 0, stdout);
+	hists__fprintf(base, true, 0, 0, stdout);
 }
 
 static void data_process(void)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 19e3a78..a81f0c9 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -191,18 +191,8 @@ static int hpp__width_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
 
 static double baseline_percent(struct hist_entry *he)
 {
-	struct hist_entry *pair = hist_entry__next_pair(he);
-	struct hists *pair_hists = pair ? pair->hists : NULL;
-	double percent = 0.0;
-
-	if (pair) {
-		u64 total_period = pair_hists->stats.total_period;
-		u64 base_period  = pair->stat.period;
-
-		percent = 100.0 * base_period / total_period;
-	}
-
-	return percent;
+	struct hists *hists = he->hists;
+	return 100.0 * he->stat.period / hists->stats.total_period;
 }
 
 static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
@@ -210,10 +200,8 @@ static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
 {
 	double percent = baseline_percent(he);
 
-	if (hist_entry__has_pairs(he))
-		return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
-	else
-		return scnprintf(hpp->buf, hpp->size, "        ");
+	return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%",
+				      percent);
 }
 
 static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
@@ -222,10 +210,7 @@ static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
 	double percent = baseline_percent(he);
 	const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
 
-	if (hist_entry__has_pairs(he) || symbol_conf.field_sep)
-		return scnprintf(hpp->buf, hpp->size, fmt, percent);
-	else
-		return scnprintf(hpp->buf, hpp->size, "            ");
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
-- 
1.7.11.7


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

* [PATCH 07/14] perf hists: Marking dummy hists entries
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (5 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 06/14] perf diff: Switching the base hists to be pairs head Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2012-12-13 13:09 ` [PATCH 08/14] perf diff: Display data file info ahead of the diff output Jiri Olsa
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

It does not make sense to make some computation (ratio, wdiff),
when the hist_entry is 'dummy' - added via hists__link.

Adding dummy field to struct hist_entry which indicates
that it was added by hists__link and avoiding some of
the processing for such entries.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c   | 21 ++++++++++++++-------
 tools/perf/util/hist.c |  1 +
 tools/perf/util/sort.h |  3 +++
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index a81f0c9..7da98ac 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -200,8 +200,11 @@ static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
 {
 	double percent = baseline_percent(he);
 
-	return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%",
-				      percent);
+	if (!he->dummy)
+		return percent_color_snprintf(hpp->buf, hpp->size,
+					      " %6.2f%%", percent);
+	else
+		return scnprintf(hpp->buf, hpp->size, "        ");
 }
 
 static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
@@ -210,7 +213,10 @@ static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
 	double percent = baseline_percent(he);
 	const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
 
-	return scnprintf(hpp->buf, hpp->size, fmt, percent);
+	if (!he->dummy)
+		return scnprintf(hpp->buf, hpp->size, fmt, percent);
+	else
+		return scnprintf(hpp->buf, hpp->size, "            ");
 }
 
 static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
@@ -309,8 +315,7 @@ static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
 			diff = he->diff.period_ratio_delta;
 		else
 			diff = perf_diff__compute_delta(he, pair);
-	} else
-		diff = perf_diff__period_percent(he, he->stat.period);
+	}
 
 	if (fabs(diff) >= 0.01)
 		scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
@@ -340,7 +345,8 @@ static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
 	char buf[32] = " ";
 	double ratio = 0.0;
 
-	if (pair) {
+	/* No point for ratio number if we are dummy.. */
+	if (!he->dummy && pair) {
 		if (he->diff.computed)
 			ratio = he->diff.period_ratio;
 		else
@@ -375,7 +381,8 @@ static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
 	char buf[32] = " ";
 	s64 wdiff = 0;
 
-	if (pair) {
+	/* No point for wdiff number if we are dummy.. */
+	if (!he->dummy && pair) {
 		if (he->diff.computed)
 			wdiff = he->diff.wdiff;
 		else
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 37179af..677d1c9 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -761,6 +761,7 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists,
 		rb_link_node(&he->rb_node_in, parent, p);
 		rb_insert_color(&he->rb_node_in, root);
 		hists__inc_nr_entries(hists, he);
+		he->dummy = true;
 	}
 out:
 	return he;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 7425f8d..a7f6e05 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -86,6 +86,9 @@ struct hist_entry {
 
 	struct hist_entry_diff	diff;
 
+	/* We are added by hists__add_dummy_entry. */
+	bool			dummy;
+
 	/* XXX These two should move to some tree widget lib */
 	u16			row_offset;
 	u16			nr_rows;
-- 
1.7.11.7


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

* [PATCH 08/14] perf diff: Display data file info ahead of the diff output
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (6 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 07/14] perf hists: Marking dummy hists entries Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2012-12-13 13:09 ` [PATCH 09/14] perf diff: Move diff related columns into diff command Jiri Olsa
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Data files are referenced through the index of the file
on the command line. Adding list of data files for each
index to ease up navigation for user.

It's displayed only if in verbose mode.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index a3e3f7a..15a68fb 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -482,6 +482,21 @@ static void hists__process(struct hists *base, struct hists *new)
 	hists__fprintf(base, true, 0, 0, stdout);
 }
 
+static void data__fprintf(void)
+{
+	struct data__file *d;
+	int i;
+
+	fprintf(stdout, "# Data files:\n");
+
+	data__for_each_file(i, d)
+		fprintf(stdout, "#  [%d] %s %s\n",
+			d->idx, d->file,
+			!d->idx ? "(Baseline)" : "");
+
+	fprintf(stdout, "#\n");
+}
+
 static void data_process(void)
 {
 	struct perf_evlist *evlist_old = data__files[0].session->evlist;
@@ -501,6 +516,9 @@ static void data_process(void)
 
 		first = false;
 
+		if (verbose)
+			data__fprintf();
+
 		hists__process(&evsel_old->hists, &evsel_new->hists);
 	}
 }
-- 
1.7.11.7


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

* [PATCH 09/14] perf diff: Move diff related columns into diff command
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (7 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 08/14] perf diff: Display data file info ahead of the diff output Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2012-12-13 13:09 ` [PATCH 11/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Moving diff related columns into diff command, because they
are not used by any other command.

Also moving the column entry functions under generic one
with baseline as an exception.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 306 +++++++++++++++++++++++++++++++++++++++++++---
 tools/perf/ui/hist.c      | 209 +------------------------------
 tools/perf/util/hist.h    |   7 +-
 3 files changed, 292 insertions(+), 230 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 15a68fb..504b66e 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -18,6 +18,27 @@
 #include "util/util.h"
 
 #include <stdlib.h>
+#include <math.h>
+
+/* Diff command specific HPP columns. */
+enum {
+	PERF_HPP_DIFF__BASELINE,
+	PERF_HPP_DIFF__PERIOD,
+	PERF_HPP_DIFF__PERIOD_BASELINE,
+	PERF_HPP_DIFF__DELTA,
+	PERF_HPP_DIFF__RATIO,
+	PERF_HPP_DIFF__WEIGHTED_DIFF,
+	PERF_HPP_DIFF__FORMULA,
+
+	PERF_HPP_DIFF__MAX_INDEX
+};
+
+struct diff_hpp_fmt {
+	struct perf_hpp_fmt	 fmt;
+	int			 idx;
+	char			*header;
+	int			 header_width;
+};
 
 struct data__file {
 	struct perf_session	*session;
@@ -60,6 +81,47 @@ const char *compute_names[COMPUTE_MAX] = {
 
 static int compute;
 
+static int compute_2_hpp[COMPUTE_MAX] = {
+	[COMPUTE_DELTA]		= PERF_HPP_DIFF__DELTA,
+	[COMPUTE_RATIO]		= PERF_HPP_DIFF__RATIO,
+	[COMPUTE_WEIGHTED_DIFF]	= PERF_HPP_DIFF__WEIGHTED_DIFF,
+};
+
+#define MAX_COL_WIDTH 100
+
+static struct header_column {
+	const char *name;
+	int width;
+} columns[PERF_HPP_DIFF__MAX_INDEX] = {
+	[PERF_HPP_DIFF__BASELINE] = {
+		.name  = "Baseline",
+	},
+	[PERF_HPP_DIFF__PERIOD] = {
+		.name  = "Period",
+		.width = 14,
+	},
+	[PERF_HPP_DIFF__PERIOD_BASELINE] = {
+		.name  = "Base period",
+		.width = 14,
+	},
+	[PERF_HPP_DIFF__DELTA] = {
+		.name  = "Delta",
+		.width = 7,
+	},
+	[PERF_HPP_DIFF__RATIO] = {
+		.name  = "Ratio",
+		.width = 14,
+	},
+	[PERF_HPP_DIFF__WEIGHTED_DIFF] = {
+		.name  = "Weighted diff",
+		.width = 14,
+	},
+	[PERF_HPP_DIFF__FORMULA] = {
+		.name  = "Formula",
+		.width = MAX_COL_WIDTH,
+	}
+};
+
 static int setup_compute_opt_wdiff(char *opt)
 {
 	char *w1_str = opt;
@@ -597,34 +659,246 @@ static const struct option options[] = {
 	OPT_END()
 };
 
-static void ui_init(void)
+static double baseline_percent(struct hist_entry *he)
 {
-	/*
-	 * Display baseline/delta/ratio
-	 * formula/periods columns.
-	 */
-	perf_hpp__column_enable(PERF_HPP__BASELINE);
+	struct hists *hists = he->hists;
+	return 100.0 * he->stat.period / hists->stats.total_period;
+}
 
-	switch (compute) {
-	case COMPUTE_DELTA:
-		perf_hpp__column_enable(PERF_HPP__DELTA);
+static int hpp__color_baseline(struct perf_hpp_fmt *fmt,
+			       struct perf_hpp *hpp, struct hist_entry *he)
+{
+	struct diff_hpp_fmt *dfmt =
+		container_of(fmt, struct diff_hpp_fmt, fmt);
+	double percent = baseline_percent(he);
+	char pfmt[20] = " ";
+
+	if (!he->dummy) {
+		scnprintf(pfmt, 20, "%%%d.2f%%%%", dfmt->header_width - 1);
+		return percent_color_snprintf(hpp->buf, hpp->size,
+					      pfmt, percent);
+	} else
+		return scnprintf(hpp->buf, hpp->size, "%*s",
+				 dfmt->header_width, pfmt);
+}
+
+static int hpp__entry_baseline(struct hist_entry *he, char *buf, size_t size)
+{
+	double percent = baseline_percent(he);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%6.2f%%";
+	int ret = 0;
+
+	if (!he->dummy)
+		ret = scnprintf(buf, size, fmt, percent);
+
+	return ret;
+}
+
+static void
+hpp__entry_unpair(struct hist_entry *he, int idx, char *buf, size_t size)
+{
+	switch (idx) {
+	case PERF_HPP_DIFF__PERIOD_BASELINE:
+		scnprintf(buf, size, "%" PRIu64, he->stat.period);
 		break;
-	case COMPUTE_RATIO:
-		perf_hpp__column_enable(PERF_HPP__RATIO);
+
+	default:
 		break;
-	case COMPUTE_WEIGHTED_DIFF:
-		perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF);
+	}
+}
+
+static void
+hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
+		int idx, char *buf, size_t size)
+{
+	double diff;
+	double ratio;
+	s64 wdiff;
+
+	switch (idx) {
+	case PERF_HPP_DIFF__DELTA:
+		if (pair->diff.computed)
+			diff = pair->diff.period_ratio_delta;
+		else
+			diff = perf_diff__compute_delta(he, pair);
+
+		if (fabs(diff) >= 0.01)
+			scnprintf(buf, size, "%+4.2F%%", diff);
+		break;
+
+	case PERF_HPP_DIFF__RATIO:
+		/* No point for ratio number if we are dummy.. */
+		if (he->dummy)
+			break;
+
+		if (pair->diff.computed)
+			ratio = pair->diff.period_ratio;
+		else
+			ratio = perf_diff__compute_ratio(he, pair);
+
+		if (ratio > 0.0)
+			scnprintf(buf, size, "%14.6F", ratio);
+		break;
+
+	case PERF_HPP_DIFF__WEIGHTED_DIFF:
+		/* No point for wdiff number if we are dummy.. */
+		if (he->dummy)
+			break;
+
+		if (pair->diff.computed)
+			wdiff = pair->diff.wdiff;
+		else
+			wdiff = perf_diff__compute_wdiff(he, pair);
+
+		if (wdiff != 0)
+			scnprintf(buf, size, "%14ld", wdiff);
+		break;
+
+	case PERF_HPP_DIFF__FORMULA:
+		perf_diff__formula(he, pair, buf, size);
 		break;
+
+	case PERF_HPP_DIFF__PERIOD:
+		scnprintf(buf, size, "%" PRIu64, pair->stat.period);
+		break;
+
 	default:
 		BUG_ON(1);
 	};
+}
+
+static void
+__hpp__entry_global(struct hist_entry *he, int idx, char *buf, size_t size)
+{
+	struct hist_entry *pair = hist_entry__next_pair(he);
+
+	/* baseline is special */
+	if (idx == PERF_HPP_DIFF__BASELINE)
+		hpp__entry_baseline(he, buf, size);
+	else {
+		if (pair)
+			hpp__entry_pair(he, pair, idx, buf, size);
+		else
+			hpp__entry_unpair(he, idx, buf, size);
+	}
+}
+
+static int hpp__entry_global(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
+			     struct hist_entry *he)
+{
+	struct diff_hpp_fmt *dfmt =
+		container_of(_fmt, struct diff_hpp_fmt, fmt);
+	char buf[MAX_COL_WIDTH] = " ";
+
+	__hpp__entry_global(he, dfmt->idx, buf, MAX_COL_WIDTH);
+
+	if (symbol_conf.field_sep)
+		return scnprintf(hpp->buf, hpp->size, "%s", buf);
+	else
+		return scnprintf(hpp->buf, hpp->size, "%*s",
+				 dfmt->header_width, buf);
+}
+
+static int hpp__header(struct perf_hpp_fmt *fmt,
+		       struct perf_hpp *hpp)
+{
+	struct diff_hpp_fmt *dfmt =
+		container_of(fmt, struct diff_hpp_fmt, fmt);
+
+	BUG_ON(!dfmt->header);
+	return scnprintf(hpp->buf, hpp->size, dfmt->header);
+}
+
+static int hpp__width(struct perf_hpp_fmt *fmt,
+		      struct perf_hpp *hpp __maybe_unused)
+{
+	struct diff_hpp_fmt *dfmt =
+		container_of(fmt, struct diff_hpp_fmt, fmt);
+
+	BUG_ON(dfmt->header_width <= 0);
+	return dfmt->header_width;
+}
+
+#define hpp__color_global hpp__entry_global
+
+#define FMT(_i, _entry, _color)					\
+	[_i] = {						\
+		.fmt = {					\
+			.header	= hpp__header,			\
+			.width	= hpp__width,			\
+			.entry	= hpp__entry_ ## _entry,	\
+			.color	= hpp__color_ ## _color,	\
+		},						\
+		.idx = _i,					\
+	}
+
+#define FMT_GLOBAL(_i)	 FMT(_i, global, global)
+#define FMT_BASELINE(_i) FMT(_i, global, baseline)
+
+static struct diff_hpp_fmt diff_fmt[] = {
+	FMT_BASELINE(PERF_HPP_DIFF__BASELINE),
+	FMT_GLOBAL(PERF_HPP_DIFF__PERIOD),
+	FMT_GLOBAL(PERF_HPP_DIFF__PERIOD_BASELINE),
+	FMT_GLOBAL(PERF_HPP_DIFF__DELTA),
+	FMT_GLOBAL(PERF_HPP_DIFF__RATIO),
+	FMT_GLOBAL(PERF_HPP_DIFF__WEIGHTED_DIFF),
+	FMT_GLOBAL(PERF_HPP_DIFF__FORMULA),
+};
+
+static void init_header(struct diff_hpp_fmt *dfmt)
+{
+#define MAX_HEADER_NAME 100
+	char buf_indent[MAX_HEADER_NAME];
+	char buf[MAX_HEADER_NAME];
+	const char *header = NULL;
+	int width = 0;
+
+	BUG_ON(dfmt->idx >= PERF_HPP_DIFF__MAX_INDEX);
+	header = columns[dfmt->idx].name;
+	width  = columns[dfmt->idx].width;
+
+	/* Only our defined HPP fmts should appear here. */
+	BUG_ON(!header);
+
+#define NAME (data__files_cnt > 2 ? buf : header)
+	dfmt->header_width = width;
+	width = (int) strlen(NAME);
+	if (dfmt->header_width < width)
+		dfmt->header_width = width;
+
+	scnprintf(buf_indent, MAX_HEADER_NAME, "%*s",
+		  dfmt->header_width, NAME);
+
+	dfmt->header = strdup(buf_indent);
+#undef MAX_HEADER_NAME
+#undef NAME
+}
+
+static void column_enable(unsigned col)
+{
+	struct diff_hpp_fmt *dfmt;
+
+	BUG_ON(col >= PERF_HPP_DIFF__MAX_INDEX);
+	dfmt = &diff_fmt[col];
+	init_header(dfmt);
+	perf_hpp__column_register(&dfmt->fmt);
+}
+
+static void ui_init(void)
+{
+	/*
+	 * Display baseline/delta/ratio/
+	 * formula/periods columns.
+	 */
+	column_enable(PERF_HPP_DIFF__BASELINE);
+	column_enable(compute_2_hpp[compute]);
 
 	if (show_formula)
-		perf_hpp__column_enable(PERF_HPP__FORMULA);
+		column_enable(PERF_HPP_DIFF__FORMULA);
 
 	if (show_period) {
-		perf_hpp__column_enable(PERF_HPP__PERIOD);
-		perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE);
+		column_enable(PERF_HPP_DIFF__PERIOD);
+		column_enable(PERF_HPP_DIFF__PERIOD_BASELINE);
 	}
 }
 
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 7da98ac..0db3b44 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -177,48 +177,6 @@ hpp__entry_overhead_guest_us(struct perf_hpp_fmt *_fmt __maybe_unused,
 	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
-static int hpp__header_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
-				struct perf_hpp *hpp)
-{
-	return scnprintf(hpp->buf, hpp->size, "Baseline");
-}
-
-static int hpp__width_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
-			       struct perf_hpp *hpp __maybe_unused)
-{
-	return 8;
-}
-
-static double baseline_percent(struct hist_entry *he)
-{
-	struct hists *hists = he->hists;
-	return 100.0 * he->stat.period / hists->stats.total_period;
-}
-
-static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
-			       struct perf_hpp *hpp, struct hist_entry *he)
-{
-	double percent = baseline_percent(he);
-
-	if (!he->dummy)
-		return percent_color_snprintf(hpp->buf, hpp->size,
-					      " %6.2f%%", percent);
-	else
-		return scnprintf(hpp->buf, hpp->size, "        ");
-}
-
-static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
-			       struct perf_hpp *hpp, struct hist_entry *he)
-{
-	double percent = baseline_percent(he);
-	const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
-
-	if (!he->dummy)
-		return scnprintf(hpp->buf, hpp->size, fmt, percent);
-	else
-		return scnprintf(hpp->buf, hpp->size, "            ");
-}
-
 static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
 			       struct perf_hpp *hpp)
 {
@@ -263,165 +221,6 @@ static int hpp__entry_period(struct perf_hpp_fmt *_fmt __maybe_unused,
 	return scnprintf(hpp->buf, hpp->size, fmt, he->stat.period);
 }
 
-static int hpp__header_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
-				       struct perf_hpp *hpp)
-{
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
-
-	return scnprintf(hpp->buf, hpp->size, fmt, "Period Base");
-}
-
-static int hpp__width_period_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
-				      struct perf_hpp *hpp __maybe_unused)
-{
-	return 12;
-}
-
-static int hpp__entry_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
-				      struct perf_hpp *hpp,
-				      struct hist_entry *he)
-{
-	struct hist_entry *pair = hist_entry__next_pair(he);
-	u64 period = pair ? pair->stat.period : 0;
-	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
-
-	return scnprintf(hpp->buf, hpp->size, fmt, period);
-}
-
-static int hpp__header_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
-			     struct perf_hpp *hpp)
-{
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
-
-	return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
-}
-
-static int hpp__width_delta(struct perf_hpp_fmt *fmt __maybe_unused,
-			    struct perf_hpp *hpp __maybe_unused)
-{
-	return 7;
-}
-
-static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
-			    struct perf_hpp *hpp, struct hist_entry *he)
-{
-	struct hist_entry *pair = hist_entry__next_pair(he);
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
-	char buf[32] = " ";
-	double diff = 0.0;
-
-	if (pair) {
-		if (he->diff.computed)
-			diff = he->diff.period_ratio_delta;
-		else
-			diff = perf_diff__compute_delta(he, pair);
-	}
-
-	if (fabs(diff) >= 0.01)
-		scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
-
-	return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
-			     struct perf_hpp *hpp)
-{
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
-
-	return scnprintf(hpp->buf, hpp->size, fmt, "Ratio");
-}
-
-static int hpp__width_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
-			    struct perf_hpp *hpp __maybe_unused)
-{
-	return 14;
-}
-
-static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
-			    struct perf_hpp *hpp, struct hist_entry *he)
-{
-	struct hist_entry *pair = hist_entry__next_pair(he);
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
-	char buf[32] = " ";
-	double ratio = 0.0;
-
-	/* No point for ratio number if we are dummy.. */
-	if (!he->dummy && pair) {
-		if (he->diff.computed)
-			ratio = he->diff.period_ratio;
-		else
-			ratio = perf_diff__compute_ratio(he, pair);
-	}
-
-	if (ratio > 0.0)
-		scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
-
-	return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
-			     struct perf_hpp *hpp)
-{
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
-
-	return scnprintf(hpp->buf, hpp->size, fmt, "Weighted diff");
-}
-
-static int hpp__width_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
-			    struct perf_hpp *hpp __maybe_unused)
-{
-	return 14;
-}
-
-static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
-			    struct perf_hpp *hpp, struct hist_entry *he)
-{
-	struct hist_entry *pair = hist_entry__next_pair(he);
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
-	char buf[32] = " ";
-	s64 wdiff = 0;
-
-	/* No point for wdiff number if we are dummy.. */
-	if (!he->dummy && pair) {
-		if (he->diff.computed)
-			wdiff = he->diff.wdiff;
-		else
-			wdiff = perf_diff__compute_wdiff(he, pair);
-	}
-
-	if (wdiff != 0)
-		scnprintf(buf, sizeof(buf), "%14ld", wdiff);
-
-	return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
-			       struct perf_hpp *hpp)
-{
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%70s";
-
-	return scnprintf(hpp->buf, hpp->size, fmt, "Formula");
-}
-
-static int hpp__width_formula(struct perf_hpp_fmt *fmt __maybe_unused,
-			      struct perf_hpp *hpp __maybe_unused)
-{
-	return 70;
-}
-
-static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
-			      struct perf_hpp *hpp, struct hist_entry *he)
-{
-	struct hist_entry *pair = hist_entry__next_pair(he);
-	const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
-	char buf[96] = " ";
-
-	if (pair)
-		perf_diff__formula(he, pair, buf, sizeof(buf));
-
-	return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
 #define HPP__COLOR_PRINT_FNS(_name)			\
 	{						\
 		.header	= hpp__header_ ## _name,	\
@@ -438,19 +237,13 @@ static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
 	}
 
 struct perf_hpp_fmt perf_hpp__format[] = {
-	HPP__COLOR_PRINT_FNS(baseline),
 	HPP__COLOR_PRINT_FNS(overhead),
 	HPP__COLOR_PRINT_FNS(overhead_sys),
 	HPP__COLOR_PRINT_FNS(overhead_us),
 	HPP__COLOR_PRINT_FNS(overhead_guest_sys),
 	HPP__COLOR_PRINT_FNS(overhead_guest_us),
 	HPP__PRINT_FNS(samples),
-	HPP__PRINT_FNS(period),
-	HPP__PRINT_FNS(period_baseline),
-	HPP__PRINT_FNS(delta),
-	HPP__PRINT_FNS(ratio),
-	HPP__PRINT_FNS(wdiff),
-	HPP__PRINT_FNS(formula)
+	HPP__PRINT_FNS(period)
 };
 
 LIST_HEAD(perf_hpp__list);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index ae6754d..c60dbe7 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -145,7 +145,7 @@ extern struct list_head perf_hpp__list;
 extern struct perf_hpp_fmt perf_hpp__format[];
 
 enum {
-	PERF_HPP__BASELINE,
+	/* Matches perf_hpp__format array. */
 	PERF_HPP__OVERHEAD,
 	PERF_HPP__OVERHEAD_SYS,
 	PERF_HPP__OVERHEAD_US,
@@ -153,11 +153,6 @@ enum {
 	PERF_HPP__OVERHEAD_GUEST_US,
 	PERF_HPP__SAMPLES,
 	PERF_HPP__PERIOD,
-	PERF_HPP__PERIOD_BASELINE,
-	PERF_HPP__DELTA,
-	PERF_HPP__RATIO,
-	PERF_HPP__WEIGHTED_DIFF,
-	PERF_HPP__FORMULA,
 
 	PERF_HPP__MAX_INDEX
 };
-- 
1.7.11.7


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

* [PATCH 11/14] perf diff: Change diff command to work over multiple data files
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (8 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 09/14] perf diff: Move diff related columns into diff command Jiri Olsa
@ 2012-12-13 13:09 ` Jiri Olsa
  2012-12-13 13:10 ` [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
  2012-12-17  4:55 ` Namhyung Kim
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

Adding diff command the flexibility to specify multiple data
files on input. If not input file is given the standard behaviour
stands and diff inspects 'perf.data' and 'perf.data.old' files.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 99 +++++++++++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 33 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 3b3d21f..25a41e8 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -44,6 +44,7 @@ struct data__file {
 	struct perf_session	*session;
 	const char		*file;
 	int			 idx;
+	struct hists		*hists;
 	struct diff_hpp_fmt	 fmt[PERF_HPP_DIFF__MAX_INDEX];
 };
 
@@ -56,6 +57,7 @@ static int data__files_cnt;
 	     i++, d = &data__files[i])
 
 #define data__for_each_file(i, d) data__for_each_file_start(i, d, 0)
+#define data__for_each_file_new(i, d) data__for_each_file_start(i, d, 1)
 
 static char diff__default_sort_order[] = "dso,symbol";
 static bool force;
@@ -526,23 +528,19 @@ static void hists__compute_resort(struct hists *hists)
 	}
 }
 
-static void hists__process(struct hists *base, struct hists *new)
+static void hists__process(struct hists *hists)
 {
-	hists__match(base, new);
-
 	if (show_baseline_only)
-		hists__baseline_only(base);
-	else
-		hists__link(base, new);
+		hists__baseline_only(hists);
 
 	if (sort_compute) {
-		hists__precompute(base);
-		hists__compute_resort(base);
+		hists__precompute(hists);
+		hists__compute_resort(hists);
 	} else {
-		hists__output_resort(base);
+		hists__output_resort(hists);
 	}
 
-	hists__fprintf(base, true, 0, 0, stdout);
+	hists__fprintf(hists, true, 0, 0, stdout);
 }
 
 static void data__fprintf(void)
@@ -562,27 +560,40 @@ static void data__fprintf(void)
 
 static void data_process(void)
 {
-	struct perf_evlist *evlist_old = data__files[0].session->evlist;
-	struct perf_evlist *evlist_new = data__files[1].session->evlist;
-	struct perf_evsel *evsel_old;
+	struct perf_evlist *evlist_base = data__files[0].session->evlist;
+	struct perf_evsel *evsel_base;
 	bool first = true;
 
-	list_for_each_entry(evsel_old, &evlist_old->entries, node) {
-		struct perf_evsel *evsel_new;
+	list_for_each_entry(evsel_base, &evlist_base->entries, node) {
+		struct data__file *d;
+		int i;
 
-		evsel_new = evsel_match(evsel_old, evlist_new);
-		if (!evsel_new)
-			continue;
+		data__for_each_file_new(i, d) {
+			struct perf_evlist *evlist = d->session->evlist;
+			struct perf_evsel *evsel;
+
+			evsel = evsel_match(evsel_base, evlist);
+			if (!evsel)
+				continue;
+
+			d->hists = &evsel->hists;
+
+			hists__match(&evsel_base->hists, &evsel->hists);
+
+			if (!show_baseline_only)
+				hists__link(&evsel_base->hists,
+					    &evsel->hists);
+		}
 
 		fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
-			perf_evsel__name(evsel_old));
+			perf_evsel__name(evsel_base));
 
 		first = false;
 
-		if (verbose)
+		if (data__files_cnt > 2)
 			data__fprintf();
 
-		hists__process(&evsel_old->hists, &evsel_new->hists);
+		hists__process(&evsel_base->hists);
 	}
 }
 
@@ -781,10 +792,29 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
 	};
 }
 
+static struct hist_entry *get_pair(struct hist_entry *he,
+				   struct diff_hpp_fmt *dfmt)
+{
+	void *ptr = dfmt - dfmt->idx;
+	struct data__file *d = container_of(ptr, struct data__file, fmt);
+
+	if (hist_entry__has_pairs(he)) {
+		struct hist_entry *pair;
+
+		list_for_each_entry(pair, &he->pairs.head, pairs.node)
+			if (pair->hists == d->hists)
+				return pair;
+	}
+
+	return NULL;
+}
+
 static void
-__hpp__entry_global(struct hist_entry *he, int idx, char *buf, size_t size)
+__hpp__entry_global(struct hist_entry *he, struct diff_hpp_fmt *dfmt,
+		    char *buf, size_t size)
 {
-	struct hist_entry *pair = hist_entry__next_pair(he);
+	struct hist_entry *pair = get_pair(he, dfmt);
+	int idx = dfmt->idx;
 
 	/* baseline is special */
 	if (idx == PERF_HPP_DIFF__BASELINE)
@@ -804,7 +834,7 @@ static int hpp__entry_global(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
 		container_of(_fmt, struct diff_hpp_fmt, fmt);
 	char buf[MAX_COL_WIDTH] = " ";
 
-	__hpp__entry_global(he, dfmt->idx, buf, MAX_COL_WIDTH);
+	__hpp__entry_global(he, dfmt, buf, MAX_COL_WIDTH);
 
 	if (symbol_conf.field_sep)
 		return scnprintf(hpp->buf, hpp->size, "%s", buf);
@@ -833,7 +863,7 @@ static int hpp__width(struct perf_hpp_fmt *fmt,
 	return dfmt->header_width;
 }
 
-static void init_header(struct diff_hpp_fmt *dfmt)
+static void init_header(struct data__file *d, struct diff_hpp_fmt *dfmt)
 {
 #define MAX_HEADER_NAME 100
 	char buf_indent[MAX_HEADER_NAME];
@@ -848,6 +878,9 @@ static void init_header(struct diff_hpp_fmt *dfmt)
 	/* Only our defined HPP fmts should appear here. */
 	BUG_ON(!header);
 
+	if (data__files_cnt > 2)
+		scnprintf(buf, MAX_HEADER_NAME, "%s/%d", header, d->idx);
+
 #define NAME (data__files_cnt > 2 ? buf : header)
 	dfmt->header_width = width;
 	width = (int) strlen(NAME);
@@ -877,7 +910,7 @@ static void data__hpp_register(struct data__file *d, int idx)
 	if (idx == PERF_HPP_DIFF__BASELINE)
 		fmt->color = hpp__color_baseline;
 
-	init_header(dfmt);
+	init_header(d, dfmt);
 	perf_hpp__column_register(fmt);
 }
 
@@ -922,18 +955,18 @@ static int data_init(int argc, const char **argv)
 		"perf.data.old",
 		"perf.data",
 	};
+	bool use_default = true;
 	int i;
 
 	data__files_cnt = 2;
 
 	if (argc) {
-		if (argc > 2)
-			usage_with_options(diff_usage, options);
-		if (argc == 2) {
-			defaults[0] = argv[0];
-			defaults[1] = argv[1];
-		} else
+		if (argc == 1)
 			defaults[1] = argv[0];
+		else {
+			data__files_cnt = argc;
+			use_default = false;
+		}
 	} else if (symbol_conf.default_guest_vmlinux_name ||
 		   symbol_conf.default_guest_kallsyms) {
 		defaults[0] = "perf.data.host";
@@ -945,7 +978,7 @@ static int data_init(int argc, const char **argv)
 		return -ENOMEM;
 
 	data__for_each_file(i, d) {
-		d->file = defaults[i];
+		d->file = use_default ? defaults[i] : argv[i];
 		d->idx  = i;
 	}
 
-- 
1.7.11.7


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

* Re: [PATCH 00/14] perf, diff: multiple perf.data file support
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (9 preceding siblings ...)
  2012-12-13 13:09 ` [PATCH 11/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
@ 2012-12-13 13:10 ` Jiri Olsa
  2012-12-17  4:55 ` Namhyung Kim
  11 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2012-12-13 13:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim

On Thu, Dec 13, 2012 at 02:08:58PM +0100, Jiri Olsa wrote:
> hi,
> adding support to display diff for more than 2 perf.data files.
> 
> The basic idea is describe in the initial RFC in here:
> https://lkml.org/lkml/2012/11/28/200
> 
> It's rebased on current Arnaldo's perf core with Namhyung's
> changes.

and it's available in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
perf/multi12

jirka

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

* Re: [PATCH 00/14] perf, diff: multiple perf.data file support
  2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
                   ` (10 preceding siblings ...)
  2012-12-13 13:10 ` [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
@ 2012-12-17  4:55 ` Namhyung Kim
  11 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2012-12-17  4:55 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker

Hi Jiri,

On Thu, 13 Dec 2012 14:08:58 +0100, Jiri Olsa wrote:
> hi,
> adding support to display diff for more than 2 perf.data files.
>
> The basic idea is describe in the initial RFC in here:
> https://lkml.org/lkml/2012/11/28/200
>
> It's rebased on current Arnaldo's perf core with Namhyung's
> changes.

For the whole series:

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

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

* Re: [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute
  2012-12-13 13:08 ` [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute Jiri Olsa
@ 2013-05-23  9:17   ` Arnaldo Carvalho de Melo
  2013-05-31 11:44   ` [tip:perf/core] " tip-bot for Jiri Olsa
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-05-23  9:17 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Paul Mackerras,
	Corey Ashford, Frederic Weisbecker, Namhyung Kim

Em Thu, Dec 13, 2012 at 02:08:59PM +0100, Jiri Olsa escreveu:

> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c

>  	while (next != NULL) {
> -		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
> -		struct hist_entry *pair = hist_entry__next_pair(he);
> +		struct hist_entry *he, *pair;
>  
> -		next = rb_next(&he->rb_node);
> +		he   = rb_entry(next, struct hist_entry, rb_node_in);
> +		pair = hist_entry__next_pair(he);
> +
> +		next = rb_next(&he->rb_node_in);


To ease review please try to make the patch as minimal as possible, i.e.
the above could be done as:

-		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node_in);
 		struct hist_entry *pair = hist_entry__next_pair(he);

-		next = rb_next(&he->rb_node);
+		next = rb_next(&he->rb_node_in);

See how we can more quickly see what happened? I.e. just replacing
'rb_node' with 'rb_node_in' :-)

- Arnaldo

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

* Re: [PATCH 03/14] perf tools: Add struct perf_hpp_fmt into hpp callbacks
  2012-12-13 13:09 ` [PATCH 03/14] perf tools: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
@ 2013-05-23  9:56   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-05-23  9:56 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Paul Mackerras,
	Corey Ashford, Frederic Weisbecker, Namhyung Kim

Em Thu, Dec 13, 2012 at 02:09:01PM +0100, Jiri Olsa escreveu:
> Adding 'struct perf_hpp_fmt' into hpp callbacks, so commands
> can access their private data.

>  tools/perf/ui/gtk/browser.c    |  10 +--

Doesn't apply here, can you please take a look?

- Arnaldo

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

* Re: [PATCH 04/14] perf tools: Centralize default columns init in perf_hpp__init
  2012-12-13 13:09 ` [PATCH 04/14] perf tools: Centralize default columns init in perf_hpp__init Jiri Olsa
@ 2013-05-23 10:01   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-05-23 10:01 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Paul Mackerras,
	Corey Ashford, Frederic Weisbecker, Namhyung Kim

Em Thu, Dec 13, 2012 at 02:09:02PM +0100, Jiri Olsa escreveu:
> Now when diff command is separated from other standard outputs,
> we can use perf_hpp__init to initialize all standard columns.
> 
> Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
> and removing extra enable calls.

Also doesn't apply, couldn't find this function:

void perf_gtk__init_hpp(void);

But it has a prototype in tools/perf/ui/gtk/gtk.h, perhaps its one of
those macro templates? Leftover? please check,

- Arnaldo

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

* [tip:perf/core] perf diff: Use internal rb tree for hists__precompute
  2012-12-13 13:08 ` [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute Jiri Olsa
  2013-05-23  9:17   ` Arnaldo Carvalho de Melo
@ 2013-05-31 11:44   ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-05-31 11:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
	jolsa, fweisbec, tglx, cjashfor, mingo

Commit-ID:  367c53c08f84bb554a3aae18b65e5419fe4b164a
Gitweb:     http://git.kernel.org/tip/367c53c08f84bb554a3aae18b65e5419fe4b164a
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 13 Dec 2012 14:08:59 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:24:02 +0300

perf diff: Use internal rb tree for hists__precompute

There's missing change for hists__precompute to iterate either
entries_collapsed or entries_in tree. The change was initiated
for hists_compute_resort function in commit:

  66f97ed perf diff: Use internal rb tree for compute resort

but was missing for hists__precompute function changes.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1355404152-16523-2-git-send-email-jolsa@redhat.com
[ committer note: Reduce patch size, no functional change ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-diff.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index a9d63c1..da8f8eb 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -323,13 +323,20 @@ static void hists__baseline_only(struct hists *hists)
 
 static void hists__precompute(struct hists *hists)
 {
-	struct rb_node *next = rb_first(&hists->entries);
+	struct rb_root *root;
+	struct rb_node *next;
+
+	if (sort__need_collapse)
+		root = &hists->entries_collapsed;
+	else
+		root = hists->entries_in;
 
+	next = rb_first(root);
 	while (next != NULL) {
-		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node_in);
 		struct hist_entry *pair = hist_entry__next_pair(he);
 
-		next = rb_next(&he->rb_node);
+		next = rb_next(&he->rb_node_in);
 		if (!pair)
 			continue;
 

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

* [tip:perf/core] perf hists: Rename hist_entry__add_pair arguments
  2012-12-13 13:09 ` [PATCH 02/14] perf hists: Rename hist_entry__add_pair arguments Jiri Olsa
@ 2013-05-31 11:46   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-05-31 11:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
	jolsa, fweisbec, tglx, cjashfor, mingo

Commit-ID:  4d23322a0b8d0f40819dc02ea15a732a78b0a1c0
Gitweb:     http://git.kernel.org/tip/4d23322a0b8d0f40819dc02ea15a732a78b0a1c0
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 13 Dec 2012 14:09:00 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:24:02 +0300

perf hists: Rename hist_entry__add_pair arguments

The current logic is to attach pair to the leader hist_entry.

Arguments of hist_entry__add_pair function were placed the other way
round.. driving me crazy.

I.e. list_add_tail expects (new_node, head).

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1355404152-16523-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 51f1b5a..45ac84c 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -117,10 +117,10 @@ static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
 	return NULL;
 }
 
-static inline void hist_entry__add_pair(struct hist_entry *he,
-					struct hist_entry *pair)
+static inline void hist_entry__add_pair(struct hist_entry *pair,
+					struct hist_entry *he)
 {
-	list_add_tail(&he->pairs.head, &pair->pairs.node);
+	list_add_tail(&pair->pairs.node, &he->pairs.head);
 }
 
 enum sort_mode {

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

end of thread, other threads:[~2013-05-31 11:47 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-13 13:08 [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
2012-12-13 13:08 ` [PATCH 01/14] perf diff: Use internal rb tree for hists__precompute Jiri Olsa
2013-05-23  9:17   ` Arnaldo Carvalho de Melo
2013-05-31 11:44   ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-12-13 13:09 ` [PATCH 02/14] perf hists: Rename hist_entry__add_pair arguments Jiri Olsa
2013-05-31 11:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-12-13 13:09 ` [PATCH 03/14] perf tools: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
2013-05-23  9:56   ` Arnaldo Carvalho de Melo
2012-12-13 13:09 ` [PATCH 04/14] perf tools: Centralize default columns init in perf_hpp__init Jiri Olsa
2013-05-23 10:01   ` Arnaldo Carvalho de Melo
2012-12-13 13:09 ` [PATCH 05/14] perf diff: Introducing diff_data object to hold files Jiri Olsa
2012-12-13 13:09 ` [PATCH 06/14] perf diff: Switching the base hists to be pairs head Jiri Olsa
2012-12-13 13:09 ` [PATCH 07/14] perf hists: Marking dummy hists entries Jiri Olsa
2012-12-13 13:09 ` [PATCH 08/14] perf diff: Display data file info ahead of the diff output Jiri Olsa
2012-12-13 13:09 ` [PATCH 09/14] perf diff: Move diff related columns into diff command Jiri Olsa
2012-12-13 13:09 ` [PATCH 11/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
2012-12-13 13:10 ` [PATCH 00/14] perf, diff: multiple perf.data file support Jiri Olsa
2012-12-17  4:55 ` Namhyung Kim

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