On Sun, Mar 06, 2016 at 07:43:25PM +0100, Jiri Olsa wrote: > On Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim wrote: > > SNIP > > > > > @@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists > > *hists) > > static void hists_evsel__exit(struct perf_evsel *evsel) > > { > > struct hists *hists = evsel__hists(evsel); > > + struct perf_hpp_fmt *fmt, *pos; > > + struct perf_hpp_list_node *node; > > > > hists__delete_all_entries(hists); > > + > > + list_for_each_entry(node, &hists->hpp_formats, list) { > > + perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos) { > > + list_del(&fmt->list); > > + free(fmt); > > + } > > + list_del(&node->list); > > + free(node); > > + } > > } > > > > static int hists_evsel__init(struct perf_evsel *evsel) > > diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h > > index f4ef513527ba..3cab9dc20822 100644 > > --- a/tools/perf/util/hist.h > > +++ b/tools/perf/util/hist.h > > @@ -78,6 +78,7 @@ struct hists { > > u16 col_len[HISTC_NR_COLS]; > > int socket_filter; > > struct perf_hpp_list *hpp_list; > > + struct list_head hpp_formats; > > I've been thinking.. should hpp_formats and hpp_list be merged? something > like: > > struct perf_hpp_list { > struct list_head nodes; > int levels; > } > > sturct perf_hpp_list_node { > int level; > struct list_head fields; > struct list_head sorts; > int nr_sort_keys; > }; > > it seems wrong to me that your hierarchy code and current one > got in separate paths.. we could have the new hierarchy support > struct above being used in current non-hierarchy code just > by using single level > > I haven't thought all this through.. just an idea ;-)
Yes, I also think that we need to converge on the same data structure eventually. But I think it's too intrusive to be merged at once. Also keeping up with the latest perf changes is a PITA. ;-) So I decided to use it on the hierarchy first, and get tested in the wild for some time. After that we can convert the non-hierarchy code to use same path and data structure gradually IMHO. Thanks, Namhyung