Re: [PATCH 2/7] perf hist: Introduce hists__match_hierarchy()

[Namhyung Kim]

Hi Arnaldo,

On Mon, Sep 12, 2016 at 11:25:30AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 12, 2016 at 03:19:53PM +0900, Namhyung Kim escreveu:
> > The hists__match_hierarchy() is to find matching hist entries in a
> > group.  It needs to search all matching children in the hierarchy.
> 
> Can you please enrich this changeset log? Define "matching", an example
> of how the output of a hierarchy + group would help people trying to
> understand the changes you're making, for instance.

Ok.  How about this?


The hists__match_hierarchy() is to find matching hist entries in a
group.  A matching entry has same values for all sort keys given.
With event group, a leader event should show other members in a
group.  So each entry in the leader should be able to find its pair
entries which have same values.  With hierarchy mode, it needs to
search all matching children in a hierarchy.

An example output below:

  #               Overhead  Command / Shared Object / Symbol
  # ......................  ..................................
  #
      25.74%  27.18%        sh
         19.96%  24.14%        libc-2.24.so
            9.55%  14.64%        [.] __strcmp_sse2
            1.54%   0.00%        [.] __tfind
            1.07%   1.13%        [.] _int_malloc
  ...

In the above example, two overhead were shown - one for leader and
another for member.  They were matched since their command, dso and
symbol have same values.


Thanks,
Namhyung


>  
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > ---
> >  tools/perf/util/hist.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> > 
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index de15dbcdcecf..be3f5ce31303 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -2174,6 +2174,51 @@ static struct hist_entry *hists__find_entry(struct hists *hists,
> >  	return NULL;
> >  }
> >  
> > +static struct hist_entry *hists__find_hierarchy_entry(struct rb_root *root,
> > +						      struct hist_entry *he)
> > +{
> > +	struct rb_node *n = root->rb_node;
> > +
> > +	while (n) {
> > +		struct hist_entry *iter;
> > +		struct perf_hpp_fmt *fmt;
> > +		int64_t cmp = 0;
> > +
> > +		iter = rb_entry(n, struct hist_entry, rb_node_in);
> > +		perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
> > +			cmp = fmt->collapse(fmt, iter, he);
> > +			if (cmp)
> > +				break;
> > +		}
> > +
> > +		if (cmp < 0)
> > +			n = n->rb_left;
> > +		else if (cmp > 0)
> > +			n = n->rb_right;
> > +		else
> > +			return iter;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static void hists__match_hierarchy(struct rb_root *leader_root,
> > +				   struct rb_root *other_root)
> > +{
> > +	struct rb_node *nd;
> > +	struct hist_entry *pos, *pair;
> > +
> > +	for (nd = rb_first(leader_root); nd; nd = rb_next(nd)) {
> > +		pos  = rb_entry(nd, struct hist_entry, rb_node_in);
> > +		pair = hists__find_hierarchy_entry(other_root, pos);
> > +
> > +		if (pair) {
> > +			hist_entry__add_pair(pair, pos);
> > +			hists__match_hierarchy(&pos->hroot_in, &pair->hroot_in);
> > +		}
> > +	}
> > +}
> > +
> >  /*
> >   * Look for pairs to link to the leader buckets (hist_entries):
> >   */
> > @@ -2183,6 +2228,12 @@ void hists__match(struct hists *leader, struct hists *other)
> >  	struct rb_node *nd;
> >  	struct hist_entry *pos, *pair;
> >  
> > +	if (symbol_conf.report_hierarchy) {
> > +		/* hierarchy report always collapses entries */
> > +		return hists__match_hierarchy(&leader->entries_collapsed,
> > +					      &other->entries_collapsed);
> > +	}
> > +
> >  	if (hists__has(leader, need_collapse))
> >  		root = &leader->entries_collapsed;
> >  	else
> > -- 
> > 2.9.3