[PATCHSET 0/7] perf tools: Support multiple keys in a single hierarchy level (v3)

[PATCHSET 0/7] perf tools: Support multiple keys in a single hierarchy level (v3)

[Namhyung Kim]

Hello,

This implements what Arnaldo suggested in previous discussion of
hierarchy patchset [1].  Originally each level in a hierarchy can have
a single sort key in it, but with this patches it's possible to have
more than one sort keys using a syntax similar to event grouping.  I
added the struct perf_hpp_list_node and carry it to group output
formats (hpp_fmt) in a single level.

 * Changes from v2)
  - fix segfault on perf top  (Arnaldo)
  - add hpp_list_node->skip  (Jiri)

 * Changes from v1)
  - use '{ }' to group sort keys  (Arnaldo)
  - cleanup hpp_list_node creation  (Jiri)


Example below shows how 4 sort keys are used for 2 levels.  As you can
see, the first level shows pid and comm of previous (switched) task
and the second level shows pid and comm of next task.

  $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
   --percent-limit 1 -i perf.data.sched
  ...
  #    Overhead  prev_pid+prev_comm / next_pid+next_comm
  # ...........  .......................................
  #
      22.36%     0  swapper/0
          9.48%     17773  transmission-gt
          5.25%     109  kworker/0:1H
          1.53%     6524  Xephyr
      21.39%     17773  transmission-gt
          9.52%     0  swapper/0
          9.04%     0  swapper/2
          1.78%     0  swapper/3


It's available on the 'perf/hierarchy-multi-v3' branch in my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Any feedbacks are welcome

Thanks,
Namhyung


[1] https://lkml.org/lkml/2016/2/24/1041


Namhyung Kim (7):
  perf tools: Introduce perf_hpp__setup_hists_formats()
  perf tools: Use own hpp_list for hierarchy mode
  perf tools: Support multiple sort keys in a hierarchy level
  perf tools: Fix indent for multiple hierarchy sort key
  perf report: Use hierarchy hpp list on stdio
  perf hists browser: Use hierarchy hpp list
  perf report: Use hierarchy hpp list on gtk

 tools/perf/ui/browsers/hists.c | 147 +++++++++++++++++++----------------
 tools/perf/ui/gtk/hists.c      |  73 +++++++++++-------
 tools/perf/ui/hist.c           |  69 +++++++++++++++++
 tools/perf/ui/stdio/hist.c     | 171 +++++++++++++++++++++--------------------
 tools/perf/util/hist.c         |  72 +++++++++++------
 tools/perf/util/hist.h         |  13 ++++
 tools/perf/util/sort.c         |  74 +++++++++++++++---
 tools/perf/util/sort.h         |   1 +
 8 files changed, 408 insertions(+), 212 deletions(-)

-- 
2.7.2

[PATCH v3 1/7] perf tools: Introduce perf_hpp__setup_hists_formats()

[Namhyung Kim]

The perf_hpp__setup_hists_formats() is to build hists-specific output
formats (and sort keys).  Currently it's only used in order to build the
output format in a hierarchy with same sort keys, but it could be used
with different sort keys in non-hierarchy mode later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c   | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.c | 12 ++++++++++
 tools/perf/util/hist.h | 11 +++++++++
 tools/perf/util/sort.c | 32 +++++++++++++++++++++++++
 4 files changed, 118 insertions(+)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 7c0585c146e1..3a15e844f89a 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -5,6 +5,7 @@
 #include "../util/util.h"
 #include "../util/sort.h"
 #include "../util/evsel.h"
+#include "../util/evlist.h"
 
 /* hist period print (hpp) functions */
 
@@ -715,3 +716,65 @@ void perf_hpp__set_user_width(const char *width_list_str)
 			break;
 	}
 }
+
+static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
+{
+	struct perf_hpp_list_node *node = NULL;
+	struct perf_hpp_fmt *fmt_copy;
+	bool found = false;
+
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		if (node->level == fmt->level) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		node = malloc(sizeof(*node));
+		if (node == NULL)
+			return -1;
+
+		node->level = fmt->level;
+		perf_hpp_list__init(&node->hpp);
+
+		list_add_tail(&node->list, &hists->hpp_formats);
+	}
+
+	fmt_copy = perf_hpp_fmt__dup(fmt);
+	if (fmt_copy == NULL)
+		return -1;
+
+	list_add_tail(&fmt_copy->list, &node->hpp.fields);
+	list_add_tail(&fmt_copy->sort_list, &node->hpp.sorts);
+
+	return 0;
+}
+
+int perf_hpp__setup_hists_formats(struct perf_hpp_list *list,
+				  struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+	struct perf_hpp_fmt *fmt;
+	struct hists *hists;
+	int ret;
+
+	if (!symbol_conf.report_hierarchy)
+		return 0;
+
+	evlist__for_each(evlist, evsel) {
+		hists = evsel__hists(evsel);
+
+		perf_hpp_list__for_each_sort_list(list, fmt) {
+			if (perf_hpp__is_dynamic_entry(fmt) &&
+			    !perf_hpp__defined_dynamic_entry(fmt, hists))
+				continue;
+
+			ret = add_hierarchy_fmt(hists, fmt);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	return 0;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 4b8b67bc0cd8..fea92fcb6903 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2105,6 +2105,7 @@ int __hists__init(struct hists *hists, struct perf_hpp_list *hpp_list)
 	pthread_mutex_init(&hists->lock, NULL);
 	hists->socket_filter = -1;
 	hists->hpp_list = hpp_list;
+	INIT_LIST_HEAD(&hists->hpp_formats);
 	return 0;
 }
 
@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)
 static void hists_evsel__exit(struct perf_evsel *evsel)
 {
 	struct hists *hists = evsel__hists(evsel);
+	struct perf_hpp_fmt *fmt, *pos;
+	struct perf_hpp_list_node *node, *tmp;
 
 	hists__delete_all_entries(hists);
+
+	list_for_each_entry_safe(node, tmp, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos) {
+			list_del(&fmt->list);
+			free(fmt);
+		}
+		list_del(&node->list);
+		free(node);
+	}
 }
 
 static int hists_evsel__init(struct perf_evsel *evsel)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index f4ef513527ba..3cab9dc20822 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -78,6 +78,7 @@ struct hists {
 	u16			col_len[HISTC_NR_COLS];
 	int			socket_filter;
 	struct perf_hpp_list	*hpp_list;
+	struct list_head	hpp_formats;
 	int			nr_sort_keys;
 };
 
@@ -244,6 +245,12 @@ struct perf_hpp_list {
 
 extern struct perf_hpp_list perf_hpp_list;
 
+struct perf_hpp_list_node {
+	struct list_head	list;
+	struct perf_hpp_list	hpp;
+	int			level;
+};
+
 void perf_hpp_list__column_register(struct perf_hpp_list *list,
 				    struct perf_hpp_fmt *format);
 void perf_hpp_list__register_sort_field(struct perf_hpp_list *list,
@@ -299,6 +306,8 @@ void perf_hpp__cancel_cumulate(void);
 void perf_hpp__setup_output_field(struct perf_hpp_list *list);
 void perf_hpp__reset_output_field(struct perf_hpp_list *list);
 void perf_hpp__append_sort_keys(struct perf_hpp_list *list);
+int perf_hpp__setup_hists_formats(struct perf_hpp_list *list,
+				  struct perf_evlist *evlist);
 
 
 bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
@@ -308,6 +317,8 @@ bool perf_hpp__is_trace_entry(struct perf_hpp_fmt *fmt);
 bool perf_hpp__is_srcline_entry(struct perf_hpp_fmt *fmt);
 bool perf_hpp__is_srcfile_entry(struct perf_hpp_fmt *fmt);
 
+struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt);
+
 int hist_entry__filter(struct hist_entry *he, int type, const void *arg);
 
 static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ab6eb7ca8c60..71d45d147376 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1908,6 +1908,34 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field,
 	return hde;
 }
 
+struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt)
+{
+	struct perf_hpp_fmt *new_fmt = NULL;
+
+	if (perf_hpp__is_sort_entry(fmt)) {
+		struct hpp_sort_entry *hse, *new_hse;
+
+		hse = container_of(fmt, struct hpp_sort_entry, hpp);
+		new_hse = memdup(hse, sizeof(*hse));
+		if (new_hse)
+			new_fmt = &new_hse->hpp;
+	} else if (perf_hpp__is_dynamic_entry(fmt)) {
+		struct hpp_dynamic_entry *hde, *new_hde;
+
+		hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
+		new_hde = memdup(hde, sizeof(*hde));
+		if (new_hde)
+			new_fmt = &new_hde->hpp;
+	} else {
+		new_fmt = memdup(fmt, sizeof(*fmt));
+	}
+
+	INIT_LIST_HEAD(&new_fmt->list);
+	INIT_LIST_HEAD(&new_fmt->sort_list);
+
+	return new_fmt;
+}
+
 static int parse_field_name(char *str, char **event, char **field, char **opt)
 {
 	char *event_name, *field_name, *opt_name;
@@ -2700,6 +2728,10 @@ int setup_sorting(struct perf_evlist *evlist)
 	/* and then copy output fields to sort keys */
 	perf_hpp__append_sort_keys(&perf_hpp_list);
 
+	/* setup hists-specific output fields */
+	if (perf_hpp__setup_hists_formats(&perf_hpp_list, evlist) < 0)
+		return -1;
+
 	return 0;
 }
 
-- 
2.7.2

[tip:perf/core] perf hists: Introduce perf_hpp__setup_hists_formats()

[tip-bot for Namhyung Kim]

Commit-ID:  c3bc0c436899d01c3a09fddb308d487cc032fbd2
Gitweb:     http://git.kernel.org/tip/c3bc0c436899d01c3a09fddb308d487cc032fbd2
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:45 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:19 +0100

perf hists: Introduce perf_hpp__setup_hists_formats()

The perf_hpp__setup_hists_formats() is to build hists-specific output
formats (and sort keys).  Currently it's only used in order to build the
output format in a hierarchy with same sort keys, but it could be used
with different sort keys in non-hierarchy mode later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-2-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/ui/hist.c   | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.c | 12 ++++++++++
 tools/perf/util/hist.h | 11 +++++++++
 tools/perf/util/sort.c | 32 +++++++++++++++++++++++++
 4 files changed, 118 insertions(+)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 7c0585c..3a15e84 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -5,6 +5,7 @@
 #include "../util/util.h"
 #include "../util/sort.h"
 #include "../util/evsel.h"
+#include "../util/evlist.h"
 
 /* hist period print (hpp) functions */
 
@@ -715,3 +716,65 @@ void perf_hpp__set_user_width(const char *width_list_str)
 			break;
 	}
 }
+
+static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
+{
+	struct perf_hpp_list_node *node = NULL;
+	struct perf_hpp_fmt *fmt_copy;
+	bool found = false;
+
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		if (node->level == fmt->level) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		node = malloc(sizeof(*node));
+		if (node == NULL)
+			return -1;
+
+		node->level = fmt->level;
+		perf_hpp_list__init(&node->hpp);
+
+		list_add_tail(&node->list, &hists->hpp_formats);
+	}
+
+	fmt_copy = perf_hpp_fmt__dup(fmt);
+	if (fmt_copy == NULL)
+		return -1;
+
+	list_add_tail(&fmt_copy->list, &node->hpp.fields);
+	list_add_tail(&fmt_copy->sort_list, &node->hpp.sorts);
+
+	return 0;
+}
+
+int perf_hpp__setup_hists_formats(struct perf_hpp_list *list,
+				  struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+	struct perf_hpp_fmt *fmt;
+	struct hists *hists;
+	int ret;
+
+	if (!symbol_conf.report_hierarchy)
+		return 0;
+
+	evlist__for_each(evlist, evsel) {
+		hists = evsel__hists(evsel);
+
+		perf_hpp_list__for_each_sort_list(list, fmt) {
+			if (perf_hpp__is_dynamic_entry(fmt) &&
+			    !perf_hpp__defined_dynamic_entry(fmt, hists))
+				continue;
+
+			ret = add_hierarchy_fmt(hists, fmt);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	return 0;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 4b8b67b..fea92fc 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2105,6 +2105,7 @@ int __hists__init(struct hists *hists, struct perf_hpp_list *hpp_list)
 	pthread_mutex_init(&hists->lock, NULL);
 	hists->socket_filter = -1;
 	hists->hpp_list = hpp_list;
+	INIT_LIST_HEAD(&hists->hpp_formats);
 	return 0;
 }
 
@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)
 static void hists_evsel__exit(struct perf_evsel *evsel)
 {
 	struct hists *hists = evsel__hists(evsel);
+	struct perf_hpp_fmt *fmt, *pos;
+	struct perf_hpp_list_node *node, *tmp;
 
 	hists__delete_all_entries(hists);
+
+	list_for_each_entry_safe(node, tmp, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos) {
+			list_del(&fmt->list);
+			free(fmt);
+		}
+		list_del(&node->list);
+		free(node);
+	}
 }
 
 static int hists_evsel__init(struct perf_evsel *evsel)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index f4ef513..3cab9dc 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -78,6 +78,7 @@ struct hists {
 	u16			col_len[HISTC_NR_COLS];
 	int			socket_filter;
 	struct perf_hpp_list	*hpp_list;
+	struct list_head	hpp_formats;
 	int			nr_sort_keys;
 };
 
@@ -244,6 +245,12 @@ struct perf_hpp_list {
 
 extern struct perf_hpp_list perf_hpp_list;
 
+struct perf_hpp_list_node {
+	struct list_head	list;
+	struct perf_hpp_list	hpp;
+	int			level;
+};
+
 void perf_hpp_list__column_register(struct perf_hpp_list *list,
 				    struct perf_hpp_fmt *format);
 void perf_hpp_list__register_sort_field(struct perf_hpp_list *list,
@@ -299,6 +306,8 @@ void perf_hpp__cancel_cumulate(void);
 void perf_hpp__setup_output_field(struct perf_hpp_list *list);
 void perf_hpp__reset_output_field(struct perf_hpp_list *list);
 void perf_hpp__append_sort_keys(struct perf_hpp_list *list);
+int perf_hpp__setup_hists_formats(struct perf_hpp_list *list,
+				  struct perf_evlist *evlist);
 
 
 bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
@@ -308,6 +317,8 @@ bool perf_hpp__is_trace_entry(struct perf_hpp_fmt *fmt);
 bool perf_hpp__is_srcline_entry(struct perf_hpp_fmt *fmt);
 bool perf_hpp__is_srcfile_entry(struct perf_hpp_fmt *fmt);
 
+struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt);
+
 int hist_entry__filter(struct hist_entry *he, int type, const void *arg);
 
 static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ab6eb7c..71d45d1 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1908,6 +1908,34 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field,
 	return hde;
 }
 
+struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt)
+{
+	struct perf_hpp_fmt *new_fmt = NULL;
+
+	if (perf_hpp__is_sort_entry(fmt)) {
+		struct hpp_sort_entry *hse, *new_hse;
+
+		hse = container_of(fmt, struct hpp_sort_entry, hpp);
+		new_hse = memdup(hse, sizeof(*hse));
+		if (new_hse)
+			new_fmt = &new_hse->hpp;
+	} else if (perf_hpp__is_dynamic_entry(fmt)) {
+		struct hpp_dynamic_entry *hde, *new_hde;
+
+		hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
+		new_hde = memdup(hde, sizeof(*hde));
+		if (new_hde)
+			new_fmt = &new_hde->hpp;
+	} else {
+		new_fmt = memdup(fmt, sizeof(*fmt));
+	}
+
+	INIT_LIST_HEAD(&new_fmt->list);
+	INIT_LIST_HEAD(&new_fmt->sort_list);
+
+	return new_fmt;
+}
+
 static int parse_field_name(char *str, char **event, char **field, char **opt)
 {
 	char *event_name, *field_name, *opt_name;
@@ -2700,6 +2728,10 @@ int setup_sorting(struct perf_evlist *evlist)
 	/* and then copy output fields to sort keys */
 	perf_hpp__append_sort_keys(&perf_hpp_list);
 
+	/* setup hists-specific output fields */
+	if (perf_hpp__setup_hists_formats(&perf_hpp_list, evlist) < 0)
+		return -1;
+
 	return 0;
 }
 

[PATCH v3 2/7] perf tools: Use own hpp_list for hierarchy mode

[Namhyung Kim]

Now each hists has its own hpp lists in hierarchy.  So instead of having
a pointer to a single perf_hpp_fmt in a hist entry, make it point the
hpp_list for its level.  This will be used to support multiple sort keys
in a single hierarchy level.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 45 +++++++++++++++++--------------
 tools/perf/ui/gtk/hists.c      | 20 +++++++++-----
 tools/perf/ui/hist.c           |  5 ++++
 tools/perf/ui/stdio/hist.c     | 44 +++++++++++++++----------------
 tools/perf/util/hist.c         | 60 ++++++++++++++++++++++++------------------
 tools/perf/util/hist.h         |  1 +
 tools/perf/util/sort.h         |  1 +
 7 files changed, 103 insertions(+), 73 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 5ffffcb1e3c5..928b4825b752 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1388,25 +1388,26 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 					      HE_COLORSET_NORMAL);
 		}
 
-		ui_browser__write_nstring(&browser->b, "", 2);
-		width -= 2;
+		perf_hpp_list__for_each_format(entry->hpp_list, fmt) {
+			ui_browser__write_nstring(&browser->b, "", 2);
+			width -= 2;
 
-		/*
-		 * No need to call hist_entry__snprintf_alignment()
-		 * since this fmt is always the last column in the
-		 * hierarchy mode.
-		 */
-		fmt = entry->fmt;
-		if (fmt->color) {
-			width -= fmt->color(fmt, &hpp, entry);
-		} else {
-			int i = 0;
+			/*
+			 * No need to call hist_entry__snprintf_alignment()
+			 * since this fmt is always the last column in the
+			 * hierarchy mode.
+			 */
+			if (fmt->color) {
+				width -= fmt->color(fmt, &hpp, entry);
+			} else {
+				int i = 0;
 
-			width -= fmt->entry(fmt, &hpp, entry);
-			ui_browser__printf(&browser->b, "%s", ltrim(s));
+				width -= fmt->entry(fmt, &hpp, entry);
+				ui_browser__printf(&browser->b, "%s", ltrim(s));
 
-			while (isspace(s[i++]))
-				width++;
+				while (isspace(s[i++]))
+					width++;
+			}
 		}
 	}
 
@@ -1934,7 +1935,7 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 	struct perf_hpp_fmt *fmt;
 	bool first = true;
 	int ret;
-	int hierarchy_indent = (nr_sort_keys + 1) * HIERARCHY_INDENT;
+	int hierarchy_indent = nr_sort_keys * HIERARCHY_INDENT;
 
 	printed = fprintf(fp, "%*s", level * HIERARCHY_INDENT, "");
 
@@ -1962,9 +1963,13 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 	ret = scnprintf(hpp.buf, hpp.size, "%*s", hierarchy_indent, "");
 	advance_hpp(&hpp, ret);
 
-	fmt = he->fmt;
-	ret = fmt->entry(fmt, &hpp, he);
-	advance_hpp(&hpp, ret);
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		ret = scnprintf(hpp.buf, hpp.size, "  ");
+		advance_hpp(&hpp, ret);
+
+		ret = fmt->entry(fmt, &hpp, he);
+		advance_hpp(&hpp, ret);
+	}
 
 	printed += fprintf(fp, "%s\n", rtrim(s));
 
diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index a5758fdfbe1f..4534e2d7669c 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -412,6 +412,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		GtkTreeIter iter;
 		float percent;
+		char *bf;
 
 		he = rb_entry(node, struct hist_entry, rb_node);
 		if (he->filtered)
@@ -437,13 +438,20 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 			gtk_tree_store_set(store, &iter, col_idx++, hpp->buf, -1);
 		}
 
-		fmt = he->fmt;
-		if (fmt->color)
-			fmt->color(fmt, hpp, he);
-		else
-			fmt->entry(fmt, hpp, he);
+		bf = hpp->buf;
+		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+			int ret;
+
+			if (fmt->color)
+				ret = fmt->color(fmt, hpp, he);
+			else
+				ret = fmt->entry(fmt, hpp, he);
+
+			snprintf(hpp->buf + ret, hpp->size - ret, "  ");
+			advance_hpp(hpp, ret + 2);
+		}
 
-		gtk_tree_store_set(store, &iter, col_idx, rtrim(hpp->buf), -1);
+		gtk_tree_store_set(store, &iter, col_idx, rtrim(bf), -1);
 
 		if (!he->leaf) {
 			perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 3a15e844f89a..95795ef4209b 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -722,6 +722,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 	struct perf_hpp_list_node *node = NULL;
 	struct perf_hpp_fmt *fmt_copy;
 	bool found = false;
+	bool skip = perf_hpp__should_skip(fmt, hists);
 
 	list_for_each_entry(node, &hists->hpp_formats, list) {
 		if (node->level == fmt->level) {
@@ -735,6 +736,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 		if (node == NULL)
 			return -1;
 
+		node->skip = skip;
 		node->level = fmt->level;
 		perf_hpp_list__init(&node->hpp);
 
@@ -745,6 +747,9 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 	if (fmt_copy == NULL)
 		return -1;
 
+	if (!skip)
+		node->skip = false;
+
 	list_add_tail(&fmt_copy->list, &node->hpp.fields);
 	list_add_tail(&fmt_copy->sort_list, &node->hpp.sorts);
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 6d06fbb365b6..073642a63cc9 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -451,33 +451,33 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 		advance_hpp(hpp, ret);
 	}
 
-	if (sep)
-		ret = scnprintf(hpp->buf, hpp->size, "%s", sep);
-	else
+	if (!sep)
 		ret = scnprintf(hpp->buf, hpp->size, "%*s",
-				(nr_sort_key - 1) * HIERARCHY_INDENT + 2, "");
+				(nr_sort_key - 1) * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
 	printed += fprintf(fp, "%s", buf);
 
-	hpp->buf  = buf;
-	hpp->size = size;
-
-	/*
-	 * No need to call hist_entry__snprintf_alignment() since this
-	 * fmt is always the last column in the hierarchy mode.
-	 */
-	fmt = he->fmt;
-	if (perf_hpp__use_color() && fmt->color)
-		fmt->color(fmt, hpp, he);
-	else
-		fmt->entry(fmt, hpp, he);
-
-	/*
-	 * dynamic entries are right-aligned but we want left-aligned
-	 * in the hierarchy mode
-	 */
-	printed += fprintf(fp, "%s\n", ltrim(buf));
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		hpp->buf  = buf;
+		hpp->size = size;
+
+		/*
+		 * No need to call hist_entry__snprintf_alignment() since this
+		 * fmt is always the last column in the hierarchy mode.
+		 */
+		if (perf_hpp__use_color() && fmt->color)
+			fmt->color(fmt, hpp, he);
+		else
+			fmt->entry(fmt, hpp, he);
+
+		/*
+		 * dynamic entries are right-aligned but we want left-aligned
+		 * in the hierarchy mode
+		 */
+		printed += fprintf(fp, "%s%s", sep ?: "  ", ltrim(buf));
+	}
+	printed += putc('\n', fp);
 
 	if (symbol_conf.use_callchain && he->leaf) {
 		u64 total = hists__total_period(hists);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fea92fcb6903..29da9e0d8db9 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1091,18 +1091,25 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he);
 static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 						 struct rb_root *root,
 						 struct hist_entry *he,
-						 struct perf_hpp_fmt *fmt)
+						 struct perf_hpp_list *hpp_list)
 {
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter, *new;
+	struct perf_hpp_fmt *fmt;
 	int64_t cmp;
 
 	while (*p != NULL) {
 		parent = *p;
 		iter = rb_entry(parent, struct hist_entry, rb_node_in);
 
-		cmp = fmt->collapse(fmt, iter, he);
+		cmp = 0;
+		perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+			cmp = fmt->collapse(fmt, iter, he);
+			if (cmp)
+				break;
+		}
+
 		if (!cmp) {
 			he_stat__add_stat(&iter->stat, &he->stat);
 			return iter;
@@ -1121,24 +1128,26 @@ static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 	hists__apply_filters(hists, new);
 	hists->nr_entries++;
 
-	/* save related format for output */
-	new->fmt = fmt;
+	/* save related format list for output */
+	new->hpp_list = hpp_list;
 
 	/* some fields are now passed to 'new' */
-	if (perf_hpp__is_trace_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-		he->trace_output = NULL;
-	else
-		new->trace_output = NULL;
+	perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+		if (perf_hpp__is_trace_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
+			he->trace_output = NULL;
+		else
+			new->trace_output = NULL;
 
-	if (perf_hpp__is_srcline_entry(fmt))
-		he->srcline = NULL;
-	else
-		new->srcline = NULL;
+		if (perf_hpp__is_srcline_entry(fmt))
+			he->srcline = NULL;
+		else
+			new->srcline = NULL;
 
-	if (perf_hpp__is_srcfile_entry(fmt))
-		he->srcfile = NULL;
-	else
-		new->srcfile = NULL;
+		if (perf_hpp__is_srcfile_entry(fmt))
+			he->srcfile = NULL;
+		else
+			new->srcfile = NULL;
+	}
 
 	rb_link_node(&new->rb_node_in, parent, p);
 	rb_insert_color(&new->rb_node_in, root);
@@ -1149,21 +1158,19 @@ static int hists__hierarchy_insert_entry(struct hists *hists,
 					 struct rb_root *root,
 					 struct hist_entry *he)
 {
-	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
 	struct hist_entry *new_he = NULL;
 	struct hist_entry *parent = NULL;
 	int depth = 0;
 	int ret = 0;
 
-	hists__for_each_sort_list(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) &&
-		    !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		/* skip period (overhead) and elided columns */
+		if (node->level == 0 || node->skip)
 			continue;
 
 		/* insert copy of 'he' for each fmt into the hierarchy */
-		new_he = hierarchy_insert_entry(hists, root, he, fmt);
+		new_he = hierarchy_insert_entry(hists, root, he, &node->hpp);
 		if (new_he == NULL) {
 			ret = -1;
 			break;
@@ -1358,6 +1365,7 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
+	struct perf_hpp_fmt *fmt;
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1373,8 +1381,10 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
 	rb_insert_color(&he->rb_node, root);
 
 	/* update column width of dynamic entry */
-	if (perf_hpp__is_dynamic_entry(he->fmt))
-		he->fmt->sort(he->fmt, he, NULL);
+	perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
+		if (perf_hpp__is_dynamic_entry(fmt))
+			fmt->sort(fmt, he, NULL);
+	}
 }
 
 static void hists__hierarchy_output_resort(struct hists *hists,
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 3cab9dc20822..2209188d729c 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -249,6 +249,7 @@ struct perf_hpp_list_node {
 	struct list_head	list;
 	struct perf_hpp_list	hpp;
 	int			level;
+	bool			skip;
 };
 
 void perf_hpp_list__column_register(struct perf_hpp_list *list,
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 25a5529a94e4..ea1f722cffea 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -130,6 +130,7 @@ struct hist_entry {
 	u32			raw_size;
 	void			*trace_output;
 	struct perf_hpp_fmt	*fmt;
+	struct perf_hpp_list	*hpp_list;
 	struct hist_entry	*parent_he;
 	union {
 		/* this is for hierarchical entry structure */
-- 
2.7.2

[tip:perf/core] perf hists: Use own hpp_list for hierarchy mode

[tip-bot for Namhyung Kim]

Commit-ID:  1b2dbbf41a0f4cf7a5662bccb9a18128d16e5ffb
Gitweb:     http://git.kernel.org/tip/1b2dbbf41a0f4cf7a5662bccb9a18128d16e5ffb
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:46 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:19 +0100

perf hists: Use own hpp_list for hierarchy mode

Now each hists has its own hpp lists in hierarchy.  So instead of having
a pointer to a single perf_hpp_fmt in a hist entry, make it point the
hpp_list for its level.  This will be used to support multiple sort keys
in a single hierarchy level.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-3-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 45 +++++++++++++++++--------------
 tools/perf/ui/gtk/hists.c      | 20 +++++++++-----
 tools/perf/ui/hist.c           |  5 ++++
 tools/perf/ui/stdio/hist.c     | 44 +++++++++++++++----------------
 tools/perf/util/hist.c         | 60 ++++++++++++++++++++++++------------------
 tools/perf/util/hist.h         |  1 +
 tools/perf/util/sort.h         |  1 +
 7 files changed, 103 insertions(+), 73 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 5ffffcb..928b482 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1388,25 +1388,26 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 					      HE_COLORSET_NORMAL);
 		}
 
-		ui_browser__write_nstring(&browser->b, "", 2);
-		width -= 2;
+		perf_hpp_list__for_each_format(entry->hpp_list, fmt) {
+			ui_browser__write_nstring(&browser->b, "", 2);
+			width -= 2;
 
-		/*
-		 * No need to call hist_entry__snprintf_alignment()
-		 * since this fmt is always the last column in the
-		 * hierarchy mode.
-		 */
-		fmt = entry->fmt;
-		if (fmt->color) {
-			width -= fmt->color(fmt, &hpp, entry);
-		} else {
-			int i = 0;
+			/*
+			 * No need to call hist_entry__snprintf_alignment()
+			 * since this fmt is always the last column in the
+			 * hierarchy mode.
+			 */
+			if (fmt->color) {
+				width -= fmt->color(fmt, &hpp, entry);
+			} else {
+				int i = 0;
 
-			width -= fmt->entry(fmt, &hpp, entry);
-			ui_browser__printf(&browser->b, "%s", ltrim(s));
+				width -= fmt->entry(fmt, &hpp, entry);
+				ui_browser__printf(&browser->b, "%s", ltrim(s));
 
-			while (isspace(s[i++]))
-				width++;
+				while (isspace(s[i++]))
+					width++;
+			}
 		}
 	}
 
@@ -1934,7 +1935,7 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 	struct perf_hpp_fmt *fmt;
 	bool first = true;
 	int ret;
-	int hierarchy_indent = (nr_sort_keys + 1) * HIERARCHY_INDENT;
+	int hierarchy_indent = nr_sort_keys * HIERARCHY_INDENT;
 
 	printed = fprintf(fp, "%*s", level * HIERARCHY_INDENT, "");
 
@@ -1962,9 +1963,13 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 	ret = scnprintf(hpp.buf, hpp.size, "%*s", hierarchy_indent, "");
 	advance_hpp(&hpp, ret);
 
-	fmt = he->fmt;
-	ret = fmt->entry(fmt, &hpp, he);
-	advance_hpp(&hpp, ret);
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		ret = scnprintf(hpp.buf, hpp.size, "  ");
+		advance_hpp(&hpp, ret);
+
+		ret = fmt->entry(fmt, &hpp, he);
+		advance_hpp(&hpp, ret);
+	}
 
 	printed += fprintf(fp, "%s\n", rtrim(s));
 
diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index a5758fd..4534e2d 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -412,6 +412,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		GtkTreeIter iter;
 		float percent;
+		char *bf;
 
 		he = rb_entry(node, struct hist_entry, rb_node);
 		if (he->filtered)
@@ -437,13 +438,20 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 			gtk_tree_store_set(store, &iter, col_idx++, hpp->buf, -1);
 		}
 
-		fmt = he->fmt;
-		if (fmt->color)
-			fmt->color(fmt, hpp, he);
-		else
-			fmt->entry(fmt, hpp, he);
+		bf = hpp->buf;
+		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+			int ret;
+
+			if (fmt->color)
+				ret = fmt->color(fmt, hpp, he);
+			else
+				ret = fmt->entry(fmt, hpp, he);
+
+			snprintf(hpp->buf + ret, hpp->size - ret, "  ");
+			advance_hpp(hpp, ret + 2);
+		}
 
-		gtk_tree_store_set(store, &iter, col_idx, rtrim(hpp->buf), -1);
+		gtk_tree_store_set(store, &iter, col_idx, rtrim(bf), -1);
 
 		if (!he->leaf) {
 			perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 3a15e84..95795ef 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -722,6 +722,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 	struct perf_hpp_list_node *node = NULL;
 	struct perf_hpp_fmt *fmt_copy;
 	bool found = false;
+	bool skip = perf_hpp__should_skip(fmt, hists);
 
 	list_for_each_entry(node, &hists->hpp_formats, list) {
 		if (node->level == fmt->level) {
@@ -735,6 +736,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 		if (node == NULL)
 			return -1;
 
+		node->skip = skip;
 		node->level = fmt->level;
 		perf_hpp_list__init(&node->hpp);
 
@@ -745,6 +747,9 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 	if (fmt_copy == NULL)
 		return -1;
 
+	if (!skip)
+		node->skip = false;
+
 	list_add_tail(&fmt_copy->list, &node->hpp.fields);
 	list_add_tail(&fmt_copy->sort_list, &node->hpp.sorts);
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 6d06fbb..073642a 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -451,33 +451,33 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 		advance_hpp(hpp, ret);
 	}
 
-	if (sep)
-		ret = scnprintf(hpp->buf, hpp->size, "%s", sep);
-	else
+	if (!sep)
 		ret = scnprintf(hpp->buf, hpp->size, "%*s",
-				(nr_sort_key - 1) * HIERARCHY_INDENT + 2, "");
+				(nr_sort_key - 1) * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
 	printed += fprintf(fp, "%s", buf);
 
-	hpp->buf  = buf;
-	hpp->size = size;
-
-	/*
-	 * No need to call hist_entry__snprintf_alignment() since this
-	 * fmt is always the last column in the hierarchy mode.
-	 */
-	fmt = he->fmt;
-	if (perf_hpp__use_color() && fmt->color)
-		fmt->color(fmt, hpp, he);
-	else
-		fmt->entry(fmt, hpp, he);
-
-	/*
-	 * dynamic entries are right-aligned but we want left-aligned
-	 * in the hierarchy mode
-	 */
-	printed += fprintf(fp, "%s\n", ltrim(buf));
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		hpp->buf  = buf;
+		hpp->size = size;
+
+		/*
+		 * No need to call hist_entry__snprintf_alignment() since this
+		 * fmt is always the last column in the hierarchy mode.
+		 */
+		if (perf_hpp__use_color() && fmt->color)
+			fmt->color(fmt, hpp, he);
+		else
+			fmt->entry(fmt, hpp, he);
+
+		/*
+		 * dynamic entries are right-aligned but we want left-aligned
+		 * in the hierarchy mode
+		 */
+		printed += fprintf(fp, "%s%s", sep ?: "  ", ltrim(buf));
+	}
+	printed += putc('\n', fp);
 
 	if (symbol_conf.use_callchain && he->leaf) {
 		u64 total = hists__total_period(hists);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fea92fc..29da9e0 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1091,18 +1091,25 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he);
 static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 						 struct rb_root *root,
 						 struct hist_entry *he,
-						 struct perf_hpp_fmt *fmt)
+						 struct perf_hpp_list *hpp_list)
 {
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter, *new;
+	struct perf_hpp_fmt *fmt;
 	int64_t cmp;
 
 	while (*p != NULL) {
 		parent = *p;
 		iter = rb_entry(parent, struct hist_entry, rb_node_in);
 
-		cmp = fmt->collapse(fmt, iter, he);
+		cmp = 0;
+		perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+			cmp = fmt->collapse(fmt, iter, he);
+			if (cmp)
+				break;
+		}
+
 		if (!cmp) {
 			he_stat__add_stat(&iter->stat, &he->stat);
 			return iter;
@@ -1121,24 +1128,26 @@ static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 	hists__apply_filters(hists, new);
 	hists->nr_entries++;
 
-	/* save related format for output */
-	new->fmt = fmt;
+	/* save related format list for output */
+	new->hpp_list = hpp_list;
 
 	/* some fields are now passed to 'new' */
-	if (perf_hpp__is_trace_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-		he->trace_output = NULL;
-	else
-		new->trace_output = NULL;
+	perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+		if (perf_hpp__is_trace_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
+			he->trace_output = NULL;
+		else
+			new->trace_output = NULL;
 
-	if (perf_hpp__is_srcline_entry(fmt))
-		he->srcline = NULL;
-	else
-		new->srcline = NULL;
+		if (perf_hpp__is_srcline_entry(fmt))
+			he->srcline = NULL;
+		else
+			new->srcline = NULL;
 
-	if (perf_hpp__is_srcfile_entry(fmt))
-		he->srcfile = NULL;
-	else
-		new->srcfile = NULL;
+		if (perf_hpp__is_srcfile_entry(fmt))
+			he->srcfile = NULL;
+		else
+			new->srcfile = NULL;
+	}
 
 	rb_link_node(&new->rb_node_in, parent, p);
 	rb_insert_color(&new->rb_node_in, root);
@@ -1149,21 +1158,19 @@ static int hists__hierarchy_insert_entry(struct hists *hists,
 					 struct rb_root *root,
 					 struct hist_entry *he)
 {
-	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
 	struct hist_entry *new_he = NULL;
 	struct hist_entry *parent = NULL;
 	int depth = 0;
 	int ret = 0;
 
-	hists__for_each_sort_list(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) &&
-		    !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		/* skip period (overhead) and elided columns */
+		if (node->level == 0 || node->skip)
 			continue;
 
 		/* insert copy of 'he' for each fmt into the hierarchy */
-		new_he = hierarchy_insert_entry(hists, root, he, fmt);
+		new_he = hierarchy_insert_entry(hists, root, he, &node->hpp);
 		if (new_he == NULL) {
 			ret = -1;
 			break;
@@ -1358,6 +1365,7 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
+	struct perf_hpp_fmt *fmt;
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1373,8 +1381,10 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
 	rb_insert_color(&he->rb_node, root);
 
 	/* update column width of dynamic entry */
-	if (perf_hpp__is_dynamic_entry(he->fmt))
-		he->fmt->sort(he->fmt, he, NULL);
+	perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
+		if (perf_hpp__is_dynamic_entry(fmt))
+			fmt->sort(fmt, he, NULL);
+	}
 }
 
 static void hists__hierarchy_output_resort(struct hists *hists,
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 3cab9dc..2209188 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -249,6 +249,7 @@ struct perf_hpp_list_node {
 	struct list_head	list;
 	struct perf_hpp_list	hpp;
 	int			level;
+	bool			skip;
 };
 
 void perf_hpp_list__column_register(struct perf_hpp_list *list,
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 25a5529..ea1f722 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -130,6 +130,7 @@ struct hist_entry {
 	u32			raw_size;
 	void			*trace_output;
 	struct perf_hpp_fmt	*fmt;
+	struct perf_hpp_list	*hpp_list;
 	struct hist_entry	*parent_he;
 	union {
 		/* this is for hierarchical entry structure */

[PATCH v3 3/7] perf tools: Support multiple sort keys in a hierarchy level

[Namhyung Kim]

This implements having multiple sort keys in a single hierarchy level.
Originally only single sort key is supported for each level, but now
using the group syntax with '{ }', it can set more than one sort key in
one level.  Note that now it needs to quote in order to prevent shell
interpretation.

For example:

  $ perf report --hierarchy -s '{comm,dso},sym'
  ...
  #       Overhead  Command / Shared Object / Symbol
  # ..............  ..........................................
  #
      48.67%        swapper          [kernel.vmlinux]
         34.42%        [k] intel_idle
          1.30%        [k] __tick_nohz_idle_enter
          1.03%        [k] cpuidle_reflect
       8.87%        firefox          libpthread-2.22.so
          6.60%        [.] __GI___libc_recvmsg
          1.18%        [.] pthread_cond_signal@@GLIBC_2.3.2
          1.09%        [.] 0x000000000000ff4b
       6.11%        Xorg             libc-2.22.so
          5.27%        [.] __memcpy_sse2_unaligned

In the above example, the command name and the shared object name are
shown on the same line but the symbol name is on the different line.
Since the first two are grouped by '{}', they are in the same level.

Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 71d45d147376..041f236379e0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)
 	char *tmp, *tok;
 	int ret = 0;
 	int level = 0;
+	int next_level = 1;
+	bool in_group = false;
+
+	do {
+		tok = str;
+		tmp = strpbrk(str, "{}, ");
+		if (tmp) {
+			if (in_group)
+				next_level = level;
+			else
+				next_level = level + 1;
+
+			if (*tmp == '{')
+				in_group = true;
+			else if (*tmp == '}')
+				in_group = false;
+
+			*tmp = '\0';
+			str = tmp + 1;
+		}
 
-	for (tok = strtok_r(str, ", ", &tmp);
-			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		ret = sort_dimension__add(tok, evlist, level++);
-		if (ret == -EINVAL) {
-			error("Invalid --sort key: `%s'", tok);
-			break;
-		} else if (ret == -ESRCH) {
-			error("Unknown --sort key: `%s'", tok);
-			break;
+		if (*tok) {
+			ret = sort_dimension__add(tok, evlist, level);
+			if (ret == -EINVAL) {
+				error("Invalid --sort key: `%s'", tok);
+				break;
+			} else if (ret == -ESRCH) {
+				error("Unknown --sort key: `%s'", tok);
+				break;
+			}
 		}
-	}
+
+		level = next_level;
+	} while (tmp);
 
 	return ret;
 }
-- 
2.7.2

Re: [PATCH v3 3/7] perf tools: Support multiple sort keys in a hierarchy level

[Arnaldo Carvalho de Melo]

Em Mon, Mar 07, 2016 at 11:35:04PM +0900, Namhyung Kim escreveu:
> This implements having multiple sort keys in a single hierarchy level.
> Originally only single sort key is supported for each level, but now
> using the group syntax with '{ }', it can set more than one sort key in
> one level.  Note that now it needs to quote in order to prevent shell
> interpretation.
> 
> For example:
> 
>   $ perf report --hierarchy -s '{comm,dso},sym'
>   ...
>   #       Overhead  Command / Shared Object / Symbol

It works, but there is no difference on the one line above, the header,
to help differentiate the above from:


    $ perf report --hierarchy -s 'comm,dso,sym'

Also we don't warn the user that such groupings only make sense when
used with --hierarchy.

Anyway, applying the patch, those are things that can be done on top.

- Arnaldo


>   # ..............  ..........................................
>   #
>       48.67%        swapper          [kernel.vmlinux]
>          34.42%        [k] intel_idle
>           1.30%        [k] __tick_nohz_idle_enter
>           1.03%        [k] cpuidle_reflect
>        8.87%        firefox          libpthread-2.22.so
>           6.60%        [.] __GI___libc_recvmsg
>           1.18%        [.] pthread_cond_signal@@GLIBC_2.3.2
>           1.09%        [.] 0x000000000000ff4b
>        6.11%        Xorg             libc-2.22.so
>           5.27%        [.] __memcpy_sse2_unaligned
> 
> In the above example, the command name and the shared object name are
> shown on the same line but the symbol name is on the different line.
> Since the first two are grouped by '{}', they are in the same level.
> 
> Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 42 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 71d45d147376..041f236379e0 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)
>  	char *tmp, *tok;
>  	int ret = 0;
>  	int level = 0;
> +	int next_level = 1;
> +	bool in_group = false;
> +
> +	do {
> +		tok = str;
> +		tmp = strpbrk(str, "{}, ");
> +		if (tmp) {
> +			if (in_group)
> +				next_level = level;
> +			else
> +				next_level = level + 1;
> +
> +			if (*tmp == '{')
> +				in_group = true;
> +			else if (*tmp == '}')
> +				in_group = false;
> +
> +			*tmp = '\0';
> +			str = tmp + 1;
> +		}
>  
> -	for (tok = strtok_r(str, ", ", &tmp);
> -			tok; tok = strtok_r(NULL, ", ", &tmp)) {
> -		ret = sort_dimension__add(tok, evlist, level++);
> -		if (ret == -EINVAL) {
> -			error("Invalid --sort key: `%s'", tok);
> -			break;
> -		} else if (ret == -ESRCH) {
> -			error("Unknown --sort key: `%s'", tok);
> -			break;
> +		if (*tok) {
> +			ret = sort_dimension__add(tok, evlist, level);
> +			if (ret == -EINVAL) {
> +				error("Invalid --sort key: `%s'", tok);
> +				break;
> +			} else if (ret == -ESRCH) {
> +				error("Unknown --sort key: `%s'", tok);
> +				break;
> +			}
>  		}
> -	}
> +
> +		level = next_level;
> +	} while (tmp);
>  
>  	return ret;
>  }
> -- 
> 2.7.2

[tip:perf/core] perf hists: Support multiple sort keys in a hierarchy level

[tip-bot for Namhyung Kim]

Commit-ID:  a23f37e864609f0887c1cb77c4d5b62586484a61
Gitweb:     http://git.kernel.org/tip/a23f37e864609f0887c1cb77c4d5b62586484a61
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:47 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:20 +0100

perf hists: Support multiple sort keys in a hierarchy level

This implements having multiple sort keys in a single hierarchy level.
Originally only single sort key is supported for each level, but now
using the group syntax with '{ }', it can set more than one sort key in
one level.  Note that now it needs to quote in order to prevent shell
interpretation.

For example:

  $ perf report --hierarchy -s '{comm,dso},sym'
  ...
  #       Overhead  Command / Shared Object / Symbol
  # ..............  ..........................................
  #
      48.67%        swapper          [kernel.vmlinux]
         34.42%        [k] intel_idle
          1.30%        [k] __tick_nohz_idle_enter
          1.03%        [k] cpuidle_reflect
       8.87%        firefox          libpthread-2.22.so
          6.60%        [.] __GI___libc_recvmsg
          1.18%        [.] pthread_cond_signal@@GLIBC_2.3.2
          1.09%        [.] 0x000000000000ff4b
       6.11%        Xorg             libc-2.22.so
          5.27%        [.] __memcpy_sse2_unaligned

In the above example, the command name and the shared object name are
shown on the same line but the symbol name is on the different line.
Since the first two are grouped by '{}', they are in the same level.

Suggested-and-Tested=by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-4-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/util/sort.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 71d45d1..041f236 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)
 	char *tmp, *tok;
 	int ret = 0;
 	int level = 0;
+	int next_level = 1;
+	bool in_group = false;
+
+	do {
+		tok = str;
+		tmp = strpbrk(str, "{}, ");
+		if (tmp) {
+			if (in_group)
+				next_level = level;
+			else
+				next_level = level + 1;
+
+			if (*tmp == '{')
+				in_group = true;
+			else if (*tmp == '}')
+				in_group = false;
+
+			*tmp = '\0';
+			str = tmp + 1;
+		}
 
-	for (tok = strtok_r(str, ", ", &tmp);
-			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		ret = sort_dimension__add(tok, evlist, level++);
-		if (ret == -EINVAL) {
-			error("Invalid --sort key: `%s'", tok);
-			break;
-		} else if (ret == -ESRCH) {
-			error("Unknown --sort key: `%s'", tok);
-			break;
+		if (*tok) {
+			ret = sort_dimension__add(tok, evlist, level);
+			if (ret == -EINVAL) {
+				error("Invalid --sort key: `%s'", tok);
+				break;
+			} else if (ret == -ESRCH) {
+				error("Unknown --sort key: `%s'", tok);
+				break;
+			}
 		}
-	}
+
+		level = next_level;
+	} while (tmp);
 
 	return ret;
 }

[PATCH v3 4/7] perf tools: Fix indent for multiple hierarchy sort key

[Namhyung Kim]

When multiple sort keys are used in a single hierarchy, it should indent
using number of hierarchy levels instead of number of sort keys.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 23 ++++++++++-------------
 tools/perf/ui/hist.c           |  1 +
 tools/perf/ui/stdio/hist.c     | 26 +++++++++++---------------
 tools/perf/util/hist.h         |  1 +
 4 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 928b4825b752..2f02ce79bd9d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1280,7 +1280,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 					      struct hist_entry *entry,
 					      unsigned short row,
-					      int level, int nr_sort_keys)
+					      int level)
 {
 	int printed = 0;
 	int width = browser->b.width;
@@ -1294,7 +1294,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		.current_entry	= current_entry,
 	};
 	int column = 0;
-	int hierarchy_indent = (nr_sort_keys - 1) * HIERARCHY_INDENT;
+	int hierarchy_indent = (entry->hists->nr_hpp_node - 2) * HIERARCHY_INDENT;
 
 	if (current_entry) {
 		browser->he_selection = entry;
@@ -1436,8 +1436,7 @@ show_callchain:
 }
 
 static int hist_browser__show_no_entry(struct hist_browser *browser,
-				       unsigned short row,
-				       int level, int nr_sort_keys)
+				       unsigned short row, int level)
 {
 	int width = browser->b.width;
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
@@ -1445,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	int column = 0;
 	int ret;
 	struct perf_hpp_fmt *fmt;
+	int indent = browser->hists->nr_hpp_node - 2;
 
 	if (current_entry) {
 		browser->he_selection = NULL;
@@ -1485,8 +1485,8 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 		width -= ret;
 	}
 
-	ui_browser__write_nstring(&browser->b, "", nr_sort_keys * HIERARCHY_INDENT);
-	width -= nr_sort_keys * HIERARCHY_INDENT;
+	ui_browser__write_nstring(&browser->b, "", indent * HIERARCHY_INDENT);
+	width -= indent * HIERARCHY_INDENT;
 
 	if (column >= browser->b.horiz_scroll) {
 		char buf[32];
@@ -1553,7 +1553,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	struct perf_hpp_fmt *fmt;
 	size_t ret = 0;
 	int column = 0;
-	int nr_sort_keys = hists->nr_sort_keys;
+	int indent = hists->nr_hpp_node - 2;
 	bool first = true;
 
 	ret = scnprintf(buf, size, " ");
@@ -1577,7 +1577,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	}
 
 	ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
-			(nr_sort_keys - 1) * HIERARCHY_INDENT, "");
+			indent * HIERARCHY_INDENT, "");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
@@ -1645,7 +1645,6 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 	u16 header_offset = 0;
 	struct rb_node *nd;
 	struct hist_browser *hb = container_of(browser, struct hist_browser, b);
-	int nr_sort = hb->hists->nr_sort_keys;
 
 	if (hb->show_headers) {
 		hist_browser__show_headers(hb);
@@ -1672,14 +1671,12 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 
 		if (symbol_conf.report_hierarchy) {
 			row += hist_browser__show_hierarchy_entry(hb, h, row,
-								  h->depth,
-								  nr_sort);
+								  h->depth);
 			if (row == browser->rows)
 				break;
 
 			if (h->has_no_entry) {
-				hist_browser__show_no_entry(hb, row, h->depth,
-							    nr_sort);
+				hist_browser__show_no_entry(hb, row, h->depth);
 				row++;
 			}
 		} else {
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 95795ef4209b..f03c4f70438f 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -740,6 +740,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 		node->level = fmt->level;
 		perf_hpp_list__init(&node->hpp);
 
+		hists->nr_hpp_node++;
 		list_add_tail(&node->list, &hists->hpp_formats);
 	}
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 073642a63cc9..543d7137cc0c 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -412,7 +412,7 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 
 static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 					 struct perf_hpp *hpp,
-					 int nr_sort_key, struct hists *hists,
+					 struct hists *hists,
 					 FILE *fp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -453,7 +453,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 
 	if (!sep)
 		ret = scnprintf(hpp->buf, hpp->size, "%*s",
-				(nr_sort_key - 1) * HIERARCHY_INDENT, "");
+				(hists->nr_hpp_node - 2) * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
 	printed += fprintf(fp, "%s", buf);
@@ -504,12 +504,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
 
-	if (symbol_conf.report_hierarchy) {
-		int nr_sort = hists->nr_sort_keys;
-
-		return hist_entry__hierarchy_fprintf(he, &hpp, nr_sort,
-						     hists, fp);
-	}
+	if (symbol_conf.report_hierarchy)
+		return hist_entry__hierarchy_fprintf(he, &hpp, hists, fp);
 
 	hist_entry__snprintf(he, &hpp);
 
@@ -521,29 +517,29 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	return ret;
 }
 
-static int print_hierarchy_indent(const char *sep, int nr_sort,
+static int print_hierarchy_indent(const char *sep, int indent,
 				  const char *line, FILE *fp)
 {
-	if (sep != NULL || nr_sort < 1)
+	if (sep != NULL || indent < 2)
 		return 0;
 
-	return fprintf(fp, "%-.*s", (nr_sort - 1) * HIERARCHY_INDENT, line);
+	return fprintf(fp, "%-.*s", (indent - 2) * HIERARCHY_INDENT, line);
 }
 
 static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 				  const char *sep, FILE *fp)
 {
 	bool first = true;
-	int nr_sort;
+	int indent;
 	int depth;
 	unsigned width = 0;
 	unsigned header_width = 0;
 	struct perf_hpp_fmt *fmt;
 
-	nr_sort = hists->nr_sort_keys;
+	indent = hists->nr_hpp_node;
 
 	/* preserve max indent depth for column headers */
-	print_hierarchy_indent(sep, nr_sort, spaces, fp);
+	print_hierarchy_indent(sep, indent, spaces, fp);
 
 	hists__for_each_format(hists, fmt) {
 		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
@@ -582,7 +578,7 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 	fprintf(fp, "\n# ");
 
 	/* preserve max indent depth for initial dots */
-	print_hierarchy_indent(sep, nr_sort, dots, fp);
+	print_hierarchy_indent(sep, indent, dots, fp);
 
 	first = true;
 	hists__for_each_format(hists, fmt) {
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2209188d729c..2cb017f28f9e 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -80,6 +80,7 @@ struct hists {
 	struct perf_hpp_list	*hpp_list;
 	struct list_head	hpp_formats;
 	int			nr_sort_keys;
+	int			nr_hpp_node;
 };
 
 struct hist_entry_iter;
-- 
2.7.2

[tip:perf/core] perf hists: Fix indent for multiple hierarchy sort key

[tip-bot for Namhyung Kim]

Commit-ID:  2dbbe9f26c082be5aa0e8ba5480e7bac43b2c4f0
Gitweb:     http://git.kernel.org/tip/2dbbe9f26c082be5aa0e8ba5480e7bac43b2c4f0
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:48 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:20 +0100

perf hists: Fix indent for multiple hierarchy sort key

When multiple sort keys are used in a single hierarchy, it should indent
using number of hierarchy levels instead of number of sort keys.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-5-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 23 ++++++++++-------------
 tools/perf/ui/hist.c           |  1 +
 tools/perf/ui/stdio/hist.c     | 26 +++++++++++---------------
 tools/perf/util/hist.h         |  1 +
 4 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 928b482..2f02ce7 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1280,7 +1280,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 					      struct hist_entry *entry,
 					      unsigned short row,
-					      int level, int nr_sort_keys)
+					      int level)
 {
 	int printed = 0;
 	int width = browser->b.width;
@@ -1294,7 +1294,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		.current_entry	= current_entry,
 	};
 	int column = 0;
-	int hierarchy_indent = (nr_sort_keys - 1) * HIERARCHY_INDENT;
+	int hierarchy_indent = (entry->hists->nr_hpp_node - 2) * HIERARCHY_INDENT;
 
 	if (current_entry) {
 		browser->he_selection = entry;
@@ -1436,8 +1436,7 @@ show_callchain:
 }
 
 static int hist_browser__show_no_entry(struct hist_browser *browser,
-				       unsigned short row,
-				       int level, int nr_sort_keys)
+				       unsigned short row, int level)
 {
 	int width = browser->b.width;
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
@@ -1445,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	int column = 0;
 	int ret;
 	struct perf_hpp_fmt *fmt;
+	int indent = browser->hists->nr_hpp_node - 2;
 
 	if (current_entry) {
 		browser->he_selection = NULL;
@@ -1485,8 +1485,8 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 		width -= ret;
 	}
 
-	ui_browser__write_nstring(&browser->b, "", nr_sort_keys * HIERARCHY_INDENT);
-	width -= nr_sort_keys * HIERARCHY_INDENT;
+	ui_browser__write_nstring(&browser->b, "", indent * HIERARCHY_INDENT);
+	width -= indent * HIERARCHY_INDENT;
 
 	if (column >= browser->b.horiz_scroll) {
 		char buf[32];
@@ -1553,7 +1553,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	struct perf_hpp_fmt *fmt;
 	size_t ret = 0;
 	int column = 0;
-	int nr_sort_keys = hists->nr_sort_keys;
+	int indent = hists->nr_hpp_node - 2;
 	bool first = true;
 
 	ret = scnprintf(buf, size, " ");
@@ -1577,7 +1577,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	}
 
 	ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
-			(nr_sort_keys - 1) * HIERARCHY_INDENT, "");
+			indent * HIERARCHY_INDENT, "");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
@@ -1645,7 +1645,6 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 	u16 header_offset = 0;
 	struct rb_node *nd;
 	struct hist_browser *hb = container_of(browser, struct hist_browser, b);
-	int nr_sort = hb->hists->nr_sort_keys;
 
 	if (hb->show_headers) {
 		hist_browser__show_headers(hb);
@@ -1672,14 +1671,12 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 
 		if (symbol_conf.report_hierarchy) {
 			row += hist_browser__show_hierarchy_entry(hb, h, row,
-								  h->depth,
-								  nr_sort);
+								  h->depth);
 			if (row == browser->rows)
 				break;
 
 			if (h->has_no_entry) {
-				hist_browser__show_no_entry(hb, row, h->depth,
-							    nr_sort);
+				hist_browser__show_no_entry(hb, row, h->depth);
 				row++;
 			}
 		} else {
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 95795ef..f03c4f7 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -740,6 +740,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 		node->level = fmt->level;
 		perf_hpp_list__init(&node->hpp);
 
+		hists->nr_hpp_node++;
 		list_add_tail(&node->list, &hists->hpp_formats);
 	}
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 073642a..543d713 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -412,7 +412,7 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 
 static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 					 struct perf_hpp *hpp,
-					 int nr_sort_key, struct hists *hists,
+					 struct hists *hists,
 					 FILE *fp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -453,7 +453,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 
 	if (!sep)
 		ret = scnprintf(hpp->buf, hpp->size, "%*s",
-				(nr_sort_key - 1) * HIERARCHY_INDENT, "");
+				(hists->nr_hpp_node - 2) * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
 	printed += fprintf(fp, "%s", buf);
@@ -504,12 +504,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
 
-	if (symbol_conf.report_hierarchy) {
-		int nr_sort = hists->nr_sort_keys;
-
-		return hist_entry__hierarchy_fprintf(he, &hpp, nr_sort,
-						     hists, fp);
-	}
+	if (symbol_conf.report_hierarchy)
+		return hist_entry__hierarchy_fprintf(he, &hpp, hists, fp);
 
 	hist_entry__snprintf(he, &hpp);
 
@@ -521,29 +517,29 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	return ret;
 }
 
-static int print_hierarchy_indent(const char *sep, int nr_sort,
+static int print_hierarchy_indent(const char *sep, int indent,
 				  const char *line, FILE *fp)
 {
-	if (sep != NULL || nr_sort < 1)
+	if (sep != NULL || indent < 2)
 		return 0;
 
-	return fprintf(fp, "%-.*s", (nr_sort - 1) * HIERARCHY_INDENT, line);
+	return fprintf(fp, "%-.*s", (indent - 2) * HIERARCHY_INDENT, line);
 }
 
 static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 				  const char *sep, FILE *fp)
 {
 	bool first = true;
-	int nr_sort;
+	int indent;
 	int depth;
 	unsigned width = 0;
 	unsigned header_width = 0;
 	struct perf_hpp_fmt *fmt;
 
-	nr_sort = hists->nr_sort_keys;
+	indent = hists->nr_hpp_node;
 
 	/* preserve max indent depth for column headers */
-	print_hierarchy_indent(sep, nr_sort, spaces, fp);
+	print_hierarchy_indent(sep, indent, spaces, fp);
 
 	hists__for_each_format(hists, fmt) {
 		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
@@ -582,7 +578,7 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 	fprintf(fp, "\n# ");
 
 	/* preserve max indent depth for initial dots */
-	print_hierarchy_indent(sep, nr_sort, dots, fp);
+	print_hierarchy_indent(sep, indent, dots, fp);
 
 	first = true;
 	hists__for_each_format(hists, fmt) {
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2209188..2cb017f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -80,6 +80,7 @@ struct hists {
 	struct perf_hpp_list	*hpp_list;
 	struct list_head	hpp_formats;
 	int			nr_sort_keys;
+	int			nr_hpp_node;
 };
 
 struct hist_entry_iter;

[PATCH v3 5/7] perf report: Use hierarchy hpp list on stdio

[Namhyung Kim]

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Use this info to print entries with multiple sort keys in a
single hierarchy properly.

For example, the below example shows using 4 sort keys with 2 levels.

  $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
   --percent-limit 1 -i perf.data.sched
  ...
  #    Overhead  prev_pid+prev_comm / next_pid+next_comm
  # ...........  .......................................
  #
      22.36%     0  swapper/0
          9.48%     17773  transmission-gt
          5.25%     109  kworker/0:1H
          1.53%     6524  Xephyr
      21.39%     17773  transmission-gt
          9.52%     0  swapper/0
          9.04%     0  swapper/2
          1.78%     0  swapper/3

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 103 +++++++++++++++++++++++++--------------------
 1 file changed, 57 insertions(+), 46 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 543d7137cc0c..7aff5acf3265 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -417,6 +417,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 {
 	const char *sep = symbol_conf.field_sep;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	char *buf = hpp->buf;
 	size_t size = hpp->size;
 	int ret, printed = 0;
@@ -428,10 +429,10 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
-	hists__for_each_format(he->hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		/*
 		 * If there's no field_sep, we still need
 		 * to display initial '  '.
@@ -529,50 +530,49 @@ static int print_hierarchy_indent(const char *sep, int indent,
 static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 				  const char *sep, FILE *fp)
 {
-	bool first = true;
+	bool first_node, first_col;
 	int indent;
 	int depth;
 	unsigned width = 0;
 	unsigned header_width = 0;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 
 	indent = hists->nr_hpp_node;
 
 	/* preserve max indent depth for column headers */
 	print_hierarchy_indent(sep, indent, spaces, fp);
 
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
-		if (!first)
-			fprintf(fp, "%s", sep ?: "  ");
-		else
-			first = false;
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
 
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		fmt->header(fmt, hpp, hists_to_evsel(hists));
-		fprintf(fp, "%s", hpp->buf);
+		fprintf(fp, "%s%s", hpp->buf, sep ?: "  ");
 	}
 
 	/* combine sort headers with ' / ' */
-	first = true;
-	hists__for_each_format(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (!first)
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node)
 			header_width += fprintf(fp, " / ");
-		else {
-			fprintf(fp, "%s", sep ?: "  ");
-			first = false;
-		}
+		first_node = false;
 
-		fmt->header(fmt, hpp, hists_to_evsel(hists));
-		rtrim(hpp->buf);
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				header_width += fprintf(fp, "+");
+			first_col = false;
+
+			fmt->header(fmt, hpp, hists_to_evsel(hists));
+			rtrim(hpp->buf);
 
-		header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+		}
 	}
 
 	fprintf(fp, "\n# ");
@@ -580,29 +580,35 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 	/* preserve max indent depth for initial dots */
 	print_hierarchy_indent(sep, indent, dots, fp);
 
-	first = true;
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
 
-		if (!first)
-			fprintf(fp, "%s", sep ?: "  ");
-		else
-			first = false;
+	first_col = true;
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+		if (!first_col)
+			fprintf(fp, "%s", sep ?: "..");
+		first_col = false;
 
 		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
 		fprintf(fp, "%.*s", width, dots);
 	}
 
 	depth = 0;
-	hists__for_each_format(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		first_col = true;
+		width = depth * HIERARCHY_INDENT;
 
-		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
-		width += depth * HIERARCHY_INDENT;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				width++;  /* for '+' sign between column header */
+			first_col = false;
+
+			width += fmt->width(fmt, hpp, hists_to_evsel(hists));
+		}
 
 		if (width > header_width)
 			header_width = width;
@@ -621,6 +627,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		      int max_cols, float min_pcnt, FILE *fp)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	struct rb_node *nd;
 	size_t ret = 0;
 	unsigned int width;
@@ -650,6 +657,10 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 	fprintf(fp, "# ");
 
 	if (symbol_conf.report_hierarchy) {
+		list_for_each_entry(fmt_node, &hists->hpp_formats, list) {
+			perf_hpp_list__for_each_format(&fmt_node->hpp, fmt)
+				perf_hpp__reset_width(fmt, hists);
+		}
 		nr_rows += print_hierarchy_header(hists, &dummy_hpp, sep, fp);
 		goto print_entries;
 	}
@@ -734,9 +745,9 @@ print_entries:
 		 * display "no entry >= x.xx%" message.
 		 */
 		if (!h->leaf && !hist_entry__has_hierarchy_children(h, min_pcnt)) {
-			int nr_sort = hists->nr_sort_keys;
+			int depth = hists->nr_hpp_node + h->depth + 1;
 
-			print_hierarchy_indent(sep, nr_sort + h->depth + 1, spaces, fp);
+			print_hierarchy_indent(sep, depth, spaces, fp);
 			fprintf(fp, "%*sno entry >= %.2f%%\n", indent, "", min_pcnt);
 
 			if (max_rows && ++nr_rows >= max_rows)
-- 
2.7.2

Re: [PATCH v3 5/7] perf report: Use hierarchy hpp list on stdio

[Arnaldo Carvalho de Melo]

Em Mon, Mar 07, 2016 at 11:35:06PM +0900, Namhyung Kim escreveu:
> Now hpp formats are linked using perf_hpp_list_node when hierarchy is
> enabled.  Use this info to print entries with multiple sort keys in a
> single hierarchy properly.
> 
> For example, the below example shows using 4 sort keys with 2 levels.
> 
>   $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
>    --percent-limit 1 -i perf.data.sched
>   ...
>   #    Overhead  prev_pid+prev_comm / next_pid+next_comm
>   # ...........  .......................................
>   #

Ok, this one addresses my previous comment, I think the next one will do it for
the TUI, good. :-)


-#    Overhead  prev_pid / prev_comm / next_pid / next_comm
-# ...........  ...........................................
+#    Overhead  prev_pid+prev_comm / next_pid+next_comm
+# ...........  .......................................


>       22.36%     0  swapper/0
>           9.48%     17773  transmission-gt
>           5.25%     109  kworker/0:1H
>           1.53%     6524  Xephyr
>       21.39%     17773  transmission-gt
>           9.52%     0  swapper/0
>           9.04%     0  swapper/2
>           1.78%     0  swapper/3
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/stdio/hist.c | 103 +++++++++++++++++++++++++--------------------
>  1 file changed, 57 insertions(+), 46 deletions(-)
> 
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index 543d7137cc0c..7aff5acf3265 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -417,6 +417,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
>  {
>  	const char *sep = symbol_conf.field_sep;
>  	struct perf_hpp_fmt *fmt;
> +	struct perf_hpp_list_node *fmt_node;
>  	char *buf = hpp->buf;
>  	size_t size = hpp->size;
>  	int ret, printed = 0;
> @@ -428,10 +429,10 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
>  	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
>  	advance_hpp(hpp, ret);
>  
> -	hists__for_each_format(he->hists, fmt) {
> -		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
> -			break;
> -
> +	/* the first hpp_list_node is for overhead columns */
> +	fmt_node = list_first_entry(&hists->hpp_formats,
> +				    struct perf_hpp_list_node, list);
> +	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
>  		/*
>  		 * If there's no field_sep, we still need
>  		 * to display initial '  '.
> @@ -529,50 +530,49 @@ static int print_hierarchy_indent(const char *sep, int indent,
>  static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
>  				  const char *sep, FILE *fp)
>  {
> -	bool first = true;
> +	bool first_node, first_col;
>  	int indent;
>  	int depth;
>  	unsigned width = 0;
>  	unsigned header_width = 0;
>  	struct perf_hpp_fmt *fmt;
> +	struct perf_hpp_list_node *fmt_node;
>  
>  	indent = hists->nr_hpp_node;
>  
>  	/* preserve max indent depth for column headers */
>  	print_hierarchy_indent(sep, indent, spaces, fp);
>  
> -	hists__for_each_format(hists, fmt) {
> -		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
> -			break;
> -
> -		if (!first)
> -			fprintf(fp, "%s", sep ?: "  ");
> -		else
> -			first = false;
> +	/* the first hpp_list_node is for overhead columns */
> +	fmt_node = list_first_entry(&hists->hpp_formats,
> +				    struct perf_hpp_list_node, list);
>  
> +	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
>  		fmt->header(fmt, hpp, hists_to_evsel(hists));
> -		fprintf(fp, "%s", hpp->buf);
> +		fprintf(fp, "%s%s", hpp->buf, sep ?: "  ");
>  	}
>  
>  	/* combine sort headers with ' / ' */
> -	first = true;
> -	hists__for_each_format(hists, fmt) {
> -		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
> -			continue;
> -		if (perf_hpp__should_skip(fmt, hists))
> -			continue;
> -
> -		if (!first)
> +	first_node = true;
> +	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
> +		if (!first_node)
>  			header_width += fprintf(fp, " / ");
> -		else {
> -			fprintf(fp, "%s", sep ?: "  ");
> -			first = false;
> -		}
> +		first_node = false;
>  
> -		fmt->header(fmt, hpp, hists_to_evsel(hists));
> -		rtrim(hpp->buf);
> +		first_col = true;
> +		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
> +			if (perf_hpp__should_skip(fmt, hists))
> +				continue;
> +
> +			if (!first_col)
> +				header_width += fprintf(fp, "+");
> +			first_col = false;
> +
> +			fmt->header(fmt, hpp, hists_to_evsel(hists));
> +			rtrim(hpp->buf);
>  
> -		header_width += fprintf(fp, "%s", ltrim(hpp->buf));
> +			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
> +		}
>  	}
>  
>  	fprintf(fp, "\n# ");
> @@ -580,29 +580,35 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
>  	/* preserve max indent depth for initial dots */
>  	print_hierarchy_indent(sep, indent, dots, fp);
>  
> -	first = true;
> -	hists__for_each_format(hists, fmt) {
> -		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
> -			break;
> +	/* the first hpp_list_node is for overhead columns */
> +	fmt_node = list_first_entry(&hists->hpp_formats,
> +				    struct perf_hpp_list_node, list);
>  
> -		if (!first)
> -			fprintf(fp, "%s", sep ?: "  ");
> -		else
> -			first = false;
> +	first_col = true;
> +	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
> +		if (!first_col)
> +			fprintf(fp, "%s", sep ?: "..");
> +		first_col = false;
>  
>  		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
>  		fprintf(fp, "%.*s", width, dots);
>  	}
>  
>  	depth = 0;
> -	hists__for_each_format(hists, fmt) {
> -		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
> -			continue;
> -		if (perf_hpp__should_skip(fmt, hists))
> -			continue;
> +	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
> +		first_col = true;
> +		width = depth * HIERARCHY_INDENT;
>  
> -		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
> -		width += depth * HIERARCHY_INDENT;
> +		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
> +			if (perf_hpp__should_skip(fmt, hists))
> +				continue;
> +
> +			if (!first_col)
> +				width++;  /* for '+' sign between column header */
> +			first_col = false;
> +
> +			width += fmt->width(fmt, hpp, hists_to_evsel(hists));
> +		}
>  
>  		if (width > header_width)
>  			header_width = width;
> @@ -621,6 +627,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
>  		      int max_cols, float min_pcnt, FILE *fp)
>  {
>  	struct perf_hpp_fmt *fmt;
> +	struct perf_hpp_list_node *fmt_node;
>  	struct rb_node *nd;
>  	size_t ret = 0;
>  	unsigned int width;
> @@ -650,6 +657,10 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
>  	fprintf(fp, "# ");
>  
>  	if (symbol_conf.report_hierarchy) {
> +		list_for_each_entry(fmt_node, &hists->hpp_formats, list) {
> +			perf_hpp_list__for_each_format(&fmt_node->hpp, fmt)
> +				perf_hpp__reset_width(fmt, hists);
> +		}
>  		nr_rows += print_hierarchy_header(hists, &dummy_hpp, sep, fp);
>  		goto print_entries;
>  	}
> @@ -734,9 +745,9 @@ print_entries:
>  		 * display "no entry >= x.xx%" message.
>  		 */
>  		if (!h->leaf && !hist_entry__has_hierarchy_children(h, min_pcnt)) {
> -			int nr_sort = hists->nr_sort_keys;
> +			int depth = hists->nr_hpp_node + h->depth + 1;
>  
> -			print_hierarchy_indent(sep, nr_sort + h->depth + 1, spaces, fp);
> +			print_hierarchy_indent(sep, depth, spaces, fp);
>  			fprintf(fp, "%*sno entry >= %.2f%%\n", indent, "", min_pcnt);
>  
>  			if (max_rows && ++nr_rows >= max_rows)
> -- 
> 2.7.2

Re: [PATCH v3 5/7] perf report: Use hierarchy hpp list on stdio

[Arnaldo Carvalho de Melo]

Em Mon, Mar 07, 2016 at 03:06:04PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Mar 07, 2016 at 11:35:06PM +0900, Namhyung Kim escreveu:
> > Now hpp formats are linked using perf_hpp_list_node when hierarchy is
> > enabled.  Use this info to print entries with multiple sort keys in a
> > single hierarchy properly.
> > 
> > For example, the below example shows using 4 sort keys with 2 levels.
> > 
> >   $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
> >    --percent-limit 1 -i perf.data.sched
> >   ...
> >   #    Overhead  prev_pid+prev_comm / next_pid+next_comm
> >   # ...........  .......................................
> >   #
> 
> Ok, this one addresses my previous comment, I think the next one will do it for
> the TUI, good. :-)
> 
> 
> -#    Overhead  prev_pid / prev_comm / next_pid / next_comm
> -# ...........  ...........................................
> +#    Overhead  prev_pid+prev_comm / next_pid+next_comm
> +# ...........  .......................................

It ends up adding an extra space to the other cases, harmless, I guess:

@@ -68,7 +68,7 @@
 # Samples: 1K of event 'sched:sched_stat_sleep'
 # Event count (approx.): 0
 #
-# Overhead
+# Overhead  
  ........
 #

Do you test like:

   perf report ... > /tmp/before
   make
   perf report ... > /tmp/after
   diff /tmp/before /tmp/after

?

[tip:perf/core] perf report: Use hierarchy hpp list on stdio

[tip-bot for Namhyung Kim]

Commit-ID:  f58c95e344c26223c6503e6ecb0c1e11806d91e0
Gitweb:     http://git.kernel.org/tip/f58c95e344c26223c6503e6ecb0c1e11806d91e0
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:49 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:20 +0100

perf report: Use hierarchy hpp list on stdio

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Use this info to print entries with multiple sort keys in a
single hierarchy properly.

For example, the below example shows using 4 sort keys with 2 levels.

  $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
   --percent-limit 1 -i perf.data.sched
  ...
  #    Overhead  prev_pid+prev_comm / next_pid+next_comm
  # ...........  .......................................
  #
      22.36%     0  swapper/0
          9.48%     17773  transmission-gt
          5.25%     109  kworker/0:1H
          1.53%     6524  Xephyr
      21.39%     17773  transmission-gt
          9.52%     0  swapper/0
          9.04%     0  swapper/2
          1.78%     0  swapper/3

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-6-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 103 +++++++++++++++++++++++++--------------------
 1 file changed, 57 insertions(+), 46 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 543d713..7aff5ac 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -417,6 +417,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 {
 	const char *sep = symbol_conf.field_sep;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	char *buf = hpp->buf;
 	size_t size = hpp->size;
 	int ret, printed = 0;
@@ -428,10 +429,10 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
-	hists__for_each_format(he->hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		/*
 		 * If there's no field_sep, we still need
 		 * to display initial '  '.
@@ -529,50 +530,49 @@ static int print_hierarchy_indent(const char *sep, int indent,
 static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 				  const char *sep, FILE *fp)
 {
-	bool first = true;
+	bool first_node, first_col;
 	int indent;
 	int depth;
 	unsigned width = 0;
 	unsigned header_width = 0;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 
 	indent = hists->nr_hpp_node;
 
 	/* preserve max indent depth for column headers */
 	print_hierarchy_indent(sep, indent, spaces, fp);
 
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
-		if (!first)
-			fprintf(fp, "%s", sep ?: "  ");
-		else
-			first = false;
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
 
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		fmt->header(fmt, hpp, hists_to_evsel(hists));
-		fprintf(fp, "%s", hpp->buf);
+		fprintf(fp, "%s%s", hpp->buf, sep ?: "  ");
 	}
 
 	/* combine sort headers with ' / ' */
-	first = true;
-	hists__for_each_format(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (!first)
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node)
 			header_width += fprintf(fp, " / ");
-		else {
-			fprintf(fp, "%s", sep ?: "  ");
-			first = false;
-		}
+		first_node = false;
 
-		fmt->header(fmt, hpp, hists_to_evsel(hists));
-		rtrim(hpp->buf);
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				header_width += fprintf(fp, "+");
+			first_col = false;
+
+			fmt->header(fmt, hpp, hists_to_evsel(hists));
+			rtrim(hpp->buf);
 
-		header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+		}
 	}
 
 	fprintf(fp, "\n# ");
@@ -580,29 +580,35 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 	/* preserve max indent depth for initial dots */
 	print_hierarchy_indent(sep, indent, dots, fp);
 
-	first = true;
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
 
-		if (!first)
-			fprintf(fp, "%s", sep ?: "  ");
-		else
-			first = false;
+	first_col = true;
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+		if (!first_col)
+			fprintf(fp, "%s", sep ?: "..");
+		first_col = false;
 
 		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
 		fprintf(fp, "%.*s", width, dots);
 	}
 
 	depth = 0;
-	hists__for_each_format(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		first_col = true;
+		width = depth * HIERARCHY_INDENT;
 
-		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
-		width += depth * HIERARCHY_INDENT;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				width++;  /* for '+' sign between column header */
+			first_col = false;
+
+			width += fmt->width(fmt, hpp, hists_to_evsel(hists));
+		}
 
 		if (width > header_width)
 			header_width = width;
@@ -621,6 +627,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		      int max_cols, float min_pcnt, FILE *fp)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	struct rb_node *nd;
 	size_t ret = 0;
 	unsigned int width;
@@ -650,6 +657,10 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 	fprintf(fp, "# ");
 
 	if (symbol_conf.report_hierarchy) {
+		list_for_each_entry(fmt_node, &hists->hpp_formats, list) {
+			perf_hpp_list__for_each_format(&fmt_node->hpp, fmt)
+				perf_hpp__reset_width(fmt, hists);
+		}
 		nr_rows += print_hierarchy_header(hists, &dummy_hpp, sep, fp);
 		goto print_entries;
 	}
@@ -734,9 +745,9 @@ print_entries:
 		 * display "no entry >= x.xx%" message.
 		 */
 		if (!h->leaf && !hist_entry__has_hierarchy_children(h, min_pcnt)) {
-			int nr_sort = hists->nr_sort_keys;
+			int depth = hists->nr_hpp_node + h->depth + 1;
 
-			print_hierarchy_indent(sep, nr_sort + h->depth + 1, spaces, fp);
+			print_hierarchy_indent(sep, depth, spaces, fp);
 			fprintf(fp, "%*sno entry >= %.2f%%\n", indent, "", min_pcnt);
 
 			if (max_rows && ++nr_rows >= max_rows)

[PATCH v3 6/7] perf hists browser: Use hierarchy hpp list

[Namhyung Kim]

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Like in stdio, use this info to print entries with multiple
sort keys in a single hierarchy properly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 81 +++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 36 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 2f02ce79bd9d..e0e217ec856b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1289,6 +1289,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 	off_t row_offset = entry->row_offset;
 	bool first = true;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	struct hpp_arg arg = {
 		.b		= &browser->b,
 		.current_entry	= current_entry,
@@ -1320,7 +1321,10 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 	ui_browser__write_nstring(&browser->b, "", level * HIERARCHY_INDENT);
 	width -= level * HIERARCHY_INDENT;
 
-	hists__for_each_format(entry->hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&entry->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		char s[2048];
 		struct perf_hpp hpp = {
 			.buf		= s,
@@ -1332,10 +1336,6 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		    column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		if (current_entry && browser->b.navkeypressed) {
 			ui_browser__set_color(&browser->b,
 					      HE_COLORSET_SELECTED);
@@ -1444,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	int column = 0;
 	int ret;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	int indent = browser->hists->nr_hpp_node - 2;
 
 	if (current_entry) {
@@ -1461,15 +1462,14 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	ui_browser__write_nstring(&browser->b, "", level * HIERARCHY_INDENT);
 	width -= level * HIERARCHY_INDENT;
 
-	hists__for_each_format(browser->hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&browser->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (perf_hpp__should_skip(fmt, browser->hists) ||
 		    column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		ret = fmt->width(fmt, NULL, hists_to_evsel(browser->hists));
 
 		if (first) {
@@ -1551,22 +1551,23 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 		.size   = size,
 	};
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	size_t ret = 0;
 	int column = 0;
 	int indent = hists->nr_hpp_node - 2;
-	bool first = true;
+	bool first_node, first_col;
 
 	ret = scnprintf(buf, size, " ");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-	hists__for_each_format(hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
@@ -1581,34 +1582,42 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-	hists__for_each_format(hists, fmt) {
-		char *start;
-
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (first) {
-			first = false;
-		} else {
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node) {
 			ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, " / ");
 			if (advance_hpp_check(&dummy_hpp, ret))
 				break;
 		}
+		first_node = false;
 
-		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
-		dummy_hpp.buf[ret] = '\0';
-		rtrim(dummy_hpp.buf);
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			char *start;
 
-		start = ltrim(dummy_hpp.buf);
-		ret = strlen(start);
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
 
-		if (start != dummy_hpp.buf)
-			memmove(dummy_hpp.buf, start, ret + 1);
+			if (!first_col) {
+				ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "+");
+				if (advance_hpp_check(&dummy_hpp, ret))
+					break;
+			}
+			first_col = false;
 
-		if (advance_hpp_check(&dummy_hpp, ret))
-			break;
+			ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
+			dummy_hpp.buf[ret] = '\0';
+			rtrim(dummy_hpp.buf);
+
+			start = ltrim(dummy_hpp.buf);
+			ret = strlen(start);
+
+			if (start != dummy_hpp.buf)
+				memmove(dummy_hpp.buf, start, ret + 1);
+
+			if (advance_hpp_check(&dummy_hpp, ret))
+				break;
+		}
 	}
 
 	return ret;
@@ -1676,7 +1685,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 				break;
 
 			if (h->has_no_entry) {
-				hist_browser__show_no_entry(hb, row, h->depth);
+				hist_browser__show_no_entry(hb, row, h->depth + 1);
 				row++;
 			}
 		} else {
-- 
2.7.2

[tip:perf/core] perf hists browser: Use hierarchy hpp list

[tip-bot for Namhyung Kim]

Commit-ID:  a61a22f6845f9e86e0ca60d1d256a35ca12312ef
Gitweb:     http://git.kernel.org/tip/a61a22f6845f9e86e0ca60d1d256a35ca12312ef
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:50 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:21 +0100

perf hists browser: Use hierarchy hpp list

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Like in stdio, use this info to print entries with multiple
sort keys in a single hierarchy properly.

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-7-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 81 +++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 36 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 2f02ce7..e0e217e 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1289,6 +1289,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 	off_t row_offset = entry->row_offset;
 	bool first = true;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	struct hpp_arg arg = {
 		.b		= &browser->b,
 		.current_entry	= current_entry,
@@ -1320,7 +1321,10 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 	ui_browser__write_nstring(&browser->b, "", level * HIERARCHY_INDENT);
 	width -= level * HIERARCHY_INDENT;
 
-	hists__for_each_format(entry->hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&entry->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		char s[2048];
 		struct perf_hpp hpp = {
 			.buf		= s,
@@ -1332,10 +1336,6 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		    column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		if (current_entry && browser->b.navkeypressed) {
 			ui_browser__set_color(&browser->b,
 					      HE_COLORSET_SELECTED);
@@ -1444,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	int column = 0;
 	int ret;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	int indent = browser->hists->nr_hpp_node - 2;
 
 	if (current_entry) {
@@ -1461,15 +1462,14 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	ui_browser__write_nstring(&browser->b, "", level * HIERARCHY_INDENT);
 	width -= level * HIERARCHY_INDENT;
 
-	hists__for_each_format(browser->hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&browser->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (perf_hpp__should_skip(fmt, browser->hists) ||
 		    column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		ret = fmt->width(fmt, NULL, hists_to_evsel(browser->hists));
 
 		if (first) {
@@ -1551,22 +1551,23 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 		.size   = size,
 	};
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	size_t ret = 0;
 	int column = 0;
 	int indent = hists->nr_hpp_node - 2;
-	bool first = true;
+	bool first_node, first_col;
 
 	ret = scnprintf(buf, size, " ");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-	hists__for_each_format(hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
@@ -1581,34 +1582,42 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-	hists__for_each_format(hists, fmt) {
-		char *start;
-
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (first) {
-			first = false;
-		} else {
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node) {
 			ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, " / ");
 			if (advance_hpp_check(&dummy_hpp, ret))
 				break;
 		}
+		first_node = false;
 
-		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
-		dummy_hpp.buf[ret] = '\0';
-		rtrim(dummy_hpp.buf);
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			char *start;
 
-		start = ltrim(dummy_hpp.buf);
-		ret = strlen(start);
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
 
-		if (start != dummy_hpp.buf)
-			memmove(dummy_hpp.buf, start, ret + 1);
+			if (!first_col) {
+				ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "+");
+				if (advance_hpp_check(&dummy_hpp, ret))
+					break;
+			}
+			first_col = false;
 
-		if (advance_hpp_check(&dummy_hpp, ret))
-			break;
+			ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
+			dummy_hpp.buf[ret] = '\0';
+			rtrim(dummy_hpp.buf);
+
+			start = ltrim(dummy_hpp.buf);
+			ret = strlen(start);
+
+			if (start != dummy_hpp.buf)
+				memmove(dummy_hpp.buf, start, ret + 1);
+
+			if (advance_hpp_check(&dummy_hpp, ret))
+				break;
+		}
 	}
 
 	return ret;
@@ -1676,7 +1685,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 				break;
 
 			if (h->has_no_entry) {
-				hist_browser__show_no_entry(hb, row, h->depth);
+				hist_browser__show_no_entry(hb, row, h->depth + 1);
 				row++;
 			}
 		} else {

[PATCH v3 7/7] perf report: Use hierarchy hpp list on gtk

[Namhyung Kim]

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Like in stdio, use this info to print entries with multiple
sort keys in a single hierarchy properly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/gtk/hists.c | 55 ++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index 4534e2d7669c..bd9bf7e343b1 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -407,7 +407,9 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 	struct rb_node *node;
 	struct hist_entry *he;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	u64 total = hists__total_period(hists);
+	int size;
 
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		GtkTreeIter iter;
@@ -425,11 +427,11 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 		gtk_tree_store_append(store, &iter, parent);
 
 		col_idx = 0;
-		hists__for_each_format(hists, fmt) {
-			if (perf_hpp__is_sort_entry(fmt) ||
-			    perf_hpp__is_dynamic_entry(fmt))
-				break;
 
+		/* the first hpp_list_node is for overhead columns */
+		fmt_node = list_first_entry(&hists->hpp_formats,
+					    struct perf_hpp_list_node, list);
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 			if (fmt->color)
 				fmt->color(fmt, hpp, he);
 			else
@@ -439,6 +441,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 		}
 
 		bf = hpp->buf;
+		size = hpp->size;
 		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
 			int ret;
 
@@ -451,9 +454,12 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 			advance_hpp(hpp, ret + 2);
 		}
 
-		gtk_tree_store_set(store, &iter, col_idx, rtrim(bf), -1);
+		gtk_tree_store_set(store, &iter, col_idx, ltrim(rtrim(bf)), -1);
 
 		if (!he->leaf) {
+			hpp->buf = bf;
+			hpp->size = size;
+
 			perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
 							store, &iter, hpp,
 							min_pcnt);
@@ -486,6 +492,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 				     float min_pcnt)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	GType col_types[MAX_COLUMNS];
 	GtkCellRenderer *renderer;
 	GtkTreeStore *store;
@@ -494,7 +501,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 	int nr_cols = 0;
 	char s[512];
 	char buf[512];
-	bool first = true;
+	bool first_node, first_col;
 	struct perf_hpp hpp = {
 		.buf		= s,
 		.size		= sizeof(s),
@@ -514,11 +521,11 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 	renderer = gtk_cell_renderer_text_new();
 
 	col_idx = 0;
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
 
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
 							    -1, fmt->name,
 							    renderer, "markup",
@@ -527,20 +534,24 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 
 	/* construct merged column header since sort keys share single column */
 	buf[0] = '\0';
-	hists__for_each_format(hists ,fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) &&
-		    !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (first)
-			first = false;
-		else
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node)
 			strcat(buf, " / ");
+		first_node = false;
 
-		fmt->header(fmt, &hpp, hists_to_evsel(hists));
-		strcat(buf, rtrim(hpp.buf));
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp ,fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				strcat(buf, "+");
+			first_col = false;
+
+			fmt->header(fmt, &hpp, hists_to_evsel(hists));
+			strcat(buf, ltrim(rtrim(hpp.buf)));
+		}
 	}
 
 	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-- 
2.7.2

[tip:perf/core] perf report: Use hierarchy hpp list on gtk

[tip-bot for Namhyung Kim]

Commit-ID:  58ecd33be90647724a78ce5e0b42f5bc482771fd
Gitweb:     http://git.kernel.org/tip/58ecd33be90647724a78ce5e0b42f5bc482771fd
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 7 Mar 2016 16:44:51 -0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 8 Mar 2016 10:11:21 +0100

perf report: Use hierarchy hpp list on gtk

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Like in stdio, use this info to print entries with multiple
sort keys in a single hierarchy properly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-8-git-send-email-namhyung@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/ui/gtk/hists.c | 55 ++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index 4534e2d..bd9bf7e 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -407,7 +407,9 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 	struct rb_node *node;
 	struct hist_entry *he;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	u64 total = hists__total_period(hists);
+	int size;
 
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		GtkTreeIter iter;
@@ -425,11 +427,11 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 		gtk_tree_store_append(store, &iter, parent);
 
 		col_idx = 0;
-		hists__for_each_format(hists, fmt) {
-			if (perf_hpp__is_sort_entry(fmt) ||
-			    perf_hpp__is_dynamic_entry(fmt))
-				break;
 
+		/* the first hpp_list_node is for overhead columns */
+		fmt_node = list_first_entry(&hists->hpp_formats,
+					    struct perf_hpp_list_node, list);
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 			if (fmt->color)
 				fmt->color(fmt, hpp, he);
 			else
@@ -439,6 +441,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 		}
 
 		bf = hpp->buf;
+		size = hpp->size;
 		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
 			int ret;
 
@@ -451,9 +454,12 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 			advance_hpp(hpp, ret + 2);
 		}
 
-		gtk_tree_store_set(store, &iter, col_idx, rtrim(bf), -1);
+		gtk_tree_store_set(store, &iter, col_idx, ltrim(rtrim(bf)), -1);
 
 		if (!he->leaf) {
+			hpp->buf = bf;
+			hpp->size = size;
+
 			perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
 							store, &iter, hpp,
 							min_pcnt);
@@ -486,6 +492,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 				     float min_pcnt)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	GType col_types[MAX_COLUMNS];
 	GtkCellRenderer *renderer;
 	GtkTreeStore *store;
@@ -494,7 +501,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 	int nr_cols = 0;
 	char s[512];
 	char buf[512];
-	bool first = true;
+	bool first_node, first_col;
 	struct perf_hpp hpp = {
 		.buf		= s,
 		.size		= sizeof(s),
@@ -514,11 +521,11 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 	renderer = gtk_cell_renderer_text_new();
 
 	col_idx = 0;
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
 
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
 							    -1, fmt->name,
 							    renderer, "markup",
@@ -527,20 +534,24 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 
 	/* construct merged column header since sort keys share single column */
 	buf[0] = '\0';
-	hists__for_each_format(hists ,fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) &&
-		    !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (first)
-			first = false;
-		else
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node)
 			strcat(buf, " / ");
+		first_node = false;
 
-		fmt->header(fmt, &hpp, hists_to_evsel(hists));
-		strcat(buf, rtrim(hpp.buf));
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp ,fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				strcat(buf, "+");
+			first_col = false;
+
+			fmt->header(fmt, &hpp, hists_to_evsel(hists));
+			strcat(buf, ltrim(rtrim(hpp.buf)));
+		}
 	}
 
 	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),