Re: [PATCH 05/21] perf hists: Accumulate hist entry stat based onthe callchain

From: Jiri Olsa
Date: Sun Jan 05 2014 - 12:15:57 EST


On Sun, Jan 05, 2014 at 05:58:31PM +0100, Jiri Olsa wrote:
> On Tue, Dec 24, 2013 at 05:22:11PM +0900, Namhyung Kim wrote:
> > From: Namhyung Kim <namhyung.kim@xxxxxxx>
> >
> > Call __hists__add_entry() for each callchain node to get an
> > accumulated stat for an entry. Introduce new cumulative_iter ops to
> > process them properly.
> >
> > Cc: Arun Sharma <asharma@xxxxxx>
> > Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > ---
> > tools/perf/builtin-report.c | 103 +++++++++++++++++++++++++++++++++++++++++++-
> > tools/perf/ui/stdio/hist.c | 2 +-
> > 2 files changed, 103 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 29fe19071d24..4fde0ab82498 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -360,6 +360,97 @@ iter_finish_normal_entry(struct add_entry_iter *iter, struct addr_location *al)
> > return hist_entry__append_callchain(he, sample);
> > }
> >
> > +static int
> > +iter_prepare_cumulative_entry(struct add_entry_iter *iter,
> > + struct machine *machine __maybe_unused,
> > + struct perf_evsel *evsel,
> > + struct addr_location *al __maybe_unused,
> > + struct perf_sample *sample)
> > +{
> > + callchain_cursor_commit(&callchain_cursor);
> > +
> > + iter->evsel = evsel;
> > + iter->sample = sample;
> > + return 0;
> > +}
> > +
> > +static int
> > +iter_add_single_cumulative_entry(struct add_entry_iter *iter,
> > + struct addr_location *al)
> > +{
> > + struct perf_evsel *evsel = iter->evsel;
> > + struct perf_sample *sample = iter->sample;
> > + struct hist_entry *he;
> > +
> > + he = __hists__add_entry(&evsel->hists, al, iter->parent, NULL, NULL,
> > + sample->period, sample->weight,
> > + sample->transaction, true);
> > + if (he == NULL)
> > + return -ENOMEM;
> > +
> > + /*
> > + * This is for putting parents upward during output resort iff
> > + * only a child gets sampled. See hist_entry__sort_on_period().
> > + */
> > + he->callchain->max_depth = PERF_MAX_STACK_DEPTH + 1;
>
> so you're using callchain struct to hold the entry's stack
> position for sorting.. I think we could store this info
> inside hist_entry itself, and omit 'struct callchain_root'
> size being allocated for hist_entry
>
> I checked 'struct hist_entry' and the 'position' entry seems to
> be abandonned ;-)) or we could use some unused entry (mem_info?)
> and create some union.

also perhaps above 5 lines should be part of the later commit:
perf hists: Sort hist entries by accumulated period

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/