[lkml]   [2012]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 07/18] perf hists: Exchange order of comparing items when collapsing hists
On Mon, 3 Dec 2012 11:19:52 +0100, Jiri Olsa wrote:
> On Mon, Dec 03, 2012 at 10:41:08AM +0900, Namhyung Kim wrote:
>> On Thu, 29 Nov 2012 15:52:57 -0300, Arnaldo Carvalho de Melo wrote:
>> > doesn't this now gets inconsistent with the hist_entry__collapse() call?
>> > I.e. iter first, he after, also there is the case for callchains, below,
>> > care to elaborate here?
>> I meant it by consistent with hist_entry__cmp() and didn't consider
>> he_stat__add_stat and callchain_merge things - thought that they're
>> other kind of operation.
>> I needed this change because I introduced hists__{match,link}_collapsed
>> function in the patch 8 and I found that hist_entry__cmp and
>> hist_entry__collapse received same kind of arguments in different
>> order. Sorry about missing this in the changelog.
>> However on the second thought, I feel like I don't need those _collapsed
>> functions at all and perf diff can be converted to use collapsed rb tree
>> directly instead. IIUC perf diff use those functions to match entries
>> by sort keys and do additional resort output rb tree by sort keys (IMHO
>> the function names - _name_resort and _insert_by_name - are misnomers)
>> to do the match.
>> Since output resorting (by period) is only needed for the baseline,
>> other data files doesn't need to do this additional step. So I can get
>> rid of those hists__{match,link}_collapsed functions and change plain
>> hists__{match,link} functions to use collapsed (or input) rb tree
>> directly.
>> Jiri, what do you think? What am I missing? :)
> feels like I'm missing something now :))
> right now I don't see a point in having sort__need_collapse enabled
> for group report.. there seems to be no special processing as it was
> in the initial patchset..?

Right. It's not needed anymore, will remove.

> It seems like you could do only stadard period resort (collapsed if
> needed) and afterwards run current hists__match and hists_link function
> they are now.

But hists__match/link needs the tree sorted by sort keys. That means I
have to do something like:

(1) add_hist_entry (sort by keys)
(2) (optionally) hists__collapse_resort (sort by keys)
(3) hists__output_resort (sort by period)
(4) manual resort (by keys)
(5) hists__match/link
(6) (duplicated) hists__output_resort (sort by period)

I think the step (3) and (4) are unnecessary if we use input (or
collapsed) tree directly.

> All you need at the output callback is paired leader hist entry.. where
> you do the temp array magic to get the data.

Yeah, this is what I'm saying. So we don't need to use output tree for
matching and linking entries.


 \ /
  Last update: 2012-12-03 12:41    [W:0.049 / U:5.748 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site