[PATCH 1/3] perf sort: Add 'addr' sort key

[PATCH 1/3] perf sort: Add 'addr' sort key

[Namhyung Kim]

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

New addr sort key provides a way to sort the entries by the symbol
addresses.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c |  2 ++
 tools/perf/util/hist.h |  1 +
 tools/perf/util/sort.c | 23 +++++++++++++++++++++++
 tools/perf/util/sort.h |  1 +
 4 files changed, 27 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f855941bebea..82b7542db904 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -52,6 +52,8 @@ void hists__reset_col_len(struct hists *hists)
 
 	for (col = 0; col < HISTC_NR_COLS; ++col)
 		hists__set_col_len(hists, col, 0);
+
+	hists__set_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
 }
 
 static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 848331377bdb..72c75bef0c68 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -49,6 +49,7 @@ enum hist_column {
 	HISTC_DSO_FROM,
 	HISTC_DSO_TO,
 	HISTC_SRCLINE,
+	HISTC_ADDR,
 	HISTC_NR_COLS, /* Last entry */
 };
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d41926cb9e3f..27e55562cbbb 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -334,6 +334,28 @@ struct sort_entry sort_cpu = {
 	.se_width_idx	= HISTC_CPU,
 };
 
+/* --sort addr */
+
+static int64_t
+sort__addr_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	return right->ip - left->ip;
+}
+
+static int hist_entry__addr_snprintf(struct hist_entry *self, char *bf,
+				     size_t size, unsigned int width)
+{
+	return repsep_snprintf(bf, size, "%#*"PRIx64, width, (uint64_t)self->ip);
+}
+
+struct sort_entry sort_addr = {
+	.se_header      = "Address",
+	.se_cmp	        = sort__addr_cmp,
+	.se_snprintf    = hist_entry__addr_snprintf,
+	.se_width_idx	= HISTC_ADDR,
+};
+
+
 /* sort keys for branch stacks */
 
 static int64_t
@@ -480,6 +502,7 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_PARENT, "parent", sort_parent),
 	DIM(SORT_CPU, "cpu", sort_cpu),
 	DIM(SORT_SRCLINE, "srcline", sort_srcline),
+	DIM(SORT_ADDR, "addr", sort_addr),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index b13e56f6ccbe..a24dba16cc8f 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -130,6 +130,7 @@ enum sort_type {
 	SORT_PARENT,
 	SORT_CPU,
 	SORT_SRCLINE,
+	SORT_ADDR,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
1.7.11.7

[PATCH 2/3] perf sort: Add 'addr_to/from' sort key

[Namhyung Kim]

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

New addr_{to,from} sort keys provide a way to sort the entries by the
source/target symbol addresses.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c |  2 ++
 tools/perf/util/hist.h |  2 ++
 tools/perf/util/sort.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/sort.h |  2 ++
 4 files changed, 56 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 82b7542db904..b951f4101d7e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -54,6 +54,8 @@ void hists__reset_col_len(struct hists *hists)
 		hists__set_col_len(hists, col, 0);
 
 	hists__set_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
+	hists__set_col_len(hists, HISTC_ADDR_FROM, BITS_PER_LONG / 4 + 2);
+	hists__set_col_len(hists, HISTC_ADDR_TO, BITS_PER_LONG / 4 + 2);
 }
 
 static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 72c75bef0c68..ad1462c87918 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -50,6 +50,8 @@ enum hist_column {
 	HISTC_DSO_TO,
 	HISTC_SRCLINE,
 	HISTC_ADDR,
+	HISTC_ADDR_FROM,
+	HISTC_ADDR_TO,
 	HISTC_NR_COLS, /* Last entry */
 };
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 27e55562cbbb..988d7ea3ecc2 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -428,6 +428,40 @@ static int hist_entry__sym_to_snprintf(struct hist_entry *self, char *bf,
 
 }
 
+static int64_t
+sort__addr_from_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	struct addr_map_symbol *from_l = &left->branch_info->from;
+	struct addr_map_symbol *from_r = &right->branch_info->from;
+
+	return from_r->addr - from_l->addr;
+}
+
+static int hist_entry__addr_from_snprintf(struct hist_entry *self, char *bf,
+					  size_t size, unsigned int width)
+{
+	struct addr_map_symbol *from = &self->branch_info->from;
+	return repsep_snprintf(bf, size, "%#*"PRIx64, width,
+					(uint64_t)from->addr);
+}
+
+static int64_t
+sort__addr_to_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	struct addr_map_symbol *to_l = &left->branch_info->to;
+	struct addr_map_symbol *to_r = &right->branch_info->to;
+
+	return to_r->addr - to_l->addr;
+}
+
+static int hist_entry__addr_to_snprintf(struct hist_entry *self, char *bf,
+					size_t size, unsigned int width)
+{
+	struct addr_map_symbol *to = &self->branch_info->to;
+	return repsep_snprintf(bf, size, "%#*"PRIx64, width,
+					(uint64_t)to->addr);
+}
+
 struct sort_entry sort_dso_from = {
 	.se_header	= "Source Shared Object",
 	.se_cmp		= sort__dso_from_cmp,
@@ -456,6 +490,20 @@ struct sort_entry sort_sym_to = {
 	.se_width_idx	= HISTC_SYMBOL_TO,
 };
 
+struct sort_entry sort_addr_from = {
+	.se_header	= "Source Address",
+	.se_cmp		= sort__addr_from_cmp,
+	.se_snprintf	= hist_entry__addr_from_snprintf,
+	.se_width_idx	= HISTC_ADDR_FROM,
+};
+
+struct sort_entry sort_addr_to = {
+	.se_header	= "Target Address",
+	.se_cmp		= sort__addr_to_cmp,
+	.se_snprintf	= hist_entry__addr_to_snprintf,
+	.se_width_idx	= HISTC_ADDR_TO,
+};
+
 static int64_t
 sort__mispredict_cmp(struct hist_entry *left, struct hist_entry *right)
 {
@@ -514,6 +562,8 @@ static struct sort_dimension bstack_sort_dimensions[] = {
 	DIM(SORT_DSO_TO, "dso_to", sort_dso_to),
 	DIM(SORT_SYM_FROM, "symbol_from", sort_sym_from),
 	DIM(SORT_SYM_TO, "symbol_to", sort_sym_to),
+	DIM(SORT_ADDR_FROM, "addr_from", sort_addr_from),
+	DIM(SORT_ADDR_TO, "addr_to", sort_addr_to),
 	DIM(SORT_MISPREDICT, "mispredict", sort_mispredict),
 };
 
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index a24dba16cc8f..1c049cf58f34 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -138,6 +138,8 @@ enum sort_type {
 	SORT_DSO_TO,
 	SORT_SYM_FROM,
 	SORT_SYM_TO,
+	SORT_ADDR_FROM,
+	SORT_ADDR_TO,
 	SORT_MISPREDICT,
 };
 
-- 
1.7.11.7

[PATCH 3/3] perf report: Fix alignment of symbol column when -v is given

[Namhyung Kim]

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

When -v option is given, the symbol sort key prints its address also
but it wasn't properly aligned since hists__calc_col_len() misses the
additional part.  Also it missed 2 spaces for 0x prefix when printing.

  $ perf report --stdio -v -s sym
  # Samples: 133  of event 'cycles'
  # Event count (approx.): 50536717
  #
  # Overhead                          Symbol
  # ........  ..............................
  #
      12.20%  0xffffffff81384c50 v [k] intel_idle
       7.62%  0xffffffff8170976a v [k] ftrace_caller
       7.02%  0x2d986d         B [.] 0x00000000002d986d

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

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b951f4101d7e..7d4bb1f515d1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -73,8 +73,17 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	const unsigned int unresolved_col_width = BITS_PER_LONG / 4;
 	u16 len;
 
-	if (h->ms.sym)
-		hists__new_col_len(hists, HISTC_SYMBOL, h->ms.sym->namelen + 4);
+	if (h->ms.sym) {
+		/*
+		 * +4 accounts for '[x] ' priv level info
+		 * +2 accounts for 0x prefix on raw addresses
+		 * +3 accounts for ' y ' symtab origin info
+		 */
+		len = h->ms.sym->namelen + 4;
+		if (verbose)
+			len += BITS_PER_LONG / 4 + 2 + 3;
+		hists__new_col_len(hists, HISTC_SYMBOL, len);
+	}
 	else
 		hists__set_unres_dso_col_len(hists, HISTC_DSO);
 
@@ -94,10 +103,13 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		int symlen;
 		/*
 		 * +4 accounts for '[x] ' priv level info
-		 * +2 account of 0x prefix on raw addresses
+		 * +2 accounts for 0x prefix on raw addresses
+		 * +3 accounts for ' y ' symtab origin info
 		 */
 		if (h->branch_info->from.sym) {
 			symlen = (int)h->branch_info->from.sym->namelen + 4;
+			if (verbose)
+				symlen += BITS_PER_LONG / 4 + 2 + 3;
 			hists__new_col_len(hists, HISTC_SYMBOL_FROM, symlen);
 
 			symlen = dso__name_len(h->branch_info->from.map->dso);
@@ -110,6 +122,8 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 
 		if (h->branch_info->to.sym) {
 			symlen = (int)h->branch_info->to.sym->namelen + 4;
+			if (verbose)
+				symlen += BITS_PER_LONG / 4 + 2 + 3;
 			hists__new_col_len(hists, HISTC_SYMBOL_TO, symlen);
 
 			symlen = dso__name_len(h->branch_info->to.map->dso);
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 988d7ea3ecc2..5df020f3f59b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -194,7 +194,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 	if (verbose) {
 		char o = map ? dso__symtab_origin(map->dso) : '!';
 		ret += repsep_snprintf(bf, size, "%-#*llx %c ",
-				       BITS_PER_LONG / 4, ip, o);
+				       BITS_PER_LONG / 4 + 2, ip, o);
 	}
 
 	ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", level);
-- 
1.7.11.7

Re: [PATCH 1/3] perf sort: Add 'addr' sort key

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> New addr sort key provides a way to sort the entries by the symbol
> addresses.

No objections from me - just wondering about the motivation: why would we 
want to sort by symbol address?

Perhaps to see the overhead layout/distribution within a DSO, to better 
cache-pack hot functions by placing them next to each other?

Putting a short usecase into the changelog (or even better, the 
documentation) would be nice.

Thanks,

	Ingo

Re: [PATCH 1/3] perf sort: Add 'addr' sort key

[Namhyung Kim]

Hi Ingo,

On Tue, 26 Mar 2013 15:40:23 +0100, Ingo Molnar wrote:
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> From: Namhyung Kim <namhyung.kim@lge.com>
>> 
>> New addr sort key provides a way to sort the entries by the symbol
>> addresses.
>
> No objections from me - just wondering about the motivation: why would we 
> want to sort by symbol address?

In fact, it came from bugzilla:

  https://bugzilla.kernel.org/show_bug.cgi?id=55561

>
> Perhaps to see the overhead layout/distribution within a DSO, to better 
> cache-pack hot functions by placing them next to each other?
>
> Putting a short usecase into the changelog (or even better, the 
> documentation) would be nice.

Well, I didn't thought it much, but yeah, aforementioned usecase looks
promising.  Also it might be useful to find out which part is the
hotpath in a function if you have some really big functions (possibly
due to auto-inlining or something).

Arnaldo, do you have other usecase/scenario in mind?

Thanks,
Namhyung

Re: [PATCH 1/3] perf sort: Add 'addr' sort key

[Arnaldo Carvalho de Melo]

Em Wed, Mar 27, 2013 at 03:12:34PM +0900, Namhyung Kim escreveu:
> On Tue, 26 Mar 2013 15:40:23 +0100, Ingo Molnar wrote:
> > * Namhyung Kim <namhyung@kernel.org> wrote:
> >> New addr sort key provides a way to sort the entries by the symbol
> >> addresses.

> > No objections from me - just wondering about the motivation: why would we 
> > want to sort by symbol address?
 
> In fact, it came from bugzilla:
 
>   https://bugzilla.kernel.org/show_bug.cgi?id=55561

Will Cohen asked if this was possible while trying to figure out why
some libxul.so samples were not being resolved to symbols, Will?

Namhyung, I'm using:

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=55561

Just before the: Link: entry, consider adding it when addressing some
bugzilla entry, be it from bugzilla.kernel.org or from some other
bugzilla where problems or feature requests were made.

- Arnaldo
 
> >
> > Perhaps to see the overhead layout/distribution within a DSO, to better 
> > cache-pack hot functions by placing them next to each other?
> >
> > Putting a short usecase into the changelog (or even better, the 
> > documentation) would be nice.
> 
> Well, I didn't thought it much, but yeah, aforementioned usecase looks
> promising.  Also it might be useful to find out which part is the
> hotpath in a function if you have some really big functions (possibly
> due to auto-inlining or something).
> 
> Arnaldo, do you have other usecase/scenario in mind?

See above.