Re: [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events incase of overlapping events

[kajoljain <kjain@linux.ibm.com>]

On 1/8/20 9:32 PM, Jiri Olsa wrote:
> On Wed, Jan 08, 2020 at 02:41:35PM +0530, kajoljain wrote:
>
> SNIP
>
>>>> -    int i = 0;
>>>> +    int i = 0, j = 0;
>>>>        bool leader_found;
>>>>          evlist__for_each_entry (perf_evlist, ev) {
>>>> +        j++;
>>>> +        if (j <= iterator_perf_evlist)
>>>> +            continue;
>>>>            if (!strcmp(ev->name, ids[i])) {
>>>>                if (!metric_events[i])
>>>>                    metric_events[i] = ev;
>>>> @@ -146,6 +151,7 @@ static struct evsel *find_evsel_group(struct
>>>> evlist *perf_evlist,
>>>>                }
>>>>            }
>>>>        }
>>>> +    iterator_perf_evlist = j;
>>>>          return metric_events[0];
>>>>    }
>>>>
>>> Thanks for reporting and fixing this issue.
>>>
>>> I just have one question, do we really need a *static variable* to track
>>> the matched events? Perhaps using an input parameter?
>> Hi Jin,
>>
>> The other way I come up with to solve this issue is, making change in
>> perf_evlist itself by adding some flag in event name, to keep track of
>> matched events.
>>
>> As if we change event name itself, next time when we compare it won't
>> matched. But in that case we need to remove those flag later. Which will
>> increase the
>>
>> complexity. If you have any suggestions, please let me know.
> we already keep evsel::cpu_iter for similar concept
>
> so I guess we could have some iterator_perf_evlist variable in evlist..
> that is if we don't find other solution (other than static varable)

Hi Jiri,

          Thanks for reviewing the patch. I checked 'evsel::cpu_iter' 
variable, I think it added recently and I am not able to find any 
similar kind of variable in

          evlist. Please let me know if my understanding is fine. Do you 
want me to add new variable in evlist itself or there is any other way 
possible.

Thanks,

Kajol




> thanks,
> jirka
>