[PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4)

[PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4)

[Namhyung Kim]

Hi,

This is a cleanup and refactoring patchset for the hist printing code
by adding perf_hpp__format functions and perf_hpp.  I believe it makes
the code easy to maintain and to add new features like upcoming group
viewing and callchain accumulation.

Any comments are welcome, thanks.
Namhyung

v3 -> v4:
 * Rebase to current acme/perf/core
 * The first two in the previous series have been merged
 * Rename hist_period_print to perf_hpp_fmt (Arnaldo)
 * Rename ctx->s to hpp->buf (Arnaldo)

v2 -> v3:
 * Move fprintf code to ui/stdio/hist.c (Arnaldo)
 * Add ack from Pekka


Namhyung Kim (5):
  perf hists: Introduce perf_hpp for hist period printing
  perf hists: Handle field separator properly
  perf hists: Use perf_hpp__format->width to calculate the column widths
  perf ui/browser: Use perf_hpp__format functions
  perf gtk/browser: Use perf_hpp__format functions

 tools/perf/Makefile            |   2 +
 tools/perf/builtin-diff.c      |   1 +
 tools/perf/ui/browsers/hists.c |  94 ++++++++---
 tools/perf/ui/gtk/browser.c    | 101 ++++++++++--
 tools/perf/ui/gtk/gtk.h        |   1 +
 tools/perf/ui/gtk/setup.c      |   1 +
 tools/perf/ui/hist.c           | 366 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c          |   8 +-
 tools/perf/ui/stdio/hist.c     | 239 +++++----------------------
 tools/perf/ui/tui/setup.c      |   4 +
 tools/perf/util/hist.c         |  33 ----
 tools/perf/util/hist.h         |  37 +++++
 12 files changed, 616 insertions(+), 271 deletions(-)
 create mode 100644 tools/perf/ui/hist.c

-- 
1.7.11.4

[PATCH 1/5] perf hists: Introduce perf_hpp for hist period printing

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Current hist print functions are messy because it has to consider many
of command line options and the code doing that is scattered around to
places. So when someone wants to add an option to manipulate the hist
output it'd very easy to miss to update all of them in sync. And things
getting worse as more options/features are added continuously.

So I'd like to refactor them using hpp formats and move common code to
ui/hist.c in order to make it easy to maintain and to add new features.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Makefile        |   2 +
 tools/perf/builtin-diff.c  |   1 +
 tools/perf/ui/hist.c       | 316 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c      |   8 +-
 tools/perf/ui/stdio/hist.c | 238 ++++++----------------------------
 tools/perf/util/hist.h     |  37 ++++++
 6 files changed, 402 insertions(+), 200 deletions(-)
 create mode 100644 tools/perf/ui/hist.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 722ddee61f9f..23e5c6ab4276 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -403,7 +403,9 @@ LIB_OBJS += $(OUTPUT)util/cgroup.o
 LIB_OBJS += $(OUTPUT)util/target.o
 LIB_OBJS += $(OUTPUT)util/rblist.o
 LIB_OBJS += $(OUTPUT)util/intlist.o
+
 LIB_OBJS += $(OUTPUT)ui/helpline.o
+LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index d29d350fb2b7..3fc53f8b098e 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -235,6 +235,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
 	if (symbol__init() < 0)
 		return -1;
 
+	perf_hpp__init(true, show_displacement);
 	setup_sorting(diff_usage, options);
 	setup_pager();
 
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
new file mode 100644
index 000000000000..c9a566cfd9c2
--- /dev/null
+++ b/tools/perf/ui/hist.c
@@ -0,0 +1,316 @@
+#include <math.h>
+
+#include "../util/hist.h"
+#include "../util/util.h"
+#include "../util/sort.h"
+
+
+/* hist period print (hpp) functions */
+static int hpp__header_overhead(struct perf_hpp *hpp)
+{
+	if (hpp->ptr)
+		return scnprintf(hpp->buf, hpp->size, "Baseline");
+	else
+		return scnprintf(hpp->buf, hpp->size, "Overhead");
+}
+
+static int hpp__width_overhead(struct perf_hpp *hpp __used)
+{
+	return 8;
+}
+
+static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+}
+
+static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+}
+
+static int hpp__header_overhead_sys(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, " sys  ");
+}
+
+static int hpp__width_overhead_sys(struct perf_hpp *hpp __used)
+{
+	return 6;
+}
+
+static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_sys / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_sys / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__header_overhead_us(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, " user ");
+}
+
+static int hpp__width_overhead_us(struct perf_hpp *hpp __used)
+{
+	return 6;
+}
+
+static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_us / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_us / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "guest sys");
+}
+
+static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __used)
+{
+	return 9;
+}
+
+static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
+					 struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
+					 struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "guest usr");
+}
+
+static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __used)
+{
+	return 9;
+}
+
+static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
+					struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_us / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
+					struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_us / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__header_samples(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "  Samples  ");
+}
+
+static int hpp__width_samples(struct perf_hpp *hpp __used)
+{
+	return 11;
+}
+
+static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	return scnprintf(hpp->buf, hpp->size, "%11" PRIu64, he->nr_events);
+}
+
+static int hpp__header_period(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "   Period   ");
+}
+
+static int hpp__width_period(struct perf_hpp *hpp __used)
+{
+	return 12;
+}
+
+static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	return scnprintf(hpp->buf, hpp->size, "%12" PRIu64, he->period);
+}
+
+static int hpp__header_delta(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, " Delta ");
+}
+
+static int hpp__width_delta(struct perf_hpp *hpp __used)
+{
+	return 7;
+}
+
+static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	struct hists *pair_hists = hpp->ptr;
+	u64 old_total, new_total;
+	double old_percent = 0, new_percent = 0;
+	double diff;
+	char buf[32];
+
+	old_total = pair_hists->stats.total_period;
+	if (old_total > 0)
+		old_percent = 100.0 * he->pair->period / old_total;
+
+	new_total = hpp->total_period;
+	if (new_total > 0)
+		new_percent = 100.0 * he->period / new_total;
+
+	diff = new_percent - old_percent;
+	if (fabs(diff) < 0.01)
+		return scnprintf(hpp->buf, hpp->size, "       ");
+
+	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
+	return scnprintf(hpp->buf, hpp->size, "%7.7s", buf);
+}
+
+static int hpp__header_displ(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "Displ.");
+}
+
+static int hpp__width_displ(struct perf_hpp *hpp __used)
+{
+	return 6;
+}
+
+static int hpp__entry_displ(struct perf_hpp *hpp, struct hist_entry *he __used)
+{
+	char buf[32];
+
+	if (!hpp->displacement)
+		return scnprintf(hpp->buf, hpp->size, "     ");
+
+	scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
+	return scnprintf(hpp->buf, hpp->size, "%6.6s", buf);
+}
+
+#define HPP__COLOR_PRINT_FNS(_name)		\
+	.header	= hpp__header_ ## _name,		\
+	.width	= hpp__width_ ## _name,		\
+	.color	= hpp__color_ ## _name,		\
+	.entry	= hpp__entry_ ## _name
+
+#define HPP__PRINT_FNS(_name)			\
+	.header	= hpp__header_ ## _name,		\
+	.width	= hpp__width_ ## _name,		\
+	.entry	= hpp__entry_ ## _name
+
+struct perf_hpp_fmt perf_hpp__format[] = {
+	{ .cond = true,  HPP__COLOR_PRINT_FNS(overhead) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_sys) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_us) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_guest_sys) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_guest_us) },
+	{ .cond = false, HPP__PRINT_FNS(samples) },
+	{ .cond = false, HPP__PRINT_FNS(period) },
+	{ .cond = false, HPP__PRINT_FNS(delta) },
+	{ .cond = false, HPP__PRINT_FNS(displ) }
+};
+
+#undef HPP__COLOR_PRINT_FNS
+#undef HPP__PRINT_FNS
+
+void perf_hpp__init(bool need_pair, bool show_displacement)
+{
+	if (symbol_conf.show_cpu_utilization) {
+		perf_hpp__format[PERF_HPP__OVERHEAD_SYS].cond = true;
+		perf_hpp__format[PERF_HPP__OVERHEAD_US].cond = true;
+
+		if (perf_guest) {
+			perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].cond = true;
+			perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].cond = true;
+		}
+	}
+
+	if (symbol_conf.show_nr_samples)
+		perf_hpp__format[PERF_HPP__SAMPLES].cond = true;
+
+	if (symbol_conf.show_total_period)
+		perf_hpp__format[PERF_HPP__PERIOD].cond = true;
+
+	if (need_pair) {
+		perf_hpp__format[PERF_HPP__DELTA].cond = true;
+
+		if (show_displacement)
+			perf_hpp__format[PERF_HPP__DISPL].cond = true;
+	}
+}
+
+static inline void advance_hpp(struct perf_hpp *hpp, int inc)
+{
+	hpp->buf  += inc;
+	hpp->size -= inc;
+}
+
+int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
+				bool color)
+{
+	const char *sep = symbol_conf.field_sep;
+	char *start = hpp->buf;
+	int i, ret;
+
+	if (symbol_conf.exclude_other && !he->parent)
+		return 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		if (!sep || i > 0) {
+			ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: "  ");
+			advance_hpp(hpp, ret);
+		}
+
+		if (color && perf_hpp__format[i].color)
+			ret = perf_hpp__format[i].color(hpp, he);
+		else
+			ret = perf_hpp__format[i].entry(hpp, he);
+
+		advance_hpp(hpp, ret);
+	}
+
+	return hpp->buf - start;
+}
+
+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;
+}
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index c7820e569660..bd7d460f844c 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -1,8 +1,8 @@
 #include <pthread.h>
 
-#include "../cache.h"
-#include "../debug.h"
-
+#include "../util/cache.h"
+#include "../util/debug.h"
+#include "../util/hist.h"
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -29,6 +29,8 @@ void setup_browser(bool fallback_to_pager)
 		use_browser = 0;
 		if (fallback_to_pager)
 			setup_pager();
+
+		perf_hpp__init(false, false);
 		break;
 	}
 }
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 9bf7e9e5a72e..4228b4c6b72d 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -1,5 +1,4 @@
 #include <stdio.h>
-#include <math.h>
 
 #include "../../util/util.h"
 #include "../../util/hist.h"
@@ -291,138 +290,6 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static int hist_entry__period_snprintf(struct hist_entry *he, char *s,
-				     size_t size, struct hists *pair_hists,
-				     bool show_displacement, long displacement,
-				     bool color, u64 total_period)
-{
-	u64 period, total, period_sys, period_us, period_guest_sys, period_guest_us;
-	u64 nr_events;
-	const char *sep = symbol_conf.field_sep;
-	int ret;
-
-	if (symbol_conf.exclude_other && !he->parent)
-		return 0;
-
-	if (pair_hists) {
-		period = he->pair ? he->pair->period : 0;
-		nr_events = he->pair ? he->pair->nr_events : 0;
-		total = pair_hists->stats.total_period;
-		period_sys = he->pair ? he->pair->period_sys : 0;
-		period_us = he->pair ? he->pair->period_us : 0;
-		period_guest_sys = he->pair ? he->pair->period_guest_sys : 0;
-		period_guest_us = he->pair ? he->pair->period_guest_us : 0;
-	} else {
-		period = he->period;
-		nr_events = he->nr_events;
-		total = total_period;
-		period_sys = he->period_sys;
-		period_us = he->period_us;
-		period_guest_sys = he->period_guest_sys;
-		period_guest_us = he->period_guest_us;
-	}
-
-	if (total) {
-		if (color)
-			ret = percent_color_snprintf(s, size,
-						     sep ? "%.2f" : "   %6.2f%%",
-						     (period * 100.0) / total);
-		else
-			ret = scnprintf(s, size, sep ? "%.2f" : "   %6.2f%%",
-				       (period * 100.0) / total);
-		if (symbol_conf.show_cpu_utilization) {
-			ret += percent_color_snprintf(s + ret, size - ret,
-					sep ? "%.2f" : "   %6.2f%%",
-					(period_sys * 100.0) / total);
-			ret += percent_color_snprintf(s + ret, size - ret,
-					sep ? "%.2f" : "   %6.2f%%",
-					(period_us * 100.0) / total);
-			if (perf_guest) {
-				ret += percent_color_snprintf(s + ret,
-						size - ret,
-						sep ? "%.2f" : "   %6.2f%%",
-						(period_guest_sys * 100.0) /
-								total);
-				ret += percent_color_snprintf(s + ret,
-						size - ret,
-						sep ? "%.2f" : "   %6.2f%%",
-						(period_guest_us * 100.0) /
-								total);
-			}
-		}
-	} else
-		ret = scnprintf(s, size, sep ? "%" PRIu64 : "%12" PRIu64 " ", period);
-
-	if (symbol_conf.show_nr_samples) {
-		if (sep)
-			ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, nr_events);
-		else
-			ret += scnprintf(s + ret, size - ret, "%11" PRIu64, nr_events);
-	}
-
-	if (symbol_conf.show_total_period) {
-		if (sep)
-			ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, period);
-		else
-			ret += scnprintf(s + ret, size - ret, " %12" PRIu64, period);
-	}
-
-	if (pair_hists) {
-		char bf[32];
-		double old_percent = 0, new_percent = 0, diff;
-
-		if (total > 0)
-			old_percent = (period * 100.0) / total;
-		if (total_period > 0)
-			new_percent = (he->period * 100.0) / total_period;
-
-		diff = new_percent - old_percent;
-
-		if (fabs(diff) >= 0.01)
-			scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
-		else
-			scnprintf(bf, sizeof(bf), " ");
-
-		if (sep)
-			ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
-		else
-			ret += scnprintf(s + ret, size - ret, "%11.11s", bf);
-
-		if (show_displacement) {
-			if (displacement)
-				scnprintf(bf, sizeof(bf), "%+4ld", displacement);
-			else
-				scnprintf(bf, sizeof(bf), " ");
-
-			if (sep)
-				ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
-			else
-				ret += scnprintf(s + ret, size - ret, "%6.6s", bf);
-		}
-	}
-
-	return ret;
-}
-
-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;
-}
-
 static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
 					    struct hists *hists,
 					    u64 total_period, FILE *fp)
@@ -441,18 +308,22 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
 
 static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 			       struct hists *hists, struct hists *pair_hists,
-			       bool show_displacement, long displacement,
-			       u64 total_period, FILE *fp)
+			       long displacement, u64 total_period, FILE *fp)
 {
 	char bf[512];
 	int ret;
+	struct perf_hpp hpp = {
+		.buf		= bf,
+		.size		= size,
+		.total_period	= total_period,
+		.displacement	= displacement,
+		.ptr		= pair_hists,
+	};
 
 	if (size == 0 || size > sizeof(bf))
-		size = sizeof(bf);
+		size = hpp.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
-					  show_displacement, displacement,
-					  true, total_period);
+	ret = hist_entry__period_snprintf(&hpp, he, true);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);
@@ -477,59 +348,29 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 	unsigned int width;
 	const char *sep = symbol_conf.field_sep;
 	const char *col_width = symbol_conf.col_width_list_str;
-	int nr_rows = 0;
+	int idx, nr_rows = 0;
+	char bf[64];
+	struct perf_hpp dummy_hpp = {
+		.buf	= bf,
+		.size	= sizeof(bf),
+		.ptr	= pair,
+	};
 
 	init_rem_hits();
 
 	if (!show_header)
 		goto print_entries;
 
-	fprintf(fp, "# %s", pair ? "Baseline" : "Overhead");
-
-	if (symbol_conf.show_cpu_utilization) {
-		if (sep) {
-			ret += fprintf(fp, "%csys", *sep);
-			ret += fprintf(fp, "%cus", *sep);
-			if (perf_guest) {
-				ret += fprintf(fp, "%cguest sys", *sep);
-				ret += fprintf(fp, "%cguest us", *sep);
-			}
-		} else {
-			ret += fprintf(fp, "     sys  ");
-			ret += fprintf(fp, "      us  ");
-			if (perf_guest) {
-				ret += fprintf(fp, "  guest sys  ");
-				ret += fprintf(fp, "  guest us  ");
-			}
-		}
-	}
-
-	if (symbol_conf.show_nr_samples) {
-		if (sep)
-			fprintf(fp, "%cSamples", *sep);
-		else
-			fputs("  Samples  ", fp);
-	}
-
-	if (symbol_conf.show_total_period) {
-		if (sep)
-			ret += fprintf(fp, "%cPeriod", *sep);
-		else
-			ret += fprintf(fp, "   Period    ");
-	}
+	fprintf(fp, "# ");
+	for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
+		if (!perf_hpp__format[idx].cond)
+			continue;
 
-	if (pair) {
-		if (sep)
-			ret += fprintf(fp, "%cDelta", *sep);
-		else
-			ret += fprintf(fp, "  Delta    ");
+		if (idx)
+			fprintf(fp, "%s", sep ?: "  ");
 
-		if (show_displacement) {
-			if (sep)
-				ret += fprintf(fp, "%cDisplacement", *sep);
-			else
-				ret += fprintf(fp, " Displ");
-		}
+		perf_hpp__format[idx].header(&dummy_hpp);
+		fprintf(fp, "%s", bf);
 	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
@@ -561,18 +402,21 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 	if (sep)
 		goto print_entries;
 
-	fprintf(fp, "# ........");
-	if (symbol_conf.show_cpu_utilization)
-		fprintf(fp, "   .......   .......");
-	if (symbol_conf.show_nr_samples)
-		fprintf(fp, " ..........");
-	if (symbol_conf.show_total_period)
-		fprintf(fp, " ............");
-	if (pair) {
-		fprintf(fp, " ..........");
-		if (show_displacement)
-			fprintf(fp, " .....");
+	fprintf(fp, "# ");
+	for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
+		unsigned int i;
+
+		if (!perf_hpp__format[idx].cond)
+			continue;
+
+		if (idx)
+			fprintf(fp, "%s", sep ?: "  ");
+
+		width = perf_hpp__format[idx].width(&dummy_hpp);
+		for (i = 0; i < width; i++)
+			fprintf(fp, ".");
 	}
+
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		unsigned int i;
 
@@ -612,8 +456,8 @@ print_entries:
 				displacement = 0;
 			++position;
 		}
-		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
-					   displacement, total_period, fp);
+		ret += hist_entry__fprintf(h, max_cols, hists, pair, displacement,
+					   total_period, fp);
 
 		if (max_rows && ++nr_rows >= max_rows)
 			goto out;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2e650ffb7d23..4146f51124f0 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -115,6 +115,43 @@ bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
 void hists__reset_col_len(struct hists *hists);
 void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
 
+struct perf_hpp {
+	char *buf;
+	size_t size;
+	u64 total_period;
+	const char *sep;
+	long displacement;
+	void *ptr;
+};
+
+struct perf_hpp_fmt {
+	bool cond;
+	int (*header)(struct perf_hpp *hpp);
+	int (*width)(struct perf_hpp *hpp);
+	int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
+	int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+};
+
+extern struct perf_hpp_fmt perf_hpp__format[];
+
+enum {
+	PERF_HPP__OVERHEAD,
+	PERF_HPP__OVERHEAD_SYS,
+	PERF_HPP__OVERHEAD_US,
+	PERF_HPP__OVERHEAD_GUEST_SYS,
+	PERF_HPP__OVERHEAD_GUEST_US,
+	PERF_HPP__SAMPLES,
+	PERF_HPP__PERIOD,
+	PERF_HPP__DELTA,
+	PERF_HPP__DISPL,
+
+	PERF_HPP__MAX_INDEX
+};
+
+void perf_hpp__init(bool need_pair, bool show_displacement);
+int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
+				bool color);
+
 struct perf_evlist;
 
 #ifdef NO_NEWT_SUPPORT
-- 
1.7.11.4

[tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

[tip-bot for Namhyung Kim]

Commit-ID:  ea251d51d2c7d7233790123227f787c477f567f5
Gitweb:     http://git.kernel.org/tip/ea251d51d2c7d7233790123227f787c477f567f5
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 3 Sep 2012 11:53:06 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 8 Sep 2012 13:19:44 -0300

perf hists: Introduce perf_hpp for hist period printing

Current hist print functions are messy because it has to consider many
of command line options and the code doing that is scattered around to
places. So when someone wants to add an option to manipulate the hist
output it'd very easy to miss to update all of them in sync. And things
getting worse as more options/features are added continuously.

So I'd like to refactor them using hpp formats and move common code to
ui/hist.c in order to make it easy to maintain and to add new features.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346640790-17197-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile        |    2 +
 tools/perf/builtin-diff.c  |    1 +
 tools/perf/ui/hist.c       |  340 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c      |    8 +-
 tools/perf/ui/stdio/hist.c |  238 ++++++-------------------------
 tools/perf/util/hist.h     |   37 +++++
 6 files changed, 426 insertions(+), 200 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 3eda492..e4b2e8f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -403,7 +403,9 @@ LIB_OBJS += $(OUTPUT)util/cgroup.o
 LIB_OBJS += $(OUTPUT)util/target.o
 LIB_OBJS += $(OUTPUT)util/rblist.o
 LIB_OBJS += $(OUTPUT)util/intlist.o
+
 LIB_OBJS += $(OUTPUT)ui/helpline.o
+LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index e9933fd..c4c6d76 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -264,6 +264,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
 	if (symbol__init() < 0)
 		return -1;
 
+	perf_hpp__init(true, show_displacement);
 	setup_sorting(diff_usage, options);
 	setup_pager();
 
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
new file mode 100644
index 0000000..8ccd1f2
--- /dev/null
+++ b/tools/perf/ui/hist.c
@@ -0,0 +1,340 @@
+#include <math.h>
+
+#include "../util/hist.h"
+#include "../util/util.h"
+#include "../util/sort.h"
+
+
+/* hist period print (hpp) functions */
+static int hpp__header_overhead(struct perf_hpp *hpp)
+{
+	if (hpp->ptr)
+		return scnprintf(hpp->buf, hpp->size, "Baseline");
+	else
+		return scnprintf(hpp->buf, hpp->size, "Overhead");
+}
+
+static int hpp__width_overhead(struct perf_hpp *hpp __used)
+{
+	return 8;
+}
+
+static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period / hpp->total_period;
+
+	if (hpp->ptr) {
+		struct hists *old_hists = hpp->ptr;
+		u64 total_period = old_hists->stats.total_period;
+		u64 base_period = he->pair ? he->pair->period : 0;
+
+		if (total_period)
+			percent = 100.0 * base_period / total_period;
+		else
+			percent = 0.0;
+	}
+
+	return percent_color_snprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+}
+
+static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period / hpp->total_period;
+
+	if (hpp->ptr) {
+		struct hists *old_hists = hpp->ptr;
+		u64 total_period = old_hists->stats.total_period;
+		u64 base_period = he->pair ? he->pair->period : 0;
+
+		if (total_period)
+			percent = 100.0 * base_period / total_period;
+		else
+			percent = 0.0;
+	}
+
+	return scnprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+}
+
+static int hpp__header_overhead_sys(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, " sys  ");
+}
+
+static int hpp__width_overhead_sys(struct perf_hpp *hpp __used)
+{
+	return 6;
+}
+
+static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_sys / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_sys / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__header_overhead_us(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, " user ");
+}
+
+static int hpp__width_overhead_us(struct perf_hpp *hpp __used)
+{
+	return 6;
+}
+
+static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_us / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_us / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+}
+
+static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "guest sys");
+}
+
+static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __used)
+{
+	return 9;
+}
+
+static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
+					 struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
+					 struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "guest usr");
+}
+
+static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __used)
+{
+	return 9;
+}
+
+static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
+					struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_us / hpp->total_period;
+	return percent_color_snprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
+					struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_us / hpp->total_period;
+	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+}
+
+static int hpp__header_samples(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "  Samples  ");
+}
+
+static int hpp__width_samples(struct perf_hpp *hpp __used)
+{
+	return 11;
+}
+
+static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	return scnprintf(hpp->buf, hpp->size, "%11" PRIu64, he->nr_events);
+}
+
+static int hpp__header_period(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "   Period   ");
+}
+
+static int hpp__width_period(struct perf_hpp *hpp __used)
+{
+	return 12;
+}
+
+static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	return scnprintf(hpp->buf, hpp->size, "%12" PRIu64, he->period);
+}
+
+static int hpp__header_delta(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, " Delta ");
+}
+
+static int hpp__width_delta(struct perf_hpp *hpp __used)
+{
+	return 7;
+}
+
+static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
+{
+	struct hists *pair_hists = hpp->ptr;
+	u64 old_total, new_total;
+	double old_percent = 0, new_percent = 0;
+	double diff;
+	char buf[32];
+
+	old_total = pair_hists->stats.total_period;
+	if (old_total > 0 && he->pair)
+		old_percent = 100.0 * he->pair->period / old_total;
+
+	new_total = hpp->total_period;
+	if (new_total > 0)
+		new_percent = 100.0 * he->period / new_total;
+
+	diff = new_percent - old_percent;
+	if (fabs(diff) < 0.01)
+		return scnprintf(hpp->buf, hpp->size, "       ");
+
+	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
+	return scnprintf(hpp->buf, hpp->size, "%7.7s", buf);
+}
+
+static int hpp__header_displ(struct perf_hpp *hpp)
+{
+	return scnprintf(hpp->buf, hpp->size, "Displ.");
+}
+
+static int hpp__width_displ(struct perf_hpp *hpp __used)
+{
+	return 6;
+}
+
+static int hpp__entry_displ(struct perf_hpp *hpp, struct hist_entry *he __used)
+{
+	char buf[32];
+
+	if (!hpp->displacement)
+		return scnprintf(hpp->buf, hpp->size, "     ");
+
+	scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
+	return scnprintf(hpp->buf, hpp->size, "%6.6s", buf);
+}
+
+#define HPP__COLOR_PRINT_FNS(_name)		\
+	.header	= hpp__header_ ## _name,		\
+	.width	= hpp__width_ ## _name,		\
+	.color	= hpp__color_ ## _name,		\
+	.entry	= hpp__entry_ ## _name
+
+#define HPP__PRINT_FNS(_name)			\
+	.header	= hpp__header_ ## _name,		\
+	.width	= hpp__width_ ## _name,		\
+	.entry	= hpp__entry_ ## _name
+
+struct perf_hpp_fmt perf_hpp__format[] = {
+	{ .cond = true,  HPP__COLOR_PRINT_FNS(overhead) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_sys) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_us) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_guest_sys) },
+	{ .cond = false, HPP__COLOR_PRINT_FNS(overhead_guest_us) },
+	{ .cond = false, HPP__PRINT_FNS(samples) },
+	{ .cond = false, HPP__PRINT_FNS(period) },
+	{ .cond = false, HPP__PRINT_FNS(delta) },
+	{ .cond = false, HPP__PRINT_FNS(displ) }
+};
+
+#undef HPP__COLOR_PRINT_FNS
+#undef HPP__PRINT_FNS
+
+void perf_hpp__init(bool need_pair, bool show_displacement)
+{
+	if (symbol_conf.show_cpu_utilization) {
+		perf_hpp__format[PERF_HPP__OVERHEAD_SYS].cond = true;
+		perf_hpp__format[PERF_HPP__OVERHEAD_US].cond = true;
+
+		if (perf_guest) {
+			perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].cond = true;
+			perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].cond = true;
+		}
+	}
+
+	if (symbol_conf.show_nr_samples)
+		perf_hpp__format[PERF_HPP__SAMPLES].cond = true;
+
+	if (symbol_conf.show_total_period)
+		perf_hpp__format[PERF_HPP__PERIOD].cond = true;
+
+	if (need_pair) {
+		perf_hpp__format[PERF_HPP__DELTA].cond = true;
+
+		if (show_displacement)
+			perf_hpp__format[PERF_HPP__DISPL].cond = true;
+	}
+}
+
+static inline void advance_hpp(struct perf_hpp *hpp, int inc)
+{
+	hpp->buf  += inc;
+	hpp->size -= inc;
+}
+
+int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
+				bool color)
+{
+	const char *sep = symbol_conf.field_sep;
+	char *start = hpp->buf;
+	int i, ret;
+
+	if (symbol_conf.exclude_other && !he->parent)
+		return 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		if (!sep || i > 0) {
+			ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: "  ");
+			advance_hpp(hpp, ret);
+		}
+
+		if (color && perf_hpp__format[i].color)
+			ret = perf_hpp__format[i].color(hpp, he);
+		else
+			ret = perf_hpp__format[i].entry(hpp, he);
+
+		advance_hpp(hpp, ret);
+	}
+
+	return hpp->buf - start;
+}
+
+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;
+}
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index c7820e5..bd7d460 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -1,8 +1,8 @@
 #include <pthread.h>
 
-#include "../cache.h"
-#include "../debug.h"
-
+#include "../util/cache.h"
+#include "../util/debug.h"
+#include "../util/hist.h"
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -29,6 +29,8 @@ void setup_browser(bool fallback_to_pager)
 		use_browser = 0;
 		if (fallback_to_pager)
 			setup_pager();
+
+		perf_hpp__init(false, false);
 		break;
 	}
 }
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 9bf7e9e..4228b4c 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -1,5 +1,4 @@
 #include <stdio.h>
-#include <math.h>
 
 #include "../../util/util.h"
 #include "../../util/hist.h"
@@ -291,138 +290,6 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static int hist_entry__period_snprintf(struct hist_entry *he, char *s,
-				     size_t size, struct hists *pair_hists,
-				     bool show_displacement, long displacement,
-				     bool color, u64 total_period)
-{
-	u64 period, total, period_sys, period_us, period_guest_sys, period_guest_us;
-	u64 nr_events;
-	const char *sep = symbol_conf.field_sep;
-	int ret;
-
-	if (symbol_conf.exclude_other && !he->parent)
-		return 0;
-
-	if (pair_hists) {
-		period = he->pair ? he->pair->period : 0;
-		nr_events = he->pair ? he->pair->nr_events : 0;
-		total = pair_hists->stats.total_period;
-		period_sys = he->pair ? he->pair->period_sys : 0;
-		period_us = he->pair ? he->pair->period_us : 0;
-		period_guest_sys = he->pair ? he->pair->period_guest_sys : 0;
-		period_guest_us = he->pair ? he->pair->period_guest_us : 0;
-	} else {
-		period = he->period;
-		nr_events = he->nr_events;
-		total = total_period;
-		period_sys = he->period_sys;
-		period_us = he->period_us;
-		period_guest_sys = he->period_guest_sys;
-		period_guest_us = he->period_guest_us;
-	}
-
-	if (total) {
-		if (color)
-			ret = percent_color_snprintf(s, size,
-						     sep ? "%.2f" : "   %6.2f%%",
-						     (period * 100.0) / total);
-		else
-			ret = scnprintf(s, size, sep ? "%.2f" : "   %6.2f%%",
-				       (period * 100.0) / total);
-		if (symbol_conf.show_cpu_utilization) {
-			ret += percent_color_snprintf(s + ret, size - ret,
-					sep ? "%.2f" : "   %6.2f%%",
-					(period_sys * 100.0) / total);
-			ret += percent_color_snprintf(s + ret, size - ret,
-					sep ? "%.2f" : "   %6.2f%%",
-					(period_us * 100.0) / total);
-			if (perf_guest) {
-				ret += percent_color_snprintf(s + ret,
-						size - ret,
-						sep ? "%.2f" : "   %6.2f%%",
-						(period_guest_sys * 100.0) /
-								total);
-				ret += percent_color_snprintf(s + ret,
-						size - ret,
-						sep ? "%.2f" : "   %6.2f%%",
-						(period_guest_us * 100.0) /
-								total);
-			}
-		}
-	} else
-		ret = scnprintf(s, size, sep ? "%" PRIu64 : "%12" PRIu64 " ", period);
-
-	if (symbol_conf.show_nr_samples) {
-		if (sep)
-			ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, nr_events);
-		else
-			ret += scnprintf(s + ret, size - ret, "%11" PRIu64, nr_events);
-	}
-
-	if (symbol_conf.show_total_period) {
-		if (sep)
-			ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, period);
-		else
-			ret += scnprintf(s + ret, size - ret, " %12" PRIu64, period);
-	}
-
-	if (pair_hists) {
-		char bf[32];
-		double old_percent = 0, new_percent = 0, diff;
-
-		if (total > 0)
-			old_percent = (period * 100.0) / total;
-		if (total_period > 0)
-			new_percent = (he->period * 100.0) / total_period;
-
-		diff = new_percent - old_percent;
-
-		if (fabs(diff) >= 0.01)
-			scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
-		else
-			scnprintf(bf, sizeof(bf), " ");
-
-		if (sep)
-			ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
-		else
-			ret += scnprintf(s + ret, size - ret, "%11.11s", bf);
-
-		if (show_displacement) {
-			if (displacement)
-				scnprintf(bf, sizeof(bf), "%+4ld", displacement);
-			else
-				scnprintf(bf, sizeof(bf), " ");
-
-			if (sep)
-				ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
-			else
-				ret += scnprintf(s + ret, size - ret, "%6.6s", bf);
-		}
-	}
-
-	return ret;
-}
-
-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;
-}
-
 static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
 					    struct hists *hists,
 					    u64 total_period, FILE *fp)
@@ -441,18 +308,22 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
 
 static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 			       struct hists *hists, struct hists *pair_hists,
-			       bool show_displacement, long displacement,
-			       u64 total_period, FILE *fp)
+			       long displacement, u64 total_period, FILE *fp)
 {
 	char bf[512];
 	int ret;
+	struct perf_hpp hpp = {
+		.buf		= bf,
+		.size		= size,
+		.total_period	= total_period,
+		.displacement	= displacement,
+		.ptr		= pair_hists,
+	};
 
 	if (size == 0 || size > sizeof(bf))
-		size = sizeof(bf);
+		size = hpp.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
-					  show_displacement, displacement,
-					  true, total_period);
+	ret = hist_entry__period_snprintf(&hpp, he, true);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);
@@ -477,59 +348,29 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 	unsigned int width;
 	const char *sep = symbol_conf.field_sep;
 	const char *col_width = symbol_conf.col_width_list_str;
-	int nr_rows = 0;
+	int idx, nr_rows = 0;
+	char bf[64];
+	struct perf_hpp dummy_hpp = {
+		.buf	= bf,
+		.size	= sizeof(bf),
+		.ptr	= pair,
+	};
 
 	init_rem_hits();
 
 	if (!show_header)
 		goto print_entries;
 
-	fprintf(fp, "# %s", pair ? "Baseline" : "Overhead");
-
-	if (symbol_conf.show_cpu_utilization) {
-		if (sep) {
-			ret += fprintf(fp, "%csys", *sep);
-			ret += fprintf(fp, "%cus", *sep);
-			if (perf_guest) {
-				ret += fprintf(fp, "%cguest sys", *sep);
-				ret += fprintf(fp, "%cguest us", *sep);
-			}
-		} else {
-			ret += fprintf(fp, "     sys  ");
-			ret += fprintf(fp, "      us  ");
-			if (perf_guest) {
-				ret += fprintf(fp, "  guest sys  ");
-				ret += fprintf(fp, "  guest us  ");
-			}
-		}
-	}
-
-	if (symbol_conf.show_nr_samples) {
-		if (sep)
-			fprintf(fp, "%cSamples", *sep);
-		else
-			fputs("  Samples  ", fp);
-	}
-
-	if (symbol_conf.show_total_period) {
-		if (sep)
-			ret += fprintf(fp, "%cPeriod", *sep);
-		else
-			ret += fprintf(fp, "   Period    ");
-	}
+	fprintf(fp, "# ");
+	for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
+		if (!perf_hpp__format[idx].cond)
+			continue;
 
-	if (pair) {
-		if (sep)
-			ret += fprintf(fp, "%cDelta", *sep);
-		else
-			ret += fprintf(fp, "  Delta    ");
+		if (idx)
+			fprintf(fp, "%s", sep ?: "  ");
 
-		if (show_displacement) {
-			if (sep)
-				ret += fprintf(fp, "%cDisplacement", *sep);
-			else
-				ret += fprintf(fp, " Displ");
-		}
+		perf_hpp__format[idx].header(&dummy_hpp);
+		fprintf(fp, "%s", bf);
 	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
@@ -561,18 +402,21 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 	if (sep)
 		goto print_entries;
 
-	fprintf(fp, "# ........");
-	if (symbol_conf.show_cpu_utilization)
-		fprintf(fp, "   .......   .......");
-	if (symbol_conf.show_nr_samples)
-		fprintf(fp, " ..........");
-	if (symbol_conf.show_total_period)
-		fprintf(fp, " ............");
-	if (pair) {
-		fprintf(fp, " ..........");
-		if (show_displacement)
-			fprintf(fp, " .....");
+	fprintf(fp, "# ");
+	for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
+		unsigned int i;
+
+		if (!perf_hpp__format[idx].cond)
+			continue;
+
+		if (idx)
+			fprintf(fp, "%s", sep ?: "  ");
+
+		width = perf_hpp__format[idx].width(&dummy_hpp);
+		for (i = 0; i < width; i++)
+			fprintf(fp, ".");
 	}
+
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		unsigned int i;
 
@@ -612,8 +456,8 @@ print_entries:
 				displacement = 0;
 			++position;
 		}
-		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
-					   displacement, total_period, fp);
+		ret += hist_entry__fprintf(h, max_cols, hists, pair, displacement,
+					   total_period, fp);
 
 		if (max_rows && ++nr_rows >= max_rows)
 			goto out;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2e650ff..4146f51 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -115,6 +115,43 @@ bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
 void hists__reset_col_len(struct hists *hists);
 void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
 
+struct perf_hpp {
+	char *buf;
+	size_t size;
+	u64 total_period;
+	const char *sep;
+	long displacement;
+	void *ptr;
+};
+
+struct perf_hpp_fmt {
+	bool cond;
+	int (*header)(struct perf_hpp *hpp);
+	int (*width)(struct perf_hpp *hpp);
+	int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
+	int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+};
+
+extern struct perf_hpp_fmt perf_hpp__format[];
+
+enum {
+	PERF_HPP__OVERHEAD,
+	PERF_HPP__OVERHEAD_SYS,
+	PERF_HPP__OVERHEAD_US,
+	PERF_HPP__OVERHEAD_GUEST_SYS,
+	PERF_HPP__OVERHEAD_GUEST_US,
+	PERF_HPP__SAMPLES,
+	PERF_HPP__PERIOD,
+	PERF_HPP__DELTA,
+	PERF_HPP__DISPL,
+
+	PERF_HPP__MAX_INDEX
+};
+
+void perf_hpp__init(bool need_pair, bool show_displacement);
+int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
+				bool color);
+
 struct perf_evlist;
 
 #ifdef NO_NEWT_SUPPORT

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

[Robert Richter]

On 09.09.12 01:54:39, tip-bot for Namhyung Kim wrote:
> Commit-ID:  ea251d51d2c7d7233790123227f787c477f567f5
> Gitweb:     http://git.kernel.org/tip/ea251d51d2c7d7233790123227f787c477f567f5
> Author:     Namhyung Kim <namhyung.kim@lge.com>
> AuthorDate: Mon, 3 Sep 2012 11:53:06 +0900
> Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
> CommitDate: Sat, 8 Sep 2012 13:19:44 -0300
> 
> perf hists: Introduce perf_hpp for hist period printing
> 
> Current hist print functions are messy because it has to consider many
> of command line options and the code doing that is scattered around to
> places. So when someone wants to add an option to manipulate the hist
> output it'd very easy to miss to update all of them in sync. And things
> getting worse as more options/features are added continuously.
> 
> So I'd like to refactor them using hpp formats and move common code to
> ui/hist.c in order to make it easy to maintain and to add new features.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Link: http://lkml.kernel.org/r/1346640790-17197-2-git-send-email-namhyung@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/Makefile        |    2 +
>  tools/perf/builtin-diff.c  |    1 +
>  tools/perf/ui/hist.c       |  340 ++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/ui/setup.c      |    8 +-
>  tools/perf/ui/stdio/hist.c |  238 ++++++-------------------------
>  tools/perf/util/hist.h     |   37 +++++
>  6 files changed, 426 insertions(+), 200 deletions(-)

This patch breaks perf-record/report that the number of samples can't
be shown in pipe mode:

 # perf record -e cycles -aq sleep 1 ; perf report -n --sort comm,dso | sed '/%/q;d' ; \
   perf record -e cycles -aq sleep 1 | perf report -n --sort comm,dso | sed '/%/q;d'
     99.86%        11804       swapper  [kernel.kallsyms]
     91.57%      swapper  [kernel.kallsyms]
           ^^^^^^
           number of samples missing

Moving and changing the code at the same time make the patch
unreviewable. So no clue that's the problem here.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

[Arnaldo Carvalho de Melo]

Em Wed, Sep 12, 2012 at 07:26:41PM +0200, Robert Richter escreveu:
> On 09.09.12 01:54:39, tip-bot for Namhyung Kim wrote:
> 
>  # perf record -e cycles -aq sleep 1 ; perf report -n --sort comm,dso | sed '/%/q;d' ; \
>    perf record -e cycles -aq sleep 1 | perf report -n --sort comm,dso | sed '/%/q;d'
>      99.86%        11804       swapper  [kernel.kallsyms]
>      91.57%      swapper  [kernel.kallsyms]
>            ^^^^^^
>            number of samples missing
> 
> Moving and changing the code at the same time make the patch
> unreviewable. So no clue that's the problem here.

Yeah, that was unfortunate (movind and changing), we need to try hard
not to do that...

Anyway, Namhyung, can you take a look at this?

- Arnaldo

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

[Namhyung Kim]

Hi Arnaldo and Robert,

On Wed, 12 Sep 2012 15:48:15 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 12, 2012 at 07:26:41PM +0200, Robert Richter escreveu:
>> On 09.09.12 01:54:39, tip-bot for Namhyung Kim wrote:
>> 
>>  # perf record -e cycles -aq sleep 1 ; perf report -n --sort comm,dso | sed '/%/q;d' ; \
>>    perf record -e cycles -aq sleep 1 | perf report -n --sort comm,dso | sed '/%/q;d'
>>      99.86%        11804       swapper  [kernel.kallsyms]
>>      91.57%      swapper  [kernel.kallsyms]
>>            ^^^^^^
>>            number of samples missing
>> 
>> Moving and changing the code at the same time make the patch
>> unreviewable. So no clue that's the problem here.
>
> Yeah, that was unfortunate (movind and changing), we need to try hard
> not to do that...

Okay, will be careful next time.


>
> Anyway, Namhyung, can you take a look at this?

Sure, I'll take a look.

Thanks,
Namhyung

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

[Arnaldo Carvalho de Melo]

Em Thu, Sep 13, 2012 at 11:43:08AM +0900, Namhyung Kim escreveu:
> On Wed, 12 Sep 2012 15:48:15 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Sep 12, 2012 at 07:26:41PM +0200, Robert Richter escreveu:
> >> Moving and changing the code at the same time make the patch
> >> unreviewable. So no clue that's the problem here.

> > Yeah, that was unfortunate (movind and changing), we need to try hard
> > not to do that...

> Okay, will be careful next time.

Its a neverending discipline to follow :-\ But it can be fun, keep it
up!

> > Anyway, Namhyung, can you take a look at this?
 
> Sure, I'll take a look.

Thanks!

- Arnaldo

[PATCH] perf record: Add missing perf_hpp__init for pipe-mode

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

The perf_hpp__init() function was only called from setup_browser() so
that the pipe-mode missed the initialization thus didn't respond to
related options.  Fix it.

Reported-by: Robert Richter <robert.richter@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 97b2e6300f4c..279155a47d1c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -689,8 +689,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	if (strcmp(report.input_name, "-") != 0)
 		setup_browser(true);
-	else
+	else {
 		use_browser = 0;
+		perf_hpp__init(false, false);
+	}
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,
-- 
1.7.11.4

Re: [PATCH] perf record: Add missing perf_hpp__init for pipe-mode

[Namhyung Kim]

Oops, please do s/record/report/ on the subject line, sorry! ;-)

Also I don't see the issue on the other users of the setup_browser() -
i.e. perf annotate and perf top - since they call it unconditionally.

Thanks,
Namhyung

Re: [PATCH] perf record: Add missing perf_hpp__init for pipe-mode

[Robert Richter]

On 13.09.12 13:14:30, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> The perf_hpp__init() function was only called from setup_browser() so
> that the pipe-mode missed the initialization thus didn't respond to
> related options.  Fix it.
> 
> Reported-by: Robert Richter <robert.richter@amd.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-report.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

This patch fixes it.

Thanks,

-Robert

> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 97b2e6300f4c..279155a47d1c 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -689,8 +689,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
>  
>  	if (strcmp(report.input_name, "-") != 0)
>  		setup_browser(true);
> -	else
> +	else {
>  		use_browser = 0;
> +		perf_hpp__init(false, false);
> +	}
>  
>  	/*
>  	 * Only in the newt browser we are doing integrated annotation,
> -- 
> 1.7.11.4
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

[tip:perf/core] perf report: Add missing perf_hpp__init for pipe-mode

[tip-bot for Namhyung Kim]

Commit-ID:  60e5c706b3ea56f87afc2a4a3096118d28f9cc24
Gitweb:     http://git.kernel.org/tip/60e5c706b3ea56f87afc2a4a3096118d28f9cc24
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 13 Sep 2012 13:14:30 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:11:33 -0300

perf report: Add missing perf_hpp__init for pipe-mode

The perf_hpp__init() function was only called from setup_browser() so
that the pipe-mode missed the initialization thus didn't respond to
related options.  Fix it.

Reported-by: Robert Richter <robert.richter@amd.com>
Tested-by: Robert Richter <robert.richter@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Robert Richter <robert.richter@amd.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-tip-commits@vger.kernel.org
Link: http://lkml.kernel.org/r/87txv28spl.fsf_-_@sejong.aot.lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index b6696dd..1da243d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -689,8 +689,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	if (strcmp(report.input_name, "-") != 0)
 		setup_browser(true);
-	else
+	else {
 		use_browser = 0;
+		perf_hpp__init(false, false);
+	}
 
 	setup_sorting(report_usage, options);
 

[PATCH 2/5] perf hists: Handle field separator properly

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

When a field separator is given, the output format doesn't need to be
fancy like aligning to column length, coloring the percent value and
so on.  And since there's a slight difference to normal format, fix it
not to break backward compatibility.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c       | 75 ++++++++++++++++++++++++++++++----------------
 tools/perf/ui/stdio/hist.c |  3 +-
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index c9a566cfd9c2..802a8659c15a 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -8,10 +8,9 @@
 /* hist period print (hpp) functions */
 static int hpp__header_overhead(struct perf_hpp *hpp)
 {
-	if (hpp->ptr)
-		return scnprintf(hpp->buf, hpp->size, "Baseline");
-	else
-		return scnprintf(hpp->buf, hpp->size, "Overhead");
+	const char *fmt = hpp->ptr ? "Baseline" : "Overhead";
+
+	return scnprintf(hpp->buf, hpp->size, fmt);
 }
 
 static int hpp__width_overhead(struct perf_hpp *hpp __used)
@@ -28,12 +27,16 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_sys(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " sys  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "sys");
 }
 
 static int hpp__width_overhead_sys(struct perf_hpp *hpp __used)
@@ -50,12 +53,16 @@ static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_us(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " user ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "user");
 }
 
 static int hpp__width_overhead_us(struct perf_hpp *hpp __used)
@@ -72,7 +79,9 @@ static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
@@ -96,7 +105,9 @@ static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
 					 struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
@@ -120,12 +131,16 @@ static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
 					struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_samples(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "  Samples  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Samples");
 }
 
 static int hpp__width_samples(struct perf_hpp *hpp __used)
@@ -135,12 +150,16 @@ static int hpp__width_samples(struct perf_hpp *hpp __used)
 
 static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%11" PRIu64, he->nr_events);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%11" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->nr_events);
 }
 
 static int hpp__header_period(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "   Period   ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Period");
 }
 
 static int hpp__width_period(struct perf_hpp *hpp __used)
@@ -150,12 +169,16 @@ static int hpp__width_period(struct perf_hpp *hpp __used)
 
 static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%12" PRIu64, he->period);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->period);
 }
 
 static int hpp__header_delta(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " Delta ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
 }
 
 static int hpp__width_delta(struct perf_hpp *hpp __used)
@@ -169,7 +192,8 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	u64 old_total, new_total;
 	double old_percent = 0, new_percent = 0;
 	double diff;
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
+	char buf[32] = " ";
 
 	old_total = pair_hists->stats.total_period;
 	if (old_total > 0)
@@ -180,11 +204,10 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 		new_percent = 100.0 * he->period / new_total;
 
 	diff = new_percent - old_percent;
-	if (fabs(diff) < 0.01)
-		return scnprintf(hpp->buf, hpp->size, "       ");
+	if (fabs(diff) >= 0.01)
+		scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
 
-	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
-	return scnprintf(hpp->buf, hpp->size, "%7.7s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 static int hpp__header_displ(struct perf_hpp *hpp)
@@ -199,13 +222,13 @@ static int hpp__width_displ(struct perf_hpp *hpp __used)
 
 static int hpp__entry_displ(struct perf_hpp *hpp, struct hist_entry *he __used)
 {
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
+	char buf[32] = " ";
 
-	if (!hpp->displacement)
-		return scnprintf(hpp->buf, hpp->size, "     ");
+	if (hpp->displacement)
+		scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
 
-	scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
-	return scnprintf(hpp->buf, hpp->size, "%6.6s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 #define HPP__COLOR_PRINT_FNS(_name)		\
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 4228b4c6b72d..882461a42830 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -319,11 +319,12 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.displacement	= displacement,
 		.ptr		= pair_hists,
 	};
+	bool color = !symbol_conf.field_sep;
 
 	if (size == 0 || size > sizeof(bf))
 		size = hpp.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(&hpp, he, true);
+	ret = hist_entry__period_snprintf(&hpp, he, color);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);
-- 
1.7.11.4

[tip:perf/core] perf hists: Handle field separator properly

[tip-bot for Namhyung Kim]

Commit-ID:  9ffad987ff565999d91fc2783dd77f08094a743b
Gitweb:     http://git.kernel.org/tip/9ffad987ff565999d91fc2783dd77f08094a743b
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 3 Sep 2012 11:53:07 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 8 Sep 2012 13:19:55 -0300

perf hists: Handle field separator properly

When a field separator is given, the output format doesn't need to be
fancy like aligning to column length, coloring the percent value and so
on.  And since there's a slight difference to normal format, fix it not
to break backward compatibility.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346640790-17197-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/hist.c       |   74 ++++++++++++++++++++++++++++---------------
 tools/perf/ui/stdio/hist.c |    3 +-
 2 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 8ccd1f2..009adf20 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -8,10 +8,9 @@
 /* hist period print (hpp) functions */
 static int hpp__header_overhead(struct perf_hpp *hpp)
 {
-	if (hpp->ptr)
-		return scnprintf(hpp->buf, hpp->size, "Baseline");
-	else
-		return scnprintf(hpp->buf, hpp->size, "Overhead");
+	const char *fmt = hpp->ptr ? "Baseline" : "Overhead";
+
+	return scnprintf(hpp->buf, hpp->size, fmt);
 }
 
 static int hpp__width_overhead(struct perf_hpp *hpp __used)
@@ -40,6 +39,7 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period / hpp->total_period;
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%%";
 
 	if (hpp->ptr) {
 		struct hists *old_hists = hpp->ptr;
@@ -52,12 +52,14 @@ static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 			percent = 0.0;
 	}
 
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_sys(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " sys  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "sys");
 }
 
 static int hpp__width_overhead_sys(struct perf_hpp *hpp __used)
@@ -74,12 +76,16 @@ static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_us(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " user ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "user");
 }
 
 static int hpp__width_overhead_us(struct perf_hpp *hpp __used)
@@ -96,7 +102,9 @@ static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
@@ -120,7 +128,9 @@ static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
 					 struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
@@ -144,12 +154,16 @@ static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
 					struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_samples(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "  Samples  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Samples");
 }
 
 static int hpp__width_samples(struct perf_hpp *hpp __used)
@@ -159,12 +173,16 @@ static int hpp__width_samples(struct perf_hpp *hpp __used)
 
 static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%11" PRIu64, he->nr_events);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%11" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->nr_events);
 }
 
 static int hpp__header_period(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "   Period   ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Period");
 }
 
 static int hpp__width_period(struct perf_hpp *hpp __used)
@@ -174,12 +192,16 @@ static int hpp__width_period(struct perf_hpp *hpp __used)
 
 static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%12" PRIu64, he->period);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->period);
 }
 
 static int hpp__header_delta(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " Delta ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
 }
 
 static int hpp__width_delta(struct perf_hpp *hpp __used)
@@ -193,7 +215,8 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	u64 old_total, new_total;
 	double old_percent = 0, new_percent = 0;
 	double diff;
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
+	char buf[32] = " ";
 
 	old_total = pair_hists->stats.total_period;
 	if (old_total > 0 && he->pair)
@@ -204,11 +227,10 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 		new_percent = 100.0 * he->period / new_total;
 
 	diff = new_percent - old_percent;
-	if (fabs(diff) < 0.01)
-		return scnprintf(hpp->buf, hpp->size, "       ");
+	if (fabs(diff) >= 0.01)
+		scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
 
-	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
-	return scnprintf(hpp->buf, hpp->size, "%7.7s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 static int hpp__header_displ(struct perf_hpp *hpp)
@@ -223,13 +245,13 @@ static int hpp__width_displ(struct perf_hpp *hpp __used)
 
 static int hpp__entry_displ(struct perf_hpp *hpp, struct hist_entry *he __used)
 {
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
+	char buf[32] = " ";
 
-	if (!hpp->displacement)
-		return scnprintf(hpp->buf, hpp->size, "     ");
+	if (hpp->displacement)
+		scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
 
-	scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
-	return scnprintf(hpp->buf, hpp->size, "%6.6s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 #define HPP__COLOR_PRINT_FNS(_name)		\
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 4228b4c..882461a 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -319,11 +319,12 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.displacement	= displacement,
 		.ptr		= pair_hists,
 	};
+	bool color = !symbol_conf.field_sep;
 
 	if (size == 0 || size > sizeof(bf))
 		size = hpp.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(&hpp, he, true);
+	ret = hist_entry__period_snprintf(&hpp, he, color);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);

[PATCH 3/5] perf hists: Use perf_hpp__format->width to calculate the column widths

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c   | 27 +++++++++++++++++++++++++++
 tools/perf/util/hist.c | 33 ---------------------------------
 2 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 802a8659c15a..16dc486d02be 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -337,3 +337,30 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
 
 	return ret;
 }
+
+/*
+ * See hists__fprintf to match the column widths
+ */
+unsigned int hists__sort_list_width(struct hists *hists)
+{
+	struct sort_entry *se;
+	int i, ret = 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+		if (i)
+			ret += 2;
+
+		ret += perf_hpp__format[i].width(NULL);
+	}
+
+	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 */
+		ret += 3 + BITS_PER_LONG / 4;
+
+	return ret;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b1817f15bb87..0ba65ad07cd1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -563,39 +563,6 @@ void hists__output_resort_threaded(struct hists *hists)
 	return __hists__output_resort(hists, true);
 }
 
-/*
- * See hists__fprintf to match the column widths
- */
-unsigned int hists__sort_list_width(struct hists *hists)
-{
-	struct sort_entry *se;
-	int ret = 9; /* total % */
-
-	if (symbol_conf.show_cpu_utilization) {
-		ret += 7; /* count_sys % */
-		ret += 6; /* count_us % */
-		if (perf_guest) {
-			ret += 13; /* count_guest_sys % */
-			ret += 12; /* count_guest_us % */
-		}
-	}
-
-	if (symbol_conf.show_nr_samples)
-		ret += 11;
-
-	if (symbol_conf.show_total_period)
-		ret += 13;
-
-	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 */
-		ret += 3 + BITS_PER_LONG / 4;
-
-	return ret;
-}
-
 static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h,
 				       enum hist_filter filter)
 {
-- 
1.7.11.4

[tip:perf/core] perf hists: Use perf_hpp__format-> width to calculate the column widths

[tip-bot for Namhyung Kim]

Commit-ID:  7e62ef44e89e7b7f2c48090a048f2a5dffa838c7
Gitweb:     http://git.kernel.org/tip/7e62ef44e89e7b7f2c48090a048f2a5dffa838c7
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 3 Sep 2012 11:53:08 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 8 Sep 2012 13:20:05 -0300

perf hists: Use perf_hpp__format->width to calculate the column widths

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346640790-17197-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/hist.c   |   27 +++++++++++++++++++++++++++
 tools/perf/util/hist.c |   33 ---------------------------------
 2 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 009adf20..031b349 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -360,3 +360,30 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
 
 	return ret;
 }
+
+/*
+ * See hists__fprintf to match the column widths
+ */
+unsigned int hists__sort_list_width(struct hists *hists)
+{
+	struct sort_entry *se;
+	int i, ret = 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+		if (i)
+			ret += 2;
+
+		ret += perf_hpp__format[i].width(NULL);
+	}
+
+	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 */
+		ret += 3 + BITS_PER_LONG / 4;
+
+	return ret;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b1817f1..0ba65ad 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -563,39 +563,6 @@ void hists__output_resort_threaded(struct hists *hists)
 	return __hists__output_resort(hists, true);
 }
 
-/*
- * See hists__fprintf to match the column widths
- */
-unsigned int hists__sort_list_width(struct hists *hists)
-{
-	struct sort_entry *se;
-	int ret = 9; /* total % */
-
-	if (symbol_conf.show_cpu_utilization) {
-		ret += 7; /* count_sys % */
-		ret += 6; /* count_us % */
-		if (perf_guest) {
-			ret += 13; /* count_guest_sys % */
-			ret += 12; /* count_guest_us % */
-		}
-	}
-
-	if (symbol_conf.show_nr_samples)
-		ret += 11;
-
-	if (symbol_conf.show_total_period)
-		ret += 13;
-
-	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 */
-		ret += 3 + BITS_PER_LONG / 4;
-
-	return ret;
-}
-
 static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h,
 				       enum hist_filter filter)
 {

[PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Override hpp->color functions for TUI. Because line coloring is done
outside of the function, it just sets the percent value and pass it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |  4 ++
 2 files changed, 76 insertions(+), 22 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 81bd8c2af730..144d7be2872e 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 
+extern void hist_browser__init_hpp(void);
+
 static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);
 
@@ -563,14 +565,47 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 	return row - first_row;
 }
 
+#define HPP__COLOR_FN(_name, _field)					\
+static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp,	\
+					     struct hist_entry *he)	\
+{									\
+	double percent = 100.0 * he->_field / hpp->total_period;	\
+	*(double *)hpp->ptr = percent;					\
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);	\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void hist_browser__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				hist_browser__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				hist_browser__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				hist_browser__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				hist_browser__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				hist_browser__hpp_color_overhead_guest_us;
+}
+
 static int hist_browser__show_entry(struct hist_browser *browser,
 				    struct hist_entry *entry,
 				    unsigned short row)
 {
 	char s[256];
 	double percent;
-	int printed = 0;
-	int width = browser->b.width - 6; /* The percentage */
+	int i, printed = 0;
+	int width = browser->b.width;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;
@@ -586,35 +621,50 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
-		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
+		struct perf_hpp hpp = {
+			.buf		= s,
+			.size		= sizeof(s),
+			.total_period	= browser->hists->stats.total_period,
+		};
 
-		ui_browser__set_percent_color(&browser->b, percent, current_entry);
 		ui_browser__gotorc(&browser->b, row, 0);
-		if (symbol_conf.use_callchain) {
-			slsmg_printf("%c ", folded_sign);
-			width -= 2;
-		}
 
-		slsmg_printf(" %5.2f%%", percent);
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		/* The scroll bar isn't being used */
-		if (!browser->b.navkeypressed)
-			width += 1;
+			if (i) {
+				slsmg_printf("  ");
+				width -= 2;
+			}
 
-		if (!current_entry || !browser->b.navkeypressed)
-			ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			if (perf_hpp__format[i].color) {
+				hpp.ptr = &percent;
+				/* It will set percent for us. See HPP__COLOR_FN above. */
+				width -= perf_hpp__format[i].color(&hpp, entry);
 
-		if (symbol_conf.show_nr_samples) {
-			slsmg_printf(" %11u", entry->nr_events);
-			width -= 12;
-		}
+				ui_browser__set_percent_color(&browser->b, percent, current_entry);
+
+				if (i == 0 && symbol_conf.use_callchain) {
+					slsmg_printf("%c ", folded_sign);
+					width -= 2;
+				}
+
+				slsmg_printf("%s", s);
 
-		if (symbol_conf.show_total_period) {
-			slsmg_printf(" %12" PRIu64, entry->period);
-			width -= 13;
+				if (!current_entry || !browser->b.navkeypressed)
+					ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			} else {
+				width -= perf_hpp__format[i].entry(&hpp, entry);
+				slsmg_printf("%s", s);
+			}
 		}
 
+		/* The scroll bar isn't being used */
+		if (!browser->b.navkeypressed)
+			width += 1;
+
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 4c936e09931c..4dc0887c04f1 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -15,6 +15,8 @@ static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
 
+extern void hist_browser__init_hpp(void);
+
 void ui__refresh_dimensions(bool force)
 {
 	if (force || ui__need_resize) {
@@ -124,6 +126,8 @@ int ui__init(void)
 	signal(SIGTERM, ui__signal);
 
 	perf_error__register(&perf_tui_eops);
+
+	hist_browser__init_hpp();
 out:
 	return err;
 }
-- 
1.7.11.4

Re: [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

[Arnaldo Carvalho de Melo]

Em Mon, Sep 03, 2012 at 11:53:09AM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> Override hpp->color functions for TUI. Because line coloring is done
> outside of the function, it just sets the percent value and pass it.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
>  tools/perf/ui/tui/setup.c      |  4 ++
>  2 files changed, 76 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 81bd8c2af730..144d7be2872e 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -28,6 +28,8 @@ struct hist_browser {
>  	bool		     has_symbols;
>  };
>  
> +extern void hist_browser__init_hpp(void);

I folded the patch below so that we have the lines starting at column 1
like before, i.e. we were using " %5.2f" for the overhead and now you
changed it to "%5.2f".

- Arnaldo

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 144d7be..5a5739b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -605,7 +605,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	char s[256];
 	double percent;
 	int i, printed = 0;
-	int width = browser->b.width;
+	int width = browser->b.width - 1;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;
@@ -627,7 +627,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			.total_period	= browser->hists->stats.total_period,
 		};
 
-		ui_browser__gotorc(&browser->b, row, 0);
+		ui_browser__gotorc(&browser->b, row, 1);
 
 		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
 			if (!perf_hpp__format[i].cond)

Re: [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

[Namhyung Kim]

On Fri, 7 Sep 2012 17:32:18 -0700, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 03, 2012 at 11:53:09AM +0900, Namhyung Kim escreveu:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>> 
>> Override hpp->color functions for TUI. Because line coloring is done
>> outside of the function, it just sets the percent value and pass it.
>> 
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> ---
>>  tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
>>  tools/perf/ui/tui/setup.c      |  4 ++
>>  2 files changed, 76 insertions(+), 22 deletions(-)
>> 
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index 81bd8c2af730..144d7be2872e 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -28,6 +28,8 @@ struct hist_browser {
>>  	bool		     has_symbols;
>>  };
>>  
>> +extern void hist_browser__init_hpp(void);
>
> I folded the patch below so that we have the lines starting at column 1
> like before, i.e. we were using " %5.2f" for the overhead and now you
> changed it to "%5.2f".

Looks good to me.

Thanks,
Namhyung

>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 144d7be..5a5739b 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -605,7 +605,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
>  	char s[256];
>  	double percent;
>  	int i, printed = 0;
> -	int width = browser->b.width;
> +	int width = browser->b.width - 1;
>  	char folded_sign = ' ';
>  	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
>  	off_t row_offset = entry->row_offset;
> @@ -627,7 +627,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
>  			.total_period	= browser->hists->stats.total_period,
>  		};
>  
> -		ui_browser__gotorc(&browser->b, row, 0);
> +		ui_browser__gotorc(&browser->b, row, 1);
>  
>  		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
>  			if (!perf_hpp__format[i].cond)

Re: [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

[Namhyung Kim]

On Sat, 08 Sep 2012 23:05:43 +0900, Namhyung Kim wrote:
> On Fri, 7 Sep 2012 17:32:18 -0700, Arnaldo Carvalho de Melo wrote:
>> I folded the patch below so that we have the lines starting at column 1
>> like before, i.e. we were using " %5.2f" for the overhead and now you
>> changed it to "%5.2f".
>
> Looks good to me.

It wasn't good actually :-/  Simply because it cannot refresh the
character at column 0 anymore.  I will send a fix soon.

Thanks,
Namhyung

[tip:perf/core] perf hists browser: Use perf_hpp__format functions

[tip-bot for Namhyung Kim]

Commit-ID:  f5951d56a2ab4181b96806a4746895aaa5a3d72e
Gitweb:     http://git.kernel.org/tip/f5951d56a2ab4181b96806a4746895aaa5a3d72e
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 3 Sep 2012 11:53:09 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 8 Sep 2012 13:20:13 -0300

perf hists browser: Use perf_hpp__format functions

Override hpp->color functions for TUI. Because line coloring is done
outside of the function, it just sets the percent value and pass it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346640790-17197-5-git-send-email-namhyung@kernel.org
[ committer note: Keep previous layout by showing the overhead at column 1 ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c |   96 ++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |    4 ++
 2 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 81bd8c2..5a5739b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 
+extern void hist_browser__init_hpp(void);
+
 static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);
 
@@ -563,14 +565,47 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 	return row - first_row;
 }
 
+#define HPP__COLOR_FN(_name, _field)					\
+static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp,	\
+					     struct hist_entry *he)	\
+{									\
+	double percent = 100.0 * he->_field / hpp->total_period;	\
+	*(double *)hpp->ptr = percent;					\
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);	\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void hist_browser__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				hist_browser__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				hist_browser__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				hist_browser__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				hist_browser__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				hist_browser__hpp_color_overhead_guest_us;
+}
+
 static int hist_browser__show_entry(struct hist_browser *browser,
 				    struct hist_entry *entry,
 				    unsigned short row)
 {
 	char s[256];
 	double percent;
-	int printed = 0;
-	int width = browser->b.width - 6; /* The percentage */
+	int i, printed = 0;
+	int width = browser->b.width - 1;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;
@@ -586,35 +621,50 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
-		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
+		struct perf_hpp hpp = {
+			.buf		= s,
+			.size		= sizeof(s),
+			.total_period	= browser->hists->stats.total_period,
+		};
 
-		ui_browser__set_percent_color(&browser->b, percent, current_entry);
-		ui_browser__gotorc(&browser->b, row, 0);
-		if (symbol_conf.use_callchain) {
-			slsmg_printf("%c ", folded_sign);
-			width -= 2;
-		}
+		ui_browser__gotorc(&browser->b, row, 1);
 
-		slsmg_printf(" %5.2f%%", percent);
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		/* The scroll bar isn't being used */
-		if (!browser->b.navkeypressed)
-			width += 1;
+			if (i) {
+				slsmg_printf("  ");
+				width -= 2;
+			}
 
-		if (!current_entry || !browser->b.navkeypressed)
-			ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			if (perf_hpp__format[i].color) {
+				hpp.ptr = &percent;
+				/* It will set percent for us. See HPP__COLOR_FN above. */
+				width -= perf_hpp__format[i].color(&hpp, entry);
 
-		if (symbol_conf.show_nr_samples) {
-			slsmg_printf(" %11u", entry->nr_events);
-			width -= 12;
-		}
+				ui_browser__set_percent_color(&browser->b, percent, current_entry);
+
+				if (i == 0 && symbol_conf.use_callchain) {
+					slsmg_printf("%c ", folded_sign);
+					width -= 2;
+				}
+
+				slsmg_printf("%s", s);
 
-		if (symbol_conf.show_total_period) {
-			slsmg_printf(" %12" PRIu64, entry->period);
-			width -= 13;
+				if (!current_entry || !browser->b.navkeypressed)
+					ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			} else {
+				width -= perf_hpp__format[i].entry(&hpp, entry);
+				slsmg_printf("%s", s);
+			}
 		}
 
+		/* The scroll bar isn't being used */
+		if (!browser->b.navkeypressed)
+			width += 1;
+
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 4c936e0..4dc0887 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -15,6 +15,8 @@ static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
 
+extern void hist_browser__init_hpp(void);
+
 void ui__refresh_dimensions(bool force)
 {
 	if (force || ui__need_resize) {
@@ -124,6 +126,8 @@ int ui__init(void)
 	signal(SIGTERM, ui__signal);
 
 	perf_error__register(&perf_tui_eops);
+
+	hist_browser__init_hpp();
 out:
 	return err;
 }

[PATCH 5/5] perf gtk/browser: Use perf_hpp__format functions

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Now we can support color using pango markup with this change.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/gtk/browser.c | 101 +++++++++++++++++++++++++++++++++++++-------
 tools/perf/ui/gtk/gtk.h     |   1 +
 tools/perf/ui/gtk/setup.c   |   1 +
 3 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 26b5b652a8cd..3c16ab50e0f8 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -36,6 +36,57 @@ static void perf_gtk__resize_window(GtkWidget *window)
 	gtk_window_resize(GTK_WINDOW(window), width, height);
 }
 
+static const char *perf_gtk__get_percent_color(double percent)
+{
+	if (percent >= MIN_RED)
+		return "<span fgcolor='red'>";
+	if (percent >= MIN_GREEN)
+		return "<span fgcolor='dark green'>";
+	return NULL;
+}
+
+#define HPP__COLOR_FN(_name, _field)						\
+static int perf_gtk__hpp_color_ ## _name(struct perf_hpp *hpp,			\
+					 struct hist_entry *he)			\
+{										\
+	double percent = 100.0 * he->_field / hpp->total_period;		\
+	const char *markup;							\
+	int ret = 0;								\
+										\
+	markup = perf_gtk__get_percent_color(percent);				\
+	if (markup)								\
+		ret += scnprintf(hpp->buf, hpp->size, "%s", markup);		\
+	ret += scnprintf(hpp->buf + ret, hpp->size - ret, "%5.2f%%", percent); 	\
+	if (markup)								\
+		ret += scnprintf(hpp->buf + ret, hpp->size - ret, "</span>"); 	\
+										\
+	return ret;								\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void perf_gtk__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				perf_gtk__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				perf_gtk__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				perf_gtk__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				perf_gtk__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				perf_gtk__hpp_color_overhead_guest_us;
+}
+
 static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 {
 	GType col_types[MAX_COLUMNS];
@@ -43,15 +94,25 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	struct sort_entry *se;
 	GtkListStore *store;
 	struct rb_node *nd;
-	u64 total_period;
 	GtkWidget *view;
-	int col_idx;
+	int i, col_idx;
 	int nr_cols;
+	char s[512];
+
+	struct perf_hpp hpp = {
+		.buf		= s,
+		.size		= sizeof(s),
+		.total_period	= hists->stats.total_period,
+	};
 
 	nr_cols = 0;
 
-	/* The percentage column */
-	col_types[nr_cols++] = G_TYPE_STRING;
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		col_types[nr_cols++] = G_TYPE_STRING;
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)
@@ -68,11 +129,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	col_idx = 0;
 
-	/* The percentage column */
-	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-						    -1, "Overhead (%)",
-						    renderer, "text",
-						    col_idx++, NULL);
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		perf_hpp__format[i].header(&hpp);
+
+		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+							    -1, s,
+							    renderer, "markup",
+							    col_idx++, NULL);
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)
@@ -88,13 +155,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	g_object_unref(GTK_TREE_MODEL(store));
 
-	total_period = hists->stats.total_period;
-
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 		GtkTreeIter iter;
-		double percent;
-		char s[512];
 
 		if (h->filtered)
 			continue;
@@ -103,11 +166,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 		col_idx = 0;
 
-		percent = (h->period * 100.0) / total_period;
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+			if (perf_hpp__format[i].color)
+				perf_hpp__format[i].color(&hpp, h);
+			else
+				perf_hpp__format[i].entry(&hpp, h);
 
-		gtk_list_store_set(store, &iter, col_idx++, s, -1);
+			gtk_list_store_set(store, &iter, col_idx++, s, -1);
+		}
 
 		list_for_each_entry(se, &hist_entry__sort_list, list) {
 			if (se->elide)
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 793cb6116ddf..687af0bba187 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -30,6 +30,7 @@ struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
 int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
 
 void perf_gtk__init_helpline(void);
+void perf_gtk__init_hpp(void);
 
 #ifndef HAVE_GTK_INFO_BAR
 static inline GtkWidget *perf_gtk__setup_info_bar(void)
diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index ec1ee26b485a..26429437e19e 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -8,6 +8,7 @@ int perf_gtk__init(void)
 {
 	perf_error__register(&perf_gtk_eops);
 	perf_gtk__init_helpline();
+	perf_gtk__init_hpp();
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 
-- 
1.7.11.4

[tip:perf/core] perf gtk/browser: Use perf_hpp__format functions

[tip-bot for Namhyung Kim]

Commit-ID:  12ceaded6b276c13d905cf09f0c1a1322022ea58
Gitweb:     http://git.kernel.org/tip/12ceaded6b276c13d905cf09f0c1a1322022ea58
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 3 Sep 2012 11:53:10 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 8 Sep 2012 13:20:21 -0300

perf gtk/browser: Use perf_hpp__format functions

Now we can support color using pango markup with this change.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346640790-17197-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/browser.c |  101 ++++++++++++++++++++++++++++++++++++-------
 tools/perf/ui/gtk/gtk.h     |    1 +
 tools/perf/ui/gtk/setup.c   |    1 +
 3 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 26b5b65..3c16ab5 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -36,6 +36,57 @@ static void perf_gtk__resize_window(GtkWidget *window)
 	gtk_window_resize(GTK_WINDOW(window), width, height);
 }
 
+static const char *perf_gtk__get_percent_color(double percent)
+{
+	if (percent >= MIN_RED)
+		return "<span fgcolor='red'>";
+	if (percent >= MIN_GREEN)
+		return "<span fgcolor='dark green'>";
+	return NULL;
+}
+
+#define HPP__COLOR_FN(_name, _field)						\
+static int perf_gtk__hpp_color_ ## _name(struct perf_hpp *hpp,			\
+					 struct hist_entry *he)			\
+{										\
+	double percent = 100.0 * he->_field / hpp->total_period;		\
+	const char *markup;							\
+	int ret = 0;								\
+										\
+	markup = perf_gtk__get_percent_color(percent);				\
+	if (markup)								\
+		ret += scnprintf(hpp->buf, hpp->size, "%s", markup);		\
+	ret += scnprintf(hpp->buf + ret, hpp->size - ret, "%5.2f%%", percent); 	\
+	if (markup)								\
+		ret += scnprintf(hpp->buf + ret, hpp->size - ret, "</span>"); 	\
+										\
+	return ret;								\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void perf_gtk__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				perf_gtk__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				perf_gtk__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				perf_gtk__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				perf_gtk__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				perf_gtk__hpp_color_overhead_guest_us;
+}
+
 static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 {
 	GType col_types[MAX_COLUMNS];
@@ -43,15 +94,25 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	struct sort_entry *se;
 	GtkListStore *store;
 	struct rb_node *nd;
-	u64 total_period;
 	GtkWidget *view;
-	int col_idx;
+	int i, col_idx;
 	int nr_cols;
+	char s[512];
+
+	struct perf_hpp hpp = {
+		.buf		= s,
+		.size		= sizeof(s),
+		.total_period	= hists->stats.total_period,
+	};
 
 	nr_cols = 0;
 
-	/* The percentage column */
-	col_types[nr_cols++] = G_TYPE_STRING;
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		col_types[nr_cols++] = G_TYPE_STRING;
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)
@@ -68,11 +129,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	col_idx = 0;
 
-	/* The percentage column */
-	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-						    -1, "Overhead (%)",
-						    renderer, "text",
-						    col_idx++, NULL);
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		perf_hpp__format[i].header(&hpp);
+
+		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+							    -1, s,
+							    renderer, "markup",
+							    col_idx++, NULL);
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)
@@ -88,13 +155,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	g_object_unref(GTK_TREE_MODEL(store));
 
-	total_period = hists->stats.total_period;
-
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 		GtkTreeIter iter;
-		double percent;
-		char s[512];
 
 		if (h->filtered)
 			continue;
@@ -103,11 +166,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 		col_idx = 0;
 
-		percent = (h->period * 100.0) / total_period;
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+			if (perf_hpp__format[i].color)
+				perf_hpp__format[i].color(&hpp, h);
+			else
+				perf_hpp__format[i].entry(&hpp, h);
 
-		gtk_list_store_set(store, &iter, col_idx++, s, -1);
+			gtk_list_store_set(store, &iter, col_idx++, s, -1);
+		}
 
 		list_for_each_entry(se, &hist_entry__sort_list, list) {
 			if (se->elide)
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 793cb61..687af0b 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -30,6 +30,7 @@ struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
 int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
 
 void perf_gtk__init_helpline(void);
+void perf_gtk__init_hpp(void);
 
 #ifndef HAVE_GTK_INFO_BAR
 static inline GtkWidget *perf_gtk__setup_info_bar(void)
diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index ec1ee26..2642943 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -8,6 +8,7 @@ int perf_gtk__init(void)
 {
 	perf_error__register(&perf_gtk_eops);
 	perf_gtk__init_helpline();
+	perf_gtk__init_hpp();
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 

Re: [PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4)

[Arnaldo Carvalho de Melo]

Em Mon, Sep 03, 2012 at 11:53:05AM +0900, Namhyung Kim escreveu:
> This is a cleanup and refactoring patchset for the hist printing code
> by adding perf_hpp__format functions and perf_hpp.  I believe it makes
> the code easy to maintain and to add new features like upcoming group
> viewing and callchain accumulation.

I applied this patch series to then get some patches from Jiri's 'perf
diff' series, so that he can use what you did here, as you noticed the
overlap when reviewing his series.

But then 'perf diff' segfaults :-\

I left your patch series + with that overhead column fixlet at my tree,
branch tmp.perf/hpp.

The segfaults happens here:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000498837 in hpp__entry_delta (hpp=0x7fffffffdeb0, he=0xcd6310) at ui/hist.c:200
200			old_percent = 100.0 * he->pair->period / old_total;
Missing separate debuginfos, use: debuginfo-install atk-1.28.0-2.el6.x86_64 bzip2-libs-1.0.5-7.el6_0.x86_64 elfutils-libelf-0.152-1.el6.x86_64 elfutils-libs-0.152-1.el6.x86_64 expat-2.0.1-11.el6_2.x86_64 fontconfig-2.8.0-3.el6.x86_64 freetype-2.3.11-6.el6_2.9.x86_64 glib2-2.22.5-7.el6.x86_64 gtk2-2.18.9-10.el6.x86_64 libX11-1.3-2.el6.x86_64 libXau-1.0.5-1.el6.x86_64 libXcomposite-0.4.1-2.el6.x86_64 libXcursor-1.1.10-2.el6.x86_64 libXdamage-1.1.2-1.el6.x86_64 libXext-1.1-3.el6.x86_64 libXfixes-4.0.4-1.el6.x86_64 libXi-1.3-3.el6.x86_64 libXinerama-1.1-1.el6.x86_64 libXrandr-1.3.0-4.el6.x86_64 libXrender-0.9.5-1.el6.x86_64 libpng-1.2.49-1.el6_2.x86_64 libselinux-2.0.94-5.3.el6.x86_64 libxcb-1.5-1.el6.x86_64 nss-softokn-freebl-3.12.9-11.el6.x86_64 pango-1.28.1-3.el6_0.5.x86_64 perl-libs-5.10.1-127.el6.x86_64 pixman-0.18.4-1.el6_0.1.x86_64 xz-libs-4.999.9-0.3.beta.20091007git.el6.x86_64 zlib-1.2.3-27.el6.x86_64
(gdb) p he->pair
$1 = (struct hist_entry *) 0x0

Please try with:

  perf record -a usleep 1
  perf record -a usleep 1
  perf diff

it will use perf.data.old and perf.data and will segfault in that branch.

- Arnaldo

Re: [PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4)

[Namhyung Kim]

Hi, Arnaldo

On Fri, 7 Sep 2012 17:48:54 -0700, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 03, 2012 at 11:53:05AM +0900, Namhyung Kim escreveu:
>> This is a cleanup and refactoring patchset for the hist printing code
>> by adding perf_hpp__format functions and perf_hpp.  I believe it makes
>> the code easy to maintain and to add new features like upcoming group
>> viewing and callchain accumulation.
>
> I applied this patch series to then get some patches from Jiri's 'perf
> diff' series, so that he can use what you did here, as you noticed the
> overlap when reviewing his series.
>
> But then 'perf diff' segfaults :-\
>
> I left your patch series + with that overhead column fixlet at my tree,
> branch tmp.perf/hpp.
>
> The segfaults happens here:
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000498837 in hpp__entry_delta (hpp=0x7fffffffdeb0, he=0xcd6310) at ui/hist.c:200
> 200			old_percent = 100.0 * he->pair->period / old_total;
> Missing separate debuginfos, use: debuginfo-install atk-1.28.0-2.el6.x86_64 bzip2-libs-1.0.5-7.el6_0.x86_64 elfutils-libelf-0.152-1.el6.x86_64 elfutils-libs-0.152-1.el6.x86_64 expat-2.0.1-11.el6_2.x86_64 fontconfig-2.8.0-3.el6.x86_64 freetype-2.3.11-6.el6_2.9.x86_64 glib2-2.22.5-7.el6.x86_64 gtk2-2.18.9-10.el6.x86_64 libX11-1.3-2.el6.x86_64 libXau-1.0.5-1.el6.x86_64 libXcomposite-0.4.1-2.el6.x86_64 libXcursor-1.1.10-2.el6.x86_64 libXdamage-1.1.2-1.el6.x86_64 libXext-1.1-3.el6.x86_64 libXfixes-4.0.4-1.el6.x86_64 libXi-1.3-3.el6.x86_64 libXinerama-1.1-1.el6.x86_64 libXrandr-1.3.0-4.el6.x86_64 libXrender-0.9.5-1.el6.x86_64 libpng-1.2.49-1.el6_2.x86_64 libselinux-2.0.94-5.3.el6.x86_64 libxcb-1.5-1.el6.x86_64 nss-softokn-freebl-3.12.9-11.el6.x86_64 pango-1.28.1-3.el6_0.5.x86_64 perl-libs-5.10.1
>  -127.el6.x86_64 pixman-0.18.4-1.el6_0.1.x86_64 xz-libs-4.999.9-0.3.beta.20091007git.el6.x86_64 zlib-1.2.3-27.el6.x86_64
> (gdb) p he->pair
> $1 = (struct hist_entry *) 0x0
>
> Please try with:
>
>   perf record -a usleep 1
>   perf record -a usleep 1
>   perf diff
>
> it will use perf.data.old and perf.data and will segfault in that branch.

I see the problem.  I overlooked he->pair can be NULL when perf diff is
running.  So the fix will be adding a NULL check before the line.  In
case of NULL, it will default to 0, so no problem.

Thanks,
Namhyung


diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 16dc486d02be..3c2701fa6be1 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -196,7 +196,7 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	char buf[32] = " ";
 
 	old_total = pair_hists->stats.total_period;
-	if (old_total > 0)
+	if (old_total > 0 && he->pair)
 		old_percent = 100.0 * he->pair->period / old_total;
 
 	new_total = hpp->total_period;

Re: [PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4)

[Arnaldo Carvalho de Melo]

Em Sat, Sep 08, 2012 at 11:00:36PM +0900, Namhyung Kim escreveu:
> On Fri, 7 Sep 2012 17:48:54 -0700, Arnaldo Carvalho de Melo wrote:
> > Please try with:

> >   perf record -a usleep 1
> >   perf record -a usleep 1
> >   perf diff

> > it will use perf.data.old and perf.data and will segfault in that branch.

> I see the problem.  I overlooked he->pair can be NULL when perf diff is
> running.  So the fix will be adding a NULL check before the line.  In
> case of NULL, it will default to 0, so no problem.

Ok, now it doesn't segfaults, but the diff results seems bogus:

Withour this patch + your fix:

[root@sandy ~]# perf diff
# Event 'cycles'
#
# Baseline  Delta          Shared Object                                  Symbol
# ........ ..........  .................  ......................................
#
     0.00%    +39.61%  [kernel.kallsyms]  [k] __alloc_pages_nodemask            
    19.82%     +6.10%  [kernel.kallsyms]  [k] intel_idle                        
     0.00%     +8.61%  [kernel.kallsyms]  [k] generic_exec_single               
     0.00%     +6.39%  [kernel.kallsyms]  [k] list_del                          
     0.00%     +6.15%  [kernel.kallsyms]  [k] nr_iowait_cpu                     
     0.00%     +5.98%  [kernel.kallsyms]  [k] selinux_inode_permission          
     0.00%     +5.23%  libc-2.12.so       [.] _dl_addr                          
     0.00%     +0.76%  [kernel.kallsyms]  [k] group_sched_in                    
     0.10%     +0.32%  [kernel.kallsyms]  [k] native_write_msr_safe             
     0.00%     +0.40%  [kernel.kallsyms]  [k] smp_call_function_single_interrupt
     0.14%     +0.19%  [kernel.kallsyms]  [k] csd_unlock                        
     0.00%     +0.20%  [kernel.kallsyms]  [k] perf_pmu_enable                   

With your patch:

[root@sandy ~]# perf diff
# Event 'cycles'
#
# Baseline   Delta       Shared Object                                  Symbol
# ........  .......  .................  ......................................
#
    39.61%  +39.61%  [kernel.kallsyms]  [k] __alloc_pages_nodemask            
    25.92%   +6.10%  [kernel.kallsyms]  [k] intel_idle                        
     8.61%   +8.61%  [kernel.kallsyms]  [k] generic_exec_single               
     6.39%   +6.39%  [kernel.kallsyms]  [k] list_del                          
     6.15%   +6.15%  [kernel.kallsyms]  [k] nr_iowait_cpu                     
     5.98%   +5.98%  [kernel.kallsyms]  [k] selinux_inode_permission          
     5.23%   +5.23%  libc-2.12.so       [.] _dl_addr                          
     0.76%   +0.76%  [kernel.kallsyms]  [k] group_sched_in                    
     0.42%   +0.32%  [kernel.kallsyms]  [k] native_write_msr_safe             
     0.40%   +0.40%  [kernel.kallsyms]  [k] smp_call_function_single_interrupt
     0.33%   +0.19%  [kernel.kallsyms]  [k] csd_unlock                        
     0.20%   +0.20%  [kernel.kallsyms]  [k] perf_pmu_enable                   
[root@sandy ~]#

The deltas are the same, but the baseline completely changed and are bogus.

So please try again and make sure that the results are kept.

- Arnaldo

[PATCH 0/5] perf tools: Cleanup hist printing code (v4)

[Namhyung Kim]

Hi,

This is a cleanup and refactoring patchset for the hist printing code
by adding perf_hpp__format functions and perf_hpp.  I believe it makes
the code easy to maintain and to add new features like upcoming group
viewing and callchain accumulation.

Any comments are welcome, thanks.
Namhyung

v3 -> v4:
 * Rebase to current acme/perf/core
 * The first two in the previous series have been merged
 * Rename hist_period_print to perf_hpp_fmt (Arnaldo)
 * Rename ctx->s to hpp->buf (Arnaldo)

v2 -> v3:
 * Move fprintf code to ui/stdio/hist.c (Arnaldo)
 * Add ack from Pekka


Namhyung Kim (5):
  perf hists: Introduce perf_hpp for hist period printing
  perf hists: Handle field separator properly
  perf hists: Use perf_hpp__format->width to calculate the column widths
  perf ui/browser: Use perf_hpp__format functions
  perf gtk/browser: Use perf_hpp__format functions

 tools/perf/Makefile            |   2 +
 tools/perf/builtin-diff.c      |   1 +
 tools/perf/ui/browsers/hists.c |  94 ++++++++---
 tools/perf/ui/gtk/browser.c    | 101 ++++++++++--
 tools/perf/ui/gtk/gtk.h        |   1 +
 tools/perf/ui/gtk/setup.c      |   1 +
 tools/perf/ui/hist.c           | 366 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c          |   8 +-
 tools/perf/ui/stdio/hist.c     | 239 +++++----------------------
 tools/perf/ui/tui/setup.c      |   4 +
 tools/perf/util/hist.c         |  33 ----
 tools/perf/util/hist.h         |  37 +++++
 12 files changed, 616 insertions(+), 271 deletions(-)
 create mode 100644 tools/perf/ui/hist.c

-- 
1.7.11.4

[PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Override hpp->color functions for TUI. Because line coloring is done
outside of the function, it just sets the percent value and pass it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |  4 ++
 2 files changed, 76 insertions(+), 22 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 81bd8c2af730..144d7be2872e 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 
+extern void hist_browser__init_hpp(void);
+
 static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);
 
@@ -563,14 +565,47 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 	return row - first_row;
 }
 
+#define HPP__COLOR_FN(_name, _field)					\
+static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp,	\
+					     struct hist_entry *he)	\
+{									\
+	double percent = 100.0 * he->_field / hpp->total_period;	\
+	*(double *)hpp->ptr = percent;					\
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);	\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void hist_browser__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				hist_browser__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				hist_browser__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				hist_browser__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				hist_browser__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				hist_browser__hpp_color_overhead_guest_us;
+}
+
 static int hist_browser__show_entry(struct hist_browser *browser,
 				    struct hist_entry *entry,
 				    unsigned short row)
 {
 	char s[256];
 	double percent;
-	int printed = 0;
-	int width = browser->b.width - 6; /* The percentage */
+	int i, printed = 0;
+	int width = browser->b.width;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;
@@ -586,35 +621,50 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
-		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
+		struct perf_hpp hpp = {
+			.buf		= s,
+			.size		= sizeof(s),
+			.total_period	= browser->hists->stats.total_period,
+		};
 
-		ui_browser__set_percent_color(&browser->b, percent, current_entry);
 		ui_browser__gotorc(&browser->b, row, 0);
-		if (symbol_conf.use_callchain) {
-			slsmg_printf("%c ", folded_sign);
-			width -= 2;
-		}
 
-		slsmg_printf(" %5.2f%%", percent);
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		/* The scroll bar isn't being used */
-		if (!browser->b.navkeypressed)
-			width += 1;
+			if (i) {
+				slsmg_printf("  ");
+				width -= 2;
+			}
 
-		if (!current_entry || !browser->b.navkeypressed)
-			ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			if (perf_hpp__format[i].color) {
+				hpp.ptr = &percent;
+				/* It will set percent for us. See HPP__COLOR_FN above. */
+				width -= perf_hpp__format[i].color(&hpp, entry);
 
-		if (symbol_conf.show_nr_samples) {
-			slsmg_printf(" %11u", entry->nr_events);
-			width -= 12;
-		}
+				ui_browser__set_percent_color(&browser->b, percent, current_entry);
+
+				if (i == 0 && symbol_conf.use_callchain) {
+					slsmg_printf("%c ", folded_sign);
+					width -= 2;
+				}
+
+				slsmg_printf("%s", s);
 
-		if (symbol_conf.show_total_period) {
-			slsmg_printf(" %12" PRIu64, entry->period);
-			width -= 13;
+				if (!current_entry || !browser->b.navkeypressed)
+					ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			} else {
+				width -= perf_hpp__format[i].entry(&hpp, entry);
+				slsmg_printf("%s", s);
+			}
 		}
 
+		/* The scroll bar isn't being used */
+		if (!browser->b.navkeypressed)
+			width += 1;
+
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 4c936e09931c..4dc0887c04f1 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -15,6 +15,8 @@ static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
 
+extern void hist_browser__init_hpp(void);
+
 void ui__refresh_dimensions(bool force)
 {
 	if (force || ui__need_resize) {
@@ -124,6 +126,8 @@ int ui__init(void)
 	signal(SIGTERM, ui__signal);
 
 	perf_error__register(&perf_tui_eops);
+
+	hist_browser__init_hpp();
 out:
 	return err;
 }
-- 
1.7.11.4