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

Liang, Kan
2024-08-06T14:11:16Z

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
>>
>