Messages in this thread | | | Date | Wed, 3 Apr 2024 14:57:32 -0400 | Subject | Re: [PATCH v1] perf metrics: Remove the "No_group" metric group | From | "Liang, Kan" <> |
| |
On 2024-04-03 2:31 p.m., Ian Rogers wrote: > On Wed, Apr 3, 2024 at 10:59 AM Liang, Kan <kan.liang@linux.intel.com> wrote: >> >> >> >> On 2024-04-03 12:46 p.m., Ian Rogers wrote: >>> Rather than place metrics without a metric group in "No_group" place >>> them in a a metric group that is their name. Still allow such metrics >>> to be selected if "No_group" is passed, this change just impacts perf >>> list. >> >> So it looks like the "No_group" is not completely removed. >> They are just not seen in the perf list, but users can still use it via >> perf stat -M No_group, right? >> >> If so, why we want to remove it from perf list? Where can the end user >> know which metrics are included in the No_group? >> >> If the No_group is useless, why not completely remove it? > > Agreed. For command line argument deprecation we usually keep the > option but hide it from help with PARSE_OPT_HIDDEN, so I was trying to > follow that pattern albeit that a metric group isn't a command line > option it's an option to an option. >
Perf list has a deprecated option to show the deprecated events. The "No_group" should be a deprecated metrics group.
If so, to follow the same pattern, I think perf list should still display the "No_group" with the --deprecated option at least.
Thanks, Kan
> Thanks, > Ian > >> Thanks, >> Kan >> >>> >>> Signed-off-by: Ian Rogers <irogers@google.com> >>> --- >>> tools/perf/util/metricgroup.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c >>> index 79ef6095ab28..6ec083af14a1 100644 >>> --- a/tools/perf/util/metricgroup.c >>> +++ b/tools/perf/util/metricgroup.c >>> @@ -455,7 +455,7 @@ static int metricgroup__add_to_mep_groups(const struct pmu_metric *pm, >>> const char *g; >>> char *omg, *mg; >>> >>> - mg = strdup(pm->metric_group ?: "No_group"); >>> + mg = strdup(pm->metric_group ?: pm->metric_name); >>> if (!mg) >>> return -ENOMEM; >>> omg = mg; >>> @@ -466,7 +466,7 @@ static int metricgroup__add_to_mep_groups(const struct pmu_metric *pm, >>> if (strlen(g)) >>> me = mep_lookup(groups, g, pm->metric_name); >>> else >>> - me = mep_lookup(groups, "No_group", pm->metric_name); >>> + me = mep_lookup(groups, pm->metric_name, pm->metric_name); >>> >>> if (me) { >>> me->metric_desc = pm->desc;
| |