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

Namhyung Kim
2024-08-03T00:15:08Z

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.

Thanks,
Namhyung

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