Messages in this thread | | | Subject | Re: [PATCH v7 1/3] perf report: Change sort order by a specified event in group | From | "Jin, Yao" <> | Date | Thu, 19 Mar 2020 09:14:26 +0800 |
| |
On 3/19/2020 2:45 AM, Arnaldo Carvalho de Melo wrote: > Em Thu, Feb 20, 2020 at 09:36:14AM +0800, Jin Yao escreveu: >> When performing "perf report --group", it shows the event group information >> together. By default, the output is sorted by the first event in group. >> >> It would be nice for user to select any event for sorting. This patch >> introduces a new option "--group-sort-idx" to sort the output by the >> event at the index n in event group. >> >> For example, >> >> Before: > > Tested and applied, I also did the following simplifications, to try and > follow naming conventions and use less lines to do the same thing: > > diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c > index 35224b366305..025f4c7f96bf 100644 > --- a/tools/perf/ui/hist.c > +++ b/tools/perf/ui/hist.c > @@ -151,44 +151,35 @@ static int field_cmp(u64 field_a, u64 field_b) > return 0; > } > > -static int pair_fields_alloc(struct hist_entry *a, struct hist_entry *b, > - hpp_field_fn get_field, int nr_members, > - u64 **fields_a, u64 **fields_b) > +static int hist_entry__new_pair(struct hist_entry *a, struct hist_entry *b, > + hpp_field_fn get_field, int nr_members, > + u64 **fields_a, u64 **fields_b) > { > - struct evsel *evsel; > + u64 *fa = calloc(nr_members, sizeof(*fa)), > + *fb = calloc(nr_members, sizeof(*fb)); > struct hist_entry *pair; > - u64 *fa, *fb; > - int ret = -1; > - > - fa = calloc(nr_members, sizeof(*fa)); > - fb = calloc(nr_members, sizeof(*fb)); > > if (!fa || !fb) > - goto out; > + goto out_free; > > list_for_each_entry(pair, &a->pairs.head, pairs.node) { > - evsel = hists_to_evsel(pair->hists); > + struct evsel *evsel = hists_to_evsel(pair->hists); > fa[perf_evsel__group_idx(evsel)] = get_field(pair); > } > > list_for_each_entry(pair, &b->pairs.head, pairs.node) { > - evsel = hists_to_evsel(pair->hists); > + struct evsel *evsel = hists_to_evsel(pair->hists); > fb[perf_evsel__group_idx(evsel)] = get_field(pair); > } > > *fields_a = fa; > *fields_b = fb; > - ret = 0; > - > -out: > - if (ret != 0) { > - free(fa); > - free(fb); > - *fields_a = NULL; > - *fields_b = NULL; > - } > - > - return ret; > + return 0; > +out_free: > + free(fa); > + free(fb); > + *fields_a = *fields_b = NULL; > + return -1; > } > > static int __hpp__group_sort_idx(struct hist_entry *a, struct hist_entry *b, > @@ -206,8 +197,7 @@ static int __hpp__group_sort_idx(struct hist_entry *a, struct hist_entry *b, > if (idx < 1 || idx >= nr_members) > return cmp; > > - ret = pair_fields_alloc(a, b, get_field, nr_members, > - &fields_a, &fields_b); > + ret = hist_entry__new_pair(a, b, get_field, nr_members, &fields_a, &fields_b); > if (ret) { > ret = cmp; > goto out; > @@ -254,8 +244,7 @@ static int __hpp__sort(struct hist_entry *a, struct hist_entry *b, > return ret; > > nr_members = evsel->core.nr_members; > - i = pair_fields_alloc(a, b, get_field, nr_members, > - &fields_a, &fields_b); > + i = hist_entry__new_pair(a, b, get_field, nr_members, &fields_a, &fields_b); > if (i) > goto out; > >
Thanks for refining the patch!
Thanks Jin Yao
| |