Messages in this thread | | | Date | Mon, 7 Mar 2016 14:15:02 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats() |
| |
On Mon, Mar 07, 2016 at 09:59:34PM +0900, Namhyung Kim wrote: > 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. ;-)
heh, amen ;-)
> > 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.
ok, let's keep it in mind at least..
thanks. jirka
| |