Re: [PATCH v2 4/5] perf tools: Support dynamic sort keys for -F/--fields

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Jan 05, 2016 at 07:58:37PM +0900, Namhyung Kim escreveu:
> Now dynamic sort keys are supported for tracepoint events, add it to
> output fields too.

Must be the end of the day, but:

  [root@zoo ~]# perf report -h -F -s

   Usage: perf report [<options>]

    -F, --fields <key[,keys...]>
                          output field(s): overhead, period, sample plus all of sort keys
    -s, --sort <key[,key2...]>
                          sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ... Please refer the man page for the complete list.

  [root@zoo ~]#

So, with this file, for instance:

  [root@zoo ~]# perf evlist -v
  sched:sched_switch: type: 2, size: 112, config: 0x10b, { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU|PERIOD|RAW, disabled: 1, inherit: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1
  [root@zoo ~]#

I thought that these would be equivalent:

  [root@zoo ~]# perf report -s prev_comm,next_comm | head -20 | tail -15
  # Samples: 27K of event 'sched:sched_switch'
  # Event count (approx.): 27997
  #
  # Overhead         prev_comm         next_comm
  # ........  ................  ................
  #
    10.97%           firefox         swapper/0
    10.97%         swapper/0           firefox
    10.23%          Xorg.bin         swapper/2
    10.23%         swapper/2          Xorg.bin
    10.20%           firefox         swapper/3
    10.20%         swapper/3           firefox
     5.75%         swapper/1          Xorg.bin
     5.74%          Xorg.bin         swapper/1
     3.93%          Xorg.bin         swapper/0
  [root@zoo ~]#

  [root@zoo ~]# perf report -s prev_comm,next_comm -F overhead | head -20 | tail -15
  # Samples: 27K of event 'sched:sched_switch'
  # Event count (approx.): 27997
  #
  # Overhead         prev_comm         next_comm
  # ........  ................  ................
  #
     0.05%             xchat         swapper/1
     0.11%             xchat         swapper/0
     0.01%        watchdog/3         swapper/3
     0.01%        watchdog/2         swapper/2
     0.01%        watchdog/1         swapper/1
     0.01%        watchdog/0         swapper/0
     0.01%             virsh         swapper/2
     0.08%               tor         swapper/3
     0.02%               tor         swapper/1
  [root@zoo ~]#

What am I doing wrong or didn't understood about those options?

- Arnaldo
 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 51 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index a8a958827e39..07312ceed6eb 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1908,7 +1908,7 @@ static struct perf_evsel *find_evsel(struct perf_evlist *evlist, char *event_nam
>  
>  static int __dynamic_dimension__add(struct perf_evsel *evsel,
>  				    struct format_field *field,
> -				    bool raw_trace)
> +				    bool raw_trace, bool is_sort_key)
>  {
>  	struct hpp_dynamic_entry *hde;
>  
> @@ -1918,18 +1918,24 @@ static int __dynamic_dimension__add(struct perf_evsel *evsel,
>  
>  	hde->raw_trace = raw_trace;
>  
> -	perf_hpp__register_sort_field(&hde->hpp);
> +	if (is_sort_key)
> +		perf_hpp__register_sort_field(&hde->hpp);
> +	else
> +		perf_hpp__column_register(&hde->hpp);
> +
>  	return 0;
>  }
>  
> -static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace)
> +static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace,
> +			    bool is_sort_key)
>  {
>  	int ret;
>  	struct format_field *field;
>  
>  	field = evsel->tp_format->format.fields;
>  	while (field) {
> -		ret = __dynamic_dimension__add(evsel, field, raw_trace);
> +		ret = __dynamic_dimension__add(evsel, field, raw_trace,
> +					       is_sort_key);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -1938,7 +1944,8 @@ static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace)
>  	return 0;
>  }
>  
> -static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
> +static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace,
> +				  bool is_sort_key)
>  {
>  	int ret;
>  	struct perf_evsel *evsel;
> @@ -1947,7 +1954,7 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
>  		if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
>  			continue;
>  
> -		ret = add_evsel_fields(evsel, raw_trace);
> +		ret = add_evsel_fields(evsel, raw_trace, is_sort_key);
>  		if (ret < 0)
>  			return ret;
>  	}
> @@ -1955,7 +1962,8 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
>  }
>  
>  static int add_all_matching_fields(struct perf_evlist *evlist,
> -				   char *field_name, bool raw_trace)
> +				   char *field_name, bool raw_trace,
> +				   bool is_sort_key)
>  {
>  	int ret = -ESRCH;
>  	struct perf_evsel *evsel;
> @@ -1969,14 +1977,16 @@ static int add_all_matching_fields(struct perf_evlist *evlist,
>  		if (field == NULL)
>  			continue;
>  
> -		ret = __dynamic_dimension__add(evsel, field, raw_trace);
> +		ret = __dynamic_dimension__add(evsel, field, raw_trace,
> +					       is_sort_key);
>  		if (ret < 0)
>  			break;
>  	}
>  	return ret;
>  }
>  
> -static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
> +static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok,
> +			     bool is_sort_key)
>  {
>  	char *str, *event_name, *field_name, *opt_name;
>  	struct perf_evsel *evsel;
> @@ -2006,12 +2016,13 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
>  	}
>  
>  	if (!strcmp(field_name, "trace_fields")) {
> -		ret = add_all_dynamic_fields(evlist, raw_trace);
> +		ret = add_all_dynamic_fields(evlist, raw_trace, is_sort_key);
>  		goto out;
>  	}
>  
>  	if (event_name == NULL) {
> -		ret = add_all_matching_fields(evlist, field_name, raw_trace);
> +		ret = add_all_matching_fields(evlist, field_name, raw_trace,
> +					      is_sort_key);
>  		goto out;
>  	}
>  
> @@ -2029,7 +2040,7 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
>  	}
>  
>  	if (!strcmp(field_name, "*")) {
> -		ret = add_evsel_fields(evsel, raw_trace);
> +		ret = add_evsel_fields(evsel, raw_trace, is_sort_key);
>  	} else {
>  		field = pevent_find_any_field(evsel->tp_format, field_name);
>  		if (field == NULL) {
> @@ -2038,7 +2049,8 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
>  			return -ENOENT;
>  		}
>  
> -		ret = __dynamic_dimension__add(evsel, field, raw_trace);
> +		ret = __dynamic_dimension__add(evsel, field, raw_trace,
> +					       is_sort_key);
>  	}
>  
>  out:
> @@ -2182,7 +2194,7 @@ static int sort_dimension__add(const char *tok,
>  		return 0;
>  	}
>  
> -	if (!add_dynamic_entry(evlist, tok))
> +	if (!add_dynamic_entry(evlist, tok, true))
>  		return 0;
>  
>  	return -ESRCH;
> @@ -2387,7 +2399,7 @@ void sort__setup_elide(FILE *output)
>  	}
>  }
>  
> -static int output_field_add(char *tok)
> +static int output_field_add(struct perf_evlist *evlist, char *tok)
>  {
>  	unsigned int i;
>  
> @@ -2427,6 +2439,9 @@ static int output_field_add(char *tok)
>  		return __sort_dimension__add_output(sd);
>  	}
>  
> +	if (!add_dynamic_entry(evlist, tok, false))
> +		return 0;
> +
>  	return -ESRCH;
>  }
>  
> @@ -2452,7 +2467,7 @@ bool is_strict_order(const char *order)
>  	return order && (*order != '+');
>  }
>  
> -static int __setup_output_field(void)
> +static int __setup_output_field(struct perf_evlist *evlist)
>  {
>  	char *tmp, *tok, *str, *strp;
>  	int ret = -EINVAL;
> @@ -2476,7 +2491,7 @@ static int __setup_output_field(void)
>  
>  	for (tok = strtok_r(strp, ", ", &tmp);
>  			tok; tok = strtok_r(NULL, ", ", &tmp)) {
> -		ret = output_field_add(tok);
> +		ret = output_field_add(evlist, tok);
>  		if (ret == -EINVAL) {
>  			error("Invalid --fields key: `%s'", tok);
>  			break;
> @@ -2513,7 +2528,7 @@ int setup_sorting(struct perf_evlist *evlist)
>  	if (sort__mode != SORT_MODE__DIFF)
>  		perf_hpp__init();
>  
> -	err = __setup_output_field();
> +	err = __setup_output_field(evlist);
>  	if (err < 0)
>  		return err;
>  
> -- 
> 2.6.4