[5/8] perf stat,jevents: Introduce Default tags for the default mode

Message ID 20230607162700.3234712-6-kan.liang@linux.intel.com
State New
Headers
Series New metricgroup output in perf stat default mode |

Commit Message

Liang, Kan June 7, 2023, 4:26 p.m. UTC
  From: Kan Liang <kan.liang@linux.intel.com>

Introduce a new metricgroup, Default, to tag all the metric groups which
will be collected in the default mode.

Add a new field, DefaultMetricgroupName, in the JSON file to indicate
the real metric group name. It will be printed in the default output
to replace the event names.

There is nothing changed for the output format.

On SPR, both TopdownL1 and TopdownL2 are displayed in the default
output.

On ARM, Intel ICL and later platforms (before SPR), only TopdownL1 is
displayed in the default output.

Suggested-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/builtin-stat.c          | 4 ++--
 tools/perf/pmu-events/jevents.py   | 5 +++--
 tools/perf/pmu-events/pmu-events.h | 1 +
 tools/perf/util/metricgroup.c      | 3 +++
 4 files changed, 9 insertions(+), 4 deletions(-)
  

Comments

Ian Rogers June 13, 2023, 7:59 p.m. UTC | #1
On Wed, Jun 7, 2023 at 9:27 AM <kan.liang@linux.intel.com> wrote:
>
> From: Kan Liang <kan.liang@linux.intel.com>
>
> Introduce a new metricgroup, Default, to tag all the metric groups which
> will be collected in the default mode.
>
> Add a new field, DefaultMetricgroupName, in the JSON file to indicate
> the real metric group name. It will be printed in the default output
> to replace the event names.
>
> There is nothing changed for the output format.
>
> On SPR, both TopdownL1 and TopdownL2 are displayed in the default
> output.
>
> On ARM, Intel ICL and later platforms (before SPR), only TopdownL1 is
> displayed in the default output.
>
> Suggested-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>  tools/perf/builtin-stat.c          | 4 ++--
>  tools/perf/pmu-events/jevents.py   | 5 +++--
>  tools/perf/pmu-events/pmu-events.h | 1 +
>  tools/perf/util/metricgroup.c      | 3 +++
>  4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index c87c6897edc9..2269b3e90e9b 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2154,14 +2154,14 @@ static int add_default_attributes(void)
>                  * Add TopdownL1 metrics if they exist. To minimize
>                  * multiplexing, don't request threshold computation.
>                  */
> -               if (metricgroup__has_metric(pmu, "TopdownL1")) {
> +               if (metricgroup__has_metric(pmu, "Default")) {
>                         struct evlist *metric_evlist = evlist__new();
>                         struct evsel *metric_evsel;
>
>                         if (!metric_evlist)
>                                 return -1;
>
> -                       if (metricgroup__parse_groups(metric_evlist, pmu, "TopdownL1",
> +                       if (metricgroup__parse_groups(metric_evlist, pmu, "Default",
>                                                         /*metric_no_group=*/false,
>                                                         /*metric_no_merge=*/false,
>                                                         /*metric_no_threshold=*/true,
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 7ed258be1829..12e80bb7939b 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -54,8 +54,8 @@ _json_event_attributes = [
>  # Attributes that are in pmu_metric rather than pmu_event.
>  _json_metric_attributes = [
>      'pmu', 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold',
> -    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group', 'aggr_mode',
> -    'event_grouping'
> +    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group',
> +    'default_metricgroup_name', 'aggr_mode', 'event_grouping'
>  ]
>  # Attributes that are bools or enum int values, encoded as '0', '1',...
>  _json_enum_attributes = ['aggr_mode', 'deprecated', 'event_grouping', 'perpkg']
> @@ -307,6 +307,7 @@ class JsonEvent:
>      self.metric_name = jd.get('MetricName')
>      self.metric_group = jd.get('MetricGroup')
>      self.metricgroup_no_group = jd.get('MetricgroupNoGroup')
> +    self.default_metricgroup_name = jd.get('DefaultMetricgroupName')
>      self.event_grouping = convert_metric_constraint(jd.get('MetricConstraint'))
>      self.metric_expr = None
>      if 'MetricExpr' in jd:
> diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
> index 8cd23d656a5d..caf59f23cd64 100644
> --- a/tools/perf/pmu-events/pmu-events.h
> +++ b/tools/perf/pmu-events/pmu-events.h
> @@ -61,6 +61,7 @@ struct pmu_metric {
>         const char *desc;
>         const char *long_desc;
>         const char *metricgroup_no_group;
> +       const char *default_metricgroup_name;
>         enum aggr_mode_class aggr_mode;
>         enum metric_event_groups event_grouping;
>  };
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 74f2d8efc02d..efafa02db5e5 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -137,6 +137,8 @@ struct metric {
>          * output.
>          */
>         const char *metric_unit;
> +       /** Optional default metric group name */
> +       const char *default_metricgroup_name;

Adding a bit more to the comment would be useful, like:

Optional name of the metric group reported if the Default metric group
is being processed.

>         /** Optional null terminated array of referenced metrics. */
>         struct metric_ref *metric_refs;
>         /**
> @@ -219,6 +221,7 @@ static struct metric *metric__new(const struct pmu_metric *pm,
>
>         m->pmu = pm->pmu ?: "cpu";
>         m->metric_name = pm->metric_name;
> +       m->default_metricgroup_name = pm->default_metricgroup_name;
>         m->modifier = NULL;
>         if (modifier) {
>                 m->modifier = strdup(modifier);
> --
> 2.35.1
>
  
Liang, Kan June 13, 2023, 8:11 p.m. UTC | #2
On 2023-06-13 3:59 p.m., Ian Rogers wrote:
> On Wed, Jun 7, 2023 at 9:27 AM <kan.liang@linux.intel.com> wrote:
>>
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> Introduce a new metricgroup, Default, to tag all the metric groups which
>> will be collected in the default mode.
>>
>> Add a new field, DefaultMetricgroupName, in the JSON file to indicate
>> the real metric group name. It will be printed in the default output
>> to replace the event names.
>>
>> There is nothing changed for the output format.
>>
>> On SPR, both TopdownL1 and TopdownL2 are displayed in the default
>> output.
>>
>> On ARM, Intel ICL and later platforms (before SPR), only TopdownL1 is
>> displayed in the default output.
>>
>> Suggested-by: Stephane Eranian <eranian@google.com>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>  tools/perf/builtin-stat.c          | 4 ++--
>>  tools/perf/pmu-events/jevents.py   | 5 +++--
>>  tools/perf/pmu-events/pmu-events.h | 1 +
>>  tools/perf/util/metricgroup.c      | 3 +++
>>  4 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index c87c6897edc9..2269b3e90e9b 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -2154,14 +2154,14 @@ static int add_default_attributes(void)
>>                  * Add TopdownL1 metrics if they exist. To minimize
>>                  * multiplexing, don't request threshold computation.
>>                  */
>> -               if (metricgroup__has_metric(pmu, "TopdownL1")) {
>> +               if (metricgroup__has_metric(pmu, "Default")) {
>>                         struct evlist *metric_evlist = evlist__new();
>>                         struct evsel *metric_evsel;
>>
>>                         if (!metric_evlist)
>>                                 return -1;
>>
>> -                       if (metricgroup__parse_groups(metric_evlist, pmu, "TopdownL1",
>> +                       if (metricgroup__parse_groups(metric_evlist, pmu, "Default",
>>                                                         /*metric_no_group=*/false,
>>                                                         /*metric_no_merge=*/false,
>>                                                         /*metric_no_threshold=*/true,
>> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
>> index 7ed258be1829..12e80bb7939b 100755
>> --- a/tools/perf/pmu-events/jevents.py
>> +++ b/tools/perf/pmu-events/jevents.py
>> @@ -54,8 +54,8 @@ _json_event_attributes = [
>>  # Attributes that are in pmu_metric rather than pmu_event.
>>  _json_metric_attributes = [
>>      'pmu', 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold',
>> -    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group', 'aggr_mode',
>> -    'event_grouping'
>> +    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group',
>> +    'default_metricgroup_name', 'aggr_mode', 'event_grouping'
>>  ]
>>  # Attributes that are bools or enum int values, encoded as '0', '1',...
>>  _json_enum_attributes = ['aggr_mode', 'deprecated', 'event_grouping', 'perpkg']
>> @@ -307,6 +307,7 @@ class JsonEvent:
>>      self.metric_name = jd.get('MetricName')
>>      self.metric_group = jd.get('MetricGroup')
>>      self.metricgroup_no_group = jd.get('MetricgroupNoGroup')
>> +    self.default_metricgroup_name = jd.get('DefaultMetricgroupName')
>>      self.event_grouping = convert_metric_constraint(jd.get('MetricConstraint'))
>>      self.metric_expr = None
>>      if 'MetricExpr' in jd:
>> diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
>> index 8cd23d656a5d..caf59f23cd64 100644
>> --- a/tools/perf/pmu-events/pmu-events.h
>> +++ b/tools/perf/pmu-events/pmu-events.h
>> @@ -61,6 +61,7 @@ struct pmu_metric {
>>         const char *desc;
>>         const char *long_desc;
>>         const char *metricgroup_no_group;
>> +       const char *default_metricgroup_name;
>>         enum aggr_mode_class aggr_mode;
>>         enum metric_event_groups event_grouping;
>>  };
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index 74f2d8efc02d..efafa02db5e5 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -137,6 +137,8 @@ struct metric {
>>          * output.
>>          */
>>         const char *metric_unit;
>> +       /** Optional default metric group name */
>> +       const char *default_metricgroup_name;
> 
> Adding a bit more to the comment would be useful, like:
> 
> Optional name of the metric group reported if the Default metric group
> is being processed.

Sure.

Thanks,
Kan
> 
>>         /** Optional null terminated array of referenced metrics. */
>>         struct metric_ref *metric_refs;
>>         /**
>> @@ -219,6 +221,7 @@ static struct metric *metric__new(const struct pmu_metric *pm,
>>
>>         m->pmu = pm->pmu ?: "cpu";
>>         m->metric_name = pm->metric_name;
>> +       m->default_metricgroup_name = pm->default_metricgroup_name;
>>         m->modifier = NULL;
>>         if (modifier) {
>>                 m->modifier = strdup(modifier);
>> --
>> 2.35.1
>>
  

Patch

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index c87c6897edc9..2269b3e90e9b 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2154,14 +2154,14 @@  static int add_default_attributes(void)
 		 * Add TopdownL1 metrics if they exist. To minimize
 		 * multiplexing, don't request threshold computation.
 		 */
-		if (metricgroup__has_metric(pmu, "TopdownL1")) {
+		if (metricgroup__has_metric(pmu, "Default")) {
 			struct evlist *metric_evlist = evlist__new();
 			struct evsel *metric_evsel;
 
 			if (!metric_evlist)
 				return -1;
 
-			if (metricgroup__parse_groups(metric_evlist, pmu, "TopdownL1",
+			if (metricgroup__parse_groups(metric_evlist, pmu, "Default",
 							/*metric_no_group=*/false,
 							/*metric_no_merge=*/false,
 							/*metric_no_threshold=*/true,
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 7ed258be1829..12e80bb7939b 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -54,8 +54,8 @@  _json_event_attributes = [
 # Attributes that are in pmu_metric rather than pmu_event.
 _json_metric_attributes = [
     'pmu', 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold',
-    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group', 'aggr_mode',
-    'event_grouping'
+    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group',
+    'default_metricgroup_name', 'aggr_mode', 'event_grouping'
 ]
 # Attributes that are bools or enum int values, encoded as '0', '1',...
 _json_enum_attributes = ['aggr_mode', 'deprecated', 'event_grouping', 'perpkg']
@@ -307,6 +307,7 @@  class JsonEvent:
     self.metric_name = jd.get('MetricName')
     self.metric_group = jd.get('MetricGroup')
     self.metricgroup_no_group = jd.get('MetricgroupNoGroup')
+    self.default_metricgroup_name = jd.get('DefaultMetricgroupName')
     self.event_grouping = convert_metric_constraint(jd.get('MetricConstraint'))
     self.metric_expr = None
     if 'MetricExpr' in jd:
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index 8cd23d656a5d..caf59f23cd64 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -61,6 +61,7 @@  struct pmu_metric {
 	const char *desc;
 	const char *long_desc;
 	const char *metricgroup_no_group;
+	const char *default_metricgroup_name;
 	enum aggr_mode_class aggr_mode;
 	enum metric_event_groups event_grouping;
 };
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 74f2d8efc02d..efafa02db5e5 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -137,6 +137,8 @@  struct metric {
 	 * output.
 	 */
 	const char *metric_unit;
+	/** Optional default metric group name */
+	const char *default_metricgroup_name;
 	/** Optional null terminated array of referenced metrics. */
 	struct metric_ref *metric_refs;
 	/**
@@ -219,6 +221,7 @@  static struct metric *metric__new(const struct pmu_metric *pm,
 
 	m->pmu = pm->pmu ?: "cpu";
 	m->metric_name = pm->metric_name;
+	m->default_metricgroup_name = pm->default_metricgroup_name;
 	m->modifier = NULL;
 	if (modifier) {
 		m->modifier = strdup(modifier);