Re: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy()

[Namhyung Kim]

On Mon, Sep 19, 2016 at 4:59 PM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> On Tue, Sep 13, 2016 at 04:45:47PM +0900, Namhyung Kim wrote:
>> The hists__link_hierarchy() is to support hierarchy report with event
>> group.  When it matches leader event and other members (with the
>> hists__match_hierarchy), it also needs to link unmatched member entries
>> with a dummy leader event so that it can show up in the output.
>
> SNIP
>
>>
>> +static int hists__link_hierarchy(struct hists *leader_hists,
>> +                              struct hist_entry *parent,
>> +                              struct rb_root *leader_root,
>> +                              struct rb_root *other_root)
>> +{
>> +     struct rb_node *nd;
>> +     struct hist_entry *pos, *leader;
>> +
>> +     for (nd = rb_first(other_root); nd; nd = rb_next(nd)) {
>> +             pos = rb_entry(nd, struct hist_entry, rb_node_in);
>> +
>> +             if (hist_entry__has_pairs(pos)) {
>> +                     bool found = false;
>> +
>> +                     list_for_each_entry(leader, &pos->pairs.head, pairs.node) {
>> +                             if (leader->hists == leader_hists) {
>> +                                     found = true;
>> +                                     break;
>> +                             }
>> +                     }
>> +                     if (!found)
>> +                             return -1;
>> +             } else {
>> +                     leader = add_dummy_hierarchy_entry(leader_hists,
>> +                                                        leader_root, pos);
>
> could we call hists__add_dummy_entry in here? seems like there's only
> difference in function arguments.. we could safe one function ;-)

Maybe, but I don't see much value doing it.  IMHO this hists
match/link thing is already complex.  Combining normal and hierarchy
case in a single function will increase the complexity so I'd like to
keep it separate for hierarchy case.

Thanks,
Namhyung