Re: [PATCH 5/9] perf evsel: Assign abbr name for the branch counter events

["Liang, Kan" <kan.liang@linux.intel.com>]

On 2024-08-02 8:14 p.m., Namhyung Kim wrote:
> On Wed, Jul 3, 2024 at 1:03 PM <kan.liang@linux.intel.com> wrote:
>>
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> There could be several branch counter events. If perf tool output the
>> result via the format "event name + a number", the line could be very
>> long and hard to read.
>>
>> An abbreviation is introduced to replace the full event name in the
>> display. The abbreviation starts from 'A' to 'Z9', which can support
>> up to 286 events. The same abbreviation will be assigned if the same
>> events are found in the evlist. The next patch will utilize the
>> abbreviation name to show the branch counter events in the output.
>>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>  tools/perf/util/evlist.c | 53 +++++++++++++++++++++++++++++++++++++++-
>>  tools/perf/util/evsel.h  |  4 +++
>>  2 files changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 6f5311d01a14..028169dcb53d 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -33,6 +33,7 @@
>>  #include "util/bpf-filter.h"
>>  #include "util/stat.h"
>>  #include "util/util.h"
>> +#include "util/env.h"
>>  #include <signal.h>
>>  #include <unistd.h>
>>  #include <sched.h>
>> @@ -1262,15 +1263,65 @@ u64 evlist__combined_branch_type(struct evlist *evlist)
>>         return branch_type;
>>  }
>>
>> +static struct evsel *
>> +evlist__find_dup_event_from_prev(struct evlist *evlist, struct evsel *event)
>> +{
>> +       struct evsel *pos;
>> +
>> +       evlist__for_each_entry(evlist, pos) {
>> +               if (event == pos)
>> +                       break;
>> +               if ((pos->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS) &&
>> +                   !strcmp(pos->name, event->name))
>> +                       return pos;
>> +       }
>> +       return NULL;
>> +}
>> +
>> +#define MAX_NR_ABBR_NAME       (26 * 11)
>> +
>> +/*
>> + * The abbr name is from A to Z9. If the number of event
>> + * which requires the branch counter > MAX_NR_ABBR_NAME,
>> + * return NA.
>> + */
>> +static char *evlist__new_abbr_name(void)
>> +{
>> +       static int idx;
>> +       char str[3];
>> +       int i = idx / 26;
>> +
>> +       if (idx >= MAX_NR_ABBR_NAME)
>> +               return strdup("NA");
>> +
>> +       str[0] = 'A' + (idx % 26);
>> +
>> +       if (!i)
>> +               str[1] = '\0';
>> +       else {
>> +               str[1] = '0' + i - 1;
>> +               str[2] = '\0';
>> +       }
>> +
>> +       idx++;
>> +       return strdup(str);
>> +}
>> +
>>  void evlist__update_br_cntr(struct evlist *evlist)
>>  {
>> -       struct evsel *evsel;
>> +       struct evsel *evsel, *dup;
>>         int i = 0;
>>
>>         evlist__for_each_entry(evlist, evsel) {
>>                 if (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS) {
>>                         evsel->br_cntr_idx = i++;
>>                         evsel__leader(evsel)->br_cntr_nr++;
>> +
>> +                       dup = evlist__find_dup_event_from_prev(evlist, evsel);
>> +                       if (dup)
>> +                               evsel->abbr_name = strdup(dup->abbr_name);
>> +                       else
>> +                               evsel->abbr_name = evlist__new_abbr_name();
>>                 }
>>         }
>>         evlist->nr_br_cntr = i;
>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> index a733d3407b35..bf37442002aa 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -152,9 +152,13 @@ struct evsel {
>>          * br_cntr_idx: The idx of the branch counter event in the evlist
>>          * br_cntr_nr:  The number of the branch counter event in the group
>>          *              (Only available for the leader event)
>> +        * abbr_name:   The abbreviation name assigned to an event which is
>> +        *              logged by the branch counter.
>>          */
>>         int                     br_cntr_idx;
>>         int                     br_cntr_nr;
>> +       char                    *abbr_name;
> 
> I think it's better to have an array (of 4 characters?) instead of a
> pointer as it's supposed to be a short string.

Sure. I think 3 characters should be enough, since the abbr name is only
from A to Z9.

Thanks,
Kan
> 
> Thanks,
> Namhyung
> 
>> +
>>         /*
>>          * bpf_counter_ops serves two use cases:
>>          *   1. perf-stat -b          counting events used byBPF programs
>> --
>> 2.38.1
>>
>