Re: [PATCH 10/28] perf report: Cache cumulative callchains

Jiri Olsa
2014-01-14T13:17:53Z

On Tue, Jan 14, 2014 at 08:55:50AM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> Thanks for reminding me. :)
>
> On Thu, 9 Jan 2014 19:06:28 +0100, Jiri Olsa wrote:
> > On Wed, Jan 08, 2014 at 05:46:15PM +0900, Namhyung Kim wrote:
>
> [SNIP]
> >> + /*
> >> + * Check if there's duplicate entries in the callchain.
> >> + * It's possible that it has cycles or recursive calls.
> >> + */
> >> + for (i = 0; i < iter->curr; i++) {
> >> + if (hist_entry__cmp(he_cache[i], &he_tmp) == 0)
> >
> > we need here:
> > iter->he = he_cache[i];
> >
> >> + return 0;
> >> + }
> >
> > otherwise iter->he and al are not in sync and
> > hist_entry__inc_addr_samples fails in hist_iter_cb
>
> Right. But the point of the he_cache is to skip duplicate entries. So
> I'd like to change it like setting it to NULL and check it before the
> callback function:
>
> while (iter->next_entry(iter, al)) {
> err = iter->add_next_entry(iter, al);
> if (err)
> break;
>
> if (iter->he && iter->add_entry_cb) {
> err = iter->add_entry_cb(iter, al, ...);
> if (err)
> break;
> }
> }
>
> What do you think?

yep, better ;-) I saw you sent v6.. I'll check that

jirka