Messages in this thread | | | Date | Wed, 6 Jan 2016 08:57:11 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH v2 4/5] perf tools: Support dynamic sort keys for -F/--fields |
| |
On Tue, Jan 05, 2016 at 07:16:43PM -0300, Arnaldo Carvalho de Melo wrote: > 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?
Yes, this is subtle. The --sort option didn't really sort data but just aggregate it. By adding -F/--fields option, the aggregation and sorting can be separated. Behavior of the -s/--sort option is kept if it's not used with -F/--fields (for backward compatibility). However if it used with -F/--fields, it has the original meaning of sorting.
IOW, 'perf report -s prev_comm,next_comm' is actually same as 'perf report -F overhead,prev_comm,next_comm -s overhead'.
You run 'perf report -F overhead -s prev_com,next_comm', so it sorts the data using alphabetical order of prev_comm and then next_comm (But it should be reversed, will fix).
Please see the description of the original patch posting.
https://lkml.org/lkml/2014/5/19/84
Thanks, Namhyung
> > - 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
| |