Re: [PATCH] perf report: Allow sorting by symbol size.

[Arnaldo Carvalho de Melo]

Em Fri, Feb 24, 2017 at 01:32:56PM +0000, charles.baylis@xxxxxxxxxx escreveu:
> From: Charles Baylis <charles.baylis@xxxxxxxxxx>
> 
> Add new sort key 'symbol_size' to allow user to sort by
> symbol size, or (more usefully) display the symbol size
> using --fields=...,symbol_size.
> 
> Signed-off-by: Charles Baylis <charles.baylis@xxxxxxxxxx>
> ---
>  tools/perf/Documentation/perf-report.txt |  1 +
>  tools/perf/util/hist.h                   |  1 +
>  tools/perf/util/sort.c                   | 46 ++++++++++++++++++++++++++++++++
>  tools/perf/util/sort.h                   |  1 +
>  4 files changed, 49 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index f2914f0..d2a8c15 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -76,6 +76,7 @@ OPTIONS
>  	- pid: command and tid of the task
>  	- dso: name of library or module executed at the time of sample
>  	- symbol: name of function executed at the time of sample
> +	- symbol_size: size of function executed at the time of sample
>  	- parent: name of function matched to the parent regex filter. Unmatched
>  	entries are displayed as "[other]".
>  	- cpu: cpu number the task ran at the time of sample
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 28c216e..2e839bf 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -57,6 +57,7 @@ enum hist_column {
>  	HISTC_SRCLINE_FROM,
>  	HISTC_SRCLINE_TO,
>  	HISTC_TRACE,
> +	HISTC_SYM_SIZE,
>  	HISTC_NR_COLS, /* Last entry */
>  };
>  
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index df622f4..0ad397c 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1396,6 +1396,51 @@ struct sort_entry sort_transaction = {
>  	.se_width_idx	= HISTC_TRANSACTION,
>  };
>  
> +/* --sort symbol_size */
> +
> +static int64_t _sort__sym_size_cmp(struct symbol *sym_l, struct symbol *sym_r)
> +{
> +	int64_t size_l = sym_l != NULL ? sym_l->end - sym_l->start : 0;
> +	int64_t size_r = sym_r != NULL ? sym_r->end - sym_r->start : 0;

We have symbol__size(), no need to open code it, I'll fix it

> +
> +	return size_l < size_r ? -1 :
> +		size_l == size_r ? 0 : 1;
> +}
> +
> +static int64_t
> +sort__sym_size_cmp(struct hist_entry *left, struct hist_entry *right)
> +{
> +	return _sort__sym_size_cmp(right->ms.sym, left->ms.sym);
> +}
> +
> +static int _hist_entry__sym_size_snprintf(struct symbol *sym, char *bf,
> +					  size_t bf_size, unsigned int width)
> +{
> +	if (sym) {
> +		int64_t sym_size = sym->end - sym->start;


Ditto

> +
> +		return repsep_snprintf(bf, bf_size, "%*lld", width,
> +					(long long)sym_size);

Humm, why use lld instead of plain d, that way you can get rid of that
(long long)? I'm fixing this as well

> +	} else {
> +		return repsep_snprintf(bf, bf_size, "%*s", width,
> +					"unknown");
> +	}
> +}
> +
> +static int hist_entry__sym_size_snprintf(struct hist_entry *he, char *bf,
> +					 size_t size, unsigned int width)
> +{
> +	return _hist_entry__sym_size_snprintf(he->ms.sym, bf, size, width);
> +}
> +
> +struct sort_entry sort_sym_size = {
> +	.se_header	= "Symbol size",
> +	.se_cmp		= sort__sym_size_cmp,
> +	.se_snprintf	= hist_entry__sym_size_snprintf,
> +	.se_width_idx	= HISTC_SYM_SIZE,
> +};
> +
> +
>  struct sort_dimension {
>  	const char		*name;
>  	struct sort_entry	*entry;
> @@ -1418,6 +1463,7 @@ static struct sort_dimension common_sort_dimensions[] = {
>  	DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
>  	DIM(SORT_TRANSACTION, "transaction", sort_transaction),
>  	DIM(SORT_TRACE, "trace", sort_trace),
> +	DIM(SORT_SYM_SIZE, "symbol_size", sort_sym_size),
>  };
>  
>  #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 7aff317..acb2c57 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -211,6 +211,7 @@ enum sort_type {
>  	SORT_GLOBAL_WEIGHT,
>  	SORT_TRANSACTION,
>  	SORT_TRACE,
> +	SORT_SYM_SIZE,
>  
>  	/* branch stack specific sort keys */
>  	__SORT_BRANCH_STACK,
> -- 
> 2.7.4