linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy
@ 2016-03-08 15:06 Namhyung Kim
  2016-03-08 15:06 ` [PATCH 2/6] perf tools: Add more sort entry check functions Namhyung Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Namhyung Kim @ 2016-03-08 15:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern,
	Andi Kleen, Stephane Eranian, Wang Nan

When hierarchy mode is enabled each output format is in a separate hpp
list.  So when applying filter it should check all formats in the list.
Currently it only checks a single ->fmt field which was not set properly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 041f236379e0..2e803847046c 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1602,16 +1602,30 @@ int hist_entry__filter(struct hist_entry *he, int type, const void *arg)
 {
 	struct perf_hpp_fmt *fmt;
 	struct hpp_sort_entry *hse;
+	int ret = -1;
+	int r;
 
-	fmt = he->fmt;
-	if (fmt == NULL || !perf_hpp__is_sort_entry(fmt))
-		return -1;
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		if (fmt == NULL || !perf_hpp__is_sort_entry(fmt))
+			continue;
 
-	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	if (hse->se->se_filter == NULL)
-		return -1;
+		hse = container_of(fmt, struct hpp_sort_entry, hpp);
+		if (hse->se->se_filter == NULL)
+			continue;
 
-	return hse->se->se_filter(he, type, arg);
+		/*
+		 * hist entry is filtered when all sort keys in the hpp list
+		 * is applied.  So if any sort key returns 0, it will *NOT*
+		 * filtered out.
+		 */
+		r = hse->se->se_filter(he, type, arg);
+		if (r == 0)
+			return 0;
+		if (r > 0)
+			ret = 1;
+	}
+
+	return ret;
 }
 
 static int __sort_dimension__add_hpp_sort(struct sort_dimension *sd, int level)
-- 
2.7.2

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

* [PATCH 2/6] perf tools: Add more sort entry check functions
  2016-03-08 15:06 [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
@ 2016-03-08 15:06 ` Namhyung Kim
  2016-03-08 15:06 ` [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode Namhyung Kim
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2016-03-08 15:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern,
	Andi Kleen, Stephane Eranian, Wang Nan

Those function are to check given perf_hpp_fmt is filter-related sort
entries or not.  With hierarchy mode, it needs to check filters on the
hist entries with its own hpp format list.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.h |  4 ++++
 tools/perf/util/sort.c | 50 +++++++++++++++++++-------------------------------
 2 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2cb017f28f9e..6870a1bfd762 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -318,6 +318,10 @@ bool perf_hpp__defined_dynamic_entry(struct perf_hpp_fmt *fmt, struct hists *his
 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);
+bool perf_hpp__is_thread_entry(struct perf_hpp_fmt *fmt);
+bool perf_hpp__is_comm_entry(struct perf_hpp_fmt *fmt);
+bool perf_hpp__is_dso_entry(struct perf_hpp_fmt *fmt);
+bool perf_hpp__is_sym_entry(struct perf_hpp_fmt *fmt);
 
 struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt);
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 2e803847046c..6bb831ddc6a7 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1488,38 +1488,26 @@ bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format)
 	return format->header == __sort__hpp_header;
 }
 
-bool perf_hpp__is_trace_entry(struct perf_hpp_fmt *fmt)
-{
-	struct hpp_sort_entry *hse;
+#define MK_SORT_ENTRY_CHK(key)					\
+bool perf_hpp__is_ ## key ## _entry(struct perf_hpp_fmt *fmt)	\
+{								\
+	struct hpp_sort_entry *hse;				\
+								\
+	if (!perf_hpp__is_sort_entry(fmt))			\
+		return false;					\
+								\
+	hse = container_of(fmt, struct hpp_sort_entry, hpp);	\
+	return hse->se == &sort_ ## key ;			\
+}
+
+MK_SORT_ENTRY_CHK(trace)
+MK_SORT_ENTRY_CHK(srcline)
+MK_SORT_ENTRY_CHK(srcfile)
+MK_SORT_ENTRY_CHK(thread)
+MK_SORT_ENTRY_CHK(comm)
+MK_SORT_ENTRY_CHK(dso)
+MK_SORT_ENTRY_CHK(sym)
 
-	if (!perf_hpp__is_sort_entry(fmt))
-		return false;
-
-	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	return hse->se == &sort_trace;
-}
-
-bool perf_hpp__is_srcline_entry(struct perf_hpp_fmt *fmt)
-{
-	struct hpp_sort_entry *hse;
-
-	if (!perf_hpp__is_sort_entry(fmt))
-		return false;
-
-	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	return hse->se == &sort_srcline;
-}
-
-bool perf_hpp__is_srcfile_entry(struct perf_hpp_fmt *fmt)
-{
-	struct hpp_sort_entry *hse;
-
-	if (!perf_hpp__is_sort_entry(fmt))
-		return false;
-
-	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	return hse->se == &sort_srcfile;
-}
 
 static bool __sort__hpp_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
 {
-- 
2.7.2

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

* [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode
  2016-03-08 15:06 [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
  2016-03-08 15:06 ` [PATCH 2/6] perf tools: Add more sort entry check functions Namhyung Kim
@ 2016-03-08 15:06 ` Namhyung Kim
  2016-03-09  9:13   ` Jiri Olsa
  2016-03-08 15:06 ` [PATCH 4/6] perf tools: Remove hist_entry->fmt field Namhyung Kim
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Namhyung Kim @ 2016-03-08 15:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern,
	Andi Kleen, Stephane Eranian, Wang Nan

When a command-line filter was applied in hierarchy mode, output was
broken especially when filtering on lower level.  The higher level
entries didn't show up so it's hard to see the result.

Also it needs to handle multi sort keys in a single level of hierarchy.

Before:

  $ perf report --hierarchy -s 'cpu,{dso,comm}' --comms swapper --stdio
  ...
  #    Overhead  CPU / Shared Object+Command
  # ...........  ...........................
  #
         13.79%     [kernel.vmlinux]  swapper
      31.71%     000
         13.80%     [kernel.vmlinux]  swapper
          0.43%     [e1000e]          swapper
         11.89%     [kernel.vmlinux]  swapper
          9.18%     [kernel.vmlinux]  swapper

After:

  #    Overhead  CPU / Shared Object+Command
  # ...........  ...............................
  #
      33.09%     003
         13.79%     [kernel.vmlinux]  swapper
      31.71%     000
         13.80%     [kernel.vmlinux]  swapper
          0.43%     [e1000e]          swapper
      21.90%     002
         11.89%     [kernel.vmlinux]  swapper
      13.30%     001
          9.18%     [kernel.vmlinux]  swapper

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 29da9e0d8db9..7c5eb11cabb6 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1087,10 +1087,86 @@ int hist_entry__snprintf_alignment(struct hist_entry *he, struct perf_hpp *hpp,
  */
 
 static void hists__apply_filters(struct hists *hists, struct hist_entry *he);
+static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *he,
+				       enum hist_filter type);
+
+typedef bool (*fmt_chk_fn)(struct perf_hpp_fmt *fmt);
+
+static bool check_thread_entry(struct perf_hpp_fmt *fmt)
+{
+	return perf_hpp__is_thread_entry(fmt) || perf_hpp__is_comm_entry(fmt);
+}
+
+static void hist_entry__check_and_remove_filter(struct hist_entry *he,
+						enum hist_filter type,
+						fmt_chk_fn check)
+{
+	struct perf_hpp_fmt *fmt;
+	bool type_match = false;
+	struct hist_entry *parent = he->parent_he;
+
+	switch (type) {
+	case HIST_FILTER__THREAD:
+		if (symbol_conf.comm_list == NULL &&
+		    symbol_conf.pid_list == NULL &&
+		    symbol_conf.tid_list == NULL)
+			return;
+		break;
+	case HIST_FILTER__DSO:
+		if (symbol_conf.dso_list == NULL)
+			return;
+		break;
+	case HIST_FILTER__SYMBOL:
+		if (symbol_conf.sym_list == NULL)
+			return;
+		break;
+	case HIST_FILTER__PARENT:
+	case HIST_FILTER__GUEST:
+	case HIST_FILTER__HOST:
+	case HIST_FILTER__SOCKET:
+	default:
+		return;
+	}
+
+	/* if it's filtered by own fmt, it has to have filter bits */
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		if (check(fmt)) {
+			type_match = true;
+			break;
+		}
+	}
+
+	if (type_match) {
+		if (!(he->filtered & (1 << type))) {
+			while (parent) {
+				parent->filtered &= ~(1 << type);
+				parent = parent->parent_he;
+			}
+		}
+	} else {
+		if (parent == NULL)
+			he->filtered |= (1 << type);
+		else
+			he->filtered |= (parent->filtered & (1 << type));
+	}
+}
+
+static void hist_entry__apply_hierarchy_filters(struct hist_entry *he)
+{
+	hist_entry__check_and_remove_filter(he, HIST_FILTER__THREAD,
+					    check_thread_entry);
+
+	hist_entry__check_and_remove_filter(he, HIST_FILTER__DSO,
+					    perf_hpp__is_dso_entry);
+
+	hist_entry__check_and_remove_filter(he, HIST_FILTER__SYMBOL,
+					    perf_hpp__is_sym_entry);
+}
 
 static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 						 struct rb_root *root,
 						 struct hist_entry *he,
+						 struct hist_entry *parent_he,
 						 struct perf_hpp_list *hpp_list)
 {
 	struct rb_node **p = &root->rb_node;
@@ -1125,11 +1201,13 @@ static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 	if (new == NULL)
 		return NULL;
 
-	hists__apply_filters(hists, new);
 	hists->nr_entries++;
 
 	/* save related format list for output */
 	new->hpp_list = hpp_list;
+	new->parent_he = parent_he;
+
+	hist_entry__apply_hierarchy_filters(new);
 
 	/* some fields are now passed to 'new' */
 	perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
@@ -1170,14 +1248,13 @@ static int hists__hierarchy_insert_entry(struct hists *hists,
 			continue;
 
 		/* insert copy of 'he' for each fmt into the hierarchy */
-		new_he = hierarchy_insert_entry(hists, root, he, &node->hpp);
+		new_he = hierarchy_insert_entry(hists, root, he, parent, &node->hpp);
 		if (new_he == NULL) {
 			ret = -1;
 			break;
 		}
 
 		root = &new_he->hroot_in;
-		new_he->parent_he = parent;
 		new_he->depth = depth++;
 		parent = new_he;
 	}
-- 
2.7.2

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

* [PATCH 4/6] perf tools: Remove hist_entry->fmt field
  2016-03-08 15:06 [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
  2016-03-08 15:06 ` [PATCH 2/6] perf tools: Add more sort entry check functions Namhyung Kim
  2016-03-08 15:06 ` [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode Namhyung Kim
@ 2016-03-08 15:06 ` Namhyung Kim
  2016-03-08 15:06 ` [PATCH 5/6] perf hists browser: Cleanup hist_browser__fprintf_hierarchy_entry() Namhyung Kim
  2016-03-08 15:06 ` [PATCH 6/6] perf tools: Remove nr_sort_keys field Namhyung Kim
  4 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2016-03-08 15:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern,
	Andi Kleen, Stephane Eranian, Wang Nan

It's not used anymore and the output format is accessed by the hpp_list
pointer instead when hierarchy is enabled.  Let's get rid of it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index ea1f722cffea..151afc1b6c2f 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -129,7 +129,6 @@ struct hist_entry {
 	void			*raw_data;
 	u32			raw_size;
 	void			*trace_output;
-	struct perf_hpp_fmt	*fmt;
 	struct perf_hpp_list	*hpp_list;
 	struct hist_entry	*parent_he;
 	union {
-- 
2.7.2

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

* [PATCH 5/6] perf hists browser: Cleanup hist_browser__fprintf_hierarchy_entry()
  2016-03-08 15:06 [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
                   ` (2 preceding siblings ...)
  2016-03-08 15:06 ` [PATCH 4/6] perf tools: Remove hist_entry->fmt field Namhyung Kim
@ 2016-03-08 15:06 ` Namhyung Kim
  2016-03-08 15:06 ` [PATCH 6/6] perf tools: Remove nr_sort_keys field Namhyung Kim
  4 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2016-03-08 15:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern,
	Andi Kleen, Stephane Eranian, Wang Nan

The hist_browser__fprintf_hierarchy_entry() if to dump current output
into a file so it needs to be sync-ed with the corresponding function
hist_browser__show_hierarchy_entry().  So use hists->nr_hpp_node to
indent width and use first fmt_node to print overhead columns instead of
checking whether it's a sort entry (or dynamic entry).

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

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e0e217ec856b..aed9c8f011f7 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1928,8 +1928,7 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
 
 static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 						 struct hist_entry *he,
-						 FILE *fp, int level,
-						 int nr_sort_keys)
+						 FILE *fp, int level)
 {
 	char s[8192];
 	int printed = 0;
@@ -1939,23 +1938,20 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 		.size = sizeof(s),
 	};
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	bool first = true;
 	int ret;
-	int hierarchy_indent = nr_sort_keys * HIERARCHY_INDENT;
+	int hierarchy_indent = (he->hists->nr_hpp_node - 2) * HIERARCHY_INDENT;
 
 	printed = fprintf(fp, "%*s", level * HIERARCHY_INDENT, "");
 
 	folded_sign = hist_entry__folded(he);
 	printed += fprintf(fp, "%c", folded_sign);
 
-	hists__for_each_format(he->hists, fmt) {
-		if (perf_hpp__should_skip(fmt, he->hists))
-			continue;
-
-		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(&he->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (!first) {
 			ret = scnprintf(hpp.buf, hpp.size, "  ");
 			advance_hpp(&hpp, ret);
@@ -1992,7 +1988,6 @@ static int hist_browser__fprintf(struct hist_browser *browser, FILE *fp)
 	struct rb_node *nd = hists__filter_entries(rb_first(browser->b.entries),
 						   browser->min_pcnt);
 	int printed = 0;
-	int nr_sort = browser->hists->nr_sort_keys;
 
 	while (nd) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
@@ -2000,8 +1995,7 @@ static int hist_browser__fprintf(struct hist_browser *browser, FILE *fp)
 		if (symbol_conf.report_hierarchy) {
 			printed += hist_browser__fprintf_hierarchy_entry(browser,
 									 h, fp,
-									 h->depth,
-									 nr_sort);
+									 h->depth);
 		} else {
 			printed += hist_browser__fprintf_entry(browser, h, fp);
 		}
-- 
2.7.2

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

* [PATCH 6/6] perf tools: Remove nr_sort_keys field
  2016-03-08 15:06 [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
                   ` (3 preceding siblings ...)
  2016-03-08 15:06 ` [PATCH 5/6] perf hists browser: Cleanup hist_browser__fprintf_hierarchy_entry() Namhyung Kim
@ 2016-03-08 15:06 ` Namhyung Kim
  4 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2016-03-08 15:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern,
	Andi Kleen, Stephane Eranian, Wang Nan

The nr_sort_keys field is to carry the number of sort entries in a
hpp_list or hists to determine the depth of indentation of a hist entry.
As it's only used in hierarchy mode and now we have used nr_hpp_node for
this reason, there's no need to keep it anymore.  Let's get rid of it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c   |  3 ---
 tools/perf/util/hist.h |  2 --
 tools/perf/util/sort.c | 26 --------------------------
 3 files changed, 31 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index f03c4f70438f..3baeaa6e71b5 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -515,9 +515,6 @@ void perf_hpp_list__column_register(struct perf_hpp_list *list,
 void perf_hpp_list__register_sort_field(struct perf_hpp_list *list,
 					struct perf_hpp_fmt *format)
 {
-	if (perf_hpp__is_sort_entry(format) || perf_hpp__is_dynamic_entry(format))
-		list->nr_sort_keys++;
-
 	list_add_tail(&format->sort_list, &list->sorts);
 }
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 6870a1bfd762..ead18c82294f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -79,7 +79,6 @@ struct hists {
 	int			socket_filter;
 	struct perf_hpp_list	*hpp_list;
 	struct list_head	hpp_formats;
-	int			nr_sort_keys;
 	int			nr_hpp_node;
 };
 
@@ -241,7 +240,6 @@ struct perf_hpp_fmt {
 struct perf_hpp_list {
 	struct list_head fields;
 	struct list_head sorts;
-	int nr_sort_keys;
 };
 
 extern struct perf_hpp_list perf_hpp_list;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 6bb831ddc6a7..be5fcb25496a 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2695,29 +2695,6 @@ out:
 	return ret;
 }
 
-static void evlist__set_hists_nr_sort_keys(struct perf_evlist *evlist)
-{
-	struct perf_evsel *evsel;
-
-	evlist__for_each(evlist, evsel) {
-		struct perf_hpp_fmt *fmt;
-		struct hists *hists = evsel__hists(evsel);
-
-		hists->nr_sort_keys = perf_hpp_list.nr_sort_keys;
-
-		/*
-		 * If dynamic entries were used, it might add multiple
-		 * entries to each evsel for a single field name.  Set
-		 * actual number of sort keys for each hists.
-		 */
-		perf_hpp_list__for_each_sort_list(&perf_hpp_list, fmt) {
-			if (perf_hpp__is_dynamic_entry(fmt) &&
-			    !perf_hpp__defined_dynamic_entry(fmt, hists))
-				hists->nr_sort_keys--;
-		}
-	}
-}
-
 int setup_sorting(struct perf_evlist *evlist)
 {
 	int err;
@@ -2732,9 +2709,6 @@ int setup_sorting(struct perf_evlist *evlist)
 			return err;
 	}
 
-	if (evlist != NULL)
-		evlist__set_hists_nr_sort_keys(evlist);
-
 	reset_dimensions();
 
 	/*
-- 
2.7.2

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

* Re: [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode
  2016-03-08 15:06 ` [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode Namhyung Kim
@ 2016-03-09  9:13   ` Jiri Olsa
  2016-03-09 12:44     ` Namhyung Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2016-03-09  9:13 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, David Ahern, Andi Kleen, Stephane Eranian, Wang Nan

On Wed, Mar 09, 2016 at 12:06:40AM +0900, Namhyung Kim wrote:
> When a command-line filter was applied in hierarchy mode, output was
> broken especially when filtering on lower level.  The higher level
> entries didn't show up so it's hard to see the result.
> 
> Also it needs to handle multi sort keys in a single level of hierarchy.
> 
> Before:
> 
>   $ perf report --hierarchy -s 'cpu,{dso,comm}' --comms swapper --stdio
>   ...
>   #    Overhead  CPU / Shared Object+Command
>   # ...........  ...........................
>   #
>          13.79%     [kernel.vmlinux]  swapper
>       31.71%     000
>          13.80%     [kernel.vmlinux]  swapper
>           0.43%     [e1000e]          swapper
>          11.89%     [kernel.vmlinux]  swapper
>           9.18%     [kernel.vmlinux]  swapper
> 
> After:
> 
>   #    Overhead  CPU / Shared Object+Command
>   # ...........  ...............................
>   #
>       33.09%     003
>          13.79%     [kernel.vmlinux]  swapper
>       31.71%     000
>          13.80%     [kernel.vmlinux]  swapper
>           0.43%     [e1000e]          swapper
>       21.90%     002
>          11.89%     [kernel.vmlinux]  swapper
>       13.30%     001
>           9.18%     [kernel.vmlinux]  swapper

I'm getting funny numbers when using 'F' toggle in tui mode 

[jolsa@krava perf]$ ./perf report --hierarchy -s 'cpu,{dso,comm}' --comms swapper 

Samples: 254  of event 'cycles:pp', Event count (approx.): 132263887
 Overhead     CPU / Shared Object+Command                                                                                                                     ◆
+ 69.85%      001                                                                                                                                             ▒
+ 44.28%      000                                                                                                                                             ▒
+ 41.62%      002                                                                                                                                             ▒
+ 36.80%      003                                                                                                                                             


[jolsa@krava perf]$ sudo ./perf top --hierarchy -s 'cpu,{dso,comm}' --comms swapper 

 Overhead     CPU / Shared O+Command
+ 320.64%     000
+ 179.91%     002
+ 137.05%     003
+ 88.37%      001


thanks,
jirka

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

* Re: [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode
  2016-03-09  9:13   ` Jiri Olsa
@ 2016-03-09 12:44     ` Namhyung Kim
  0 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2016-03-09 12:44 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, David Ahern, Andi Kleen, Stephane Eranian, Wang Nan

Hi Jiri,

On Wed, Mar 09, 2016 at 10:13:56AM +0100, Jiri Olsa wrote:
> On Wed, Mar 09, 2016 at 12:06:40AM +0900, Namhyung Kim wrote:
> > When a command-line filter was applied in hierarchy mode, output was
> > broken especially when filtering on lower level.  The higher level
> > entries didn't show up so it's hard to see the result.
> > 
> > Also it needs to handle multi sort keys in a single level of hierarchy.
> > 
> > Before:
> > 
> >   $ perf report --hierarchy -s 'cpu,{dso,comm}' --comms swapper --stdio
> >   ...
> >   #    Overhead  CPU / Shared Object+Command
> >   # ...........  ...........................
> >   #
> >          13.79%     [kernel.vmlinux]  swapper
> >       31.71%     000
> >          13.80%     [kernel.vmlinux]  swapper
> >           0.43%     [e1000e]          swapper
> >          11.89%     [kernel.vmlinux]  swapper
> >           9.18%     [kernel.vmlinux]  swapper
> > 
> > After:
> > 
> >   #    Overhead  CPU / Shared Object+Command
> >   # ...........  ...............................
> >   #
> >       33.09%     003
> >          13.79%     [kernel.vmlinux]  swapper
> >       31.71%     000
> >          13.80%     [kernel.vmlinux]  swapper
> >           0.43%     [e1000e]          swapper
> >       21.90%     002
> >          11.89%     [kernel.vmlinux]  swapper
> >       13.30%     001
> >           9.18%     [kernel.vmlinux]  swapper
> 
> I'm getting funny numbers when using 'F' toggle in tui mode 
> 
> [jolsa@krava perf]$ ./perf report --hierarchy -s 'cpu,{dso,comm}' --comms swapper 
> 
> Samples: 254  of event 'cycles:pp', Event count (approx.): 132263887
>  Overhead     CPU / Shared Object+Command                                                                                                                     ◆
> + 69.85%      001                                                                                                                                             ▒
> + 44.28%      000                                                                                                                                             ▒
> + 41.62%      002                                                                                                                                             ▒
> + 36.80%      003                                                                                                                                             
> 
> 
> [jolsa@krava perf]$ sudo ./perf top --hierarchy -s 'cpu,{dso,comm}' --comms swapper 
> 
>  Overhead     CPU / Shared O+Command
> + 320.64%     000
> + 179.91%     002
> + 137.05%     003
> + 88.37%      001

Hmm.. I think it's because that the total period is a sum of periods
of leaf nodes.  But if a filter is applied, sum of periods of upper
level entries can be different than sum of the lower level entries.
So it should use top-level entries periods instead IMHO.

I will send a fix.

Thanks,
Namhyung

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

end of thread, other threads:[~2016-03-09 12:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-08 15:06 [PATCH 1/6] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
2016-03-08 15:06 ` [PATCH 2/6] perf tools: Add more sort entry check functions Namhyung Kim
2016-03-08 15:06 ` [PATCH 3/6] perf tools: Fix command line filters in hierarchy mode Namhyung Kim
2016-03-09  9:13   ` Jiri Olsa
2016-03-09 12:44     ` Namhyung Kim
2016-03-08 15:06 ` [PATCH 4/6] perf tools: Remove hist_entry->fmt field Namhyung Kim
2016-03-08 15:06 ` [PATCH 5/6] perf hists browser: Cleanup hist_browser__fprintf_hierarchy_entry() Namhyung Kim
2016-03-08 15:06 ` [PATCH 6/6] perf tools: Remove nr_sort_keys field Namhyung Kim

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