linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHSET 0/7] Cleanup hist printing code (v3)
@ 2012-08-20  4:52 Namhyung Kim
  2012-08-20  4:52 ` [PATCH 1/7] perf hists: Separate out hist print functions Namhyung Kim
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML

Hello,

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

Any comments are welcome, thanks.
Namhyung

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


Namhyung Kim (7):
  perf hists: Separate out hist print functions
  perf hists: Refactor some functions
  perf hists: Introduce hist_period_print functions
  perf hists: Handle field separator properly
  perf hists: Use hpp_functions->width to calculate the column widths
  perf ui/browser: Use hist_period_print functions
  perf gtk/browser: Use hist_period_print functions

 tools/perf/Makefile            |   5 +-
 tools/perf/builtin-diff.c      |   1 +
 tools/perf/ui/browsers/hists.c |  96 ++++--
 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           | 391 +++++++++++++++++++++++
 tools/perf/ui/setup.c          |   8 +-
 tools/perf/ui/stdio/hist.c     | 498 +++++++++++++++++++++++++++++
 tools/perf/ui/tui/setup.c      |   4 +
 tools/perf/util/hist.c         | 710 ++---------------------------------------
 tools/perf/util/hist.h         |  43 ++-
 12 files changed, 1121 insertions(+), 738 deletions(-)
 create mode 100644 tools/perf/ui/hist.c
 create mode 100644 tools/perf/ui/stdio/hist.c

-- 
1.7.11.2


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

* [PATCH 1/7] perf hists: Separate out hist print functions
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  2012-08-21 16:34   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-08-20  4:52 ` [PATCH 2/7] perf hists: Refactor some functions Namhyung Kim
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim

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

Separate out those functions into ui/stdio/hist.c. This is required
for upcoming changes.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Makefile        |   3 +-
 tools/perf/ui/stdio/hist.c | 648 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.c     | 677 ++-------------------------------------------
 tools/perf/util/hist.h     |   2 +
 4 files changed, 669 insertions(+), 661 deletions(-)
 create mode 100644 tools/perf/ui/stdio/hist.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 75af93dc52ad..cb18b047c9c9 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -404,11 +404,10 @@ 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/stdio/hist.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
-
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
-
 # Benchmark modules
 BUILTIN_OBJS += $(OUTPUT)bench/sched-messaging.o
 BUILTIN_OBJS += $(OUTPUT)bench/sched-pipe.o
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
new file mode 100644
index 000000000000..7881d625e17a
--- /dev/null
+++ b/tools/perf/ui/stdio/hist.c
@@ -0,0 +1,648 @@
+#include <stdio.h>
+#include <math.h>
+
+#include "../../util/util.h"
+#include "../../util/hist.h"
+#include "../../util/sort.h"
+
+
+static size_t callchain__fprintf_left_margin(FILE *fp, int left_margin)
+{
+	int i;
+	int ret = fprintf(fp, "            ");
+
+	for (i = 0; i < left_margin; i++)
+		ret += fprintf(fp, " ");
+
+	return ret;
+}
+
+static size_t ipchain__fprintf_graph_line(FILE *fp, int depth, int depth_mask,
+					  int left_margin)
+{
+	int i;
+	size_t ret = callchain__fprintf_left_margin(fp, left_margin);
+
+	for (i = 0; i < depth; i++)
+		if (depth_mask & (1 << i))
+			ret += fprintf(fp, "|          ");
+		else
+			ret += fprintf(fp, "           ");
+
+	ret += fprintf(fp, "\n");
+
+	return ret;
+}
+
+static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
+				     int depth, int depth_mask, int period,
+				     u64 total_samples, u64 hits,
+				     int left_margin)
+{
+	int i;
+	size_t ret = 0;
+
+	ret += callchain__fprintf_left_margin(fp, left_margin);
+	for (i = 0; i < depth; i++) {
+		if (depth_mask & (1 << i))
+			ret += fprintf(fp, "|");
+		else
+			ret += fprintf(fp, " ");
+		if (!period && i == depth - 1) {
+			double percent;
+
+			percent = hits * 100.0 / total_samples;
+			ret += percent_color_fprintf(fp, "--%2.2f%%-- ", percent);
+		} else
+			ret += fprintf(fp, "%s", "          ");
+	}
+	if (chain->ms.sym)
+		ret += fprintf(fp, "%s\n", chain->ms.sym->name);
+	else
+		ret += fprintf(fp, "0x%0" PRIx64 "\n", chain->ip);
+
+	return ret;
+}
+
+static struct symbol *rem_sq_bracket;
+static struct callchain_list rem_hits;
+
+static void init_rem_hits(void)
+{
+	rem_sq_bracket = malloc(sizeof(*rem_sq_bracket) + 6);
+	if (!rem_sq_bracket) {
+		fprintf(stderr, "Not enough memory to display remaining hits\n");
+		return;
+	}
+
+	strcpy(rem_sq_bracket->name, "[...]");
+	rem_hits.ms.sym = rem_sq_bracket;
+}
+
+static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
+					 u64 total_samples, int depth,
+					 int depth_mask, int left_margin)
+{
+	struct rb_node *node, *next;
+	struct callchain_node *child;
+	struct callchain_list *chain;
+	int new_depth_mask = depth_mask;
+	u64 remaining;
+	size_t ret = 0;
+	int i;
+	uint entries_printed = 0;
+
+	remaining = total_samples;
+
+	node = rb_first(root);
+	while (node) {
+		u64 new_total;
+		u64 cumul;
+
+		child = rb_entry(node, struct callchain_node, rb_node);
+		cumul = callchain_cumul_hits(child);
+		remaining -= cumul;
+
+		/*
+		 * The depth mask manages the output of pipes that show
+		 * the depth. We don't want to keep the pipes of the current
+		 * level for the last child of this depth.
+		 * Except if we have remaining filtered hits. They will
+		 * supersede the last child
+		 */
+		next = rb_next(node);
+		if (!next && (callchain_param.mode != CHAIN_GRAPH_REL || !remaining))
+			new_depth_mask &= ~(1 << (depth - 1));
+
+		/*
+		 * But we keep the older depth mask for the line separator
+		 * to keep the level link until we reach the last child
+		 */
+		ret += ipchain__fprintf_graph_line(fp, depth, depth_mask,
+						   left_margin);
+		i = 0;
+		list_for_each_entry(chain, &child->val, list) {
+			ret += ipchain__fprintf_graph(fp, chain, depth,
+						      new_depth_mask, i++,
+						      total_samples,
+						      cumul,
+						      left_margin);
+		}
+
+		if (callchain_param.mode == CHAIN_GRAPH_REL)
+			new_total = child->children_hit;
+		else
+			new_total = total_samples;
+
+		ret += __callchain__fprintf_graph(fp, &child->rb_root, new_total,
+						  depth + 1,
+						  new_depth_mask | (1 << depth),
+						  left_margin);
+		node = next;
+		if (++entries_printed == callchain_param.print_limit)
+			break;
+	}
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL &&
+		remaining && remaining != total_samples) {
+
+		if (!rem_sq_bracket)
+			return ret;
+
+		new_depth_mask &= ~(1 << (depth - 1));
+		ret += ipchain__fprintf_graph(fp, &rem_hits, depth,
+					      new_depth_mask, 0, total_samples,
+					      remaining, left_margin);
+	}
+
+	return ret;
+}
+
+static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
+				       u64 total_samples, int left_margin)
+{
+	struct callchain_node *cnode;
+	struct callchain_list *chain;
+	u32 entries_printed = 0;
+	bool printed = false;
+	struct rb_node *node;
+	int i = 0;
+	int ret = 0;
+
+	/*
+	 * If have one single callchain root, don't bother printing
+	 * its percentage (100 % in fractal mode and the same percentage
+	 * than the hist in graph mode). This also avoid one level of column.
+	 */
+	node = rb_first(root);
+	if (node && !rb_next(node)) {
+		cnode = rb_entry(node, struct callchain_node, rb_node);
+		list_for_each_entry(chain, &cnode->val, list) {
+			/*
+			 * If we sort by symbol, the first entry is the same than
+			 * the symbol. No need to print it otherwise it appears as
+			 * displayed twice.
+			 */
+			if (!i++ && sort__first_dimension == SORT_SYM)
+				continue;
+			if (!printed) {
+				ret += callchain__fprintf_left_margin(fp, left_margin);
+				ret += fprintf(fp, "|\n");
+				ret += callchain__fprintf_left_margin(fp, left_margin);
+				ret += fprintf(fp, "---");
+				left_margin += 3;
+				printed = true;
+			} else
+				ret += callchain__fprintf_left_margin(fp, left_margin);
+
+			if (chain->ms.sym)
+				ret += fprintf(fp, " %s\n", chain->ms.sym->name);
+			else
+				ret += fprintf(fp, " %p\n", (void *)(long)chain->ip);
+
+			if (++entries_printed == callchain_param.print_limit)
+				break;
+		}
+		root = &cnode->rb_root;
+	}
+
+	ret += __callchain__fprintf_graph(fp, root, total_samples,
+					  1, 1, left_margin);
+	ret += fprintf(fp, "\n");
+
+	return ret;
+}
+
+static size_t __callchain__fprintf_flat(FILE *fp,
+					struct callchain_node *self,
+					u64 total_samples)
+{
+	struct callchain_list *chain;
+	size_t ret = 0;
+
+	if (!self)
+		return 0;
+
+	ret += __callchain__fprintf_flat(fp, self->parent, total_samples);
+
+
+	list_for_each_entry(chain, &self->val, list) {
+		if (chain->ip >= PERF_CONTEXT_MAX)
+			continue;
+		if (chain->ms.sym)
+			ret += fprintf(fp, "                %s\n", chain->ms.sym->name);
+		else
+			ret += fprintf(fp, "                %p\n",
+					(void *)(long)chain->ip);
+	}
+
+	return ret;
+}
+
+static size_t callchain__fprintf_flat(FILE *fp, struct rb_root *self,
+				      u64 total_samples)
+{
+	size_t ret = 0;
+	u32 entries_printed = 0;
+	struct rb_node *rb_node;
+	struct callchain_node *chain;
+
+	rb_node = rb_first(self);
+	while (rb_node) {
+		double percent;
+
+		chain = rb_entry(rb_node, struct callchain_node, rb_node);
+		percent = chain->hit * 100.0 / total_samples;
+
+		ret = percent_color_fprintf(fp, "           %6.2f%%\n", percent);
+		ret += __callchain__fprintf_flat(fp, chain, total_samples);
+		ret += fprintf(fp, "\n");
+		if (++entries_printed == callchain_param.print_limit)
+			break;
+
+		rb_node = rb_next(rb_node);
+	}
+
+	return ret;
+}
+
+static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
+					    u64 total_samples, int left_margin,
+					    FILE *fp)
+{
+	switch (callchain_param.mode) {
+	case CHAIN_GRAPH_REL:
+		return callchain__fprintf_graph(fp, &he->sorted_chain, he->period,
+						left_margin);
+		break;
+	case CHAIN_GRAPH_ABS:
+		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
+						left_margin);
+		break;
+	case CHAIN_FLAT:
+		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);
+		break;
+	case CHAIN_NONE:
+		break;
+	default:
+		pr_err("Bad callchain mode\n");
+	}
+
+	return 0;
+}
+
+static int hist_entry__pcnt_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__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 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)
+{
+	char bf[512];
+	int ret;
+
+	if (size == 0 || size > sizeof(bf))
+		size = sizeof(bf);
+
+	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
+					show_displacement, displacement,
+					true, total_period);
+	hist_entry__snprintf(he, bf + ret, size - ret, hists);
+	return fprintf(fp, "%s\n", bf);
+}
+
+static size_t hist_entry__fprintf_callchain(struct hist_entry *he,
+					    struct hists *hists,
+					    u64 total_period, FILE *fp)
+{
+	int left_margin = 0;
+
+	if (sort__first_dimension == SORT_COMM) {
+		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
+							 typeof(*se), list);
+		left_margin = hists__col_len(hists, se->se_width_idx);
+		left_margin -= thread__comm_len(he->thread);
+	}
+
+	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
+}
+
+size_t hists__fprintf(struct hists *hists, struct hists *pair,
+		      bool show_displacement, bool show_header, int max_rows,
+		      int max_cols, FILE *fp)
+{
+	struct sort_entry *se;
+	struct rb_node *nd;
+	size_t ret = 0;
+	u64 total_period;
+	unsigned long position = 1;
+	long displacement = 0;
+	unsigned int width;
+	const char *sep = symbol_conf.field_sep;
+	const char *col_width = symbol_conf.col_width_list_str;
+	int nr_rows = 0;
+
+	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    ");
+	}
+
+	if (pair) {
+		if (sep)
+			ret += fprintf(fp, "%cDelta", *sep);
+		else
+			ret += fprintf(fp, "  Delta    ");
+
+		if (show_displacement) {
+			if (sep)
+				ret += fprintf(fp, "%cDisplacement", *sep);
+			else
+				ret += fprintf(fp, " Displ");
+		}
+	}
+
+	list_for_each_entry(se, &hist_entry__sort_list, list) {
+		if (se->elide)
+			continue;
+		if (sep) {
+			fprintf(fp, "%c%s", *sep, se->se_header);
+			continue;
+		}
+		width = strlen(se->se_header);
+		if (symbol_conf.col_width_list_str) {
+			if (col_width) {
+				hists__set_col_len(hists, se->se_width_idx,
+						   atoi(col_width));
+				col_width = strchr(col_width, ',');
+				if (col_width)
+					++col_width;
+			}
+		}
+		if (!hists__new_col_len(hists, se->se_width_idx, width))
+			width = hists__col_len(hists, se->se_width_idx);
+		fprintf(fp, "  %*s", width, se->se_header);
+	}
+
+	fprintf(fp, "\n");
+	if (max_rows && ++nr_rows >= max_rows)
+		goto out;
+
+	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, " .....");
+	}
+	list_for_each_entry(se, &hist_entry__sort_list, list) {
+		unsigned int i;
+
+		if (se->elide)
+			continue;
+
+		fprintf(fp, "  ");
+		width = hists__col_len(hists, se->se_width_idx);
+		if (width == 0)
+			width = strlen(se->se_header);
+		for (i = 0; i < width; i++)
+			fprintf(fp, ".");
+	}
+
+	fprintf(fp, "\n");
+	if (max_rows && ++nr_rows >= max_rows)
+		goto out;
+
+	fprintf(fp, "#\n");
+	if (max_rows && ++nr_rows >= max_rows)
+		goto out;
+
+print_entries:
+	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);
+
+		if (h->filtered)
+			continue;
+
+		if (show_displacement) {
+			if (h->pair != NULL)
+				displacement = ((long)h->pair->position -
+					        (long)position);
+			else
+				displacement = 0;
+			++position;
+		}
+		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
+					   displacement, total_period, fp);
+
+		if (symbol_conf.use_callchain)
+			ret += hist_entry__fprintf_callchain(h, hists, total_period, fp);
+		if (max_rows && ++nr_rows >= max_rows)
+			goto out;
+
+		if (h->ms.map == NULL && verbose > 1) {
+			__map_groups__fprintf_maps(&h->thread->mg,
+						   MAP__FUNCTION, verbose, fp);
+			fprintf(fp, "%.10s end\n", graph_dotted_line);
+		}
+	}
+out:
+	free(rem_sq_bracket);
+
+	return ret;
+}
+
+size_t hists__fprintf_nr_events(struct hists *hists, FILE *fp)
+{
+	int i;
+	size_t ret = 0;
+
+	for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
+		const char *name;
+
+		if (hists->stats.nr_events[i] == 0)
+			continue;
+
+		name = perf_event__name(i);
+		if (!strcmp(name, "UNKNOWN"))
+			continue;
+
+		ret += fprintf(fp, "%16s events: %10d\n", name,
+			       hists->stats.nr_events[i]);
+	}
+
+	return ret;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f247ef2789a4..b1817f15bb87 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -45,7 +45,7 @@ bool hists__new_col_len(struct hists *hists, enum hist_column col, u16 len)
 	return false;
 }
 
-static void hists__reset_col_len(struct hists *hists)
+void hists__reset_col_len(struct hists *hists)
 {
 	enum hist_column col;
 
@@ -63,7 +63,7 @@ static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
 		hists__set_col_len(hists, dso, unresolved_col_width);
 }
 
-static void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
+void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 {
 	const unsigned int unresolved_col_width = BITS_PER_LONG / 4;
 	u16 len;
@@ -114,6 +114,22 @@ static void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	}
 }
 
+void hists__output_recalc_col_len(struct hists *hists, int max_rows)
+{
+	struct rb_node *next = rb_first(&hists->entries);
+	struct hist_entry *n;
+	int row = 0;
+
+	hists__reset_col_len(hists);
+
+	while (next && row++ < max_rows) {
+		n = rb_entry(next, struct hist_entry, rb_node);
+		if (!n->filtered)
+			hists__calc_col_len(hists, n);
+		next = rb_next(&n->rb_node);
+	}
+}
+
 static void hist_entry__add_cpumode_period(struct hist_entry *he,
 					   unsigned int cpumode, u64 period)
 {
@@ -547,641 +563,6 @@ void hists__output_resort_threaded(struct hists *hists)
 	return __hists__output_resort(hists, true);
 }
 
-static size_t callchain__fprintf_left_margin(FILE *fp, int left_margin)
-{
-	int i;
-	int ret = fprintf(fp, "            ");
-
-	for (i = 0; i < left_margin; i++)
-		ret += fprintf(fp, " ");
-
-	return ret;
-}
-
-static size_t ipchain__fprintf_graph_line(FILE *fp, int depth, int depth_mask,
-					  int left_margin)
-{
-	int i;
-	size_t ret = callchain__fprintf_left_margin(fp, left_margin);
-
-	for (i = 0; i < depth; i++)
-		if (depth_mask & (1 << i))
-			ret += fprintf(fp, "|          ");
-		else
-			ret += fprintf(fp, "           ");
-
-	ret += fprintf(fp, "\n");
-
-	return ret;
-}
-
-static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
-				     int depth, int depth_mask, int period,
-				     u64 total_samples, u64 hits,
-				     int left_margin)
-{
-	int i;
-	size_t ret = 0;
-
-	ret += callchain__fprintf_left_margin(fp, left_margin);
-	for (i = 0; i < depth; i++) {
-		if (depth_mask & (1 << i))
-			ret += fprintf(fp, "|");
-		else
-			ret += fprintf(fp, " ");
-		if (!period && i == depth - 1) {
-			double percent;
-
-			percent = hits * 100.0 / total_samples;
-			ret += percent_color_fprintf(fp, "--%2.2f%%-- ", percent);
-		} else
-			ret += fprintf(fp, "%s", "          ");
-	}
-	if (chain->ms.sym)
-		ret += fprintf(fp, "%s\n", chain->ms.sym->name);
-	else
-		ret += fprintf(fp, "0x%0" PRIx64 "\n", chain->ip);
-
-	return ret;
-}
-
-static struct symbol *rem_sq_bracket;
-static struct callchain_list rem_hits;
-
-static void init_rem_hits(void)
-{
-	rem_sq_bracket = malloc(sizeof(*rem_sq_bracket) + 6);
-	if (!rem_sq_bracket) {
-		fprintf(stderr, "Not enough memory to display remaining hits\n");
-		return;
-	}
-
-	strcpy(rem_sq_bracket->name, "[...]");
-	rem_hits.ms.sym = rem_sq_bracket;
-}
-
-static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-					 u64 total_samples, int depth,
-					 int depth_mask, int left_margin)
-{
-	struct rb_node *node, *next;
-	struct callchain_node *child;
-	struct callchain_list *chain;
-	int new_depth_mask = depth_mask;
-	u64 remaining;
-	size_t ret = 0;
-	int i;
-	uint entries_printed = 0;
-
-	remaining = total_samples;
-
-	node = rb_first(root);
-	while (node) {
-		u64 new_total;
-		u64 cumul;
-
-		child = rb_entry(node, struct callchain_node, rb_node);
-		cumul = callchain_cumul_hits(child);
-		remaining -= cumul;
-
-		/*
-		 * The depth mask manages the output of pipes that show
-		 * the depth. We don't want to keep the pipes of the current
-		 * level for the last child of this depth.
-		 * Except if we have remaining filtered hits. They will
-		 * supersede the last child
-		 */
-		next = rb_next(node);
-		if (!next && (callchain_param.mode != CHAIN_GRAPH_REL || !remaining))
-			new_depth_mask &= ~(1 << (depth - 1));
-
-		/*
-		 * But we keep the older depth mask for the line separator
-		 * to keep the level link until we reach the last child
-		 */
-		ret += ipchain__fprintf_graph_line(fp, depth, depth_mask,
-						   left_margin);
-		i = 0;
-		list_for_each_entry(chain, &child->val, list) {
-			ret += ipchain__fprintf_graph(fp, chain, depth,
-						      new_depth_mask, i++,
-						      total_samples,
-						      cumul,
-						      left_margin);
-		}
-
-		if (callchain_param.mode == CHAIN_GRAPH_REL)
-			new_total = child->children_hit;
-		else
-			new_total = total_samples;
-
-		ret += __callchain__fprintf_graph(fp, &child->rb_root, new_total,
-						  depth + 1,
-						  new_depth_mask | (1 << depth),
-						  left_margin);
-		node = next;
-		if (++entries_printed == callchain_param.print_limit)
-			break;
-	}
-
-	if (callchain_param.mode == CHAIN_GRAPH_REL &&
-		remaining && remaining != total_samples) {
-
-		if (!rem_sq_bracket)
-			return ret;
-
-		new_depth_mask &= ~(1 << (depth - 1));
-		ret += ipchain__fprintf_graph(fp, &rem_hits, depth,
-					      new_depth_mask, 0, total_samples,
-					      remaining, left_margin);
-	}
-
-	return ret;
-}
-
-static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-				       u64 total_samples, int left_margin)
-{
-	struct callchain_node *cnode;
-	struct callchain_list *chain;
-	u32 entries_printed = 0;
-	bool printed = false;
-	struct rb_node *node;
-	int i = 0;
-	int ret = 0;
-
-	/*
-	 * If have one single callchain root, don't bother printing
-	 * its percentage (100 % in fractal mode and the same percentage
-	 * than the hist in graph mode). This also avoid one level of column.
-	 */
-	node = rb_first(root);
-	if (node && !rb_next(node)) {
-		cnode = rb_entry(node, struct callchain_node, rb_node);
-		list_for_each_entry(chain, &cnode->val, list) {
-			/*
-			 * If we sort by symbol, the first entry is the same than
-			 * the symbol. No need to print it otherwise it appears as
-			 * displayed twice.
-			 */
-			if (!i++ && sort__first_dimension == SORT_SYM)
-				continue;
-			if (!printed) {
-				ret += callchain__fprintf_left_margin(fp, left_margin);
-				ret += fprintf(fp, "|\n");
-				ret += callchain__fprintf_left_margin(fp, left_margin);
-				ret += fprintf(fp, "---");
-				left_margin += 3;
-				printed = true;
-			} else
-				ret += callchain__fprintf_left_margin(fp, left_margin);
-
-			if (chain->ms.sym)
-				ret += fprintf(fp, " %s\n", chain->ms.sym->name);
-			else
-				ret += fprintf(fp, " %p\n", (void *)(long)chain->ip);
-
-			if (++entries_printed == callchain_param.print_limit)
-				break;
-		}
-		root = &cnode->rb_root;
-	}
-
-	ret += __callchain__fprintf_graph(fp, root, total_samples,
-					  1, 1, left_margin);
-	ret += fprintf(fp, "\n");
-
-	return ret;
-}
-
-static size_t __callchain__fprintf_flat(FILE *fp,
-					struct callchain_node *self,
-					u64 total_samples)
-{
-	struct callchain_list *chain;
-	size_t ret = 0;
-
-	if (!self)
-		return 0;
-
-	ret += __callchain__fprintf_flat(fp, self->parent, total_samples);
-
-
-	list_for_each_entry(chain, &self->val, list) {
-		if (chain->ip >= PERF_CONTEXT_MAX)
-			continue;
-		if (chain->ms.sym)
-			ret += fprintf(fp, "                %s\n", chain->ms.sym->name);
-		else
-			ret += fprintf(fp, "                %p\n",
-					(void *)(long)chain->ip);
-	}
-
-	return ret;
-}
-
-static size_t callchain__fprintf_flat(FILE *fp, struct rb_root *self,
-				      u64 total_samples)
-{
-	size_t ret = 0;
-	u32 entries_printed = 0;
-	struct rb_node *rb_node;
-	struct callchain_node *chain;
-
-	rb_node = rb_first(self);
-	while (rb_node) {
-		double percent;
-
-		chain = rb_entry(rb_node, struct callchain_node, rb_node);
-		percent = chain->hit * 100.0 / total_samples;
-
-		ret = percent_color_fprintf(fp, "           %6.2f%%\n", percent);
-		ret += __callchain__fprintf_flat(fp, chain, total_samples);
-		ret += fprintf(fp, "\n");
-		if (++entries_printed == callchain_param.print_limit)
-			break;
-
-		rb_node = rb_next(rb_node);
-	}
-
-	return ret;
-}
-
-static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
-					    u64 total_samples, int left_margin,
-					    FILE *fp)
-{
-	switch (callchain_param.mode) {
-	case CHAIN_GRAPH_REL:
-		return callchain__fprintf_graph(fp, &he->sorted_chain, he->period,
-						left_margin);
-		break;
-	case CHAIN_GRAPH_ABS:
-		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
-						left_margin);
-		break;
-	case CHAIN_FLAT:
-		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);
-		break;
-	case CHAIN_NONE:
-		break;
-	default:
-		pr_err("Bad callchain mode\n");
-	}
-
-	return 0;
-}
-
-void hists__output_recalc_col_len(struct hists *hists, int max_rows)
-{
-	struct rb_node *next = rb_first(&hists->entries);
-	struct hist_entry *n;
-	int row = 0;
-
-	hists__reset_col_len(hists);
-
-	while (next && row++ < max_rows) {
-		n = rb_entry(next, struct hist_entry, rb_node);
-		if (!n->filtered)
-			hists__calc_col_len(hists, n);
-		next = rb_next(&n->rb_node);
-	}
-}
-
-static int hist_entry__pcnt_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__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 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)
-{
-	char bf[512];
-	int ret;
-
-	if (size == 0 || size > sizeof(bf))
-		size = sizeof(bf);
-
-	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
-					show_displacement, displacement,
-					true, total_period);
-	hist_entry__snprintf(he, bf + ret, size - ret, hists);
-	return fprintf(fp, "%s\n", bf);
-}
-
-static size_t hist_entry__fprintf_callchain(struct hist_entry *he,
-					    struct hists *hists,
-					    u64 total_period, FILE *fp)
-{
-	int left_margin = 0;
-
-	if (sort__first_dimension == SORT_COMM) {
-		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
-							 typeof(*se), list);
-		left_margin = hists__col_len(hists, se->se_width_idx);
-		left_margin -= thread__comm_len(he->thread);
-	}
-
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
-}
-
-size_t hists__fprintf(struct hists *hists, struct hists *pair,
-		      bool show_displacement, bool show_header, int max_rows,
-		      int max_cols, FILE *fp)
-{
-	struct sort_entry *se;
-	struct rb_node *nd;
-	size_t ret = 0;
-	u64 total_period;
-	unsigned long position = 1;
-	long displacement = 0;
-	unsigned int width;
-	const char *sep = symbol_conf.field_sep;
-	const char *col_width = symbol_conf.col_width_list_str;
-	int nr_rows = 0;
-
-	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    ");
-	}
-
-	if (pair) {
-		if (sep)
-			ret += fprintf(fp, "%cDelta", *sep);
-		else
-			ret += fprintf(fp, "  Delta    ");
-
-		if (show_displacement) {
-			if (sep)
-				ret += fprintf(fp, "%cDisplacement", *sep);
-			else
-				ret += fprintf(fp, " Displ");
-		}
-	}
-
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (se->elide)
-			continue;
-		if (sep) {
-			fprintf(fp, "%c%s", *sep, se->se_header);
-			continue;
-		}
-		width = strlen(se->se_header);
-		if (symbol_conf.col_width_list_str) {
-			if (col_width) {
-				hists__set_col_len(hists, se->se_width_idx,
-						   atoi(col_width));
-				col_width = strchr(col_width, ',');
-				if (col_width)
-					++col_width;
-			}
-		}
-		if (!hists__new_col_len(hists, se->se_width_idx, width))
-			width = hists__col_len(hists, se->se_width_idx);
-		fprintf(fp, "  %*s", width, se->se_header);
-	}
-
-	fprintf(fp, "\n");
-	if (max_rows && ++nr_rows >= max_rows)
-		goto out;
-
-	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, " .....");
-	}
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		unsigned int i;
-
-		if (se->elide)
-			continue;
-
-		fprintf(fp, "  ");
-		width = hists__col_len(hists, se->se_width_idx);
-		if (width == 0)
-			width = strlen(se->se_header);
-		for (i = 0; i < width; i++)
-			fprintf(fp, ".");
-	}
-
-	fprintf(fp, "\n");
-	if (max_rows && ++nr_rows >= max_rows)
-		goto out;
-
-	fprintf(fp, "#\n");
-	if (max_rows && ++nr_rows >= max_rows)
-		goto out;
-
-print_entries:
-	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);
-
-		if (h->filtered)
-			continue;
-
-		if (show_displacement) {
-			if (h->pair != NULL)
-				displacement = ((long)h->pair->position -
-					        (long)position);
-			else
-				displacement = 0;
-			++position;
-		}
-		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
-					   displacement, total_period, fp);
-
-		if (symbol_conf.use_callchain)
-			ret += hist_entry__fprintf_callchain(h, hists, total_period, fp);
-		if (max_rows && ++nr_rows >= max_rows)
-			goto out;
-
-		if (h->ms.map == NULL && verbose > 1) {
-			__map_groups__fprintf_maps(&h->thread->mg,
-						   MAP__FUNCTION, verbose, fp);
-			fprintf(fp, "%.10s end\n", graph_dotted_line);
-		}
-	}
-out:
-	free(rem_sq_bracket);
-
-	return ret;
-}
-
 /*
  * See hists__fprintf to match the column widths
  */
@@ -1342,25 +723,3 @@ void hists__inc_nr_events(struct hists *hists, u32 type)
 	++hists->stats.nr_events[0];
 	++hists->stats.nr_events[type];
 }
-
-size_t hists__fprintf_nr_events(struct hists *hists, FILE *fp)
-{
-	int i;
-	size_t ret = 0;
-
-	for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
-		const char *name;
-
-		if (hists->stats.nr_events[i] == 0)
-			continue;
-
-		name = perf_event__name(i);
-		if (!strcmp(name, "UNKNOWN"))
-			continue;
-
-		ret += fprintf(fp, "%16s events: %10d\n", name,
-			       hists->stats.nr_events[i]);
-	}
-
-	return ret;
-}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 0b096c27a419..69fab7d9abcd 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -112,6 +112,8 @@ void hists__filter_by_symbol(struct hists *hists);
 u16 hists__col_len(struct hists *self, enum hist_column col);
 void hists__set_col_len(struct hists *self, enum hist_column col, u16 len);
 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_evlist;
 
-- 
1.7.11.2


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

* [PATCH 2/7] perf hists: Refactor some functions
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
  2012-08-20  4:52 ` [PATCH 1/7] perf hists: Separate out hist print functions Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  2012-08-21 16:34   ` [tip:perf/core] perf hists: Rename and move " tip-bot for Namhyung Kim
  2012-08-20  4:52 ` [PATCH 3/7] perf hists: Introduce hist_period_print functions Namhyung Kim
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim

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

Rename functions for consistency and move callchain print function
into hist_entry__fprintf().

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

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b8094692c153..81bd8c2af730 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -586,7 +586,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__snprintf(entry, s, sizeof(s), browser->hists);
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
 
 		ui_browser__set_percent_color(&browser->b, percent, current_entry);
@@ -931,7 +931,7 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
 	if (symbol_conf.use_callchain)
 		folded_sign = hist_entry__folded(he);
 
-	hist_entry__snprintf(he, s, sizeof(s), browser->hists);
+	hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists);
 	percent = (he->period * 100.0) / browser->hists->stats.total_period;
 
 	if (symbol_conf.use_callchain)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 7881d625e17a..9bf7e9e5a72e 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -291,7 +291,7 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static int hist_entry__pcnt_snprintf(struct hist_entry *he, char *s,
+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)
@@ -404,8 +404,8 @@ static int hist_entry__pcnt_snprintf(struct hist_entry *he, char *s,
 	return ret;
 }
 
-int hist_entry__snprintf(struct hist_entry *he, char *s, size_t size,
-			 struct hists *hists)
+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;
@@ -423,6 +423,22 @@ int hist_entry__snprintf(struct hist_entry *he, char *s, size_t size,
 	return ret;
 }
 
+static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
+					    struct hists *hists,
+					    u64 total_period, FILE *fp)
+{
+	int left_margin = 0;
+
+	if (sort__first_dimension == SORT_COMM) {
+		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
+							 typeof(*se), list);
+		left_margin = hists__col_len(hists, se->se_width_idx);
+		left_margin -= thread__comm_len(he->thread);
+	}
+
+	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
+}
+
 static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 			       struct hists *hists, struct hists *pair_hists,
 			       bool show_displacement, long displacement,
@@ -434,27 +450,18 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	if (size == 0 || size > sizeof(bf))
 		size = sizeof(bf);
 
-	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
-					show_displacement, displacement,
-					true, total_period);
-	hist_entry__snprintf(he, bf + ret, size - ret, hists);
-	return fprintf(fp, "%s\n", bf);
-}
+	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
+					  show_displacement, displacement,
+					  true, total_period);
+	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
-static size_t hist_entry__fprintf_callchain(struct hist_entry *he,
-					    struct hists *hists,
-					    u64 total_period, FILE *fp)
-{
-	int left_margin = 0;
+	ret = fprintf(fp, "%s\n", bf);
 
-	if (sort__first_dimension == SORT_COMM) {
-		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
-							 typeof(*se), list);
-		left_margin = hists__col_len(hists, se->se_width_idx);
-		left_margin -= thread__comm_len(he->thread);
-	}
+	if (symbol_conf.use_callchain)
+		ret += hist_entry__callchain_fprintf(he, hists,
+						     total_period, fp);
 
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
+	return ret;
 }
 
 size_t hists__fprintf(struct hists *hists, struct hists *pair,
@@ -608,8 +615,6 @@ print_entries:
 		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
 					   displacement, total_period, fp);
 
-		if (symbol_conf.use_callchain)
-			ret += hist_entry__fprintf_callchain(h, hists, 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 69fab7d9abcd..2e650ffb7d23 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -75,8 +75,8 @@ struct hist_entry *__hists__add_entry(struct hists *self,
 				      struct symbol *parent, u64 period);
 int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right);
 int64_t hist_entry__collapse(struct hist_entry *left, struct hist_entry *right);
-int hist_entry__snprintf(struct hist_entry *self, char *bf, size_t size,
-			 struct hists *hists);
+int hist_entry__sort_snprintf(struct hist_entry *self, char *bf, size_t size,
+			      struct hists *hists);
 void hist_entry__free(struct hist_entry *);
 
 struct hist_entry *__hists__add_branch_entry(struct hists *self,
-- 
1.7.11.2


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

* [PATCH 3/7] perf hists: Introduce hist_period_print functions
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
  2012-08-20  4:52 ` [PATCH 1/7] perf hists: Separate out hist print functions Namhyung Kim
  2012-08-20  4:52 ` [PATCH 2/7] perf hists: Refactor some functions Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  2012-08-20 13:08   ` Arnaldo Carvalho de Melo
  2012-08-20  4:52 ` [PATCH 4/7] perf hists: Handle field separator properly Namhyung Kim
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim

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

Current hist print functions are messy. Refactor them using the hpp
callbacks and move common code to ui/hist.c. This will make it easy 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       | 326 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c      |   8 +-
 tools/perf/ui/stdio/hist.c | 238 ++++++---------------------------
 tools/perf/util/hist.h     |  37 +++++
 6 files changed, 412 insertions(+), 200 deletions(-)
 create mode 100644 tools/perf/ui/hist.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index cb18b047c9c9..96dda3280ce9 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..8062b38f7971
--- /dev/null
+++ b/tools/perf/ui/hist.c
@@ -0,0 +1,326 @@
+#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 hpp_context *ctx)
+{
+	if (ctx->ptr)
+		return scnprintf(ctx->s, ctx->size, "Baseline");
+	else
+		return scnprintf(ctx->s, ctx->size, "Overhead");
+}
+
+static int hpp_width_overhead(struct hpp_context *ctx __used)
+{
+	return 8;
+}
+
+static int hpp_color_overhead(struct hpp_context *ctx,
+			      struct hist_entry *he)
+{
+	double percent = 100.0 * he->period / ctx->total_period;
+	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
+}
+
+static int hpp_entry_overhead(struct hpp_context *ctx,
+			      struct hist_entry *he)
+{
+	double percent = 100.0 * he->period / ctx->total_period;
+	return scnprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
+}
+
+static int hpp_header_overhead_sys(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, " sys  ");
+}
+
+static int hpp_width_overhead_sys(struct hpp_context *ctx __used)
+{
+	return 6;
+}
+
+static int hpp_color_overhead_sys(struct hpp_context *ctx,
+				  struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_sys / ctx->total_period;
+	return percent_color_snprintf(ctx->s, ctx->size, "%5.2f%%", percent);
+}
+
+static int hpp_entry_overhead_sys(struct hpp_context *ctx,
+				  struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_sys / ctx->total_period;
+	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
+}
+
+static int hpp_header_overhead_us(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, " user ");
+}
+
+static int hpp_width_overhead_us(struct hpp_context *ctx __used)
+{
+	return 6;
+}
+
+static int hpp_color_overhead_us(struct hpp_context *ctx,
+				 struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_us / ctx->total_period;
+	return percent_color_snprintf(ctx->s, ctx->size, "%5.2f%%", percent);
+}
+
+static int hpp_entry_overhead_us(struct hpp_context *ctx,
+				 struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_us / ctx->total_period;
+	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
+}
+
+static int hpp_header_overhead_guest_sys(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, "guest sys");
+}
+
+static int hpp_width_overhead_guest_sys(struct hpp_context *ctx __used)
+{
+	return 9;
+}
+
+static int hpp_color_overhead_guest_sys(struct hpp_context *ctx,
+					struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
+	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
+}
+
+static int hpp_entry_overhead_guest_sys(struct hpp_context *ctx,
+					struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
+	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
+}
+
+static int hpp_header_overhead_guest_us(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, "guest usr");
+}
+
+static int hpp_width_overhead_guest_us(struct hpp_context *ctx __used)
+{
+	return 9;
+}
+
+static int hpp_color_overhead_guest_us(struct hpp_context *ctx,
+				       struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_us / ctx->total_period;
+	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
+}
+
+static int hpp_entry_overhead_guest_us(struct hpp_context *ctx,
+				       struct hist_entry *he)
+{
+	double percent = 100.0 * he->period_guest_us / ctx->total_period;
+	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
+}
+
+static int hpp_header_samples(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, "  Samples  ");
+}
+
+static int hpp_width_samples(struct hpp_context *ctx __used)
+{
+	return 11;
+}
+
+static int hpp_entry_samples(struct hpp_context *ctx,
+			     struct hist_entry *he)
+{
+	return scnprintf(ctx->s, ctx->size, "%11" PRIu64, he->nr_events);
+}
+
+static int hpp_header_period(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, "   Period   ");
+}
+
+static int hpp_width_period(struct hpp_context *ctx __used)
+{
+	return 12;
+}
+
+static int hpp_entry_period(struct hpp_context *ctx,
+			    struct hist_entry *he)
+{
+	return scnprintf(ctx->s, ctx->size, "%12" PRIu64, he->period);
+}
+
+static int hpp_header_delta(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, " Delta ");
+}
+
+static int hpp_width_delta(struct hpp_context *ctx __used)
+{
+	return 7;
+}
+
+static int hpp_entry_delta(struct hpp_context *ctx,
+			   struct hist_entry *he)
+{
+	struct hists *pair_hists = ctx->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 = ctx->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(ctx->s, ctx->size, "       ");
+
+	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
+	return scnprintf(ctx->s, ctx->size, "%7.7s", buf);
+}
+
+static int hpp_header_displ(struct hpp_context *ctx)
+{
+	return scnprintf(ctx->s, ctx->size, "Displ.");
+}
+
+static int hpp_width_displ(struct hpp_context *ctx __used)
+{
+	return 6;
+}
+
+static int hpp_entry_displ(struct hpp_context *ctx,
+			   struct hist_entry *he __used)
+{
+	char buf[32];
+
+	if (!ctx->displacement)
+		return scnprintf(ctx->s, ctx->size, "     ");
+
+	scnprintf(buf, sizeof(buf), "%+4ld", ctx->displacement);
+	return scnprintf(ctx->s, ctx->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 hist_period_print hpp_functions[] = {
+	{ .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) {
+		hpp_functions[PERF_HPP__OVERHEAD_SYS].cond = true;
+		hpp_functions[PERF_HPP__OVERHEAD_US].cond = true;
+
+		if (perf_guest) {
+			hpp_functions[PERF_HPP__OVERHEAD_GUEST_SYS].cond = true;
+			hpp_functions[PERF_HPP__OVERHEAD_GUEST_US].cond = true;
+		}
+	}
+
+	if (symbol_conf.show_nr_samples)
+		hpp_functions[PERF_HPP__SAMPLES].cond = true;
+
+	if (symbol_conf.show_total_period)
+		hpp_functions[PERF_HPP__PERIOD].cond = true;
+
+	if (need_pair) {
+		hpp_functions[PERF_HPP__DELTA].cond = true;
+
+		if (show_displacement)
+			hpp_functions[PERF_HPP__DISPL].cond = true;
+	}
+}
+
+static inline void advance_hpp_context(struct hpp_context *ctx, int inc)
+{
+	ctx->s    += inc;
+	ctx->size -= inc;
+}
+
+int hist_entry__period_snprintf(struct hpp_context *ctx,
+				struct hist_entry *he, bool color)
+{
+	const char *sep = symbol_conf.field_sep;
+	char *start_s = ctx->s;
+	int i, ret;
+
+	if (symbol_conf.exclude_other && !he->parent)
+		return 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!hpp_functions[i].cond)
+			continue;
+
+		if (!sep || i > 0) {
+			ret = scnprintf(ctx->s, ctx->size, "%s", sep ?: "  ");
+			advance_hpp_context(ctx, ret);
+		}
+
+		if (color && hpp_functions[i].color)
+			ret = hpp_functions[i].color(ctx, he);
+		else
+			ret = hpp_functions[i].entry(ctx, he);
+
+		advance_hpp_context(ctx, ret);
+	}
+
+	return ctx->s - start_s;
+}
+
+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..c040b03cc6f4 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 hpp_context ctx = {
+		.s		= bf,
+		.size		= size,
+		.total_period	= total_period,
+		.displacement	= displacement,
+		.ptr		= pair_hists,
+	};
 
 	if (size == 0 || size > sizeof(bf))
-		size = sizeof(bf);
+		size = ctx.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
-					  show_displacement, displacement,
-					  true, total_period);
+	ret = hist_entry__period_snprintf(&ctx, 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 hpp_context dummy_ctx = {
+		.s	= 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 (!hpp_functions[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");
-		}
+		hpp_functions[idx].header(&dummy_ctx);
+		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 (!hpp_functions[idx].cond)
+			continue;
+
+		if (idx)
+			fprintf(fp, "%s", sep ?: "  ");
+
+		width = hpp_functions[idx].width(&dummy_ctx);
+		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..a387e2aa4602 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 hpp_context {
+	char *s;
+	size_t size;
+	u64 total_period;
+	const char *sep;
+	long displacement;
+	void *ptr;
+};
+
+struct hist_period_print {
+	bool cond;
+	int (*header)(struct hpp_context *ctx);
+	int (*width)(struct hpp_context *ctx);
+	int (*color)(struct hpp_context *ctx, struct hist_entry *he);
+	int (*entry)(struct hpp_context *ctx, struct hist_entry *he);
+};
+
+extern struct hist_period_print hpp_functions[];
+
+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 hpp_context *ctx,
+				struct hist_entry *he, bool color);
+
 struct perf_evlist;
 
 #ifdef NO_NEWT_SUPPORT
-- 
1.7.11.2


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

* [PATCH 4/7] perf hists: Handle field separator properly
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
                   ` (2 preceding siblings ...)
  2012-08-20  4:52 ` [PATCH 3/7] perf hists: Introduce hist_period_print functions Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  2012-08-20  4:52 ` [PATCH 5/7] perf hists: Use hpp_functions->width to calculate the column widths Namhyung Kim
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim,
	Stephane Eranian

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.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c       | 42 ++++++++++++++++++++++++++++++++++++++++--
 tools/perf/ui/stdio/hist.c |  3 ++-
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 8062b38f7971..29c787c93452 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -30,6 +30,10 @@ static int hpp_entry_overhead(struct hpp_context *ctx,
 			      struct hist_entry *he)
 {
 	double percent = 100.0 * he->period / ctx->total_period;
+
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%.2f", percent);
+
 	return scnprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
 }
 
@@ -54,6 +58,10 @@ static int hpp_entry_overhead_sys(struct hpp_context *ctx,
 				  struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_sys / ctx->total_period;
+
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%.2f", percent);
+
 	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
 }
 
@@ -78,6 +86,10 @@ static int hpp_entry_overhead_us(struct hpp_context *ctx,
 				 struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_us / ctx->total_period;
+
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%.2f", percent);
+
 	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
 }
 
@@ -102,6 +114,10 @@ static int hpp_entry_overhead_guest_sys(struct hpp_context *ctx,
 					struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
+
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%.2f", percent);
+
 	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
 }
 
@@ -126,6 +142,10 @@ static int hpp_entry_overhead_guest_us(struct hpp_context *ctx,
 				       struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_us / ctx->total_period;
+
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%.2f", percent);
+
 	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
 }
 
@@ -142,6 +162,9 @@ static int hpp_width_samples(struct hpp_context *ctx __used)
 static int hpp_entry_samples(struct hpp_context *ctx,
 			     struct hist_entry *he)
 {
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%" PRIu64, he->nr_events);
+
 	return scnprintf(ctx->s, ctx->size, "%11" PRIu64, he->nr_events);
 }
 
@@ -158,6 +181,9 @@ static int hpp_width_period(struct hpp_context *ctx __used)
 static int hpp_entry_period(struct hpp_context *ctx,
 			    struct hist_entry *he)
 {
+	if (symbol_conf.field_sep)
+		return scnprintf(ctx->s, ctx->size, "%" PRIu64, he->period);
+
 	return scnprintf(ctx->s, ctx->size, "%12" PRIu64, he->period);
 }
 
@@ -189,10 +215,16 @@ static int hpp_entry_delta(struct hpp_context *ctx,
 		new_percent = 100.0 * he->period / new_total;
 
 	diff = new_percent - old_percent;
-	if (fabs(diff) < 0.01)
+	if (fabs(diff) < 0.01) {
+		if (symbol_conf.field_sep)
+			return scnprintf(ctx->s, ctx->size, " ");
 		return scnprintf(ctx->s, ctx->size, "       ");
+	}
 
 	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
+
+	if (symbol_conf.field_sep)
+		scnprintf(ctx->s, ctx->size, "%s", buf);
 	return scnprintf(ctx->s, ctx->size, "%7.7s", buf);
 }
 
@@ -211,10 +243,16 @@ static int hpp_entry_displ(struct hpp_context *ctx,
 {
 	char buf[32];
 
-	if (!ctx->displacement)
+	if (!ctx->displacement) {
+		if (symbol_conf.field_sep)
+			return scnprintf(ctx->s, ctx->size, " ");
 		return scnprintf(ctx->s, ctx->size, "     ");
+	}
 
 	scnprintf(buf, sizeof(buf), "%+4ld", ctx->displacement);
+
+	if (symbol_conf.field_sep)
+		scnprintf(ctx->s, ctx->size, "%s", buf);
 	return scnprintf(ctx->s, ctx->size, "%6.6s", buf);
 }
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index c040b03cc6f4..0712fb5c1bb7 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 = ctx.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(&ctx, he, true);
+	ret = hist_entry__period_snprintf(&ctx, he, color);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);
-- 
1.7.11.2


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

* [PATCH 5/7] perf hists: Use hpp_functions->width to calculate the column widths
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
                   ` (3 preceding siblings ...)
  2012-08-20  4:52 ` [PATCH 4/7] perf hists: Handle field separator properly Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  2012-08-20  4:52 ` [PATCH 6/7] perf ui/browser: Use hist_period_print functions Namhyung Kim
  2012-08-20  4:52 ` [PATCH 7/7] perf gtk/browser: " Namhyung Kim
  6 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, 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 29c787c93452..6856460df7e4 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -362,3 +362,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 (!hpp_functions[i].cond)
+			continue;
+		if (i)
+			ret += 2;
+
+		ret += hpp_functions[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.2


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

* [PATCH 6/7] perf ui/browser: Use hist_period_print functions
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
                   ` (4 preceding siblings ...)
  2012-08-20  4:52 ` [PATCH 5/7] perf hists: Use hpp_functions->width to calculate the column widths Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  2012-08-20  4:52 ` [PATCH 7/7] perf gtk/browser: " Namhyung Kim
  6 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, 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..69ce68708c0b 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 hpp_context *ctx,	\
+					     struct hist_entry *he)	\
+{									\
+	double percent = 100.0 * he->_field / ctx->total_period;	\
+	*(double *)ctx->ptr = percent;					\
+	return scnprintf(ctx->s, ctx->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);
+
+	hpp_functions[PERF_HPP__OVERHEAD].color =
+				hist_browser__hpp_color_overhead;
+	hpp_functions[PERF_HPP__OVERHEAD_SYS].color =
+				hist_browser__hpp_color_overhead_sys;
+	hpp_functions[PERF_HPP__OVERHEAD_US].color =
+				hist_browser__hpp_color_overhead_us;
+	hpp_functions[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				hist_browser__hpp_color_overhead_guest_sys;
+	hpp_functions[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 hpp_context ctx = {
+			.s		= 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 (!hpp_functions[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 (hpp_functions[i].color) {
+				ctx.ptr = &percent;
+				/* It will set percent for us. See HPP_COLOR_FN above. */
+				width -= hpp_functions[i].color(&ctx, 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 -= hpp_functions[i].entry(&ctx, 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.2


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

* [PATCH 7/7] perf gtk/browser: Use hist_period_print functions
  2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
                   ` (5 preceding siblings ...)
  2012-08-20  4:52 ` [PATCH 6/7] perf ui/browser: Use hist_period_print functions Namhyung Kim
@ 2012-08-20  4:52 ` Namhyung Kim
  6 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-08-20  4:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, 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..4a890fc17b91 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 hpp_context *ctx,		\
+					 struct hist_entry *he)			\
+{										\
+	double percent = 100.0 * he->_field / ctx->total_period;		\
+	const char *markup;							\
+	int ret = 0;								\
+										\
+	markup = perf_gtk__get_percent_color(percent);				\
+	if (markup)								\
+		ret += scnprintf(ctx->s, ctx->size, "%s", markup);		\
+	ret += scnprintf(ctx->s + ret, ctx->size - ret, "%5.2f%%", percent); 	\
+	if (markup)								\
+		ret += scnprintf(ctx->s + ret, ctx->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);
+
+	hpp_functions[PERF_HPP__OVERHEAD].color =
+				perf_gtk__hpp_color_overhead;
+	hpp_functions[PERF_HPP__OVERHEAD_SYS].color =
+				perf_gtk__hpp_color_overhead_sys;
+	hpp_functions[PERF_HPP__OVERHEAD_US].color =
+				perf_gtk__hpp_color_overhead_us;
+	hpp_functions[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				perf_gtk__hpp_color_overhead_guest_sys;
+	hpp_functions[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 hpp_context ctx = {
+		.s		= 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 (!hpp_functions[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 (!hpp_functions[i].cond)
+			continue;
+
+		hpp_functions[i].header(&ctx);
+
+		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 (!hpp_functions[i].cond)
+				continue;
 
-		snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+			if (hpp_functions[i].color)
+				hpp_functions[i].color(&ctx, h);
+			else
+				hpp_functions[i].entry(&ctx, 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 ad40b3626fdb..2588da865652 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.2


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

* Re: [PATCH 3/7] perf hists: Introduce hist_period_print functions
  2012-08-20  4:52 ` [PATCH 3/7] perf hists: Introduce hist_period_print functions Namhyung Kim
@ 2012-08-20 13:08   ` Arnaldo Carvalho de Melo
  2012-08-21  8:05     ` Namhyung Kim
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-20 13:08 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim

Em Mon, Aug 20, 2012 at 01:52:07PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> Current hist print functions are messy. Refactor them using the hpp

Why? I'm not saying they aren't, just curious about the lack of an
explanation as to why they would be. Please add it to this changelog
comment.

More comments below

BTW, I applied [1,2]/7 already.

> callbacks and move common code to ui/hist.c. This will make it easy 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       | 326 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/ui/setup.c      |   8 +-
>  tools/perf/ui/stdio/hist.c | 238 ++++++---------------------------
>  tools/perf/util/hist.h     |  37 +++++
>  6 files changed, 412 insertions(+), 200 deletions(-)
>  create mode 100644 tools/perf/ui/hist.c
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index cb18b047c9c9..96dda3280ce9 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..8062b38f7971
> --- /dev/null
> +++ b/tools/perf/ui/hist.c
> @@ -0,0 +1,326 @@
> +#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 hpp_context *ctx)

The "_context" looks superfluous, why not just "perf_hpp" or just "hpp"
since probably this won't be exposed in any library outside perf itself,
right?

Also please separate the struct name from the method using __, as in
other perf classes.

So it would be:

static int hpp__header_overhead(struct hpp *hpp)

As 'ctx' also doesn't add any cue as to what it is, just like 'self',
that I've been removing over time.

> +{
> +	if (ctx->ptr)
> +		return scnprintf(ctx->s, ctx->size, "Baseline");
> +	else
> +		return scnprintf(ctx->s, ctx->size, "Overhead");
> +}
> +
> +static int hpp_width_overhead(struct hpp_context *ctx __used)
> +{
> +	return 8;
> +}
> +
> +static int hpp_color_overhead(struct hpp_context *ctx,
> +			      struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period / ctx->total_period;
> +	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
> +}
> +
> +static int hpp_entry_overhead(struct hpp_context *ctx,
> +			      struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period / ctx->total_period;
> +	return scnprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
> +}
> +
> +static int hpp_header_overhead_sys(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, " sys  ");
> +}
> +
> +static int hpp_width_overhead_sys(struct hpp_context *ctx __used)
> +{
> +	return 6;
> +}
> +
> +static int hpp_color_overhead_sys(struct hpp_context *ctx,
> +				  struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_sys / ctx->total_period;
> +	return percent_color_snprintf(ctx->s, ctx->size, "%5.2f%%", percent);
> +}
> +
> +static int hpp_entry_overhead_sys(struct hpp_context *ctx,
> +				  struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_sys / ctx->total_period;
> +	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
> +}
> +
> +static int hpp_header_overhead_us(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, " user ");
> +}
> +
> +static int hpp_width_overhead_us(struct hpp_context *ctx __used)
> +{
> +	return 6;
> +}
> +
> +static int hpp_color_overhead_us(struct hpp_context *ctx,
> +				 struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_us / ctx->total_period;
> +	return percent_color_snprintf(ctx->s, ctx->size, "%5.2f%%", percent);
> +}
> +
> +static int hpp_entry_overhead_us(struct hpp_context *ctx,
> +				 struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_us / ctx->total_period;
> +	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
> +}
> +
> +static int hpp_header_overhead_guest_sys(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, "guest sys");
> +}
> +
> +static int hpp_width_overhead_guest_sys(struct hpp_context *ctx __used)
> +{
> +	return 9;
> +}
> +
> +static int hpp_color_overhead_guest_sys(struct hpp_context *ctx,
> +					struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
> +	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
> +}
> +
> +static int hpp_entry_overhead_guest_sys(struct hpp_context *ctx,
> +					struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
> +	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
> +}
> +
> +static int hpp_header_overhead_guest_us(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, "guest usr");
> +}
> +
> +static int hpp_width_overhead_guest_us(struct hpp_context *ctx __used)
> +{
> +	return 9;
> +}
> +
> +static int hpp_color_overhead_guest_us(struct hpp_context *ctx,
> +				       struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_guest_us / ctx->total_period;
> +	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
> +}
> +
> +static int hpp_entry_overhead_guest_us(struct hpp_context *ctx,
> +				       struct hist_entry *he)
> +{
> +	double percent = 100.0 * he->period_guest_us / ctx->total_period;
> +	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
> +}
> +
> +static int hpp_header_samples(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, "  Samples  ");
> +}
> +
> +static int hpp_width_samples(struct hpp_context *ctx __used)
> +{
> +	return 11;
> +}
> +
> +static int hpp_entry_samples(struct hpp_context *ctx,
> +			     struct hist_entry *he)
> +{
> +	return scnprintf(ctx->s, ctx->size, "%11" PRIu64, he->nr_events);
> +}
> +
> +static int hpp_header_period(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, "   Period   ");
> +}
> +
> +static int hpp_width_period(struct hpp_context *ctx __used)
> +{
> +	return 12;
> +}
> +
> +static int hpp_entry_period(struct hpp_context *ctx,
> +			    struct hist_entry *he)
> +{
> +	return scnprintf(ctx->s, ctx->size, "%12" PRIu64, he->period);
> +}
> +
> +static int hpp_header_delta(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, " Delta ");
> +}
> +
> +static int hpp_width_delta(struct hpp_context *ctx __used)
> +{
> +	return 7;
> +}
> +
> +static int hpp_entry_delta(struct hpp_context *ctx,
> +			   struct hist_entry *he)
> +{
> +	struct hists *pair_hists = ctx->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 = ctx->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(ctx->s, ctx->size, "       ");
> +
> +	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
> +	return scnprintf(ctx->s, ctx->size, "%7.7s", buf);
> +}
> +
> +static int hpp_header_displ(struct hpp_context *ctx)
> +{
> +	return scnprintf(ctx->s, ctx->size, "Displ.");
> +}
> +
> +static int hpp_width_displ(struct hpp_context *ctx __used)
> +{
> +	return 6;
> +}
> +
> +static int hpp_entry_displ(struct hpp_context *ctx,
> +			   struct hist_entry *he __used)
> +{
> +	char buf[32];
> +
> +	if (!ctx->displacement)
> +		return scnprintf(ctx->s, ctx->size, "     ");
> +
> +	scnprintf(buf, sizeof(buf), "%+4ld", ctx->displacement);
> +	return scnprintf(ctx->s, ctx->size, "%6.6s", buf);
> +}
> +
> +#define HPP_COLOR_PRINT_FNS(_name)		\

HPP__

> +	.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 hist_period_print hpp_functions[] = {
> +	{ .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)

Here you used it, so lets be consistent in the other cases :)

> +{
> +	if (symbol_conf.show_cpu_utilization) {
> +		hpp_functions[PERF_HPP__OVERHEAD_SYS].cond = true;
> +		hpp_functions[PERF_HPP__OVERHEAD_US].cond = true;
> +
> +		if (perf_guest) {
> +			hpp_functions[PERF_HPP__OVERHEAD_GUEST_SYS].cond = true;
> +			hpp_functions[PERF_HPP__OVERHEAD_GUEST_US].cond = true;
> +		}
> +	}
> +
> +	if (symbol_conf.show_nr_samples)
> +		hpp_functions[PERF_HPP__SAMPLES].cond = true;
> +
> +	if (symbol_conf.show_total_period)
> +		hpp_functions[PERF_HPP__PERIOD].cond = true;
> +
> +	if (need_pair) {
> +		hpp_functions[PERF_HPP__DELTA].cond = true;
> +
> +		if (show_displacement)
> +			hpp_functions[PERF_HPP__DISPL].cond = true;
> +	}
> +}
> +
> +static inline void advance_hpp_context(struct hpp_context *ctx, int inc)
> +{
> +	ctx->s    += inc;

So far, just skimming the patch I couldn't understand what is this
'ctx->s', lets see further down...

> +	ctx->size -= inc;
> +}
> +
> +int hist_entry__period_snprintf(struct hpp_context *ctx,
> +				struct hist_entry *he, bool color)
> +{
> +	const char *sep = symbol_conf.field_sep;
> +	char *start_s = ctx->s;
> +	int i, ret;
> +
> +	if (symbol_conf.exclude_other && !he->parent)
> +		return 0;
> +
> +	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
> +		if (!hpp_functions[i].cond)
> +			continue;
> +
> +		if (!sep || i > 0) {
> +			ret = scnprintf(ctx->s, ctx->size, "%s", sep ?: "  ");
> +			advance_hpp_context(ctx, ret);
> +		}
> +
> +		if (color && hpp_functions[i].color)
> +			ret = hpp_functions[i].color(ctx, he);
> +		else
> +			ret = hpp_functions[i].entry(ctx, he);
> +
> +		advance_hpp_context(ctx, ret);

So that inc is the number of characters printed...

> +	}
> +
> +	return ctx->s - start_s;

's' is the number of characters printed? Looks like

> +}
> +
> +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..c040b03cc6f4 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 hpp_context ctx = {
> +		.s		= bf,

Oh, its not the number of chars printed, its the buffer! So why not call
it 'bf'? :-)

> +		.size		= size,

Also, what if size > sizeof(bf)?

> +		.total_period	= total_period,
> +		.displacement	= displacement,
> +		.ptr		= pair_hists,
> +	};
>  
>  	if (size == 0 || size > sizeof(bf))
> -		size = sizeof(bf);
> +		size = ctx.size = sizeof(bf);

Later on we check it...

>  
> -	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
> -					  show_displacement, displacement,
> -					  true, total_period);
> +	ret = hist_entry__period_snprintf(&ctx, 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 hpp_context dummy_ctx = {
> +		.s	= 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 (!hpp_functions[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");
> -		}
> +		hpp_functions[idx].header(&dummy_ctx);
> +		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 (!hpp_functions[idx].cond)
> +			continue;
> +
> +		if (idx)
> +			fprintf(fp, "%s", sep ?: "  ");
> +
> +		width = hpp_functions[idx].width(&dummy_ctx);
> +		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..a387e2aa4602 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 hpp_context {
> +	char *s;
> +	size_t size;
> +	u64 total_period;
> +	const char *sep;
> +	long displacement;
> +	void *ptr;
> +};

Humm, so we'll need to have this exposed here because you'll implement
gtk equivalents?

> +struct hist_period_print {
> +	bool cond;
> +	int (*header)(struct hpp_context *ctx);
> +	int (*width)(struct hpp_context *ctx);
> +	int (*color)(struct hpp_context *ctx, struct hist_entry *he);
> +	int (*entry)(struct hpp_context *ctx, struct hist_entry *he);
> +};

Using 'perf_" to separate namespaces seems required, as we'll be mixing
this with gtk stuff (qt or whatever at some point if anybody
volunteers).

But I think hist_period_print should be hpp_fmt and hpp_context renamed
to hpp_buffer or hpp_bf.

- Arnaldo

> +extern struct hist_period_print hpp_functions[];
> +
> +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 hpp_context *ctx,
> +				struct hist_entry *he, bool color);
> +
>  struct perf_evlist;
>  
>  #ifdef NO_NEWT_SUPPORT
> -- 
> 1.7.11.2

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

* Re: [PATCH 3/7] perf hists: Introduce hist_period_print functions
  2012-08-20 13:08   ` Arnaldo Carvalho de Melo
@ 2012-08-21  8:05     ` Namhyung Kim
  0 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-08-21  8:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim

On Mon, 20 Aug 2012 10:08:26 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 20, 2012 at 01:52:07PM +0900, Namhyung Kim escreveu:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>> 
>> Current hist print functions are messy. Refactor them using the hpp
>
> Why? I'm not saying they aren't, just curious about the lack of an
> explanation as to why they would be. Please add it to this changelog
> comment.

Ok, let me try it like below:

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 the using hpp callbacks and move common code to
ui/hist.c in order to make it easy to maintain and to add new features.


>
> More comments below
>
> BTW, I applied [1,2]/7 already.
>
>> callbacks and move common code to ui/hist.c. This will make it easy 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       | 326 +++++++++++++++++++++++++++++++++++++++++++++
>>  tools/perf/ui/setup.c      |   8 +-
>>  tools/perf/ui/stdio/hist.c | 238 ++++++---------------------------
>>  tools/perf/util/hist.h     |  37 +++++
>>  6 files changed, 412 insertions(+), 200 deletions(-)
>>  create mode 100644 tools/perf/ui/hist.c
>> 
>> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
>> index cb18b047c9c9..96dda3280ce9 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..8062b38f7971
>> --- /dev/null
>> +++ b/tools/perf/ui/hist.c
>> @@ -0,0 +1,326 @@
>> +#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 hpp_context *ctx)
>
> The "_context" looks superfluous, why not just "perf_hpp" or just "hpp"
> since probably this won't be exposed in any library outside perf itself,
> right?
>
> Also please separate the struct name from the method using __, as in
> other perf classes.
>
> So it would be:
>
> static int hpp__header_overhead(struct hpp *hpp)
>
> As 'ctx' also doesn't add any cue as to what it is, just like 'self',
> that I've been removing over time.
>

Ok, will do with the proposed names.


>> +{
>> +	if (ctx->ptr)
>> +		return scnprintf(ctx->s, ctx->size, "Baseline");
>> +	else
>> +		return scnprintf(ctx->s, ctx->size, "Overhead");
>> +}
>> +
>> +static int hpp_width_overhead(struct hpp_context *ctx __used)
>> +{
>> +	return 8;
>> +}
>> +
>> +static int hpp_color_overhead(struct hpp_context *ctx,
>> +			      struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period / ctx->total_period;
>> +	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
>> +}
>> +
>> +static int hpp_entry_overhead(struct hpp_context *ctx,
>> +			      struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period / ctx->total_period;
>> +	return scnprintf(ctx->s, ctx->size, "  %5.2f%%", percent);
>> +}
>> +
>> +static int hpp_header_overhead_sys(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, " sys  ");
>> +}
>> +
>> +static int hpp_width_overhead_sys(struct hpp_context *ctx __used)
>> +{
>> +	return 6;
>> +}
>> +
>> +static int hpp_color_overhead_sys(struct hpp_context *ctx,
>> +				  struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_sys / ctx->total_period;
>> +	return percent_color_snprintf(ctx->s, ctx->size, "%5.2f%%", percent);
>> +}
>> +
>> +static int hpp_entry_overhead_sys(struct hpp_context *ctx,
>> +				  struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_sys / ctx->total_period;
>> +	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
>> +}
>> +
>> +static int hpp_header_overhead_us(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, " user ");
>> +}
>> +
>> +static int hpp_width_overhead_us(struct hpp_context *ctx __used)
>> +{
>> +	return 6;
>> +}
>> +
>> +static int hpp_color_overhead_us(struct hpp_context *ctx,
>> +				 struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_us / ctx->total_period;
>> +	return percent_color_snprintf(ctx->s, ctx->size, "%5.2f%%", percent);
>> +}
>> +
>> +static int hpp_entry_overhead_us(struct hpp_context *ctx,
>> +				 struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_us / ctx->total_period;
>> +	return scnprintf(ctx->s, ctx->size, "%5.2f%%", percent);
>> +}
>> +
>> +static int hpp_header_overhead_guest_sys(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "guest sys");
>> +}
>> +
>> +static int hpp_width_overhead_guest_sys(struct hpp_context *ctx __used)
>> +{
>> +	return 9;
>> +}
>> +
>> +static int hpp_color_overhead_guest_sys(struct hpp_context *ctx,
>> +					struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
>> +	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
>> +}
>> +
>> +static int hpp_entry_overhead_guest_sys(struct hpp_context *ctx,
>> +					struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_guest_sys / ctx->total_period;
>> +	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
>> +}
>> +
>> +static int hpp_header_overhead_guest_us(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "guest usr");
>> +}
>> +
>> +static int hpp_width_overhead_guest_us(struct hpp_context *ctx __used)
>> +{
>> +	return 9;
>> +}
>> +
>> +static int hpp_color_overhead_guest_us(struct hpp_context *ctx,
>> +				       struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_guest_us / ctx->total_period;
>> +	return percent_color_snprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
>> +}
>> +
>> +static int hpp_entry_overhead_guest_us(struct hpp_context *ctx,
>> +				       struct hist_entry *he)
>> +{
>> +	double percent = 100.0 * he->period_guest_us / ctx->total_period;
>> +	return scnprintf(ctx->s, ctx->size, "  %5.2f%% ", percent);
>> +}
>> +
>> +static int hpp_header_samples(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "  Samples  ");
>> +}
>> +
>> +static int hpp_width_samples(struct hpp_context *ctx __used)
>> +{
>> +	return 11;
>> +}
>> +
>> +static int hpp_entry_samples(struct hpp_context *ctx,
>> +			     struct hist_entry *he)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "%11" PRIu64, he->nr_events);
>> +}
>> +
>> +static int hpp_header_period(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "   Period   ");
>> +}
>> +
>> +static int hpp_width_period(struct hpp_context *ctx __used)
>> +{
>> +	return 12;
>> +}
>> +
>> +static int hpp_entry_period(struct hpp_context *ctx,
>> +			    struct hist_entry *he)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "%12" PRIu64, he->period);
>> +}
>> +
>> +static int hpp_header_delta(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, " Delta ");
>> +}
>> +
>> +static int hpp_width_delta(struct hpp_context *ctx __used)
>> +{
>> +	return 7;
>> +}
>> +
>> +static int hpp_entry_delta(struct hpp_context *ctx,
>> +			   struct hist_entry *he)
>> +{
>> +	struct hists *pair_hists = ctx->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 = ctx->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(ctx->s, ctx->size, "       ");
>> +
>> +	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
>> +	return scnprintf(ctx->s, ctx->size, "%7.7s", buf);
>> +}
>> +
>> +static int hpp_header_displ(struct hpp_context *ctx)
>> +{
>> +	return scnprintf(ctx->s, ctx->size, "Displ.");
>> +}
>> +
>> +static int hpp_width_displ(struct hpp_context *ctx __used)
>> +{
>> +	return 6;
>> +}
>> +
>> +static int hpp_entry_displ(struct hpp_context *ctx,
>> +			   struct hist_entry *he __used)
>> +{
>> +	char buf[32];
>> +
>> +	if (!ctx->displacement)
>> +		return scnprintf(ctx->s, ctx->size, "     ");
>> +
>> +	scnprintf(buf, sizeof(buf), "%+4ld", ctx->displacement);
>> +	return scnprintf(ctx->s, ctx->size, "%6.6s", buf);
>> +}
>> +
>> +#define HPP_COLOR_PRINT_FNS(_name)		\
>
> HPP__
>
>> +	.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 hist_period_print hpp_functions[] = {
>> +	{ .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)
>
> Here you used it, so lets be consistent in the other cases :)
>

I was using the double '__' (and 'perf' prefix) only for the external
functions.  But if you want to use it for all cases, I can do it.


>> +{
>> +	if (symbol_conf.show_cpu_utilization) {
>> +		hpp_functions[PERF_HPP__OVERHEAD_SYS].cond = true;
>> +		hpp_functions[PERF_HPP__OVERHEAD_US].cond = true;
>> +
>> +		if (perf_guest) {
>> +			hpp_functions[PERF_HPP__OVERHEAD_GUEST_SYS].cond = true;
>> +			hpp_functions[PERF_HPP__OVERHEAD_GUEST_US].cond = true;
>> +		}
>> +	}
>> +
>> +	if (symbol_conf.show_nr_samples)
>> +		hpp_functions[PERF_HPP__SAMPLES].cond = true;
>> +
>> +	if (symbol_conf.show_total_period)
>> +		hpp_functions[PERF_HPP__PERIOD].cond = true;
>> +
>> +	if (need_pair) {
>> +		hpp_functions[PERF_HPP__DELTA].cond = true;
>> +
>> +		if (show_displacement)
>> +			hpp_functions[PERF_HPP__DISPL].cond = true;
>> +	}
>> +}
>> +
>> +static inline void advance_hpp_context(struct hpp_context *ctx, int inc)
>> +{
>> +	ctx->s    += inc;
>
> So far, just skimming the patch I couldn't understand what is this
> 'ctx->s', lets see further down...
>

It's a pointer to a internal string buffer. The name came from the
original code.


>> +	ctx->size -= inc;
>> +}
>> +
>> +int hist_entry__period_snprintf(struct hpp_context *ctx,
>> +				struct hist_entry *he, bool color)
>> +{
>> +	const char *sep = symbol_conf.field_sep;
>> +	char *start_s = ctx->s;
>> +	int i, ret;
>> +
>> +	if (symbol_conf.exclude_other && !he->parent)
>> +		return 0;
>> +
>> +	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
>> +		if (!hpp_functions[i].cond)
>> +			continue;
>> +
>> +		if (!sep || i > 0) {
>> +			ret = scnprintf(ctx->s, ctx->size, "%s", sep ?: "  ");
>> +			advance_hpp_context(ctx, ret);
>> +		}
>> +
>> +		if (color && hpp_functions[i].color)
>> +			ret = hpp_functions[i].color(ctx, he);
>> +		else
>> +			ret = hpp_functions[i].entry(ctx, he);
>> +
>> +		advance_hpp_context(ctx, ret);
>
> So that inc is the number of characters printed...
>
>> +	}
>> +
>> +	return ctx->s - start_s;
>
> 's' is the number of characters printed? Looks like
>
>> +}
>> +
>> +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..c040b03cc6f4 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 hpp_context ctx = {
>> +		.s		= bf,
>
> Oh, its not the number of chars printed, its the buffer! So why not call
> it 'bf'? :-)
>

I'd rather use 'buf'.


>> +		.size		= size,
>
> Also, what if size > sizeof(bf)?
>
>> +		.total_period	= total_period,
>> +		.displacement	= displacement,
>> +		.ptr		= pair_hists,
>> +	};
>>  
>>  	if (size == 0 || size > sizeof(bf))
>> -		size = sizeof(bf);
>> +		size = ctx.size = sizeof(bf);
>
> Later on we check it...
>
>>  
>> -	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
>> -					  show_displacement, displacement,
>> -					  true, total_period);
>> +	ret = hist_entry__period_snprintf(&ctx, 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 hpp_context dummy_ctx = {
>> +		.s	= 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 (!hpp_functions[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");
>> -		}
>> +		hpp_functions[idx].header(&dummy_ctx);
>> +		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 (!hpp_functions[idx].cond)
>> +			continue;
>> +
>> +		if (idx)
>> +			fprintf(fp, "%s", sep ?: "  ");
>> +
>> +		width = hpp_functions[idx].width(&dummy_ctx);
>> +		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..a387e2aa4602 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 hpp_context {
>> +	char *s;
>> +	size_t size;
>> +	u64 total_period;
>> +	const char *sep;
>> +	long displacement;
>> +	void *ptr;
>> +};
>
> Humm, so we'll need to have this exposed here because you'll implement
> gtk equivalents?

Yes.


>
>> +struct hist_period_print {
>> +	bool cond;
>> +	int (*header)(struct hpp_context *ctx);
>> +	int (*width)(struct hpp_context *ctx);
>> +	int (*color)(struct hpp_context *ctx, struct hist_entry *he);
>> +	int (*entry)(struct hpp_context *ctx, struct hist_entry *he);
>> +};
>
> Using 'perf_" to separate namespaces seems required, as we'll be mixing
> this with gtk stuff (qt or whatever at some point if anybody
> volunteers).
>
> But I think hist_period_print should be hpp_fmt and hpp_context renamed
> to hpp_buffer or hpp_bf.
>
> - Arnaldo
>

How about this?

struct perf_hpp_fmt {
	bool cond;
	int (*header)(struct perf_hpp *hpp);
	int (*width)(struct perf_hpp *hpp);
	...
}

Thanks,
Namhyung


>> +extern struct hist_period_print hpp_functions[];
>> +
>> +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 hpp_context *ctx,
>> +				struct hist_entry *he, bool color);
>> +
>>  struct perf_evlist;
>>  
>>  #ifdef NO_NEWT_SUPPORT
>> -- 
>> 1.7.11.2

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

* [tip:perf/core] perf hists: Separate out hist print functions
  2012-08-20  4:52 ` [PATCH 1/7] perf hists: Separate out hist print functions Namhyung Kim
@ 2012-08-21 16:34   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-08-21 16:34 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, tglx

Commit-ID:  7ccf4f9058ecff6eec11a271001d08d9024da8c0
Gitweb:     http://git.kernel.org/tip/7ccf4f9058ecff6eec11a271001d08d9024da8c0
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 20 Aug 2012 13:52:05 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 20 Aug 2012 09:46:34 -0300

perf hists: Separate out hist print functions

Separate out those functions into ui/stdio/hist.c. This is required for
upcoming changes.

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/1345438331-20234-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile        |    3 +-
 tools/perf/ui/stdio/hist.c |  648 ++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.c     |  677 ++------------------------------------------
 tools/perf/util/hist.h     |    2 +
 4 files changed, 669 insertions(+), 661 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 77e7ae3e..6bd888d 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -404,11 +404,10 @@ 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/stdio/hist.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
-
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
-
 # Benchmark modules
 BUILTIN_OBJS += $(OUTPUT)bench/sched-messaging.o
 BUILTIN_OBJS += $(OUTPUT)bench/sched-pipe.o
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
new file mode 100644
index 0000000..7881d62
--- /dev/null
+++ b/tools/perf/ui/stdio/hist.c
@@ -0,0 +1,648 @@
+#include <stdio.h>
+#include <math.h>
+
+#include "../../util/util.h"
+#include "../../util/hist.h"
+#include "../../util/sort.h"
+
+
+static size_t callchain__fprintf_left_margin(FILE *fp, int left_margin)
+{
+	int i;
+	int ret = fprintf(fp, "            ");
+
+	for (i = 0; i < left_margin; i++)
+		ret += fprintf(fp, " ");
+
+	return ret;
+}
+
+static size_t ipchain__fprintf_graph_line(FILE *fp, int depth, int depth_mask,
+					  int left_margin)
+{
+	int i;
+	size_t ret = callchain__fprintf_left_margin(fp, left_margin);
+
+	for (i = 0; i < depth; i++)
+		if (depth_mask & (1 << i))
+			ret += fprintf(fp, "|          ");
+		else
+			ret += fprintf(fp, "           ");
+
+	ret += fprintf(fp, "\n");
+
+	return ret;
+}
+
+static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
+				     int depth, int depth_mask, int period,
+				     u64 total_samples, u64 hits,
+				     int left_margin)
+{
+	int i;
+	size_t ret = 0;
+
+	ret += callchain__fprintf_left_margin(fp, left_margin);
+	for (i = 0; i < depth; i++) {
+		if (depth_mask & (1 << i))
+			ret += fprintf(fp, "|");
+		else
+			ret += fprintf(fp, " ");
+		if (!period && i == depth - 1) {
+			double percent;
+
+			percent = hits * 100.0 / total_samples;
+			ret += percent_color_fprintf(fp, "--%2.2f%%-- ", percent);
+		} else
+			ret += fprintf(fp, "%s", "          ");
+	}
+	if (chain->ms.sym)
+		ret += fprintf(fp, "%s\n", chain->ms.sym->name);
+	else
+		ret += fprintf(fp, "0x%0" PRIx64 "\n", chain->ip);
+
+	return ret;
+}
+
+static struct symbol *rem_sq_bracket;
+static struct callchain_list rem_hits;
+
+static void init_rem_hits(void)
+{
+	rem_sq_bracket = malloc(sizeof(*rem_sq_bracket) + 6);
+	if (!rem_sq_bracket) {
+		fprintf(stderr, "Not enough memory to display remaining hits\n");
+		return;
+	}
+
+	strcpy(rem_sq_bracket->name, "[...]");
+	rem_hits.ms.sym = rem_sq_bracket;
+}
+
+static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
+					 u64 total_samples, int depth,
+					 int depth_mask, int left_margin)
+{
+	struct rb_node *node, *next;
+	struct callchain_node *child;
+	struct callchain_list *chain;
+	int new_depth_mask = depth_mask;
+	u64 remaining;
+	size_t ret = 0;
+	int i;
+	uint entries_printed = 0;
+
+	remaining = total_samples;
+
+	node = rb_first(root);
+	while (node) {
+		u64 new_total;
+		u64 cumul;
+
+		child = rb_entry(node, struct callchain_node, rb_node);
+		cumul = callchain_cumul_hits(child);
+		remaining -= cumul;
+
+		/*
+		 * The depth mask manages the output of pipes that show
+		 * the depth. We don't want to keep the pipes of the current
+		 * level for the last child of this depth.
+		 * Except if we have remaining filtered hits. They will
+		 * supersede the last child
+		 */
+		next = rb_next(node);
+		if (!next && (callchain_param.mode != CHAIN_GRAPH_REL || !remaining))
+			new_depth_mask &= ~(1 << (depth - 1));
+
+		/*
+		 * But we keep the older depth mask for the line separator
+		 * to keep the level link until we reach the last child
+		 */
+		ret += ipchain__fprintf_graph_line(fp, depth, depth_mask,
+						   left_margin);
+		i = 0;
+		list_for_each_entry(chain, &child->val, list) {
+			ret += ipchain__fprintf_graph(fp, chain, depth,
+						      new_depth_mask, i++,
+						      total_samples,
+						      cumul,
+						      left_margin);
+		}
+
+		if (callchain_param.mode == CHAIN_GRAPH_REL)
+			new_total = child->children_hit;
+		else
+			new_total = total_samples;
+
+		ret += __callchain__fprintf_graph(fp, &child->rb_root, new_total,
+						  depth + 1,
+						  new_depth_mask | (1 << depth),
+						  left_margin);
+		node = next;
+		if (++entries_printed == callchain_param.print_limit)
+			break;
+	}
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL &&
+		remaining && remaining != total_samples) {
+
+		if (!rem_sq_bracket)
+			return ret;
+
+		new_depth_mask &= ~(1 << (depth - 1));
+		ret += ipchain__fprintf_graph(fp, &rem_hits, depth,
+					      new_depth_mask, 0, total_samples,
+					      remaining, left_margin);
+	}
+
+	return ret;
+}
+
+static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
+				       u64 total_samples, int left_margin)
+{
+	struct callchain_node *cnode;
+	struct callchain_list *chain;
+	u32 entries_printed = 0;
+	bool printed = false;
+	struct rb_node *node;
+	int i = 0;
+	int ret = 0;
+
+	/*
+	 * If have one single callchain root, don't bother printing
+	 * its percentage (100 % in fractal mode and the same percentage
+	 * than the hist in graph mode). This also avoid one level of column.
+	 */
+	node = rb_first(root);
+	if (node && !rb_next(node)) {
+		cnode = rb_entry(node, struct callchain_node, rb_node);
+		list_for_each_entry(chain, &cnode->val, list) {
+			/*
+			 * If we sort by symbol, the first entry is the same than
+			 * the symbol. No need to print it otherwise it appears as
+			 * displayed twice.
+			 */
+			if (!i++ && sort__first_dimension == SORT_SYM)
+				continue;
+			if (!printed) {
+				ret += callchain__fprintf_left_margin(fp, left_margin);
+				ret += fprintf(fp, "|\n");
+				ret += callchain__fprintf_left_margin(fp, left_margin);
+				ret += fprintf(fp, "---");
+				left_margin += 3;
+				printed = true;
+			} else
+				ret += callchain__fprintf_left_margin(fp, left_margin);
+
+			if (chain->ms.sym)
+				ret += fprintf(fp, " %s\n", chain->ms.sym->name);
+			else
+				ret += fprintf(fp, " %p\n", (void *)(long)chain->ip);
+
+			if (++entries_printed == callchain_param.print_limit)
+				break;
+		}
+		root = &cnode->rb_root;
+	}
+
+	ret += __callchain__fprintf_graph(fp, root, total_samples,
+					  1, 1, left_margin);
+	ret += fprintf(fp, "\n");
+
+	return ret;
+}
+
+static size_t __callchain__fprintf_flat(FILE *fp,
+					struct callchain_node *self,
+					u64 total_samples)
+{
+	struct callchain_list *chain;
+	size_t ret = 0;
+
+	if (!self)
+		return 0;
+
+	ret += __callchain__fprintf_flat(fp, self->parent, total_samples);
+
+
+	list_for_each_entry(chain, &self->val, list) {
+		if (chain->ip >= PERF_CONTEXT_MAX)
+			continue;
+		if (chain->ms.sym)
+			ret += fprintf(fp, "                %s\n", chain->ms.sym->name);
+		else
+			ret += fprintf(fp, "                %p\n",
+					(void *)(long)chain->ip);
+	}
+
+	return ret;
+}
+
+static size_t callchain__fprintf_flat(FILE *fp, struct rb_root *self,
+				      u64 total_samples)
+{
+	size_t ret = 0;
+	u32 entries_printed = 0;
+	struct rb_node *rb_node;
+	struct callchain_node *chain;
+
+	rb_node = rb_first(self);
+	while (rb_node) {
+		double percent;
+
+		chain = rb_entry(rb_node, struct callchain_node, rb_node);
+		percent = chain->hit * 100.0 / total_samples;
+
+		ret = percent_color_fprintf(fp, "           %6.2f%%\n", percent);
+		ret += __callchain__fprintf_flat(fp, chain, total_samples);
+		ret += fprintf(fp, "\n");
+		if (++entries_printed == callchain_param.print_limit)
+			break;
+
+		rb_node = rb_next(rb_node);
+	}
+
+	return ret;
+}
+
+static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
+					    u64 total_samples, int left_margin,
+					    FILE *fp)
+{
+	switch (callchain_param.mode) {
+	case CHAIN_GRAPH_REL:
+		return callchain__fprintf_graph(fp, &he->sorted_chain, he->period,
+						left_margin);
+		break;
+	case CHAIN_GRAPH_ABS:
+		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
+						left_margin);
+		break;
+	case CHAIN_FLAT:
+		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);
+		break;
+	case CHAIN_NONE:
+		break;
+	default:
+		pr_err("Bad callchain mode\n");
+	}
+
+	return 0;
+}
+
+static int hist_entry__pcnt_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__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 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)
+{
+	char bf[512];
+	int ret;
+
+	if (size == 0 || size > sizeof(bf))
+		size = sizeof(bf);
+
+	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
+					show_displacement, displacement,
+					true, total_period);
+	hist_entry__snprintf(he, bf + ret, size - ret, hists);
+	return fprintf(fp, "%s\n", bf);
+}
+
+static size_t hist_entry__fprintf_callchain(struct hist_entry *he,
+					    struct hists *hists,
+					    u64 total_period, FILE *fp)
+{
+	int left_margin = 0;
+
+	if (sort__first_dimension == SORT_COMM) {
+		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
+							 typeof(*se), list);
+		left_margin = hists__col_len(hists, se->se_width_idx);
+		left_margin -= thread__comm_len(he->thread);
+	}
+
+	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
+}
+
+size_t hists__fprintf(struct hists *hists, struct hists *pair,
+		      bool show_displacement, bool show_header, int max_rows,
+		      int max_cols, FILE *fp)
+{
+	struct sort_entry *se;
+	struct rb_node *nd;
+	size_t ret = 0;
+	u64 total_period;
+	unsigned long position = 1;
+	long displacement = 0;
+	unsigned int width;
+	const char *sep = symbol_conf.field_sep;
+	const char *col_width = symbol_conf.col_width_list_str;
+	int nr_rows = 0;
+
+	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    ");
+	}
+
+	if (pair) {
+		if (sep)
+			ret += fprintf(fp, "%cDelta", *sep);
+		else
+			ret += fprintf(fp, "  Delta    ");
+
+		if (show_displacement) {
+			if (sep)
+				ret += fprintf(fp, "%cDisplacement", *sep);
+			else
+				ret += fprintf(fp, " Displ");
+		}
+	}
+
+	list_for_each_entry(se, &hist_entry__sort_list, list) {
+		if (se->elide)
+			continue;
+		if (sep) {
+			fprintf(fp, "%c%s", *sep, se->se_header);
+			continue;
+		}
+		width = strlen(se->se_header);
+		if (symbol_conf.col_width_list_str) {
+			if (col_width) {
+				hists__set_col_len(hists, se->se_width_idx,
+						   atoi(col_width));
+				col_width = strchr(col_width, ',');
+				if (col_width)
+					++col_width;
+			}
+		}
+		if (!hists__new_col_len(hists, se->se_width_idx, width))
+			width = hists__col_len(hists, se->se_width_idx);
+		fprintf(fp, "  %*s", width, se->se_header);
+	}
+
+	fprintf(fp, "\n");
+	if (max_rows && ++nr_rows >= max_rows)
+		goto out;
+
+	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, " .....");
+	}
+	list_for_each_entry(se, &hist_entry__sort_list, list) {
+		unsigned int i;
+
+		if (se->elide)
+			continue;
+
+		fprintf(fp, "  ");
+		width = hists__col_len(hists, se->se_width_idx);
+		if (width == 0)
+			width = strlen(se->se_header);
+		for (i = 0; i < width; i++)
+			fprintf(fp, ".");
+	}
+
+	fprintf(fp, "\n");
+	if (max_rows && ++nr_rows >= max_rows)
+		goto out;
+
+	fprintf(fp, "#\n");
+	if (max_rows && ++nr_rows >= max_rows)
+		goto out;
+
+print_entries:
+	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);
+
+		if (h->filtered)
+			continue;
+
+		if (show_displacement) {
+			if (h->pair != NULL)
+				displacement = ((long)h->pair->position -
+					        (long)position);
+			else
+				displacement = 0;
+			++position;
+		}
+		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
+					   displacement, total_period, fp);
+
+		if (symbol_conf.use_callchain)
+			ret += hist_entry__fprintf_callchain(h, hists, total_period, fp);
+		if (max_rows && ++nr_rows >= max_rows)
+			goto out;
+
+		if (h->ms.map == NULL && verbose > 1) {
+			__map_groups__fprintf_maps(&h->thread->mg,
+						   MAP__FUNCTION, verbose, fp);
+			fprintf(fp, "%.10s end\n", graph_dotted_line);
+		}
+	}
+out:
+	free(rem_sq_bracket);
+
+	return ret;
+}
+
+size_t hists__fprintf_nr_events(struct hists *hists, FILE *fp)
+{
+	int i;
+	size_t ret = 0;
+
+	for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
+		const char *name;
+
+		if (hists->stats.nr_events[i] == 0)
+			continue;
+
+		name = perf_event__name(i);
+		if (!strcmp(name, "UNKNOWN"))
+			continue;
+
+		ret += fprintf(fp, "%16s events: %10d\n", name,
+			       hists->stats.nr_events[i]);
+	}
+
+	return ret;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f247ef2..b1817f1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -45,7 +45,7 @@ bool hists__new_col_len(struct hists *hists, enum hist_column col, u16 len)
 	return false;
 }
 
-static void hists__reset_col_len(struct hists *hists)
+void hists__reset_col_len(struct hists *hists)
 {
 	enum hist_column col;
 
@@ -63,7 +63,7 @@ static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
 		hists__set_col_len(hists, dso, unresolved_col_width);
 }
 
-static void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
+void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 {
 	const unsigned int unresolved_col_width = BITS_PER_LONG / 4;
 	u16 len;
@@ -114,6 +114,22 @@ static void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	}
 }
 
+void hists__output_recalc_col_len(struct hists *hists, int max_rows)
+{
+	struct rb_node *next = rb_first(&hists->entries);
+	struct hist_entry *n;
+	int row = 0;
+
+	hists__reset_col_len(hists);
+
+	while (next && row++ < max_rows) {
+		n = rb_entry(next, struct hist_entry, rb_node);
+		if (!n->filtered)
+			hists__calc_col_len(hists, n);
+		next = rb_next(&n->rb_node);
+	}
+}
+
 static void hist_entry__add_cpumode_period(struct hist_entry *he,
 					   unsigned int cpumode, u64 period)
 {
@@ -547,641 +563,6 @@ void hists__output_resort_threaded(struct hists *hists)
 	return __hists__output_resort(hists, true);
 }
 
-static size_t callchain__fprintf_left_margin(FILE *fp, int left_margin)
-{
-	int i;
-	int ret = fprintf(fp, "            ");
-
-	for (i = 0; i < left_margin; i++)
-		ret += fprintf(fp, " ");
-
-	return ret;
-}
-
-static size_t ipchain__fprintf_graph_line(FILE *fp, int depth, int depth_mask,
-					  int left_margin)
-{
-	int i;
-	size_t ret = callchain__fprintf_left_margin(fp, left_margin);
-
-	for (i = 0; i < depth; i++)
-		if (depth_mask & (1 << i))
-			ret += fprintf(fp, "|          ");
-		else
-			ret += fprintf(fp, "           ");
-
-	ret += fprintf(fp, "\n");
-
-	return ret;
-}
-
-static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
-				     int depth, int depth_mask, int period,
-				     u64 total_samples, u64 hits,
-				     int left_margin)
-{
-	int i;
-	size_t ret = 0;
-
-	ret += callchain__fprintf_left_margin(fp, left_margin);
-	for (i = 0; i < depth; i++) {
-		if (depth_mask & (1 << i))
-			ret += fprintf(fp, "|");
-		else
-			ret += fprintf(fp, " ");
-		if (!period && i == depth - 1) {
-			double percent;
-
-			percent = hits * 100.0 / total_samples;
-			ret += percent_color_fprintf(fp, "--%2.2f%%-- ", percent);
-		} else
-			ret += fprintf(fp, "%s", "          ");
-	}
-	if (chain->ms.sym)
-		ret += fprintf(fp, "%s\n", chain->ms.sym->name);
-	else
-		ret += fprintf(fp, "0x%0" PRIx64 "\n", chain->ip);
-
-	return ret;
-}
-
-static struct symbol *rem_sq_bracket;
-static struct callchain_list rem_hits;
-
-static void init_rem_hits(void)
-{
-	rem_sq_bracket = malloc(sizeof(*rem_sq_bracket) + 6);
-	if (!rem_sq_bracket) {
-		fprintf(stderr, "Not enough memory to display remaining hits\n");
-		return;
-	}
-
-	strcpy(rem_sq_bracket->name, "[...]");
-	rem_hits.ms.sym = rem_sq_bracket;
-}
-
-static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-					 u64 total_samples, int depth,
-					 int depth_mask, int left_margin)
-{
-	struct rb_node *node, *next;
-	struct callchain_node *child;
-	struct callchain_list *chain;
-	int new_depth_mask = depth_mask;
-	u64 remaining;
-	size_t ret = 0;
-	int i;
-	uint entries_printed = 0;
-
-	remaining = total_samples;
-
-	node = rb_first(root);
-	while (node) {
-		u64 new_total;
-		u64 cumul;
-
-		child = rb_entry(node, struct callchain_node, rb_node);
-		cumul = callchain_cumul_hits(child);
-		remaining -= cumul;
-
-		/*
-		 * The depth mask manages the output of pipes that show
-		 * the depth. We don't want to keep the pipes of the current
-		 * level for the last child of this depth.
-		 * Except if we have remaining filtered hits. They will
-		 * supersede the last child
-		 */
-		next = rb_next(node);
-		if (!next && (callchain_param.mode != CHAIN_GRAPH_REL || !remaining))
-			new_depth_mask &= ~(1 << (depth - 1));
-
-		/*
-		 * But we keep the older depth mask for the line separator
-		 * to keep the level link until we reach the last child
-		 */
-		ret += ipchain__fprintf_graph_line(fp, depth, depth_mask,
-						   left_margin);
-		i = 0;
-		list_for_each_entry(chain, &child->val, list) {
-			ret += ipchain__fprintf_graph(fp, chain, depth,
-						      new_depth_mask, i++,
-						      total_samples,
-						      cumul,
-						      left_margin);
-		}
-
-		if (callchain_param.mode == CHAIN_GRAPH_REL)
-			new_total = child->children_hit;
-		else
-			new_total = total_samples;
-
-		ret += __callchain__fprintf_graph(fp, &child->rb_root, new_total,
-						  depth + 1,
-						  new_depth_mask | (1 << depth),
-						  left_margin);
-		node = next;
-		if (++entries_printed == callchain_param.print_limit)
-			break;
-	}
-
-	if (callchain_param.mode == CHAIN_GRAPH_REL &&
-		remaining && remaining != total_samples) {
-
-		if (!rem_sq_bracket)
-			return ret;
-
-		new_depth_mask &= ~(1 << (depth - 1));
-		ret += ipchain__fprintf_graph(fp, &rem_hits, depth,
-					      new_depth_mask, 0, total_samples,
-					      remaining, left_margin);
-	}
-
-	return ret;
-}
-
-static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-				       u64 total_samples, int left_margin)
-{
-	struct callchain_node *cnode;
-	struct callchain_list *chain;
-	u32 entries_printed = 0;
-	bool printed = false;
-	struct rb_node *node;
-	int i = 0;
-	int ret = 0;
-
-	/*
-	 * If have one single callchain root, don't bother printing
-	 * its percentage (100 % in fractal mode and the same percentage
-	 * than the hist in graph mode). This also avoid one level of column.
-	 */
-	node = rb_first(root);
-	if (node && !rb_next(node)) {
-		cnode = rb_entry(node, struct callchain_node, rb_node);
-		list_for_each_entry(chain, &cnode->val, list) {
-			/*
-			 * If we sort by symbol, the first entry is the same than
-			 * the symbol. No need to print it otherwise it appears as
-			 * displayed twice.
-			 */
-			if (!i++ && sort__first_dimension == SORT_SYM)
-				continue;
-			if (!printed) {
-				ret += callchain__fprintf_left_margin(fp, left_margin);
-				ret += fprintf(fp, "|\n");
-				ret += callchain__fprintf_left_margin(fp, left_margin);
-				ret += fprintf(fp, "---");
-				left_margin += 3;
-				printed = true;
-			} else
-				ret += callchain__fprintf_left_margin(fp, left_margin);
-
-			if (chain->ms.sym)
-				ret += fprintf(fp, " %s\n", chain->ms.sym->name);
-			else
-				ret += fprintf(fp, " %p\n", (void *)(long)chain->ip);
-
-			if (++entries_printed == callchain_param.print_limit)
-				break;
-		}
-		root = &cnode->rb_root;
-	}
-
-	ret += __callchain__fprintf_graph(fp, root, total_samples,
-					  1, 1, left_margin);
-	ret += fprintf(fp, "\n");
-
-	return ret;
-}
-
-static size_t __callchain__fprintf_flat(FILE *fp,
-					struct callchain_node *self,
-					u64 total_samples)
-{
-	struct callchain_list *chain;
-	size_t ret = 0;
-
-	if (!self)
-		return 0;
-
-	ret += __callchain__fprintf_flat(fp, self->parent, total_samples);
-
-
-	list_for_each_entry(chain, &self->val, list) {
-		if (chain->ip >= PERF_CONTEXT_MAX)
-			continue;
-		if (chain->ms.sym)
-			ret += fprintf(fp, "                %s\n", chain->ms.sym->name);
-		else
-			ret += fprintf(fp, "                %p\n",
-					(void *)(long)chain->ip);
-	}
-
-	return ret;
-}
-
-static size_t callchain__fprintf_flat(FILE *fp, struct rb_root *self,
-				      u64 total_samples)
-{
-	size_t ret = 0;
-	u32 entries_printed = 0;
-	struct rb_node *rb_node;
-	struct callchain_node *chain;
-
-	rb_node = rb_first(self);
-	while (rb_node) {
-		double percent;
-
-		chain = rb_entry(rb_node, struct callchain_node, rb_node);
-		percent = chain->hit * 100.0 / total_samples;
-
-		ret = percent_color_fprintf(fp, "           %6.2f%%\n", percent);
-		ret += __callchain__fprintf_flat(fp, chain, total_samples);
-		ret += fprintf(fp, "\n");
-		if (++entries_printed == callchain_param.print_limit)
-			break;
-
-		rb_node = rb_next(rb_node);
-	}
-
-	return ret;
-}
-
-static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
-					    u64 total_samples, int left_margin,
-					    FILE *fp)
-{
-	switch (callchain_param.mode) {
-	case CHAIN_GRAPH_REL:
-		return callchain__fprintf_graph(fp, &he->sorted_chain, he->period,
-						left_margin);
-		break;
-	case CHAIN_GRAPH_ABS:
-		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
-						left_margin);
-		break;
-	case CHAIN_FLAT:
-		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);
-		break;
-	case CHAIN_NONE:
-		break;
-	default:
-		pr_err("Bad callchain mode\n");
-	}
-
-	return 0;
-}
-
-void hists__output_recalc_col_len(struct hists *hists, int max_rows)
-{
-	struct rb_node *next = rb_first(&hists->entries);
-	struct hist_entry *n;
-	int row = 0;
-
-	hists__reset_col_len(hists);
-
-	while (next && row++ < max_rows) {
-		n = rb_entry(next, struct hist_entry, rb_node);
-		if (!n->filtered)
-			hists__calc_col_len(hists, n);
-		next = rb_next(&n->rb_node);
-	}
-}
-
-static int hist_entry__pcnt_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__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 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)
-{
-	char bf[512];
-	int ret;
-
-	if (size == 0 || size > sizeof(bf))
-		size = sizeof(bf);
-
-	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
-					show_displacement, displacement,
-					true, total_period);
-	hist_entry__snprintf(he, bf + ret, size - ret, hists);
-	return fprintf(fp, "%s\n", bf);
-}
-
-static size_t hist_entry__fprintf_callchain(struct hist_entry *he,
-					    struct hists *hists,
-					    u64 total_period, FILE *fp)
-{
-	int left_margin = 0;
-
-	if (sort__first_dimension == SORT_COMM) {
-		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
-							 typeof(*se), list);
-		left_margin = hists__col_len(hists, se->se_width_idx);
-		left_margin -= thread__comm_len(he->thread);
-	}
-
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
-}
-
-size_t hists__fprintf(struct hists *hists, struct hists *pair,
-		      bool show_displacement, bool show_header, int max_rows,
-		      int max_cols, FILE *fp)
-{
-	struct sort_entry *se;
-	struct rb_node *nd;
-	size_t ret = 0;
-	u64 total_period;
-	unsigned long position = 1;
-	long displacement = 0;
-	unsigned int width;
-	const char *sep = symbol_conf.field_sep;
-	const char *col_width = symbol_conf.col_width_list_str;
-	int nr_rows = 0;
-
-	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    ");
-	}
-
-	if (pair) {
-		if (sep)
-			ret += fprintf(fp, "%cDelta", *sep);
-		else
-			ret += fprintf(fp, "  Delta    ");
-
-		if (show_displacement) {
-			if (sep)
-				ret += fprintf(fp, "%cDisplacement", *sep);
-			else
-				ret += fprintf(fp, " Displ");
-		}
-	}
-
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		if (se->elide)
-			continue;
-		if (sep) {
-			fprintf(fp, "%c%s", *sep, se->se_header);
-			continue;
-		}
-		width = strlen(se->se_header);
-		if (symbol_conf.col_width_list_str) {
-			if (col_width) {
-				hists__set_col_len(hists, se->se_width_idx,
-						   atoi(col_width));
-				col_width = strchr(col_width, ',');
-				if (col_width)
-					++col_width;
-			}
-		}
-		if (!hists__new_col_len(hists, se->se_width_idx, width))
-			width = hists__col_len(hists, se->se_width_idx);
-		fprintf(fp, "  %*s", width, se->se_header);
-	}
-
-	fprintf(fp, "\n");
-	if (max_rows && ++nr_rows >= max_rows)
-		goto out;
-
-	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, " .....");
-	}
-	list_for_each_entry(se, &hist_entry__sort_list, list) {
-		unsigned int i;
-
-		if (se->elide)
-			continue;
-
-		fprintf(fp, "  ");
-		width = hists__col_len(hists, se->se_width_idx);
-		if (width == 0)
-			width = strlen(se->se_header);
-		for (i = 0; i < width; i++)
-			fprintf(fp, ".");
-	}
-
-	fprintf(fp, "\n");
-	if (max_rows && ++nr_rows >= max_rows)
-		goto out;
-
-	fprintf(fp, "#\n");
-	if (max_rows && ++nr_rows >= max_rows)
-		goto out;
-
-print_entries:
-	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);
-
-		if (h->filtered)
-			continue;
-
-		if (show_displacement) {
-			if (h->pair != NULL)
-				displacement = ((long)h->pair->position -
-					        (long)position);
-			else
-				displacement = 0;
-			++position;
-		}
-		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
-					   displacement, total_period, fp);
-
-		if (symbol_conf.use_callchain)
-			ret += hist_entry__fprintf_callchain(h, hists, total_period, fp);
-		if (max_rows && ++nr_rows >= max_rows)
-			goto out;
-
-		if (h->ms.map == NULL && verbose > 1) {
-			__map_groups__fprintf_maps(&h->thread->mg,
-						   MAP__FUNCTION, verbose, fp);
-			fprintf(fp, "%.10s end\n", graph_dotted_line);
-		}
-	}
-out:
-	free(rem_sq_bracket);
-
-	return ret;
-}
-
 /*
  * See hists__fprintf to match the column widths
  */
@@ -1342,25 +723,3 @@ void hists__inc_nr_events(struct hists *hists, u32 type)
 	++hists->stats.nr_events[0];
 	++hists->stats.nr_events[type];
 }
-
-size_t hists__fprintf_nr_events(struct hists *hists, FILE *fp)
-{
-	int i;
-	size_t ret = 0;
-
-	for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
-		const char *name;
-
-		if (hists->stats.nr_events[i] == 0)
-			continue;
-
-		name = perf_event__name(i);
-		if (!strcmp(name, "UNKNOWN"))
-			continue;
-
-		ret += fprintf(fp, "%16s events: %10d\n", name,
-			       hists->stats.nr_events[i]);
-	}
-
-	return ret;
-}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 0b096c2..69fab7d 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -112,6 +112,8 @@ void hists__filter_by_symbol(struct hists *hists);
 u16 hists__col_len(struct hists *self, enum hist_column col);
 void hists__set_col_len(struct hists *self, enum hist_column col, u16 len);
 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_evlist;
 

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

* [tip:perf/core] perf hists: Rename and move some functions
  2012-08-20  4:52 ` [PATCH 2/7] perf hists: Refactor some functions Namhyung Kim
@ 2012-08-21 16:34   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-08-21 16:34 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, tglx

Commit-ID:  000078bc3ee69efb1124b8478c7527389a826074
Gitweb:     http://git.kernel.org/tip/000078bc3ee69efb1124b8478c7527389a826074
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 20 Aug 2012 13:52:06 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 20 Aug 2012 09:47:31 -0300

perf hists: Rename and move some functions

Rename functions for consistency and move callchain print function
into hist_entry__fprintf().

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/1345438331-20234-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c |    4 +-
 tools/perf/ui/stdio/hist.c     |   51 ++++++++++++++++++++++------------------
 tools/perf/util/hist.h         |    4 +-
 3 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b809469..81bd8c2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -586,7 +586,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__snprintf(entry, s, sizeof(s), browser->hists);
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
 
 		ui_browser__set_percent_color(&browser->b, percent, current_entry);
@@ -931,7 +931,7 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
 	if (symbol_conf.use_callchain)
 		folded_sign = hist_entry__folded(he);
 
-	hist_entry__snprintf(he, s, sizeof(s), browser->hists);
+	hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists);
 	percent = (he->period * 100.0) / browser->hists->stats.total_period;
 
 	if (symbol_conf.use_callchain)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 7881d62..9bf7e9e 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -291,7 +291,7 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static int hist_entry__pcnt_snprintf(struct hist_entry *he, char *s,
+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)
@@ -404,8 +404,8 @@ static int hist_entry__pcnt_snprintf(struct hist_entry *he, char *s,
 	return ret;
 }
 
-int hist_entry__snprintf(struct hist_entry *he, char *s, size_t size,
-			 struct hists *hists)
+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;
@@ -423,6 +423,22 @@ int hist_entry__snprintf(struct hist_entry *he, char *s, size_t size,
 	return ret;
 }
 
+static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
+					    struct hists *hists,
+					    u64 total_period, FILE *fp)
+{
+	int left_margin = 0;
+
+	if (sort__first_dimension == SORT_COMM) {
+		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
+							 typeof(*se), list);
+		left_margin = hists__col_len(hists, se->se_width_idx);
+		left_margin -= thread__comm_len(he->thread);
+	}
+
+	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
+}
+
 static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 			       struct hists *hists, struct hists *pair_hists,
 			       bool show_displacement, long displacement,
@@ -434,27 +450,18 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	if (size == 0 || size > sizeof(bf))
 		size = sizeof(bf);
 
-	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
-					show_displacement, displacement,
-					true, total_period);
-	hist_entry__snprintf(he, bf + ret, size - ret, hists);
-	return fprintf(fp, "%s\n", bf);
-}
+	ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
+					  show_displacement, displacement,
+					  true, total_period);
+	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
-static size_t hist_entry__fprintf_callchain(struct hist_entry *he,
-					    struct hists *hists,
-					    u64 total_period, FILE *fp)
-{
-	int left_margin = 0;
+	ret = fprintf(fp, "%s\n", bf);
 
-	if (sort__first_dimension == SORT_COMM) {
-		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
-							 typeof(*se), list);
-		left_margin = hists__col_len(hists, se->se_width_idx);
-		left_margin -= thread__comm_len(he->thread);
-	}
+	if (symbol_conf.use_callchain)
+		ret += hist_entry__callchain_fprintf(he, hists,
+						     total_period, fp);
 
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
+	return ret;
 }
 
 size_t hists__fprintf(struct hists *hists, struct hists *pair,
@@ -608,8 +615,6 @@ print_entries:
 		ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
 					   displacement, total_period, fp);
 
-		if (symbol_conf.use_callchain)
-			ret += hist_entry__fprintf_callchain(h, hists, 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 69fab7d..2e650ff 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -75,8 +75,8 @@ struct hist_entry *__hists__add_entry(struct hists *self,
 				      struct symbol *parent, u64 period);
 int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right);
 int64_t hist_entry__collapse(struct hist_entry *left, struct hist_entry *right);
-int hist_entry__snprintf(struct hist_entry *self, char *bf, size_t size,
-			 struct hists *hists);
+int hist_entry__sort_snprintf(struct hist_entry *self, char *bf, size_t size,
+			      struct hists *hists);
 void hist_entry__free(struct hist_entry *);
 
 struct hist_entry *__hists__add_branch_entry(struct hists *self,

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

end of thread, other threads:[~2012-08-21 16:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-20  4:52 [PATCHSET 0/7] Cleanup hist printing code (v3) Namhyung Kim
2012-08-20  4:52 ` [PATCH 1/7] perf hists: Separate out hist print functions Namhyung Kim
2012-08-21 16:34   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-08-20  4:52 ` [PATCH 2/7] perf hists: Refactor some functions Namhyung Kim
2012-08-21 16:34   ` [tip:perf/core] perf hists: Rename and move " tip-bot for Namhyung Kim
2012-08-20  4:52 ` [PATCH 3/7] perf hists: Introduce hist_period_print functions Namhyung Kim
2012-08-20 13:08   ` Arnaldo Carvalho de Melo
2012-08-21  8:05     ` Namhyung Kim
2012-08-20  4:52 ` [PATCH 4/7] perf hists: Handle field separator properly Namhyung Kim
2012-08-20  4:52 ` [PATCH 5/7] perf hists: Use hpp_functions->width to calculate the column widths Namhyung Kim
2012-08-20  4:52 ` [PATCH 6/7] perf ui/browser: Use hist_period_print functions Namhyung Kim
2012-08-20  4:52 ` [PATCH 7/7] perf gtk/browser: " Namhyung Kim

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