Re: [RFC PATCH v3 01/18] perf stat: Add new field in stat_config to enable hardware aware grouping.

From: Ian Rogers
Date: Tue Dec 12 2023 - 18:26:46 EST


On Tue, Dec 12, 2023 at 3:02 PM <weilin.wang@xxxxxxxxx> wrote:
>
> From: Weilin Wang <weilin.wang@xxxxxxxxx>
>
> Hardware counter and event information could be used to help creating event
> groups that better utilize hardware counters and improve multiplexing.
>
> Signed-off-by: Weilin Wang <weilin.wang@xxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

> ---
> tools/perf/builtin-stat.c | 8 ++++++++
> tools/perf/util/metricgroup.c | 5 +++++
> tools/perf/util/metricgroup.h | 1 +
> tools/perf/util/stat.h | 1 +
> 4 files changed, 15 insertions(+)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index d8e5d6f7a87a..a7a8018c3511 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2061,6 +2061,7 @@ static int add_default_attributes(void)
> stat_config.metric_no_threshold,
> stat_config.user_requested_cpu_list,
> stat_config.system_wide,
> + stat_config.hardware_aware_grouping,
> &stat_config.metric_events);
> }
>
> @@ -2094,6 +2095,7 @@ static int add_default_attributes(void)
> stat_config.metric_no_threshold,
> stat_config.user_requested_cpu_list,
> stat_config.system_wide,
> + stat_config.hardware_aware_grouping,
> &stat_config.metric_events);
> }
>
> @@ -2128,6 +2130,7 @@ static int add_default_attributes(void)
> /*metric_no_threshold=*/true,
> stat_config.user_requested_cpu_list,
> stat_config.system_wide,
> + stat_config.hardware_aware_grouping,
> &stat_config.metric_events) < 0)
> return -1;
> }
> @@ -2169,6 +2172,7 @@ static int add_default_attributes(void)
> /*metric_no_threshold=*/true,
> stat_config.user_requested_cpu_list,
> stat_config.system_wide,
> + stat_config.hardware_aware_grouping,
> &stat_config.metric_events) < 0)
> return -1;
>
> @@ -2502,6 +2506,9 @@ int cmd_stat(int argc, const char **argv)
> (const char **) stat_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
>
> + /* Set hardware_aware_grouping to false on default before completely enable it.*/
> + stat_config.hardware_aware_grouping = false;

nit: we don't normally zero values like this as the curly brace
initializer will implicitly zero all non named values being
initialized.

Thanks,
Ian

> +
> if (stat_config.csv_sep) {
> stat_config.csv_output = true;
> if (!strcmp(stat_config.csv_sep, "\\t"))
> @@ -2702,6 +2709,7 @@ int cmd_stat(int argc, const char **argv)
> stat_config.metric_no_threshold,
> stat_config.user_requested_cpu_list,
> stat_config.system_wide,
> + stat_config.hardware_aware_grouping,
> &stat_config.metric_events);
> zfree(&metrics);
> }
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 0484736d9fe4..dfbcdb517b78 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -1690,12 +1690,17 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
> bool metric_no_threshold,
> const char *user_requested_cpu_list,
> bool system_wide,
> + bool hardware_aware_grouping,
> struct rblist *metric_events)
> {
> const struct pmu_metrics_table *table = pmu_metrics_table__find();
>
> if (!table)
> return -EINVAL;
> + if (hardware_aware_grouping) {
> + pr_debug("Use hardware aware grouping instead of traditional metric grouping method\n");
> + }
> +
>
> return parse_groups(perf_evlist, pmu, str, metric_no_group, metric_no_merge,
> metric_no_threshold, user_requested_cpu_list, system_wide,
> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
> index d5325c6ec8e1..779f6ede1b51 100644
> --- a/tools/perf/util/metricgroup.h
> +++ b/tools/perf/util/metricgroup.h
> @@ -77,6 +77,7 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
> bool metric_no_threshold,
> const char *user_requested_cpu_list,
> bool system_wide,
> + bool hardware_aware_grouping,
> struct rblist *metric_events);
> int metricgroup__parse_groups_test(struct evlist *evlist,
> const struct pmu_metrics_table *table,
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index 325d0fad1842..f97d6282ba3d 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -87,6 +87,7 @@ struct perf_stat_config {
> bool metric_no_group;
> bool metric_no_merge;
> bool metric_no_threshold;
> + bool hardware_aware_grouping;
> bool stop_read_counter;
> bool iostat_run;
> char *user_requested_cpu_list;
> --
> 2.39.3
>