Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL New Jiri Olsa 2 participants 24 messages May 21, 2014 – May 22, 2014 perf events

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

hi Ingo,
please consider pulling

thanks,
jirka

The following changes since commit 6480c56130ba073df84d57d61062ec4118b10bbe:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf into perf/core (2014-05-20 08:36:09 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo

for you to fetch changes up to eca8183699964579ca8a0b8d116bd1f4da0136f7:

perf tools: Add automatic remapping of Android libraries (2014-05-21 15:03:25 +0200)

----------------------------------------------------------------
perf/core improvements and fixes:

. Android related fixes for pager and map dso resolving (Michael Lentine)

. Add -F option for specifying output fields (Namhyung Kim)

----------------------------------------------------------------
Michael Lentine (2):
perf tools: Add cat as fallback pager
perf tools: Add automatic remapping of Android libraries

Namhyung Kim (20):
perf tools: Add ->cmp(), ->collapse() and ->sort() to perf_hpp_fmt perf tools: Convert sort entries to hpp formats
perf tools: Use hpp formats to sort hist entries
perf tools: Support event grouping in hpp ->sort()
perf tools: Use hpp formats to sort final output
perf tools: Consolidate output field handling to hpp format routines perf ui: Get rid of callback from __hpp__fmt()
perf tools: Allow hpp fields to be sort keys
perf tools: Consolidate management of default sort orders perf tools: Call perf_hpp__init() before setting up GUI browsers perf report: Add -F option to specify output fields
perf tools: Add ->sort() member to struct sort_entry perf report/tui: Fix a bug when --fields/sort is given perf top: Add --fields option to specify output fields perf tools: Skip elided sort entries
perf hists: Reset width of output fields with header length perf tools: Get rid of obsolete hist_entry__sort_list perf tools: Introduce reset_output_field()
perf tests: Factor out print_hists_*()
perf tests: Add a testcase for histogram output sorting

 tools/perf/Documentation/perf-diff.txt   |   5 +-
 tools/perf/Documentation/perf-report.txt |  19 +
 tools/perf/Documentation/perf-top.txt    |  14 +-
 tools/perf/Makefile.perf                 |   1 +
 tools/perf/builtin-diff.c                |   7 +-
 tools/perf/builtin-report.c              |  41 +-
 tools/perf/builtin-top.c                 |  20 +-
 tools/perf/tests/builtin-test.c          |   4 +
 tools/perf/tests/hists_common.c          |  57 +++
 tools/perf/tests/hists_common.h          |   3 +
 tools/perf/tests/hists_filter.c          |  38 +-
 tools/perf/tests/hists_link.c            |  30 +-
 tools/perf/tests/hists_output.c          | 618 +++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h                 |   1 +
 tools/perf/ui/browsers/hists.c           | 104 +++---
 tools/perf/ui/gtk/hists.c                |  41 +-
 tools/perf/ui/hist.c                     | 244 +++++++++---
 tools/perf/ui/setup.c                    |   2 -
 tools/perf/ui/stdio/hist.c               |  79 ++--
 tools/perf/util/hist.c                   |  83 ++---
 tools/perf/util/hist.h                   |  27 +-
 tools/perf/util/map.c                    |  95 ++++-
 tools/perf/util/pager.c                  |  12 +-
 tools/perf/util/sort.c                   | 436 ++++++++++++++++++++--
 tools/perf/util/sort.h                   |   6 +
 25 files changed, 1601 insertions(+), 386 deletions(-)
 create mode 100644 tools/perf/tests/hists_output.c
author reviewer
#1 lore raw

PATCH 1/22 perf tools: Add ->cmp(), ->collapse() and ->sort() to perf_hpp_fmt

From: Namhyung Kim <[email protected]>

Those function pointers will be used to sort report output based on the selected fields. This is a preparation of later change.

---
 tools/perf/ui/hist.c   | 39 +++++++++++++++++++++++++++++++++++----
 tools/perf/util/hist.h |  3 +++
 2 files changed, 38 insertions(+), 4 deletions(-)
tools/perf/ui/hist.c +35 -4
index 0912805..d4a4f2e 100644
@@ -192,6 +192,14 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			  hpp_entry_scnprintf, true);				\
 }
 
+#define __HPP_SORT_FN(_type, _field)						\
+static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b)	\
+{										\
+	s64 __a = he_get_##_field(a);						\
+	s64 __b = he_get_##_field(b);						\
+	return __a - __b;							\
+}
+
 #define __HPP_ENTRY_RAW_FN(_type, _field)					\
 static u64 he_get_raw_##_field(struct hist_entry *he)				\
 {										\
@@ -206,16 +214,27 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			  hpp_entry_scnprintf, false);				\
 }
 
+#define __HPP_SORT_RAW_FN(_type, _field)					\
+static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b)	\
+{										\
+	s64 __a = he_get_raw_##_field(a);					\
+	s64 __b = he_get_raw_##_field(b);					\
+	return __a - __b;							\
+}
+
+
 #define HPP_PERCENT_FNS(_type, _str, _field, _min_width, _unit_width)	\
 __HPP_HEADER_FN(_type, _str, _min_width, _unit_width)			\
 __HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
 __HPP_COLOR_PERCENT_FN(_type, _field)					\
-__HPP_ENTRY_PERCENT_FN(_type, _field)
+__HPP_ENTRY_PERCENT_FN(_type, _field)					\
+__HPP_SORT_FN(_type, _field)
 
 #define HPP_RAW_FNS(_type, _str, _field, _min_width, _unit_width)	\
 __HPP_HEADER_FN(_type, _str, _min_width, _unit_width)			\
 __HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
-__HPP_ENTRY_RAW_FN(_type, _field)
+__HPP_ENTRY_RAW_FN(_type, _field)					\
+__HPP_SORT_RAW_FN(_type, _field)
 
 
 HPP_PERCENT_FNS(overhead, "Overhead", period, 8, 8)
@@ -227,19 +246,31 @@ HPP_PERCENT_FNS(overhead_guest_us, "guest usr", period_guest_us, 9, 8)
 HPP_RAW_FNS(samples, "Samples", nr_events, 12, 12)
 HPP_RAW_FNS(period, "Period", period, 12, 12)
 
+static int64_t hpp__nop_cmp(struct hist_entry *a __maybe_unused,
+			    struct hist_entry *b __maybe_unused)
+{
+	return 0;
+}
+
 #define HPP__COLOR_PRINT_FNS(_name)			\
 	{						\
 		.header	= hpp__header_ ## _name,	\
 		.width	= hpp__width_ ## _name,		\
 		.color	= hpp__color_ ## _name,		\
-		.entry	= hpp__entry_ ## _name		\
+		.entry	= hpp__entry_ ## _name,		\
+		.cmp	= hpp__nop_cmp,			\
+		.collapse = hpp__nop_cmp,		\
+		.sort	= hpp__sort_ ## _name,		\
 	}
 
 #define HPP__PRINT_FNS(_name)				\
 	{						\
 		.header	= hpp__header_ ## _name,	\
 		.width	= hpp__width_ ## _name,		\
-		.entry	= hpp__entry_ ## _name		\
+		.entry	= hpp__entry_ ## _name,		\
+		.cmp	= hpp__nop_cmp,			\
+		.collapse = hpp__nop_cmp,		\
+		.sort	= hpp__sort_ ## _name,		\
 	}
 
 struct perf_hpp_fmt perf_hpp__format[] = {
tools/perf/util/hist.h +3 -0
index 38c3e87..36dbe00 100644
@@ -160,6 +160,9 @@ struct perf_hpp_fmt {
 		     struct hist_entry *he);
 	int (*entry)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 		     struct hist_entry *he);
+	int64_t (*cmp)(struct hist_entry *a, struct hist_entry *b);
+	int64_t (*collapse)(struct hist_entry *a, struct hist_entry *b);
+	int64_t (*sort)(struct hist_entry *a, struct hist_entry *b);
 
 	struct list_head list;
 };
Signature · 1 line
1.8.3.1
author reviewer
#2 lore raw

PATCH 2/22 perf tools: Convert sort entries to hpp formats

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

This is a preparation of consolidating management of output field and sort keys.

---
 tools/perf/ui/hist.c   |  6 ++++
 tools/perf/util/hist.h |  6 ++++
 tools/perf/util/sort.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 88 insertions(+), 4 deletions(-)
tools/perf/ui/hist.c +6 -0
index d4a4f2e..a6eea66 100644
@@ -284,6 +284,7 @@ struct perf_hpp_fmt perf_hpp__format[] = {
 };
 
 LIST_HEAD(perf_hpp__list);
+LIST_HEAD(perf_hpp__sort_list);
 
 
 #undef HPP__COLOR_PRINT_FNS
@@ -325,6 +326,11 @@ void perf_hpp__column_register(struct perf_hpp_fmt *format)
 	list_add_tail(&format->list, &perf_hpp__list);
 }
 
+void perf_hpp__register_sort_field(struct perf_hpp_fmt *format)
+{
+	list_add_tail(&format->sort_list, &perf_hpp__sort_list);
+}
+
 void perf_hpp__column_enable(unsigned col)
 {
 	BUG_ON(col >= PERF_HPP__MAX_INDEX);
tools/perf/util/hist.h +6 -0
index 36dbe00..eee154a 100644
@@ -165,13 +165,18 @@ struct perf_hpp_fmt {
 	int64_t (*sort)(struct hist_entry *a, struct hist_entry *b);
 
 	struct list_head list;
+	struct list_head sort_list;
 };
 
 extern struct list_head perf_hpp__list;
+extern struct list_head perf_hpp__sort_list;
 
 #define perf_hpp__for_each_format(format) \
 	list_for_each_entry(format, &perf_hpp__list, list)
 
+#define perf_hpp__for_each_sort_list(format) \
+	list_for_each_entry(format, &perf_hpp__sort_list, sort_list)
+
 extern struct perf_hpp_fmt perf_hpp__format[];
 
 enum {
@@ -190,6 +195,7 @@ enum {
 void perf_hpp__init(void);
 void perf_hpp__column_register(struct perf_hpp_fmt *format);
 void perf_hpp__column_enable(unsigned col);
+void perf_hpp__register_sort_field(struct perf_hpp_fmt *format);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
tools/perf/util/sort.c +76 -4

106 lines not on this page · raw message

Signature · 1 line
1.8.3.1
author reviewer
#3 lore raw

PATCH 3/22 perf tools: Use hpp formats to sort hist entries

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

It wrapped sort entries to hpp functions, so using the hpp sort list to sort entries.

---
 tools/perf/util/hist.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
tools/perf/util/hist.c +6 -10
index 7f0236c..38373c9 100644
@@ -432,11 +432,11 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
 int64_t
 hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	struct sort_entry *se;
+	struct perf_hpp_fmt *fmt;
 	int64_t cmp = 0;
 
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		cmp = se->se_cmp(left, right);
+	perf_hpp__for_each_sort_list(fmt) {
+		cmp = fmt->cmp(left, right);
 		if (cmp)
 			break;
 	}
@@ -447,15 +447,11 @@ hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
 int64_t
 hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
 {
-	struct sort_entry *se;
+	struct perf_hpp_fmt *fmt;
 	int64_t cmp = 0;
 
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		int64_t (*f)(struct hist_entry *, struct hist_entry *);
-
-		f = se->se_collapse ?: se->se_cmp;
-
-		cmp = f(left, right);
+	perf_hpp__for_each_sort_list(fmt) {
+		cmp = fmt->collapse(left, right);
 		if (cmp)
 			break;
 	}
Signature · 1 line
1.8.3.1
author reviewer
#4 lore raw

PATCH 4/22 perf tools: Support event grouping in hpp ->sort()

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Move logic of hist_entry__sort_on_period to __hpp__sort() in order to support event group report.

---
 tools/perf/ui/hist.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 58 insertions(+), 6 deletions(-)
tools/perf/ui/hist.c +58 -6
index a6eea66..0299385 100644
@@ -116,6 +116,62 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 	return ret;
 }
 
+static int field_cmp(u64 field_a, u64 field_b)
+{
+	if (field_a > field_b)
+		return 1;
+	if (field_a < field_b)
+		return -1;
+	return 0;
+}
+
+static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
+		       hpp_field_fn get_field)
+{
+	s64 ret;
+	int i, nr_members;
+	struct perf_evsel *evsel;
+	struct hist_entry *pair;
+	u64 *fields_a, *fields_b;
+
+	ret = field_cmp(get_field(a), get_field(b));
+	if (ret || !symbol_conf.event_group)
+		return ret;
+
+	evsel = hists_to_evsel(a->hists);
+	if (!perf_evsel__is_group_event(evsel))
+		return ret;
+
+	nr_members = evsel->nr_members;
+	fields_a = calloc(sizeof(*fields_a), nr_members);
+	fields_b = calloc(sizeof(*fields_b), nr_members);
+
+	if (!fields_a || !fields_b)
+		goto out;
+
+	list_for_each_entry(pair, &a->pairs.head, pairs.node) {
+		evsel = hists_to_evsel(pair->hists);
+		fields_a[perf_evsel__group_idx(evsel)] = get_field(pair);
+	}
+
+	list_for_each_entry(pair, &b->pairs.head, pairs.node) {
+		evsel = hists_to_evsel(pair->hists);
+		fields_b[perf_evsel__group_idx(evsel)] = get_field(pair);
+	}
+
+	for (i = 1; i < nr_members; i++) {
+		ret = field_cmp(fields_a[i], fields_b[i]);
+		if (ret)
+			break;
+	}
+
+out:
+	free(fields_a);
+	free(fields_b);
+
+	return ret;
+}
+
 #define __HPP_HEADER_FN(_type, _str, _min_width, _unit_width) 		\
 static int hpp__header_##_type(struct perf_hpp_fmt *fmt __maybe_unused,	\
 			       struct perf_hpp *hpp,			\
@@ -195,9 +251,7 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 #define __HPP_SORT_FN(_type, _field)						\
 static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b)	\
 {										\
-	s64 __a = he_get_##_field(a);						\
-	s64 __b = he_get_##_field(b);						\
-	return __a - __b;							\
+	return __hpp__sort(a, b, he_get_##_field);				\
 }
 
 #define __HPP_ENTRY_RAW_FN(_type, _field)					\
@@ -217,9 +271,7 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 #define __HPP_SORT_RAW_FN(_type, _field)					\
 static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b)	\
 {										\
-	s64 __a = he_get_raw_##_field(a);					\
-	s64 __b = he_get_raw_##_field(b);					\
-	return __a - __b;							\
+	return __hpp__sort(a, b, he_get_raw_##_field);				\
 }
 
 
Signature · 1 line
1.8.3.1
author reviewer
#5 lore raw

PATCH 5/22 perf tools: Use hpp formats to sort final output

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Convert output sorting function to use ->sort hpp functions.

---
 tools/perf/util/hist.c | 62 +++++++-------------------------------------------
 1 file changed, 8 insertions(+), 54 deletions(-)
tools/perf/util/hist.c +8 -54
index 38373c9..c99ae4d 100644
@@ -564,64 +564,18 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog)
 	}
 }
 
-/*
- * reverse the map, sort on period.
- */
-
-static int period_cmp(u64 period_a, u64 period_b)
+static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
 {
-	if (period_a > period_b)
-		return 1;
-	if (period_a < period_b)
-		return -1;
-	return 0;
-}
-
-static int hist_entry__sort_on_period(struct hist_entry *a,
-				      struct hist_entry *b)
-{
-	int ret;
-	int i, nr_members;
-	struct perf_evsel *evsel;
-	struct hist_entry *pair;
-	u64 *periods_a, *periods_b;
-
-	ret = period_cmp(a->stat.period, b->stat.period);
-	if (ret || !symbol_conf.event_group)
-		return ret;
-
-	evsel = hists_to_evsel(a->hists);
-	nr_members = evsel->nr_members;
-	if (nr_members <= 1)
-		return ret;
-
-	periods_a = zalloc(sizeof(periods_a) * nr_members);
-	periods_b = zalloc(sizeof(periods_b) * nr_members);
-
-	if (!periods_a || !periods_b)
-		goto out;
-
-	list_for_each_entry(pair, &a->pairs.head, pairs.node) {
-		evsel = hists_to_evsel(pair->hists);
-		periods_a[perf_evsel__group_idx(evsel)] = pair->stat.period;
-	}
-
-	list_for_each_entry(pair, &b->pairs.head, pairs.node) {
-		evsel = hists_to_evsel(pair->hists);
-		periods_b[perf_evsel__group_idx(evsel)] = pair->stat.period;
-	}
+	struct perf_hpp_fmt *fmt;
+	int64_t cmp = 0;
 
-	for (i = 1; i < nr_members; i++) {
-		ret = period_cmp(periods_a[i], periods_b[i]);
-		if (ret)
+	perf_hpp__for_each_format(fmt) {
+		cmp = fmt->sort(a, b);
+		if (cmp)
 			break;
 	}
 
-out:
-	free(periods_a);
-	free(periods_b);
-
-	return ret;
+	return cmp;
 }
 
 static void hists__reset_filter_stats(struct hists *hists)
@@ -669,7 +623,7 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 		parent = *p;
 		iter = rb_entry(parent, struct hist_entry, rb_node);
 
-		if (hist_entry__sort_on_period(he, iter) > 0)
+		if (hist_entry__sort(he, iter) > 0)
 			p = &(*p)->rb_left;
 		else
 			p = &(*p)->rb_right;
Signature · 1 line
1.8.3.1
author reviewer
#6 lore raw

PATCH 6/22 perf tools: Consolidate output field handling to hpp format routines

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Until now the hpp and sort functions do similar jobs different ways. Since the sort functions converted/wrapped to hpp formats it can do the job in a uniform way.

The perf_hpp__sort_list has a list of hpp formats to sort entries and the perf_hpp__list has a list of hpp formats to print output result.

To have a backward compatibility, it automatically adds 'overhead' field in front of sort list. And then all of fields in sort list added to the output list (if it's not already there).

---
 tools/perf/ui/browsers/hists.c | 31 ++++++++++++++----------
 tools/perf/ui/gtk/hists.c      | 31 ------------------------
 tools/perf/ui/hist.c           | 26 ++++++++++++++++++++
 tools/perf/ui/stdio/hist.c     | 55 +++++++++++++-----------------------------
 tools/perf/util/hist.c         |  2 +-
 tools/perf/util/hist.h         |  1 +
 6 files changed, 63 insertions(+), 83 deletions(-)
tools/perf/ui/browsers/hists.c +18 -13
index b0861e3..847de11 100644
@@ -760,8 +760,8 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		if (!browser->b.navkeypressed)
 			width += 1;
 
-		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
-		slsmg_write_nstring(s, width);
+		slsmg_write_nstring("", width);
+
 		++row;
 		++printed;
 	} else
@@ -1104,27 +1104,32 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
 				       struct hist_entry *he, FILE *fp)
 {
 	char s[8192];
-	double percent;
 	int printed = 0;
 	char folded_sign = ' ';
+	struct perf_hpp hpp = {
+		.buf = s,
+		.size = sizeof(s),
+	};
+	struct perf_hpp_fmt *fmt;
+	bool first = true;
+	int ret;
 
 	if (symbol_conf.use_callchain)
 		folded_sign = hist_entry__folded(he);
 
-	hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists);
-	percent = (he->stat.period * 100.0) / browser->hists->stats.total_period;
-
 	if (symbol_conf.use_callchain)
 		printed += fprintf(fp, "%c ", folded_sign);
 
-	printed += fprintf(fp, " %5.2f%%", percent);
-
-	if (symbol_conf.show_nr_samples)
-		printed += fprintf(fp, " %11u", he->stat.nr_events);
-
-	if (symbol_conf.show_total_period)
-		printed += fprintf(fp, " %12" PRIu64, he->stat.period);
+	perf_hpp__for_each_format(fmt) {
+		if (!first) {
+			ret = scnprintf(hpp.buf, hpp.size, "  ");
+			advance_hpp(&hpp, ret);
+		} else
+			first = false;
 
+		ret = fmt->entry(fmt, &hpp, he);
+		advance_hpp(&hpp, ret);
+	}
 	printed += fprintf(fp, "%s\n", rtrim(s));
 
 	if (folded_sign == '-')
tools/perf/ui/gtk/hists.c +0 -31
index 91f10f3..d5c336e 100644
@@ -153,7 +153,6 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 	struct perf_hpp_fmt *fmt;
 	GType col_types[MAX_COLUMNS];
 	GtkCellRenderer *renderer;
-	struct sort_entry *se;
 	GtkTreeStore *store;
 	struct rb_node *nd;
 	GtkWidget *view;
@@ -172,16 +171,6 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 	perf_hpp__for_each_format(fmt)
 		col_types[nr_cols++] = G_TYPE_STRING;
 
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (se->elide)
-			continue;
-
-		if (se == &sort_sym)
-			sym_col = nr_cols;
-
-		col_types[nr_cols++] = G_TYPE_STRING;
-	}
-
 	store = gtk_tree_store_newv(nr_cols, col_types);
 
 	view = gtk_tree_view_new();
@@ -199,16 +188,6 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 							    col_idx++, NULL);
 	}
 
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (se->elide)
-			continue;
-
-		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-							    -1, se->se_header,
-							    renderer, "text",
-							    col_idx++, NULL);
-	}
-
 	for (col_idx = 0; col_idx < nr_cols; col_idx++) {
 		GtkTreeViewColumn *column;
 
@@ -253,16 +232,6 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 			gtk_tree_store_set(store, &iter, col_idx++, s, -1);
 		}
 
-		list_for_each_entry(se, &hist_entry__sort_list, list) {
-			if (se->elide)
-				continue;
-
-			se->se_snprintf(h, s, ARRAY_SIZE(s),
-					hists__col_len(hists, se->se_width_idx));
-
-			gtk_tree_store_set(store, &iter, col_idx++, s, -1);
-		}
-
 		if (symbol_conf.use_callchain && sort__has_sym) {
 			if (callchain_param.mode == CHAIN_GRAPH_REL)
 				total = h->stat.period;
tools/perf/ui/hist.c +26 -0
index 0299385..400437e 100644
@@ -354,6 +354,14 @@ LIST_HEAD(perf_hpp__sort_list);
 
 void perf_hpp__init(void)
 {
+	struct list_head *list;
+	int i;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		INIT_LIST_HEAD(&perf_hpp__format[i].list);
+		INIT_LIST_HEAD(&perf_hpp__format[i].sort_list);
+	}
+
 	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
 
 	if (symbol_conf.show_cpu_utilization) {
@@ -371,6 +379,13 @@ void perf_hpp__init(void)
 
 	if (symbol_conf.show_total_period)
 		perf_hpp__column_enable(PERF_HPP__PERIOD);
+
+	/* prepend overhead field for backward compatiblity.  */
+	list = &perf_hpp__format[PERF_HPP__OVERHEAD].sort_list;
+	if (list_empty(list))
+		list_add(list, &perf_hpp__sort_list);
+
+	perf_hpp__setup_output_field();
 }
 
 void perf_hpp__column_register(struct perf_hpp_fmt *format)
@@ -389,6 +404,17 @@ void perf_hpp__column_enable(unsigned col)
 	perf_hpp__column_register(&perf_hpp__format[col]);
 }
 
+void perf_hpp__setup_output_field(void)
+{
+	struct perf_hpp_fmt *fmt;
+
+	/* append sort keys to output field */
+	perf_hpp__for_each_sort_list(fmt) {
+		if (list_empty(&fmt->list))
+			perf_hpp__column_register(fmt);
+	}
+}
+
 int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
 			      struct hists *hists)
 {
tools/perf/ui/stdio/hist.c +17 -38

88 lines not on this page · raw message

tools/perf/util/hist.c +1 -1
index c99ae4d..ae13c2d 100644
@@ -569,7 +569,7 @@ static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
 	struct perf_hpp_fmt *fmt;
 	int64_t cmp = 0;
 
-	perf_hpp__for_each_format(fmt) {
+	perf_hpp__for_each_sort_list(fmt) {
 		cmp = fmt->sort(a, b);
 		if (cmp)
 			break;
tools/perf/util/hist.h +1 -0
index eee154a..e76d323 100644
@@ -196,6 +196,7 @@ void perf_hpp__init(void);
 void perf_hpp__column_register(struct perf_hpp_fmt *format);
 void perf_hpp__column_enable(unsigned col);
 void perf_hpp__register_sort_field(struct perf_hpp_fmt *format);
+void perf_hpp__setup_output_field(void);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
Signature · 1 line
1.8.3.1
author reviewer
#7 lore raw

PATCH 7/22 perf ui: Get rid of callback from __hpp__fmt()

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

The callback was used by TUI for determining color of folded sign using percent of first field/column. But it cannot be used anymore since it now support dynamic reordering of output field.

So move the logic to the hist_browser__show_entry().

---
 tools/perf/ui/browsers/hists.c | 62 ++++++++++++++++--------------------------
 tools/perf/ui/gtk/hists.c      |  2 +-
 tools/perf/ui/hist.c           | 28 ++++++-------------
 tools/perf/util/hist.h         |  4 +--
 4 files changed, 34 insertions(+), 62 deletions(-)
tools/perf/ui/browsers/hists.c +23 -39
index 847de11..37c5188 100644
@@ -616,35 +616,6 @@ struct hpp_arg {
 	bool current_entry;
 };
 
-static int __hpp__overhead_callback(struct perf_hpp *hpp, bool front)
-{
-	struct hpp_arg *arg = hpp->ptr;
-
-	if (arg->current_entry && arg->b->navkeypressed)
-		ui_browser__set_color(arg->b, HE_COLORSET_SELECTED);
-	else
-		ui_browser__set_color(arg->b, HE_COLORSET_NORMAL);
-
-	if (front) {
-		if (!symbol_conf.use_callchain)
-			return 0;
-
-		slsmg_printf("%c ", arg->folded_sign);
-		return 2;
-	}
-
-	return 0;
-}
-
-static int __hpp__color_callback(struct perf_hpp *hpp, bool front __maybe_unused)
-{
-	struct hpp_arg *arg = hpp->ptr;
-
-	if (!arg->current_entry || !arg->b->navkeypressed)
-		ui_browser__set_color(arg->b, HE_COLORSET_NORMAL);
-	return 0;
-}
-
 static int __hpp__slsmg_color_printf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	struct hpp_arg *arg = hpp->ptr;
@@ -665,7 +636,7 @@ static int __hpp__slsmg_color_printf(struct perf_hpp *hpp, const char *fmt, ...)
 	return ret;
 }
 
-#define __HPP_COLOR_PERCENT_FN(_type, _field, _cb)			\
+#define __HPP_COLOR_PERCENT_FN(_type, _field)				\
 static u64 __hpp_get_##_field(struct hist_entry *he)			\
 {									\
 	return he->stat._field;						\
@@ -676,15 +647,15 @@ hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
-	return __hpp__fmt(hpp, he, __hpp_get_##_field, _cb, " %6.2f%%",	\
+	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %6.2f%%",	\
 			  __hpp__slsmg_color_printf, true);		\
 }
 
-__HPP_COLOR_PERCENT_FN(overhead, period, __hpp__overhead_callback)
-__HPP_COLOR_PERCENT_FN(overhead_sys, period_sys, __hpp__color_callback)
-__HPP_COLOR_PERCENT_FN(overhead_us, period_us, __hpp__color_callback)
-__HPP_COLOR_PERCENT_FN(overhead_guest_sys, period_guest_sys, __hpp__color_callback)
-__HPP_COLOR_PERCENT_FN(overhead_guest_us, period_guest_us, __hpp__color_callback)
+__HPP_COLOR_PERCENT_FN(overhead, period)
+__HPP_COLOR_PERCENT_FN(overhead_sys, period_sys)
+__HPP_COLOR_PERCENT_FN(overhead_us, period_us)
+__HPP_COLOR_PERCENT_FN(overhead_guest_sys, period_guest_sys)
+__HPP_COLOR_PERCENT_FN(overhead_guest_us, period_guest_us)
 
 #undef __HPP_COLOR_PERCENT_FN
 
@@ -729,7 +700,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 
 	if (row_offset == 0) {
 		struct hpp_arg arg = {
-			.b 		= &browser->b,
+			.b		= &browser->b,
 			.folded_sign	= folded_sign,
 			.current_entry	= current_entry,
 		};
@@ -742,11 +713,24 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		ui_browser__gotorc(&browser->b, row, 0);
 
 		perf_hpp__for_each_format(fmt) {
-			if (!first) {
+			if (current_entry && browser->b.navkeypressed) {
+				ui_browser__set_color(&browser->b,
+						      HE_COLORSET_SELECTED);
+			} else {
+				ui_browser__set_color(&browser->b,
+						      HE_COLORSET_NORMAL);
+			}
+
+			if (first) {
+				if (symbol_conf.use_callchain) {
+					slsmg_printf("%c ", folded_sign);
+					width -= 2;
+				}
+				first = false;
+			} else {
 				slsmg_printf("  ");
 				width -= 2;
 			}
-			first = false;
 
 			if (fmt->color) {
 				width -= fmt->color(fmt, &hpp, entry);
tools/perf/ui/gtk/hists.c +1 -1
index d5c336e..2237245 100644
@@ -43,7 +43,7 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, NULL, " %6.2f%%",		\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %6.2f%%",			\
 			  __percent_color_snprintf, true);			\
 }
 
tools/perf/ui/hist.c +8 -20
index 400437e..e7ac794 100644
@@ -16,20 +16,15 @@
 })
 
 int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, hpp_callback_fn callback,
-	       const char *fmt, hpp_snprint_fn print_fn, bool fmt_percent)
+	       hpp_field_fn get_field, const char *fmt,
+	       hpp_snprint_fn print_fn, bool fmt_percent)
 {
-	int ret = 0;
+	int ret;
 	struct hists *hists = he->hists;
 	struct perf_evsel *evsel = hists_to_evsel(hists);
 	char *buf = hpp->buf;
 	size_t size = hpp->size;
 
-	if (callback) {
-		ret = callback(hpp, true);
-		advance_hpp(hpp, ret);
-	}
-
 	if (fmt_percent) {
 		double percent = 0.0;
 		u64 total = hists__total_period(hists);
@@ -37,9 +32,9 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 		if (total)
 			percent = 100.0 * get_field(he) / total;
 
-		ret += hpp__call_print_fn(hpp, print_fn, fmt, percent);
+		ret = hpp__call_print_fn(hpp, print_fn, fmt, percent);
 	} else
-		ret += hpp__call_print_fn(hpp, print_fn, fmt, get_field(he));
+		ret = hpp__call_print_fn(hpp, print_fn, fmt, get_field(he));
 
 	if (perf_evsel__is_group_event(evsel)) {
 		int prev_idx, idx_delta;
@@ -99,13 +94,6 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 		}
 	}
 
-	if (callback) {
-		int __ret = callback(hpp, false);
-
-		advance_hpp(hpp, __ret);
-		ret += __ret;
-	}
-
 	/*
 	 * Restore original buf and size as it's where caller expects
 	 * the result will be saved.
@@ -235,7 +223,7 @@ static u64 he_get_##_field(struct hist_entry *he)				\
 static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, NULL, " %6.2f%%",		\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %6.2f%%",			\
 			  hpp_color_scnprintf, true);				\
 }
 
@@ -244,7 +232,7 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
 	const char *fmt = symbol_conf.field_sep ? " %.2f" : " %6.2f%%";		\
-	return __hpp__fmt(hpp, he, he_get_##_field, NULL, fmt,			\
+	return __hpp__fmt(hpp, he, he_get_##_field, fmt,			\
 			  hpp_entry_scnprintf, true);				\
 }
 
@@ -264,7 +252,7 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
 	const char *fmt = symbol_conf.field_sep ? " %"PRIu64 : " %11"PRIu64;	\
-	return __hpp__fmt(hpp, he, he_get_raw_##_field, NULL, fmt,		\
+	return __hpp__fmt(hpp, he, he_get_raw_##_field, fmt,			\
 			  hpp_entry_scnprintf, false);				\
 }
 
tools/perf/util/hist.h +2 -2
index e76d323..76bb72e 100644
@@ -203,8 +203,8 @@ typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
 typedef int (*hpp_snprint_fn)(struct perf_hpp *hpp, const char *fmt, ...);
 
 int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, hpp_callback_fn callback,
-	       const char *fmt, hpp_snprint_fn print_fn, bool fmt_percent);
+	       hpp_field_fn get_field, const char *fmt,
+	       hpp_snprint_fn print_fn, bool fmt_percent);
 
 static inline void advance_hpp(struct perf_hpp *hpp, int inc)
 {
Signature · 1 line
1.8.3.1
author reviewer
#8 lore raw

PATCH 8/22 perf tools: Allow hpp fields to be sort keys

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Add overhead{,_sys,_us,_guest_sys,_guest_us}, sample and period sort keys so that they can be selected with --sort/-s option.

  $ perf report -s period,comm --stdio
  ...
  # Overhead        Period          Command
  # ........  ............  ...............
  #
      47.06%           152          swapper
      13.93%            45  qemu-system-arm
      12.38%            40         synergys
       3.72%            12          firefox
       2.48%             8            xchat
---
 tools/perf/Documentation/perf-diff.txt   |  5 ++--
 tools/perf/Documentation/perf-report.txt |  9 ++++++++
 tools/perf/Documentation/perf-top.txt    |  5 ++--
 tools/perf/builtin-diff.c                |  3 ++-
 tools/perf/builtin-report.c              |  6 ++---
 tools/perf/builtin-top.c                 |  4 ++--
 tools/perf/ui/hist.c                     |  9 ++++++--
 tools/perf/util/sort.c                   | 39 ++++++++++++++++++++++++++++++++
 8 files changed, 67 insertions(+), 13 deletions(-)
tools/perf/Documentation/perf-diff.txt +3 -2
index fbfa119..b3b8aba 100644
@@ -50,7 +50,8 @@ OPTIONS
 
 -s::
 --sort=::
-	Sort by key(s): pid, comm, dso, symbol.
+	Sort by key(s): pid, comm, dso, symbol, cpu, parent, srcline.
+	Please see description of --sort in the perf-report man page.
 
 -t::
 --field-separator=::
@@ -202,4 +203,4 @@ If specified the 'Weighted diff' column is displayed with value 'd' computed as:
 
 SEE ALSO
 --------
-linkperf:perf-record[1]
+linkperf:perf-record[1], linkperf:perf-report[1]
tools/perf/Documentation/perf-report.txt +9 -0
index 09af662..9babe91 100644
@@ -79,6 +79,15 @@ OPTIONS
 	abort cost. This is the global weight.
 	- local_weight: Local weight version of the weight above.
 	- transaction: Transaction abort flags.
+	- overhead: Overhead percentage of sample
+	- overhead_sys: Overhead percentage of sample running in system mode
+	- overhead_us: Overhead percentage of sample running in user mode
+	- overhead_guest_sys: Overhead percentage of sample running in system mode
+	on guest machine
+	- overhead_guest_us: Overhead percentage of sample running in user mode on
+	guest machine
+	- sample: Number of sample
+	- period: Raw number of event count of sample
 
 	By default, comm, dso and symbol keys are used.
 	(i.e. --sort comm,dso,symbol)
tools/perf/Documentation/perf-top.txt +3 -2
index 64ed79c..df86328 100644
@@ -113,7 +113,8 @@ Default is to monitor all CPUS.
 -s::
 --sort::
 	Sort by key(s): pid, comm, dso, symbol, parent, srcline, weight,
-	local_weight, abort, in_tx, transaction
+	local_weight, abort, in_tx, transaction, overhead, sample, period.
+	Please see description of --sort in the perf-report man page.
 
 -n::
 --show-nr-samples::
@@ -212,4 +213,4 @@ Pressing any unmapped key displays a menu, and prompts for input.
 
 SEE ALSO
 --------
-linkperf:perf-stat[1], linkperf:perf-list[1]
+linkperf:perf-stat[1], linkperf:perf-list[1], linkperf:perf-report[1]
tools/perf/builtin-diff.c +2 -1
index f3b10dc..b60c711 100644
@@ -741,7 +741,8 @@ static const struct option options[] = {
 	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
 		   "only consider these symbols"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
+		   " Please refer the man page for the complete list."),
 	OPT_STRING('t', "field-separator", &symbol_conf.field_sep, "separator",
 		   "separator for columns, no spaces will be added between "
 		   "columns '.' is reserved."),
tools/perf/builtin-report.c +2 -4
index 89c9528..d0180d5 100644
@@ -699,10 +699,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN(0, "header-only", &report.header_only,
 		    "Show only data header."),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline,"
-		   " dso_to, dso_from, symbol_to, symbol_from, mispredict,"
-		   " weight, local_weight, mem, symbol_daddr, dso_daddr, tlb, "
-		   "snoop, locked, abort, in_tx, transaction"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
+		   " Please refer the man page for the complete list."),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",
tools/perf/builtin-top.c +2 -2
index 37d3046..4fef1e4 100644
@@ -1083,8 +1083,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show counter open errors, etc)"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, weight, local_weight,"
-		   " abort, in_tx, transaction"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
+		   " Please refer the man page for the complete list."),
 	OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
 		    "Show a column with the number of samples"),
 	OPT_CALLBACK_NOOPT('g', NULL, &top.record_opts,
tools/perf/ui/hist.c +7 -2
index e7ac794..24116a4 100644
@@ -346,8 +346,13 @@ void perf_hpp__init(void)
 	int i;
 
 	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
-		INIT_LIST_HEAD(&perf_hpp__format[i].list);
-		INIT_LIST_HEAD(&perf_hpp__format[i].sort_list);
+		struct perf_hpp_fmt *fmt = &perf_hpp__format[i];
+
+		INIT_LIST_HEAD(&fmt->list);
+
+		/* sort_list may be linked by setup_sorting() */
+		if (fmt->sort_list.next == NULL)
+			INIT_LIST_HEAD(&fmt->sort_list);
 	}
 
 	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
tools/perf/util/sort.c +39 -0
index b2829f9..916652a 100644
@@ -1028,6 +1028,26 @@ static struct sort_dimension memory_sort_dimensions[] = {
 
 #undef DIM
 
+struct hpp_dimension {
+	const char		*name;
+	struct perf_hpp_fmt	*fmt;
+	int			taken;
+};
+
+#define DIM(d, n) { .name = n, .fmt = &perf_hpp__format[d], }
+
+static struct hpp_dimension hpp_sort_dimensions[] = {
+	DIM(PERF_HPP__OVERHEAD, "overhead"),
+	DIM(PERF_HPP__OVERHEAD_SYS, "overhead_sys"),
+	DIM(PERF_HPP__OVERHEAD_US, "overhead_us"),
+	DIM(PERF_HPP__OVERHEAD_GUEST_SYS, "overhead_guest_sys"),
+	DIM(PERF_HPP__OVERHEAD_GUEST_US, "overhead_guest_us"),
+	DIM(PERF_HPP__SAMPLES, "sample"),
+	DIM(PERF_HPP__PERIOD, "period"),
+};
+
+#undef DIM
+
 struct hpp_sort_entry {
 	struct perf_hpp_fmt hpp;
 	struct sort_entry *se;
@@ -1115,6 +1135,16 @@ static int __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
 	return 0;
 }
 
+static int __hpp_dimension__add(struct hpp_dimension *hd)
+{
+	if (!hd->taken) {
+		hd->taken = 1;
+
+		perf_hpp__register_sort_field(hd->fmt);
+	}
+	return 0;
+}
+
 int sort_dimension__add(const char *tok)
 {
 	unsigned int i;
@@ -1144,6 +1174,15 @@ int sort_dimension__add(const char *tok)
 		return __sort_dimension__add(sd, i);
 	}
 
+	for (i = 0; i < ARRAY_SIZE(hpp_sort_dimensions); i++) {
+		struct hpp_dimension *hd = &hpp_sort_dimensions[i];
+
+		if (strncasecmp(tok, hd->name, strlen(tok)))
+			continue;
+
+		return __hpp_dimension__add(hd);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(bstack_sort_dimensions); i++) {
 		struct sort_dimension *sd = &bstack_sort_dimensions[i];
 
Signature · 1 line
1.8.3.1
author reviewer
#9 lore raw

PATCH 9/22 perf tools: Consolidate management of default sort orders

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

The perf uses different default sort orders for different use-cases, and this was scattered throughout the code. Add get_default_sort_ order() function to handle this and change initial value of sort_order to NULL to distinguish it from user-given one.

---
 tools/perf/builtin-diff.c   |  4 ++--
 tools/perf/builtin-report.c | 18 ------------------
 tools/perf/builtin-top.c    |  3 +--
 tools/perf/util/sort.c      | 28 ++++++++++++++++++++++++++--
 tools/perf/util/sort.h      |  2 ++
 5 files changed, 31 insertions(+), 24 deletions(-)
tools/perf/builtin-diff.c +2 -2
index b60c711..8bff543 100644
@@ -60,7 +60,6 @@ static int data__files_cnt;
 #define data__for_each_file(i, d) data__for_each_file_start(i, d, 0)
 #define data__for_each_file_new(i, d) data__for_each_file_start(i, d, 1)
 
-static char diff__default_sort_order[] = "dso,symbol";
 static bool force;
 static bool show_period;
 static bool show_formula;
@@ -1142,7 +1141,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	perf_config(perf_default_config, NULL);
 
-	sort_order = diff__default_sort_order;
 	argc = parse_options(argc, argv, options, diff_usage, 0);
 
 	if (symbol__init() < 0)
@@ -1153,6 +1151,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	ui_init();
 
+	sort__mode = SORT_MODE__DIFF;
+
 	if (setup_sorting() < 0)
 		usage_with_options(diff_usage, options);
 
tools/perf/builtin-report.c +0 -18
index d0180d5..f4d640cf 100644
@@ -805,30 +805,12 @@ repeat:
 	if (branch_mode == -1 && has_br_stack)
 		sort__mode = SORT_MODE__BRANCH;
 
-	/* sort__mode could be NORMAL if --no-branch-stack */
-	if (sort__mode == SORT_MODE__BRANCH) {
-		/*
-		 * if no sort_order is provided, then specify
-		 * branch-mode specific order
-		 */
-		if (sort_order == default_sort_order)
-			sort_order = "comm,dso_from,symbol_from,"
-				     "dso_to,symbol_to";
-
-	}
 	if (report.mem_mode) {
 		if (sort__mode == SORT_MODE__BRANCH) {
 			pr_err("branch and mem mode incompatible\n");
 			goto error;
 		}
 		sort__mode = SORT_MODE__MEMORY;
-
-		/*
-		 * if no sort_order is provided, then specify
-		 * branch-mode specific order
-		 */
-		if (sort_order == default_sort_order)
-			sort_order = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked";
 	}
 
 	if (setup_sorting() < 0) {
tools/perf/builtin-top.c +1 -2
index 4fef1e4..34764b6 100644
@@ -1137,8 +1137,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (argc)
 		usage_with_options(top_usage, options);
 
-	if (sort_order == default_sort_order)
-		sort_order = "dso,symbol";
+	sort__mode = SORT_MODE__TOP;
 
 	if (setup_sorting() < 0) {
 		parse_options_usage(top_usage, options, "s", 1);
tools/perf/util/sort.c +26 -2
index 916652a..d64c1e5 100644
@@ -8,7 +8,11 @@ regex_t		parent_regex;
 const char	default_parent_pattern[] = "^sys_|^do_page_fault";
 const char	*parent_pattern = default_parent_pattern;
 const char	default_sort_order[] = "comm,dso,symbol";
-const char	*sort_order = default_sort_order;
+const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,dso_to,symbol_to";
+const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked";
+const char	default_top_sort_order[] = "dso,symbol";
+const char	default_diff_sort_order[] = "dso,symbol";
+const char	*sort_order;
 regex_t		ignore_callees_regex;
 int		have_ignore_callees = 0;
 int		sort__need_collapse = 0;
@@ -1218,11 +1222,31 @@ int sort_dimension__add(const char *tok)
 	return -ESRCH;
 }
 
+static const char *get_default_sort_order(void)
+{
+	const char *default_sort_orders[] = {
+		default_sort_order,
+		default_branch_sort_order,
+		default_mem_sort_order,
+		default_top_sort_order,
+		default_diff_sort_order,
+	};
+
+	BUG_ON(sort__mode >= ARRAY_SIZE(default_sort_orders));
+
+	return default_sort_orders[sort__mode];
+}
+
 int setup_sorting(void)
 {
-	char *tmp, *tok, *str = strdup(sort_order);
+	char *tmp, *tok, *str;
+	const char *sort_keys = sort_order;
 	int ret = 0;
 
+	if (sort_keys == NULL)
+		sort_keys = get_default_sort_order();
+
+	str = strdup(sort_keys);
 	if (str == NULL) {
 		error("Not enough memory to setup sort keys");
 		return -ENOMEM;
tools/perf/util/sort.h +2 -0
index 43e5ff4..1a72952 100644
@@ -133,6 +133,8 @@ enum sort_mode {
 	SORT_MODE__NORMAL,
 	SORT_MODE__BRANCH,
 	SORT_MODE__MEMORY,
+	SORT_MODE__TOP,
+	SORT_MODE__DIFF,
 };
 
 enum sort_type {
Signature · 1 line
1.8.3.1
author reviewer
#10 lore raw

PATCH 10/22 perf tools: Call perf_hpp__init() before setting up GUI browsers

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

So that it can be set properly prior to set up output fields. That makes easy to handle/warn errors during the setup since it doesn't need to be bothered with the GUI.

---
 tools/perf/builtin-report.c    | 6 +++---
 tools/perf/builtin-top.c       | 2 ++
 tools/perf/ui/browsers/hists.c | 2 --
 tools/perf/ui/gtk/hists.c      | 2 --
 tools/perf/ui/setup.c          | 2 --
 5 files changed, 5 insertions(+), 9 deletions(-)
tools/perf/builtin-report.c +3 -3
index f4d640cf..c4dab7a 100644
@@ -823,16 +823,16 @@ repeat:
 			goto error;
 	}
 
+	perf_hpp__init();
+
 	/* Force tty output for header output. */
 	if (report.header || report.header_only)
 		use_browser = 0;
 
 	if (strcmp(input_name, "-") != 0)
 		setup_browser(true);
-	else {
+	else
 		use_browser = 0;
-		perf_hpp__init();
-	}
 
 	if (report.header || report.header_only) {
 		perf_session__fprintf_info(session, stdout,
tools/perf/builtin-top.c +2 -0
index 34764b6..280945b 100644
@@ -1147,6 +1147,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	/* display thread wants entries to be collapsed in a different tree */
 	sort__need_collapse = 1;
 
+	perf_hpp__init();
+
 	if (top.use_stdio)
 		use_browser = 0;
 	else if (top.use_tui)
tools/perf/ui/browsers/hists.c +0 -2
index 37c5188..92d128f 100644
@@ -661,8 +661,6 @@ __HPP_COLOR_PERCENT_FN(overhead_guest_us, period_guest_us)
 
 void hist_browser__init_hpp(void)
 {
-	perf_hpp__init();
-
 	perf_hpp__format[PERF_HPP__OVERHEAD].color =
 				hist_browser__hpp_color_overhead;
 	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
tools/perf/ui/gtk/hists.c +0 -2
index 2237245..fd52669 100644
@@ -58,8 +58,6 @@ __HPP_COLOR_PERCENT_FN(overhead_guest_us, period_guest_us)
 
 void perf_gtk__init_hpp(void)
 {
-	perf_hpp__init();
-
 	perf_hpp__format[PERF_HPP__OVERHEAD].color =
 				perf_gtk__hpp_color_overhead;
 	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
tools/perf/ui/setup.c +0 -2
index 5df5140..ba51fa8 100644
@@ -86,8 +86,6 @@ void setup_browser(bool fallback_to_pager)
 		use_browser = 0;
 		if (fallback_to_pager)
 			setup_pager();
-
-		perf_hpp__init();
 		break;
 	}
 }
Signature · 1 line
1.8.3.1
author reviewer
#11 lore raw

PATCH 11/22 perf report: Add -F option to specify output fields

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

The -F/--fields option is to allow user setup output field in any order. It can receive any sort keys and following (hpp) fields:

overhead, overhead_sys, overhead_us, sample and period

If guest profiling is enabled, overhead_guest_{sys,us} will be available too.

The output fields also affect sort order unless you give -s/--sort option. And any keys specified on -s option, will also be added to the output field list automatically.

  $ perf report -F sym,sample,overhead
  ...
  #                     Symbol       Samples  Overhead
  # ..........................  ............  ........
  #
    [.] __cxa_atexit                       2     2.50%
    [.] __libc_csu_init                    4     5.00%
    [.] __new_exitfn                       3     3.75%
    [.] _dl_check_map_versions             1     1.25%
    [.] _dl_name_match_p                   4     5.00%
    [.] _dl_setup_hash                     1     1.25%
    [.] _dl_sysdep_start                   1     1.25%
    [.] _init                              5     6.25%
    [.] _setjmp                            6     7.50%
    [.] a                                  8    10.00%
    [.] b                                  8    10.00%
    [.] brk                                1     1.25%
    [.] c                                  8    10.00%

Note that, the example output above is captured after applying next patch which fixes sort/comparing behavior.

---
 tools/perf/Documentation/perf-report.txt |  10 ++
 tools/perf/builtin-report.c              |  15 ++-
 tools/perf/ui/hist.c                     |  59 ++++++++-
 tools/perf/util/hist.h                   |   4 +
 tools/perf/util/sort.c                   | 209 ++++++++++++++++++++++++++++++-
 tools/perf/util/sort.h                   |   2 +
 6 files changed, 282 insertions(+), 17 deletions(-)
tools/perf/Documentation/perf-report.txt +10 -0
index 9babe91..a1b5185 100644
@@ -107,6 +107,16 @@ OPTIONS
 	And default sort keys are changed to comm, dso_from, symbol_from, dso_to
 	and symbol_to, see '--branch-stack'.
 
+-F::
+--fields=::
+	Specify output field - multiple keys can be specified in CSV format.
+	Following fields are available:
+	overhead, overhead_sys, overhead_us, sample and period.
+	Also it can contain any sort key(s).
+
+	By default, every sort keys not specified in -F will be appended
+	automatically.
+
 -p::
 --parent=<regex>::
         A regex filter to identify parent. The parent is a caller of this
tools/perf/builtin-report.c +7 -8
index c4dab7a..bc0eec1 100644
@@ -701,6 +701,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
 		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
 		   " Please refer the man page for the complete list."),
+	OPT_STRING('F', "fields", &field_order, "key[,keys...]",
+		   "output field(s): overhead, period, sample plus all of sort keys"),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",
@@ -814,17 +816,14 @@ repeat:
 	}
 
 	if (setup_sorting() < 0) {
-		parse_options_usage(report_usage, options, "s", 1);
+		if (sort_order)
+			parse_options_usage(report_usage, options, "s", 1);
+		if (field_order)
+			parse_options_usage(sort_order ? NULL : report_usage,
+					    options, "F", 1);
 		goto error;
 	}
 
-	if (parent_pattern != default_parent_pattern) {
-		if (sort_dimension__add("parent") < 0)
-			goto error;
-	}
-
-	perf_hpp__init();
-
 	/* Force tty output for header output. */
 	if (report.header || report.header_only)
 		use_browser = 0;
tools/perf/ui/hist.c +55 -4

77 lines not on this page · raw message

tools/perf/util/hist.h +4 -0
index 76bb72e..f3713b7 100644
@@ -197,6 +197,10 @@ void perf_hpp__column_register(struct perf_hpp_fmt *format);
 void perf_hpp__column_enable(unsigned col);
 void perf_hpp__register_sort_field(struct perf_hpp_fmt *format);
 void perf_hpp__setup_output_field(void);
+void perf_hpp__append_sort_keys(void);
+
+bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
+bool perf_hpp__same_sort_entry(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
tools/perf/util/sort.c +204 -5

269 lines not on this page · raw message

tools/perf/util/sort.h +2 -0
index 1a72952..89e5057 100644
@@ -25,6 +25,7 @@
 
 extern regex_t parent_regex;
 extern const char *sort_order;
+extern const char *field_order;
 extern const char default_parent_pattern[];
 extern const char *parent_pattern;
 extern const char default_sort_order[];
@@ -191,6 +192,7 @@ extern struct sort_entry sort_thread;
 extern struct list_head hist_entry__sort_list;
 
 int setup_sorting(void);
+int setup_output_field(void);
 extern int sort_dimension__add(const char *);
 void sort__setup_elide(FILE *fp);
 
Signature · 1 line
1.8.3.1
author reviewer
#12 lore raw

PATCH 12/22 perf tools: Add ->sort() member to struct sort_entry

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Currently, what the sort_entry does is just identifying hist entries so that they can be grouped properly. However, with -F option support, it indeed needs to sort entries appropriately to be shown to users. So add ->sort() member to do it.

---
 tools/perf/util/sort.c | 27 ++++++++++++++++++++++-----
 tools/perf/util/sort.h |  1 +
 2 files changed, 23 insertions(+), 5 deletions(-)
tools/perf/util/sort.c +22 -5
index b748b02..5414ba5 100644
@@ -99,6 +99,12 @@ sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
 	return comm__str(right->comm) - comm__str(left->comm);
 }
 
+static int64_t
+sort__comm_sort(struct hist_entry *left, struct hist_entry *right)
+{
+	return strcmp(comm__str(right->comm), comm__str(left->comm));
+}
+
 static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf,
 				     size_t size, unsigned int width)
 {
@@ -109,6 +115,7 @@ struct sort_entry sort_comm = {
 	.se_header	= "Command",
 	.se_cmp		= sort__comm_cmp,
 	.se_collapse	= sort__comm_collapse,
+	.se_sort	= sort__comm_sort,
 	.se_snprintf	= hist_entry__comm_snprintf,
 	.se_width_idx	= HISTC_COMM,
 };
@@ -122,7 +129,7 @@ static int64_t _sort__dso_cmp(struct map *map_l, struct map *map_r)
 	const char *dso_name_l, *dso_name_r;
 
 	if (!dso_l || !dso_r)
-		return cmp_null(dso_l, dso_r);
+		return cmp_null(dso_r, dso_l);
 
 	if (verbose) {
 		dso_name_l = dso_l->long_name;
@@ -138,7 +145,7 @@ static int64_t _sort__dso_cmp(struct map *map_l, struct map *map_r)
 static int64_t
 sort__dso_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	return _sort__dso_cmp(left->ms.map, right->ms.map);
+	return _sort__dso_cmp(right->ms.map, left->ms.map);
 }
 
 static int _hist_entry__dso_snprintf(struct map *map, char *bf,
@@ -210,6 +217,15 @@ sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 	return _sort__sym_cmp(left->ms.sym, right->ms.sym);
 }
 
+static int64_t
+sort__sym_sort(struct hist_entry *left, struct hist_entry *right)
+{
+	if (!left->ms.sym || !right->ms.sym)
+		return cmp_null(left->ms.sym, right->ms.sym);
+
+	return strcmp(right->ms.sym->name, left->ms.sym->name);
+}
+
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 				     u64 ip, char level, char *bf, size_t size,
 				     unsigned int width)
@@ -256,6 +272,7 @@ static int hist_entry__sym_snprintf(struct hist_entry *he, char *bf,
 struct sort_entry sort_sym = {
 	.se_header	= "Symbol",
 	.se_cmp		= sort__sym_cmp,
+	.se_sort	= sort__sym_sort,
 	.se_snprintf	= hist_entry__sym_snprintf,
 	.se_width_idx	= HISTC_SYMBOL,
 };
@@ -283,7 +300,7 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
 					    map__rip_2objdump(map, right->ip));
 		}
 	}
-	return strcmp(left->srcline, right->srcline);
+	return strcmp(right->srcline, left->srcline);
 }
 
 static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
@@ -311,7 +328,7 @@ sort__parent_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!sym_l || !sym_r)
 		return cmp_null(sym_l, sym_r);
 
-	return strcmp(sym_l->name, sym_r->name);
+	return strcmp(sym_r->name, sym_l->name);
 }
 
 static int hist_entry__parent_snprintf(struct hist_entry *he, char *bf,
@@ -1126,7 +1143,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 
 	hse->hpp.cmp = sd->entry->se_cmp;
 	hse->hpp.collapse = sd->entry->se_collapse ? : sd->entry->se_cmp;
-	hse->hpp.sort = hse->hpp.collapse;
+	hse->hpp.sort = sd->entry->se_sort ? : hse->hpp.collapse;
 
 	INIT_LIST_HEAD(&hse->hpp.list);
 	INIT_LIST_HEAD(&hse->hpp.sort_list);
tools/perf/util/sort.h +1 -0
index 89e5057..f5a831c 100644
@@ -182,6 +182,7 @@ struct sort_entry {
 
 	int64_t (*se_cmp)(struct hist_entry *, struct hist_entry *);
 	int64_t (*se_collapse)(struct hist_entry *, struct hist_entry *);
+	int64_t	(*se_sort)(struct hist_entry *, struct hist_entry *);
 	int	(*se_snprintf)(struct hist_entry *he, char *bf, size_t size,
 			       unsigned int width);
 	u8	se_width_idx;
Signature · 1 line
1.8.3.1
author reviewer
#13 lore raw

PATCH 13/22 perf report/tui: Fix a bug when --fields/sort is given

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

The hists__filter_entries() function is called when down arrow key is pressed for navigating through the entries in TUI. It has a check for filtering out entries that have very small overhead (under min_pcnt).

However it just assumed the entries are sorted by the overhead so when it saw such a small overheaded entry, it just stopped navigating as an optimization. But it's not true anymore due to new --fields and --sort optoin behavior and this case users cannot go down to a next entry if ther's an entry with small overhead in-between.

---
 tools/perf/ui/browsers/hists.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
tools/perf/ui/browsers/hists.c +1 -4
index 92d128f..169224c 100644
@@ -812,10 +812,7 @@ static struct rb_node *hists__filter_entries(struct rb_node *nd,
 		if (total)
 			percent = h->stat.period * 100.0 / total;
 
-		if (percent < min_pcnt)
-			return NULL;
-
-		if (!h->filtered)
+		if (!h->filtered && percent >= min_pcnt)
 			return nd;
 
 		nd = rb_next(nd);
Signature · 1 line
1.8.3.1
author reviewer
#14 lore raw

PATCH 14/22 perf top: Add --fields option to specify output fields

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

The --fields option is to allow user setup output field in any order. It can receive any sort keys and following (hpp) fields:

overhead, overhead_sys, overhead_us, sample and period

If guest profiling is enabled, overhead_guest_{sys,us} will be available too.

More more information, please see previous patch "perf report:
Add -F option to specify output fields"

---
 tools/perf/Documentation/perf-top.txt |  9 +++++++++
 tools/perf/builtin-top.c              | 15 +++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)
tools/perf/Documentation/perf-top.txt +9 -0
index df86328..dcfa54c 100644
@@ -116,6 +116,15 @@ Default is to monitor all CPUS.
 	local_weight, abort, in_tx, transaction, overhead, sample, period.
 	Please see description of --sort in the perf-report man page.
 
+--fields=::
+	Specify output field - multiple keys can be specified in CSV format.
+	Following fields are available:
+	overhead, overhead_sys, overhead_us, sample and period.
+	Also it can contain any sort key(s).
+
+	By default, every sort keys not specified in --field will be appended
+	automatically.
+
 -n::
 --show-nr-samples::
 	Show a column with the number of samples.
tools/perf/builtin-top.c +9 -6
index 280945b..5b389ce 100644
@@ -1085,6 +1085,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
 		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
 		   " Please refer the man page for the complete list."),
+	OPT_STRING(0, "fields", &field_order, "key[,keys...]",
+		   "output field(s): overhead, period, sample plus all of sort keys"),
 	OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
 		    "Show a column with the number of samples"),
 	OPT_CALLBACK_NOOPT('g', NULL, &top.record_opts,
@@ -1138,17 +1140,18 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		usage_with_options(top_usage, options);
 
 	sort__mode = SORT_MODE__TOP;
+	/* display thread wants entries to be collapsed in a different tree */
+	sort__need_collapse = 1;
 
 	if (setup_sorting() < 0) {
-		parse_options_usage(top_usage, options, "s", 1);
+		if (sort_order)
+			parse_options_usage(top_usage, options, "s", 1);
+		if (field_order)
+			parse_options_usage(sort_order ? NULL : top_usage,
+					    options, "fields", 0);
 		goto out_delete_evlist;
 	}
 
-	/* display thread wants entries to be collapsed in a different tree */
-	sort__need_collapse = 1;
-
-	perf_hpp__init();
-
 	if (top.use_stdio)
 		use_browser = 0;
 	else if (top.use_tui)
Signature · 1 line
1.8.3.1
author reviewer
#15 lore raw

PATCH 15/22 perf tools: Skip elided sort entries

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

When it converted sort entries to hpp formats, it missed se->elide handling, so add it for compatibility.

---
 tools/perf/ui/browsers/hists.c |  6 ++++++
 tools/perf/ui/gtk/hists.c      |  6 ++++++
 tools/perf/ui/stdio/hist.c     |  9 +++++++++
 tools/perf/util/hist.c         |  9 +++++++++
 tools/perf/util/hist.h         |  1 +
 tools/perf/util/sort.c         | 11 +++++++++++
 6 files changed, 42 insertions(+)
tools/perf/ui/browsers/hists.c +6 -0
index 169224c..1c331b9 100644
@@ -711,6 +711,9 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		ui_browser__gotorc(&browser->b, row, 0);
 
 		perf_hpp__for_each_format(fmt) {
+			if (perf_hpp__should_skip(fmt))
+				continue;
+
 			if (current_entry && browser->b.navkeypressed) {
 				ui_browser__set_color(&browser->b,
 						      HE_COLORSET_SELECTED);
@@ -1100,6 +1103,9 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
 		printed += fprintf(fp, "%c ", folded_sign);
 
 	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		if (!first) {
 			ret = scnprintf(hpp.buf, hpp.size, "  ");
 			advance_hpp(&hpp, ret);
tools/perf/ui/gtk/hists.c +6 -0
index fd52669..9d90683 100644
@@ -178,6 +178,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 	col_idx = 0;
 
 	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		fmt->header(fmt, &hpp, hists_to_evsel(hists));
 
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
@@ -222,6 +225,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 		col_idx = 0;
 
 		perf_hpp__for_each_format(fmt) {
+			if (perf_hpp__should_skip(fmt))
+				continue;
+
 			if (fmt->color)
 				fmt->color(fmt, &hpp, h);
 			else
tools/perf/ui/stdio/hist.c +9 -0
index 49e2e4a..e3fdf4e 100644
@@ -318,6 +318,9 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 		return 0;
 
 	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		/*
 		 * If there's no field_sep, we still need
 		 * to display initial '  '.
@@ -407,6 +410,9 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 	fprintf(fp, "# ");
 
 	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		if (!first)
 			fprintf(fp, "%s", sep ?: "  ");
 		else
@@ -430,6 +436,9 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 	perf_hpp__for_each_format(fmt) {
 		unsigned int i;
 
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		if (!first)
 			fprintf(fp, "%s", sep ?: "  ");
 		else
tools/perf/util/hist.c +9 -0
index ae13c2d..b262b44 100644
@@ -436,6 +436,9 @@ hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
 	int64_t cmp = 0;
 
 	perf_hpp__for_each_sort_list(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		cmp = fmt->cmp(left, right);
 		if (cmp)
 			break;
@@ -451,6 +454,9 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
 	int64_t cmp = 0;
 
 	perf_hpp__for_each_sort_list(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		cmp = fmt->collapse(left, right);
 		if (cmp)
 			break;
@@ -570,6 +576,9 @@ static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
 	int64_t cmp = 0;
 
 	perf_hpp__for_each_sort_list(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
 		cmp = fmt->sort(a, b);
 		if (cmp)
 			break;
tools/perf/util/hist.h +1 -0
index f3713b7..f67feb4 100644
@@ -201,6 +201,7 @@ void perf_hpp__append_sort_keys(void);
 
 bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
 bool perf_hpp__same_sort_entry(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b);
+bool perf_hpp__should_skip(struct perf_hpp_fmt *format);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
tools/perf/util/sort.c +11 -0
index 5414ba5..0fe7cbe 100644
@@ -1358,6 +1358,17 @@ static int __setup_sorting(void)
 	return ret;
 }
 
+bool perf_hpp__should_skip(struct perf_hpp_fmt *format)
+{
+	if (perf_hpp__is_sort_entry(format)) {
+		struct hpp_sort_entry *hse;
+
+		hse = container_of(format, struct hpp_sort_entry, hpp);
+		return hse->se->elide;
+	}
+	return false;
+}
+
 static void sort_entry__setup_elide(struct sort_entry *se,
 				    struct strlist *list,
 				    const char *list_name, FILE *fp)
Signature · 1 line
1.8.3.1
author reviewer
#16 lore raw

PATCH 16/22 perf hists: Reset width of output fields with header length

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Some fields missed to set default column length so it broke align in --stdio output. Add perf_hpp__reset_width() to set it to a sane default value.

Note that this change will ignore -w/--column-widths option for now.

Before:

  $ perf report -F cpu,comm,overhead --stdio
  ...
  # CPU          Command  Overhead
  #   ...............  ........
  #
    0          firefox     2.65%
    0      kworker/0:0     1.45%
    0          swapper     5.52%
    0         synergys     0.92%
    1          firefox     4.54%

After:
# CPU Command Overhead

  # ...  ...............  ........
  #
      0          firefox     2.65%
      0      kworker/0:0     1.45%
      0          swapper     5.52%
      0         synergys     0.92%
      1          firefox     4.54%
---
 tools/perf/ui/stdio/hist.c | 21 +++------------------
 tools/perf/util/hist.h     |  1 +
 tools/perf/util/sort.c     | 12 ++++++++++++
 3 files changed, 16 insertions(+), 18 deletions(-)
tools/perf/ui/stdio/hist.c +3 -18
index e3fdf4e..cfcd3f6 100644
@@ -369,12 +369,10 @@ 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 sort_entry *se;
 	struct rb_node *nd;
 	size_t ret = 0;
 	unsigned int width;
 	const char *sep = symbol_conf.field_sep;
-	const char *col_width = symbol_conf.col_width_list_str;
 	int nr_rows = 0;
 	char bf[96];
 	struct perf_hpp dummy_hpp = {
@@ -387,22 +385,9 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 
 	init_rem_hits();
 
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (se->elide)
-			continue;
-		width = strlen(se->se_header);
-		if (symbol_conf.col_width_list_str) {
-			if (col_width) {
-				hists__set_col_len(hists, se->se_width_idx,
-						   atoi(col_width));
-				col_width = strchr(col_width, ',');
-				if (col_width)
-					++col_width;
-			}
-		}
-		if (!hists__new_col_len(hists, se->se_width_idx, width))
-			width = hists__col_len(hists, se->se_width_idx);
-	}
+
+	perf_hpp__for_each_format(fmt)
+		perf_hpp__reset_width(fmt, hists);
 
 	if (!show_header)
 		goto print_entries;
tools/perf/util/hist.h +1 -0
index f67feb4..034db76 100644
@@ -202,6 +202,7 @@ void perf_hpp__append_sort_keys(void);
 bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
 bool perf_hpp__same_sort_entry(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b);
 bool perf_hpp__should_skip(struct perf_hpp_fmt *format);
+void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
tools/perf/util/sort.c +12 -0
index 0fe7cbe..9bee728 100644
@@ -1089,6 +1089,18 @@ bool perf_hpp__same_sort_entry(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
 	return hse_a->se == hse_b->se;
 }
 
+void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
+{
+	struct hpp_sort_entry *hse;
+
+	if (!perf_hpp__is_sort_entry(fmt))
+		return;
+
+	hse = container_of(fmt, struct hpp_sort_entry, hpp);
+	hists__new_col_len(hists, hse->se->se_width_idx,
+			   strlen(hse->se->se_header));
+}
+
 static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 			      struct perf_evsel *evsel)
 {
Signature · 1 line
1.8.3.1
author reviewer
#17 lore raw

PATCH 17/22 perf tools: Get rid of obsolete hist_entry__sort_list

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Now we moved to the perf_hpp_[_sort]_list so no need to keep the old hist_entry__sort_list and sort__first_dimension. Also the hist_entry__sort_snprintf() can be gone as hist_entry__snprintf() provides the functionality.

---
 tools/perf/ui/hist.c       | 36 +++++++++---------------------------
 tools/perf/ui/stdio/hist.c | 22 +++++++++++++++-------
 tools/perf/util/sort.c     | 35 +++++++++++++++++++----------------
 3 files changed, 43 insertions(+), 50 deletions(-)
tools/perf/ui/hist.c +9 -27
index b114c66..61cf31e 100644
@@ -459,47 +459,29 @@ next:
 	}
 }
 
-int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
-			      struct hists *hists)
-{
-	const char *sep = symbol_conf.field_sep;
-	struct sort_entry *se;
-	int ret = 0;
-
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (se->elide)
-			continue;
-
-		ret += scnprintf(s + ret, size - ret, "%s", sep ?: "  ");
-		ret += se->se_snprintf(he, s + ret, size - ret,
-				       hists__col_len(hists, se->se_width_idx));
-	}
-
-	return ret;
-}
-
 /*
  * See hists__fprintf to match the column widths
  */
 unsigned int hists__sort_list_width(struct hists *hists)
 {
 	struct perf_hpp_fmt *fmt;
-	struct sort_entry *se;
-	int i = 0, ret = 0;
+	int ret = 0;
+	bool first = true;
 	struct perf_hpp dummy_hpp;
 
 	perf_hpp__for_each_format(fmt) {
-		if (i)
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
+		if (first)
+			first = false;
+		else
 			ret += 2;
 
 		ret += fmt->width(fmt, &dummy_hpp, hists_to_evsel(hists));
 	}
 
-	list_for_each_entry(se, &hist_entry__sort_list, list)
-		if (!se->elide)
-			ret += 2 + hists__col_len(hists, se->se_width_idx);
-
-	if (verbose) /* Addr + origin */
+	if (verbose && sort__has_sym) /* Addr + origin */
 		ret += 3 + BITS_PER_LONG / 4;
 
 	return ret;
tools/perf/ui/stdio/hist.c +15 -7
index cfcd3f6..9f57991 100644
@@ -183,7 +183,8 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 			 * the symbol. No need to print it otherwise it appears as
 			 * displayed twice.
 			 */
-			if (!i++ && sort__first_dimension == SORT_SYM)
+			if (!i++ && field_order == NULL &&
+			    sort_order && !prefixcmp(sort_order, "sym"))
 				continue;
 			if (!printed) {
 				ret += callchain__fprintf_left_margin(fp, left_margin);
@@ -296,13 +297,20 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
 	int left_margin = 0;
 	u64 total_period = hists->stats.total_period;
 
-	if (sort__first_dimension == SORT_COMM) {
-		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
-							 typeof(*se), list);
-		left_margin = hists__col_len(hists, se->se_width_idx);
-		left_margin -= thread__comm_len(he->thread);
-	}
+	if (field_order == NULL && (sort_order == NULL ||
+				    !prefixcmp(sort_order, "comm"))) {
+		struct perf_hpp_fmt *fmt;
+
+		perf_hpp__for_each_format(fmt) {
+			if (!perf_hpp__is_sort_entry(fmt))
+				continue;
 
+			/* must be 'comm' sort entry */
+			left_margin = fmt->width(fmt, NULL, hists_to_evsel(hists));
+			left_margin -= thread__comm_len(he->thread);
+			break;
+		}
+	}
 	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
 }
 
tools/perf/util/sort.c +19 -16
index 9bee728..d913206 100644
@@ -22,9 +22,6 @@ int		sort__has_sym = 0;
 int		sort__has_dso = 0;
 enum sort_mode	sort__mode = SORT_MODE__NORMAL;
 
-enum sort_type	sort__first_dimension;
-
-LIST_HEAD(hist_entry__sort_list);
 
 static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
 {
@@ -1190,7 +1187,7 @@ static int __sort_dimension__add_hpp_output(struct sort_dimension *sd)
 	return 0;
 }
 
-static int __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
+static int __sort_dimension__add(struct sort_dimension *sd)
 {
 	if (sd->taken)
 		return 0;
@@ -1201,10 +1198,6 @@ static int __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
 	if (sd->entry->se_collapse)
 		sort__need_collapse = 1;
 
-	if (list_empty(&hist_entry__sort_list))
-		sort__first_dimension = idx;
-
-	list_add_tail(&sd->entry->list, &hist_entry__sort_list);
 	sd->taken = 1;
 
 	return 0;
@@ -1268,7 +1261,7 @@ int sort_dimension__add(const char *tok)
 			sort__has_dso = 1;
 		}
 
-		return __sort_dimension__add(sd, i);
+		return __sort_dimension__add(sd);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(hpp_sort_dimensions); i++) {
@@ -1292,7 +1285,7 @@ int sort_dimension__add(const char *tok)
 		if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
 			sort__has_sym = 1;
 
-		__sort_dimension__add(sd, i + __SORT_BRANCH_STACK);
+		__sort_dimension__add(sd);
 		return 0;
 	}
 
@@ -1308,7 +1301,7 @@ int sort_dimension__add(const char *tok)
 		if (sd->entry == &sort_mem_daddr_sym)
 			sort__has_sym = 1;
 
-		__sort_dimension__add(sd, i + __SORT_MEMORY_MODE);
+		__sort_dimension__add(sd);
 		return 0;
 	}
 
@@ -1395,7 +1388,8 @@ static void sort_entry__setup_elide(struct sort_entry *se,
 
 void sort__setup_elide(FILE *output)
 {
-	struct sort_entry *se;
+	struct perf_hpp_fmt *fmt;
+	struct hpp_sort_entry *hse;
 
 	sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
 				"dso", output);
@@ -1436,13 +1430,22 @@ void sort__setup_elide(FILE *output)
 	 * It makes no sense to elide all of sort entries.
 	 * Just revert them to show up again.
 	 */
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (!se->elide)
+	perf_hpp__for_each_format(fmt) {
+		if (!perf_hpp__is_sort_entry(fmt))
+			continue;
+
+		hse = container_of(fmt, struct hpp_sort_entry, hpp);
+		if (!hse->se->elide)
 			return;
 	}
 
-	list_for_each_entry(se, &hist_entry__sort_list, list)
-		se->elide = false;
+	perf_hpp__for_each_format(fmt) {
+		if (!perf_hpp__is_sort_entry(fmt))
+			continue;
+
+		hse = container_of(fmt, struct hpp_sort_entry, hpp);
+		hse->se->elide = false;
+	}
 }
 
 static int output_field_add(char *tok)
Signature · 1 line
1.8.3.1
author reviewer
#18 lore raw

PATCH 18/22 perf tools: Introduce reset_output_field()

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

The reset_output_field() function is for clearing output field settings and will be used for test code in later patch.

---
 tools/perf/ui/hist.c   | 17 +++++++++++++++++
 tools/perf/util/hist.h |  7 +++++++
 tools/perf/util/sort.c | 11 +++++++++++
 tools/perf/util/sort.h |  1 +
 4 files changed, 36 insertions(+)
tools/perf/ui/hist.c +17 -0
index 61cf31e..4484f5b 100644
@@ -459,6 +459,23 @@ next:
 	}
 }
 
+void perf_hpp__reset_output_field(void)
+{
+	struct perf_hpp_fmt *fmt, *tmp;
+
+	/* reset output fields */
+	perf_hpp__for_each_format_safe(fmt, tmp) {
+		list_del_init(&fmt->list);
+		list_del_init(&fmt->sort_list);
+	}
+
+	/* reset sort keys */
+	perf_hpp__for_each_sort_list_safe(fmt, tmp) {
+		list_del_init(&fmt->list);
+		list_del_init(&fmt->sort_list);
+	}
+}
+
 /*
  * See hists__fprintf to match the column widths
  */
tools/perf/util/hist.h +7 -0
index 034db76..a8418d1 100644
@@ -174,9 +174,15 @@ extern struct list_head perf_hpp__sort_list;
 #define perf_hpp__for_each_format(format) \
 	list_for_each_entry(format, &perf_hpp__list, list)
 
+#define perf_hpp__for_each_format_safe(format, tmp)	\
+	list_for_each_entry_safe(format, tmp, &perf_hpp__list, list)
+
 #define perf_hpp__for_each_sort_list(format) \
 	list_for_each_entry(format, &perf_hpp__sort_list, sort_list)
 
+#define perf_hpp__for_each_sort_list_safe(format, tmp)	\
+	list_for_each_entry_safe(format, tmp, &perf_hpp__sort_list, sort_list)
+
 extern struct perf_hpp_fmt perf_hpp__format[];
 
 enum {
@@ -197,6 +203,7 @@ void perf_hpp__column_register(struct perf_hpp_fmt *format);
 void perf_hpp__column_enable(unsigned col);
 void perf_hpp__register_sort_field(struct perf_hpp_fmt *format);
 void perf_hpp__setup_output_field(void);
+void perf_hpp__reset_output_field(void);
 void perf_hpp__append_sort_keys(void);
 
 bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
tools/perf/util/sort.c +11 -0
index d913206..901b9be 100644
@@ -1573,3 +1573,14 @@ int setup_sorting(void)
 
 	return 0;
 }
+
+void reset_output_field(void)
+{
+	sort__need_collapse = 0;
+	sort__has_parent = 0;
+	sort__has_sym = 0;
+	sort__has_dso = 0;
+
+	reset_dimensions();
+	perf_hpp__reset_output_field();
+}
tools/perf/util/sort.h +1 -0
index f5a831c..5f38d92 100644
@@ -194,6 +194,7 @@ extern struct list_head hist_entry__sort_list;
 
 int setup_sorting(void);
 int setup_output_field(void);
+void reset_output_field(void);
 extern int sort_dimension__add(const char *);
 void sort__setup_elide(FILE *fp);
 
Signature · 1 line
1.8.3.1
author reviewer
#19 lore raw

PATCH 19/22 perf tests: Factor out print_hists_*()

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

Those print helper functions can be reused by later hist test cases so factor them out to a common location.

---
 tools/perf/tests/hists_common.c | 57 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/hists_common.h |  3 +++
 tools/perf/tests/hists_filter.c | 37 ++++----------------------
 tools/perf/tests/hists_link.c   | 29 +--------------------
 4 files changed, 66 insertions(+), 60 deletions(-)
tools/perf/tests/hists_common.c +57 -0
index 44655b3..040a85b 100644
@@ -146,3 +146,60 @@ out:
 	machine__delete(machine);
 	return NULL;
 }
+
+void print_hists_in(struct hists *hists)
+{
+	int i = 0;
+	struct rb_root *root;
+	struct rb_node *node;
+
+	if (sort__need_collapse)
+		root = &hists->entries_collapsed;
+	else
+		root = hists->entries_in;
+
+	pr_info("----- %s --------\n", __func__);
+	node = rb_first(root);
+	while (node) {
+		struct hist_entry *he;
+
+		he = rb_entry(node, struct hist_entry, rb_node_in);
+
+		if (!he->filtered) {
+			pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
+				i, thread__comm_str(he->thread),
+				he->ms.map->dso->short_name,
+				he->ms.sym->name, he->stat.period);
+		}
+
+		i++;
+		node = rb_next(node);
+	}
+}
+
+void print_hists_out(struct hists *hists)
+{
+	int i = 0;
+	struct rb_root *root;
+	struct rb_node *node;
+
+	root = &hists->entries;
+
+	pr_info("----- %s --------\n", __func__);
+	node = rb_first(root);
+	while (node) {
+		struct hist_entry *he;
+
+		he = rb_entry(node, struct hist_entry, rb_node);
+
+		if (!he->filtered) {
+			pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
+				i, thread__comm_str(he->thread),
+				he->ms.map->dso->short_name,
+				he->ms.sym->name, he->stat.period);
+		}
+
+		i++;
+		node = rb_next(node);
+	}
+}
tools/perf/tests/hists_common.h +3 -0
index 2528b8f..1415ae6 100644
@@ -41,4 +41,7 @@ struct machines;
  */
 struct machine *setup_fake_machine(struct machines *machines);
 
+void print_hists_in(struct hists *hists);
+void print_hists_out(struct hists *hists);
+
 #endif /* __PERF_TESTS__HISTS_COMMON_H__ */
tools/perf/tests/hists_filter.c +5 -32
index 4617a8b..13c8cf4 100644
@@ -98,33 +98,6 @@ out:
 	return TEST_FAIL;
 }
 
-static void print_hists(struct hists *hists)
-{
-	int i = 0;
-	struct rb_root *root;
-	struct rb_node *node;
-
-	root = &hists->entries;
-
-	pr_info("----- %s --------\n", __func__);
-	node = rb_first(root);
-	while (node) {
-		struct hist_entry *he;
-
-		he = rb_entry(node, struct hist_entry, rb_node);
-
-		if (!he->filtered) {
-			pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
-				i, thread__comm_str(he->thread),
-				he->ms.map->dso->short_name,
-				he->ms.sym->name, he->stat.period);
-		}
-
-		i++;
-		node = rb_next(node);
-	}
-}
-
 int test__hists_filter(void)
 {
 	int err = TEST_FAIL;
@@ -169,7 +142,7 @@ int test__hists_filter(void)
 
 		if (verbose > 2) {
 			pr_info("Normal histogram\n");
-			print_hists(hists);
+			print_hists_out(hists);
 		}
 
 		TEST_ASSERT_VAL("Invalid nr samples",
@@ -193,7 +166,7 @@ int test__hists_filter(void)
 
 		if (verbose > 2) {
 			pr_info("Histogram for thread filter\n");
-			print_hists(hists);
+			print_hists_out(hists);
 		}
 
 		/* normal stats should be invariant */
@@ -222,7 +195,7 @@ int test__hists_filter(void)
 
 		if (verbose > 2) {
 			pr_info("Histogram for dso filter\n");
-			print_hists(hists);
+			print_hists_out(hists);
 		}
 
 		/* normal stats should be invariant */
@@ -257,7 +230,7 @@ int test__hists_filter(void)
 
 		if (verbose > 2) {
 			pr_info("Histogram for symbol filter\n");
-			print_hists(hists);
+			print_hists_out(hists);
 		}
 
 		/* normal stats should be invariant */
@@ -284,7 +257,7 @@ int test__hists_filter(void)
 
 		if (verbose > 2) {
 			pr_info("Histogram for all filters\n");
-			print_hists(hists);
+			print_hists_out(hists);
 		}
 
 		/* normal stats should be invariant */
tools/perf/tests/hists_link.c +1 -28
index b009bbf..4e783db 100644
@@ -268,33 +268,6 @@ static int validate_link(struct hists *leader, struct hists *other)
 	return __validate_link(leader, 0) || __validate_link(other, 1);
 }
 
-static void print_hists(struct hists *hists)
-{
-	int i = 0;
-	struct rb_root *root;
-	struct rb_node *node;
-
-	if (sort__need_collapse)
-		root = &hists->entries_collapsed;
-	else
-		root = hists->entries_in;
-
-	pr_info("----- %s --------\n", __func__);
-	node = rb_first(root);
-	while (node) {
-		struct hist_entry *he;
-
-		he = rb_entry(node, struct hist_entry, rb_node_in);
-
-		pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
-			i, thread__comm_str(he->thread), he->ms.map->dso->short_name,
-			he->ms.sym->name, he->stat.period);
-
-		i++;
-		node = rb_next(node);
-	}
-}
-
 int test__hists_link(void)
 {
 	int err = -1;
@@ -336,7 +309,7 @@ int test__hists_link(void)
 		hists__collapse_resort(&evsel->hists, NULL);
 
 		if (verbose > 2)
-			print_hists(&evsel->hists);
+			print_hists_in(&evsel->hists);
 	}
 
 	first = perf_evlist__first(evlist);
Signature · 1 line
1.8.3.1
author reviewer
#20 lore raw

PATCH 20/22 perf tests: Add a testcase for histogram output sorting

In reply to Jiri Olsa

From: Namhyung Kim <[email protected]>

With new output fields option, its internal implementation was changed so add a new testcase to verify whether it breaks things.

---
 tools/perf/Makefile.perf        |   1 +
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/hists_common.c |   4 +-
 tools/perf/tests/hists_filter.c |   1 +
 tools/perf/tests/hists_link.c   |   1 +
 tools/perf/tests/hists_output.c | 618 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 7 files changed, 628 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/tests/hists_output.c
tools/perf/Makefile.perf +1 -0
index dea2d633..02f0a4d 100644
@@ -399,6 +399,7 @@ LIB_OBJS += $(OUTPUT)tests/pmu.o
 LIB_OBJS += $(OUTPUT)tests/hists_common.o
 LIB_OBJS += $(OUTPUT)tests/hists_link.o
 LIB_OBJS += $(OUTPUT)tests/hists_filter.o
+LIB_OBJS += $(OUTPUT)tests/hists_output.o
 LIB_OBJS += $(OUTPUT)tests/python-use.o
 LIB_OBJS += $(OUTPUT)tests/bp_signal.o
 LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o
tools/perf/tests/builtin-test.c +4 -0
index 5e0764b..831f52c 100644
@@ -136,6 +136,10 @@ static struct test {
 		.func = test__thread_mg_share,
 	},
 	{
+		.desc = "Test output sorting of hist entries",
+		.func = test__hists_output,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/hists_common.c +2 -2
index 040a85b..e4e01aad 100644
@@ -193,8 +193,8 @@ void print_hists_out(struct hists *hists)
 		he = rb_entry(node, struct hist_entry, rb_node);
 
 		if (!he->filtered) {
-			pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
-				i, thread__comm_str(he->thread),
+			pr_info("%2d: entry: %8s:%5d [%-8s] %20s: period = %"PRIu64"\n",
+				i, thread__comm_str(he->thread), he->thread->tid,
 				he->ms.map->dso->short_name,
 				he->ms.sym->name, he->stat.period);
 		}
tools/perf/tests/hists_filter.c +1 -0
index 13c8cf4..c5ba924 100644
@@ -283,6 +283,7 @@ int test__hists_filter(void)
 out:
 	/* tear down everything */
 	perf_evlist__delete(evlist);
+	reset_output_field();
 	machines__exit(&machines);
 
 	return err;
tools/perf/tests/hists_link.c +1 -0
index 4e783db..5ffa2c3 100644
@@ -332,6 +332,7 @@ int test__hists_link(void)
 out:
 	/* tear down everything */
 	perf_evlist__delete(evlist);
+	reset_output_field();
 	machines__exit(&machines);
 
 	return err;
tools/perf/tests/hists_output.c new file +618 -0

618 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index 8f91fb0..d76c0e2 100644
@@ -44,6 +44,7 @@ int test__dwarf_unwind(void);
 int test__hists_filter(void);
 int test__mmap_thread_lookup(void);
 int test__thread_mg_share(void);
+int test__hists_output(void);
 
 #if defined(__x86_64__) || defined(__i386__) || defined(__arm__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
Signature · 1 line
1.8.3.1
author reviewer
#21 lore raw

PATCH 21/22 perf tools: Add cat as fallback pager

In reply to Jiri Olsa

From: Michael Lentine <[email protected]>

This patch adds a fallback to cat for the pager. This is useful on environments, such as Android, where less does not exist. It is better to default to cat than to abort.

---
 tools/perf/util/pager.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
tools/perf/util/pager.c +6 -6
index 3322b84..31ee02d 100644
@@ -57,13 +57,13 @@ void setup_pager(void)
 	}
 	if (!pager)
 		pager = getenv("PAGER");
-	if (!pager) {
-		if (!access("/usr/bin/pager", X_OK))
-			pager = "/usr/bin/pager";
-	}
+	if (!(pager || access("/usr/bin/pager", X_OK)))
+		pager = "/usr/bin/pager";
+	if (!(pager || access("/usr/bin/less", X_OK)))
+		pager = "/usr/bin/less";
 	if (!pager)
-		pager = "less";
-	else if (!*pager || !strcmp(pager, "cat"))
+		pager = "cat";
+	if (!*pager || !strcmp(pager, "cat"))
 		return;
 
 	spawned_pager = 1; /* means we are emitting to terminal */
Signature · 1 line
1.8.3.1
author reviewer
#22 lore raw

PATCH 22/22 perf tools: Add automatic remapping of Android libraries

In reply to Jiri Olsa

From: Michael Lentine <[email protected]>

This patch automatically adjusts the path of MMAP records associated with Android system libraries.

The Android system is organized with system libraries found in /system/lib and user libraries in /data/app-lib. On the host system (not running Android), system libraries can be found in the downloaded NDK directory under ${NDK_ROOT}/platforms/${APP_PLATFORM}/arch-${ARCH}/usr/lib and the user libraries are installed under libs/${APP_ABI} within the apk build directory. This patch makes running the reporting tools possible on the host system using the libraries from the NDK.

[ fixed 'space required before the open parenthesis' checkpatch.pl errors ]
---
 tools/perf/util/map.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 1 deletion(-)
tools/perf/util/map.c +94 -1
index ba5f5c0c..8ccbb32 100644
@@ -32,6 +32,93 @@ static inline int is_no_dso_memory(const char *filename)
 	       !strcmp(filename, "[heap]");
 }
 
+static inline int is_android_lib(const char *filename)
+{
+	return !strncmp(filename, "/data/app-lib", 13) ||
+	       !strncmp(filename, "/system/lib", 11);
+}
+
+static inline bool replace_android_lib(const char *filename, char *newfilename)
+{
+	const char *libname;
+	char *app_abi;
+	size_t app_abi_length, new_length;
+	size_t lib_length = 0;
+
+	libname  = strrchr(filename, '/');
+	if (libname)
+		lib_length = strlen(libname);
+
+	app_abi = getenv("APP_ABI");
+	if (!app_abi)
+		return false;
+
+	app_abi_length = strlen(app_abi);
+
+	if (!strncmp(filename, "/data/app-lib", 13)) {
+		char *apk_path;
+
+		if (!app_abi_length)
+			return false;
+
+		new_length = 7 + app_abi_length + lib_length;
+
+		apk_path = getenv("APK_PATH");
+		if (apk_path) {
+			new_length += strlen(apk_path) + 1;
+			if (new_length > PATH_MAX)
+				return false;
+			snprintf(newfilename, new_length,
+				 "%s/libs/%s/%s", apk_path, app_abi, libname);
+		} else {
+			if (new_length > PATH_MAX)
+				return false;
+			snprintf(newfilename, new_length,
+				 "libs/%s/%s", app_abi, libname);
+		}
+		return true;
+	}
+
+	if (!strncmp(filename, "/system/lib/", 11)) {
+		char *ndk, *app;
+		const char *arch;
+		size_t ndk_length;
+		size_t app_length;
+
+		ndk = getenv("NDK_ROOT");
+		app = getenv("APP_PLATFORM");
+
+		if (!(ndk && app))
+			return false;
+
+		ndk_length = strlen(ndk);
+		app_length = strlen(app);
+
+		if (!(ndk_length && app_length && app_abi_length))
+			return false;
+
+		arch = !strncmp(app_abi, "arm", 3) ? "arm" :
+		       !strncmp(app_abi, "mips", 4) ? "mips" :
+		       !strncmp(app_abi, "x86", 3) ? "x86" : NULL;
+
+		if (!arch)
+			return false;
+
+		new_length = 27 + ndk_length +
+			     app_length + lib_length
+			   + strlen(arch);
+
+		if (new_length > PATH_MAX)
+			return false;
+		snprintf(newfilename, new_length,
+			"%s/platforms/%s/arch-%s/usr/lib/%s",
+			ndk, app, arch, libname);
+
+		return true;
+	}
+	return false;
+}
+
 void map__init(struct map *map, enum map_type type,
 	       u64 start, u64 end, u64 pgoff, struct dso *dso)
 {
@@ -59,8 +146,9 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
 	if (map != NULL) {
 		char newfilename[PATH_MAX];
 		struct dso *dso;
-		int anon, no_dso, vdso;
+		int anon, no_dso, vdso, android;
 
+		android = is_android_lib(filename);
 		anon = is_anon_memory(filename);
 		vdso = is_vdso_map(filename);
 		no_dso = is_no_dso_memory(filename);
@@ -75,6 +163,11 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
 			filename = newfilename;
 		}
 
+		if (android) {
+			if (replace_android_lib(filename, newfilename))
+				filename = newfilename;
+		}
+
 		if (vdso) {
 			pgoff = 0;
 			dso = vdso__dso_findnew(dsos__list);
Signature · 1 line
1.8.3.1
#23 lore raw
In reply to Jiri Olsa
Quoted 75 lines · Jiri Olsa

hi Ingo,
please consider pulling

thanks,
jirka

The following changes since commit 6480c56130ba073df84d57d61062ec4118b10bbe:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf into perf/core (2014-05-20 08:36:09 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo

for you to fetch changes up to eca8183699964579ca8a0b8d116bd1f4da0136f7:

perf tools: Add automatic remapping of Android libraries (2014-05-21 15:03:25 +0200)

----------------------------------------------------------------
perf/core improvements and fixes:

. Android related fixes for pager and map dso resolving (Michael Lentine)

. Add -F option for specifying output fields (Namhyung Kim)

----------------------------------------------------------------
Michael Lentine (2):
perf tools: Add cat as fallback pager
perf tools: Add automatic remapping of Android libraries

Namhyung Kim (20):
perf tools: Add ->cmp(), ->collapse() and ->sort() to perf_hpp_fmt perf tools: Convert sort entries to hpp formats
perf tools: Use hpp formats to sort hist entries
perf tools: Support event grouping in hpp ->sort()
perf tools: Use hpp formats to sort final output
perf tools: Consolidate output field handling to hpp format routines perf ui: Get rid of callback from __hpp__fmt()
perf tools: Allow hpp fields to be sort keys
perf tools: Consolidate management of default sort orders perf tools: Call perf_hpp__init() before setting up GUI browsers perf report: Add -F option to specify output fields
perf tools: Add ->sort() member to struct sort_entry perf report/tui: Fix a bug when --fields/sort is given perf top: Add --fields option to specify output fields perf tools: Skip elided sort entries
perf hists: Reset width of output fields with header length perf tools: Get rid of obsolete hist_entry__sort_list perf tools: Introduce reset_output_field()
perf tests: Factor out print_hists_*()
perf tests: Add a testcase for histogram output sorting

 tools/perf/Documentation/perf-diff.txt   |   5 +-
 tools/perf/Documentation/perf-report.txt |  19 +
 tools/perf/Documentation/perf-top.txt    |  14 +-
 tools/perf/Makefile.perf                 |   1 +
 tools/perf/builtin-diff.c                |   7 +-
 tools/perf/builtin-report.c              |  41 +-
 tools/perf/builtin-top.c                 |  20 +-
 tools/perf/tests/builtin-test.c          |   4 +
 tools/perf/tests/hists_common.c          |  57 +++
 tools/perf/tests/hists_common.h          |   3 +
 tools/perf/tests/hists_filter.c          |  38 +-
 tools/perf/tests/hists_link.c            |  30 +-
 tools/perf/tests/hists_output.c          | 618 +++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h                 |   1 +
 tools/perf/ui/browsers/hists.c           | 104 +++---
 tools/perf/ui/gtk/hists.c                |  41 +-
 tools/perf/ui/hist.c                     | 244 +++++++++---
 tools/perf/ui/setup.c                    |   2 -
 tools/perf/ui/stdio/hist.c               |  79 ++--
Quoted 8 lines
 tools/perf/util/hist.c                   |  83 ++---
 tools/perf/util/hist.h                   |  27 +-
 tools/perf/util/map.c                    |  95 ++++-
 tools/perf/util/pager.c                  |  12 +-
 tools/perf/util/sort.c                   | 436 ++++++++++++++++++++--
 tools/perf/util/sort.h                   |   6 +
 25 files changed, 1601 insertions(+), 386 deletions(-)
 create mode 100644 tools/perf/tests/hists_output.c

Pulled, thanks a lot Jiri!

Ingo