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

[Namhyung Kim <namhyung@kernel.org>]

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?

Thanks,
Namhyung