public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf report: Fix calculation of the symbol column width
@ 2012-03-13  5:09 Namhyung Kim
  2012-03-15 17:34 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2012-03-13  5:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML

The calculation of each column width is determining longest string or
number in the data (including header). However printing symbol column
doesn't honor the width. This is fine if symbol column is printed at
last (default behavior), but if user changes the order using -s option
the output will be messed up:

 $ perf report -s sym,dso,comm
 ...
 # Overhead  Symbol     Shared Object  Command
 # ........  ......  ................  .......
 #
     99.96%  [.] main  noploop           noploop
      0.03%  [.] memset  ld-2.12.1.so      noploop
      0.00%  [.] 0x7ff751107b70    [unknown]          :18961
      0.00%  [.] _start  ld-2.12.1.so      noploop

Also, the symbol column adds 4 characters which represents the cpu mode
before the symbol and the width of unresolved symbols were not counted.

In addition, if user gave -v option it would print raw ip value and
symbol origin character before the symbol name. After all of these
concerns are addressed, the end result will look like this:

 $ perf report -s sym,dso,comm
 ...
 # Overhead              Symbol  Shared Object  Command
 # ........  ..................  .............  .......
 #
     99.96%  [.] main            noploop        noploop
      0.03%  [.] memset          ld-2.12.1.so   noploop
      0.00%  [.] 0x7ff751107b70  [unknown]       :18961
      0.00%  [.] _start          ld-2.12.1.so   noploop

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/util/hist.c |   27 ++++++++++++++++++++-------
 tools/perf/util/sort.c |   14 ++++++++------
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6f505d1abac7..2f5f8ff16e7e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -54,16 +54,25 @@ static void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 {
 	u16 len;
 
-	if (h->ms.sym)
-		hists__new_col_len(hists, HISTC_SYMBOL, h->ms.sym->namelen);
+	if (h->ms.sym) {
+		len = h->ms.sym->namelen;
+		if (!sort_dso.elide)
+			len += 4;
+		hists__new_col_len(hists, HISTC_SYMBOL, len);
+	}
 	else {
-		const unsigned int unresolved_col_width = BITS_PER_LONG / 4;
-
-		if (hists__col_len(hists, HISTC_DSO) < unresolved_col_width &&
+		char buf[BITS_PER_LONG / 4 + 3];
+		len = snprintf(buf, BITS_PER_LONG / 4 + 2, "%#llx",
+			       (unsigned long long)h->ip);
+		if (!sort_dso.elide)
+			len += 4;
+		hists__new_col_len(hists, HISTC_SYMBOL, len);
+
+		len = strlen("[unknown]");
+		if (hists__col_len(hists, HISTC_DSO) < len &&
 		    !symbol_conf.col_width_list_str && !symbol_conf.field_sep &&
 		    !symbol_conf.dso_list)
-			hists__set_col_len(hists, HISTC_DSO,
-					   unresolved_col_width);
+			hists__set_col_len(hists, HISTC_DSO, len);
 	}
 
 	len = thread__comm_len(h->thread);
@@ -984,6 +993,8 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 		}
 		if (!hists__new_col_len(hists, se->se_width_idx, width))
 			width = hists__col_len(hists, se->se_width_idx);
+		if (verbose && se->se_width_idx == HISTC_SYMBOL)
+			width += BITS_PER_LONG / 4 + 2 + 3;
 		fprintf(fp, "  %*s", width, se->se_header);
 	}
 
@@ -1016,6 +1027,8 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 		width = hists__col_len(hists, se->se_width_idx);
 		if (width == 0)
 			width = strlen(se->se_header);
+		if (verbose && se->se_width_idx == HISTC_SYMBOL)
+			width += BITS_PER_LONG / 4 + 2 + 3;
 		for (i = 0; i < width; i++)
 			fprintf(fp, ".");
 	}
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 16da30d8d765..806752870b9c 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -167,25 +167,27 @@ sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 }
 
 static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
-				    size_t size, unsigned int width __used)
+				    size_t size, unsigned int width)
 {
 	size_t ret = 0;
 
 	if (verbose) {
 		char o = self->ms.map ? dso__symtab_origin(self->ms.map->dso) : '!';
 		ret += repsep_snprintf(bf, size, "%-#*llx %c ",
-				       BITS_PER_LONG / 4, self->ip, o);
+				       BITS_PER_LONG / 4 + 2, self->ip, o);
 	}
 
-	if (!sort_dso.elide)
+	if (!sort_dso.elide) {
 		ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", self->level);
+		width -= 4;
+	}
 
 	if (self->ms.sym)
-		ret += repsep_snprintf(bf + ret, size - ret, "%s",
+		ret += repsep_snprintf(bf + ret, size - ret, "%-*s", width,
 				       self->ms.sym->name);
 	else
-		ret += repsep_snprintf(bf + ret, size - ret, "%-#*llx",
-				       BITS_PER_LONG / 4, self->ip);
+		ret += repsep_snprintf(bf + ret, size - ret, "%-#*llx", width,
+				       self->ip);
 
 	return ret;
 }
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-03-16  8:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-13  5:09 [PATCH] perf report: Fix calculation of the symbol column width Namhyung Kim
2012-03-15 17:34 ` Arnaldo Carvalho de Melo
2012-03-16  8:26   ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox