Re: [PATCH 7/9] perf report: Add support for srcfile sort key

[Namhyung Kim]

On Mon, Aug 10, 2015 at 05:56:31PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> 
> In some cases it's useful to characterize samples by file. This is
> useful to get a higher level categorization, for example to map cost to
> subsystems.
> 
> Add a srcfile sort key to perf report. It builds on top of the existing
> srcline support.
> 
> Commiter notes:
> 
> E.g.:
> 
>   # perf record -F 10000 usleep 1
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.016 MB perf.data (13 samples) ]
>   [root@zoo ~]# perf report -s srcfile --stdio
>   # Total Lost Samples: 0
>   #
>   # Samples: 13  of event 'cycles'
>   # Event count (approx.): 869878
>   #
>   # Overhead  Source File
>   # ........  ...........
>       60.99%  .
>       20.62%  paravirt.h
>       14.23%  rmap.c
>        4.04%  signal.c
>        0.11%  msr.h
> 
>   #
> 
> The first line is collecting all the files for which srcfiles couldn't somehow
> get resolved to:
> 
>   # perf report -s srcfile,dso --stdio
>   # Total Lost Samples: 0
>   #
>   # Samples: 13  of event 'cycles'
>   # Event count (approx.): 869878
>   #
>   # Overhead  Source File  Shared Object
>   # ........  ...........  ................
>       40.97%  .            ld-2.20.so
>       20.62%  paravirt.h   [kernel.vmlinux]
>       20.02%  .            libc-2.20.so
>       14.23%  rmap.c       [kernel.vmlinux]
>        4.04%  signal.c     [kernel.vmlinux]
>        0.11%  msr.h        [kernel.vmlinux]
> 
>   #
> 
> XXX: Investigate why that is not resolving on Fedora 21, Andi says he hasn't
>      seen this on Fedora 22.
> 
> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Tested-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Link: http://lkml.kernel.org/r/1438988064-21834-1-git-send-email-andi@xxxxxxxxxxxxxx
> [ Added column length update, from 0e65bdb3f90f ('perf hists: Update the column width for the "srcline" sort key') ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
>  tools/perf/Documentation/perf-report.txt |  2 ++
>  tools/perf/util/hist.c                   |  5 +++
>  tools/perf/util/hist.h                   |  1 +
>  tools/perf/util/sort.c                   | 52 ++++++++++++++++++++++++++++++++
>  tools/perf/util/sort.h                   |  2 ++
>  5 files changed, 62 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index 1a782ef02b68..7b07d19e2d54 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -81,6 +81,8 @@ OPTIONS
>  	- cpu: cpu number the task ran at the time of sample
>  	- srcline: filename and line number executed at the time of sample.  The
>  	DWARF debugging info must be provided.
> +	- srcfile: file name of the source file of the same. Requires dwarf
> +	information.
>  	- weight: Event specific weight, e.g. memory latency or transaction
>  	abort cost. This is the global weight.
>  	- local_weight: Local weight version of the weight above.
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 608c0a7fd0f4..6bccfae334b1 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -154,6 +154,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>  	if (h->srcline)
>  		hists__new_col_len(hists, HISTC_SRCLINE, strlen(h->srcline));
>  
> +	if (h->srcfile)
> +		hists__new_col_len(hists, HISTC_SRCFILE, strlen(h->srcfile));
> +
>  	if (h->transaction)
>  		hists__new_col_len(hists, HISTC_TRANSACTION,
>  				   hist_entry__transaction_len());
> @@ -949,6 +952,8 @@ void hist_entry__delete(struct hist_entry *he)
>  
>  	zfree(&he->stat_acc);
>  	free_srcline(he->srcline);
> +	if (he->srcfile && he->srcfile[0])
> +		free(he->srcfile);
>  	free_callchain(he->callchain);
>  	free(he);
>  }
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index e2f712f85d2e..bc528d54e457 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -30,6 +30,7 @@ enum hist_column {
>  	HISTC_PARENT,
>  	HISTC_CPU,
>  	HISTC_SRCLINE,
> +	HISTC_SRCFILE,
>  	HISTC_MISPREDICT,
>  	HISTC_IN_TX,
>  	HISTC_ABORT,
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 5177088a71d3..c0c32b050e45 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -319,6 +319,57 @@ struct sort_entry sort_srcline = {
>  	.se_width_idx	= HISTC_SRCLINE,
>  };
>  
> +/* --sort srcfile */
> +
> +static char no_srcfile[1];
> +
> +static char *get_srcfile(struct hist_entry *e)
> +{
> +	char *sf, *p;
> +	struct map *map = e->ms.map;
> +
> +	sf = get_srcline(map->dso, map__rip_2objdump(map, e->ip),
> +			 e->ms.sym, true);

AFAIK get_srcline() can return a string literal SRCLINE_UNKNOWN.  It
should not be freed or altered..

Thanks,
Namhyung


> +	p = strchr(sf, ':');
> +	if (p && *sf) {
> +		*p = 0;
> +		return sf;
> +	}
> +	free(sf);
> +	return no_srcfile;
> +}
> +
> +static int64_t
> +sort__srcfile_cmp(struct hist_entry *left, struct hist_entry *right)
> +{
> +	if (!left->srcfile) {
> +		if (!left->ms.map)
> +			left->srcfile = no_srcfile;
> +		else
> +			left->srcfile = get_srcfile(left);
> +	}
> +	if (!right->srcfile) {
> +		if (!right->ms.map)
> +			right->srcfile = no_srcfile;
> +		else
> +			right->srcfile = get_srcfile(right);
> +	}
> +	return strcmp(right->srcfile, left->srcfile);
> +}
> +
> +static int hist_entry__srcfile_snprintf(struct hist_entry *he, char *bf,
> +					size_t size, unsigned int width)
> +{
> +	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcfile);
> +}
> +
> +struct sort_entry sort_srcfile = {
> +	.se_header	= "Source File",
> +	.se_cmp		= sort__srcfile_cmp,
> +	.se_snprintf	= hist_entry__srcfile_snprintf,
> +	.se_width_idx	= HISTC_SRCFILE,
> +};
> +
>  /* --sort parent */
>  
>  static int64_t
> @@ -1196,6 +1247,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_SRCFILE, "srcfile", sort_srcfile),
>  	DIM(SORT_LOCAL_WEIGHT, "local_weight", sort_local_weight),
>  	DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
>  	DIM(SORT_TRANSACTION, "transaction", sort_transaction),
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index bc6c87a76d16..3c2a399f8f5b 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -114,6 +114,7 @@ struct hist_entry {
>  		};
>  	};
>  	char			*srcline;
> +	char			*srcfile;
>  	struct symbol		*parent;
>  	struct rb_root		sorted_chain;
>  	struct branch_info	*branch_info;
> @@ -172,6 +173,7 @@ enum sort_type {
>  	SORT_PARENT,
>  	SORT_CPU,
>  	SORT_SRCLINE,
> +	SORT_SRCFILE,
>  	SORT_LOCAL_WEIGHT,
>  	SORT_GLOBAL_WEIGHT,
>  	SORT_TRANSACTION,
> -- 
> 2.1.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/