Re: [PATCH v1 05/14] perf util: Calculate the sum of all streams hits

[Jin, Yao]

On 3/10/2020 11:14 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 10, 2020 at 03:02:36PM +0800, Jin Yao escreveu:
> > We have used callchain_node->hit to measure the hot level of one
> > stream. This patch calculates the sum of hits of all streams.
> >
> > Then in next patch, we can use following formula to report hot
> > percent for one stream.
> >
> > hot percent = callchain_node->hit / sum of all hits
> >
> > Signed-off-by: Jin Yao <yao.jin@xxxxxxxxxxxxxxx>
> > ---
> >   tools/perf/util/callchain.c | 35 +++++++++++++++++++++++++++++++++++
> >   tools/perf/util/callchain.h |  1 +
> >   2 files changed, 36 insertions(+)
> >
> > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > index a9dd91268b00..040995405664 100644
> > --- a/tools/perf/util/callchain.c
> > +++ b/tools/perf/util/callchain.c
> > @@ -1685,6 +1685,39 @@ static void update_hot_streams(struct hist_entry *he,
> >   	}
> >   }
> >   
> > +static u64 count_callchain_hits(struct hist_entry *he)
> > +{
> > +	struct rb_root *root = &he->sorted_chain;
> > +	struct rb_node *rb_node = rb_first(root);
> > +	struct callchain_node *node;
> > +	u64 chain_hits = 0;
> > +
> > +	while (rb_node) {
> > +		node = rb_entry(rb_node, struct callchain_node, rb_node);
> > +		chain_hits += node->hit;
> > +		rb_node = rb_next(rb_node);
> > +	}
> > +
> > +	return chain_hits;
> > +}
> > +
> > +static u64 total_callchain_hits(struct hists *hists)
> > +{
> > +	struct rb_node *next;
> > +	u64 chain_hits = 0;
> > +
> > +	next = rb_first_cached(&hists->entries);
>
> Try to combine the variable decl line with its initial assignment,
> saving one line, i.e.:
>
> +static u64 total_callchain_hits(struct hists *hists)
> +{
> +	struct rb_node *next = rb_first_cached(&hists->entries);
> +	u64 chain_hits = 0;
> +

Yes, combining to one line is easy for reading. I will update the code.

> > +	while (next) {
> > +		struct hist_entry *he;
> > +
> > +		he = rb_entry(next, struct hist_entry, rb_node);
>
> Ditto:
>
> +		struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);

Got it.

Thanks
Jin Yao

> > +		chain_hits += count_callchain_hits(he);
> > +		next = rb_next(&he->rb_node);
> > +	}
> > +
> > +	return chain_hits;
> > +}
> > +
> >   static void get_hot_streams(struct hists *hists,
> >   			    struct callchain_streams *s)
> >   {
> > @@ -1698,6 +1731,8 @@ static void get_hot_streams(struct hists *hists,
> >   		update_hot_streams(he, s);
> >   		next = rb_next(&he->rb_node);
> >   	}
> > +
> > +	s->chain_hits = total_callchain_hits(hists);
> >   }
> >   
> >   struct callchain_streams *callchain_evsel_streams_create(struct evlist *evlist,
> > diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> > index c996ab4fb108..3c0e0b45656b 100644
> > --- a/tools/perf/util/callchain.h
> > +++ b/tools/perf/util/callchain.h
> > @@ -173,6 +173,7 @@ struct callchain_streams {
> >   	int			nr_streams_max;
> >   	int			nr_streams;
> >   	int			evsel_idx;
> > +	u64			chain_hits;
> >   };
> >   
> >   extern __thread struct callchain_cursor callchain_cursor;
> > --
> > 2.17.1