[RFC 0/3] perf tools: Add support for used defined metric

[RFC 0/3] perf tools: Add support for used defined metric

[Jiri Olsa]

hi,
Joe asked for possibility to add user defined metrics. Given that
we already have metrics support, I added --metric option that allows
to specify metric on the command line, like:

  # perf stat  --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
    (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...

The code facilitates the current metric code, and I was surprised
how easy it was, so I'm not sure I omitted something ;-)

Also available in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/metric

thoughts?
jirka

---
Jiri Olsa (3):
      perf tools: Factor metric addition into add_metric function
      perf tools: Factor metric setup code into metricgroup__setup function
      perf stat: Add --metric option

 tools/perf/Documentation/perf-stat.txt |  16 ++++++++++
 tools/perf/builtin-stat.c              |  21 +++++++++++++
 tools/perf/util/metricgroup.c          | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------
 tools/perf/util/metricgroup.h          |   2 ++
 4 files changed, 168 insertions(+), 60 deletions(-)

[PATCH 1/3] perf tools: Factor metric addition into add_metric function

[Jiri Olsa]

Factoring metric addition into add_metric function,
so it can be used to add metric from different sources
in following patches.

Link: https://lkml.kernel.org/n/tip-qw1727kbewu315mz2h0y5xfm@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/metricgroup.c | 110 +++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 48 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6a4d350d5cdb..1d01958c148d 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -396,13 +396,65 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	strlist__delete(metriclist);
 }
 
+static int add_metric(const char *name, const char *expr, const char *unit,
+		      struct strbuf *events, struct list_head *group_list)
+{
+	bool no_group = false;
+	struct egroup *eg;
+	const char **ids;
+	int idnum, j;
+
+	pr_debug("metric expr %s for %s\n", expr, name);
+
+	if (expr__find_other(expr, NULL, &ids, &idnum) < 0)
+		return -1;
+
+	if (events->len > 0)
+		strbuf_addf(events, ",");
+
+	for (j = 0; j < idnum; j++) {
+		pr_debug("found event %s\n", ids[j]);
+		/*
+		 * Duration time maps to a software event and can make
+		 * groups not count. Always use it outside a
+		 * group.
+		 */
+		if (!strcmp(ids[j], "duration_time")) {
+			if (j > 0)
+				strbuf_addf(events, "}:W,");
+			strbuf_addf(events, "duration_time");
+			no_group = true;
+			continue;
+		}
+		strbuf_addf(events, "%s%s",
+			j == 0 || no_group ? "{" : ",",
+			ids[j]);
+		no_group = false;
+	}
+
+	if (!no_group)
+		strbuf_addf(events, "}:W");
+
+	eg = malloc(sizeof(struct egroup));
+	if (!eg)
+		return -ENOMEM;
+
+	eg->ids         = ids;
+	eg->idnum       = idnum;
+	eg->metric_name = name;
+	eg->metric_expr = expr;
+	eg->metric_unit = unit;
+	list_add_tail(&eg->nd, group_list);
+	return 0;
+}
+
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				   struct list_head *group_list)
 {
 	struct pmu_events_map *map = perf_pmu__find_map(NULL);
 	struct pmu_event *pe;
 	int ret = -EINVAL;
-	int i, j;
+	int i;
 
 	if (!map)
 		return 0;
@@ -414,55 +466,17 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 			break;
 		if (!pe->metric_expr)
 			continue;
-		if (match_metric(pe->metric_group, metric) ||
-		    match_metric(pe->metric_name, metric)) {
-			const char **ids;
-			int idnum;
-			struct egroup *eg;
-			bool no_group = false;
 
-			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
-
-			if (expr__find_other(pe->metric_expr,
-					     NULL, &ids, &idnum) < 0)
-				continue;
-			if (events->len > 0)
-				strbuf_addf(events, ",");
-			for (j = 0; j < idnum; j++) {
-				pr_debug("found event %s\n", ids[j]);
-				/*
-				 * Duration time maps to a software event and can make
-				 * groups not count. Always use it outside a
-				 * group.
-				 */
-				if (!strcmp(ids[j], "duration_time")) {
-					if (j > 0)
-						strbuf_addf(events, "}:W,");
-					strbuf_addf(events, "duration_time");
-					no_group = true;
-					continue;
-				}
-				strbuf_addf(events, "%s%s",
-					j == 0 || no_group ? "{" : ",",
-					ids[j]);
-				no_group = false;
-			}
-			if (!no_group)
-				strbuf_addf(events, "}:W");
+		if (!match_metric(pe->metric_group, metric) &&
+		    !match_metric(pe->metric_name, metric))
+			continue;
 
-			eg = malloc(sizeof(struct egroup));
-			if (!eg) {
-				ret = -ENOMEM;
-				break;
-			}
-			eg->ids = ids;
-			eg->idnum = idnum;
-			eg->metric_name = pe->metric_name;
-			eg->metric_expr = pe->metric_expr;
-			eg->metric_unit = pe->unit;
-			list_add_tail(&eg->nd, group_list);
-			ret = 0;
-		}
+		ret = add_metric(pe->metric_name,
+				 pe->metric_expr,
+				 pe->unit,
+				 events, group_list);
+		if (ret)
+			break;
 	}
 	return ret;
 }
-- 
2.21.1

[PATCH 2/3] perf tools: Factor metric setup code into metricgroup__setup function

[Jiri Olsa]

Factoring metric setup code into metricgroup__setup function,
so it can be used to add metric from different sources in
following patches.

Link: https://lkml.kernel.org/n/tip-7hjix4t30ls3qqd4l60dbr2n@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/metricgroup.c | 38 +++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 1d01958c148d..abcfa3c1b4d5 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -521,32 +521,44 @@ static void metricgroup__free_egroups(struct list_head *group_list)
 	}
 }
 
+static int metricgroup__setup(struct evlist *evlist,
+			      struct rblist *metric_events,
+			      struct strbuf *extra_events,
+			      struct list_head *group_list)
+{
+	struct parse_events_error parse_error;
+	int ret;
+
+	pr_debug("adding %s\n", extra_events->buf);
+	bzero(&parse_error, sizeof(parse_error));
+
+	ret = parse_events(evlist, extra_events->buf, &parse_error);
+	if (ret) {
+		parse_events_print_error(&parse_error, extra_events->buf);
+		return ret;
+	}
+
+	if (metric_events->nr_entries == 0)
+		metricgroup__rblist_init(metric_events);
+	return metricgroup__setup_events(group_list, evlist, metric_events);
+}
+
 int metricgroup__parse_groups(const struct option *opt,
 			   const char *str,
 			   struct rblist *metric_events)
 {
-	struct parse_events_error parse_error;
 	struct evlist *perf_evlist = *(struct evlist **)opt->value;
 	struct strbuf extra_events;
 	LIST_HEAD(group_list);
 	int ret;
 
-	if (metric_events->nr_entries == 0)
-		metricgroup__rblist_init(metric_events);
 	ret = metricgroup__add_metric_list(str, &extra_events, &group_list);
 	if (ret)
 		return ret;
-	pr_debug("adding %s\n", extra_events.buf);
-	bzero(&parse_error, sizeof(parse_error));
-	ret = parse_events(perf_evlist, extra_events.buf, &parse_error);
-	if (ret) {
-		parse_events_print_error(&parse_error, extra_events.buf);
-		goto out;
-	}
+
+	ret = metricgroup__setup(perf_evlist, metric_events, &extra_events,
+				 &group_list);
 	strbuf_release(&extra_events);
-	ret = metricgroup__setup_events(&group_list, perf_evlist,
-					metric_events);
-out:
 	metricgroup__free_egroups(&group_list);
 	return ret;
 }
-- 
2.21.1

[PATCH 3/3] perf stat: Add --metric option

[Jiri Olsa]

Adding --metric option that allows to specify metric on the command
line, like:

  # perf stat  --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
    (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...

The syntax of the --metric option argument is:

  [name[/unit]=]expression

where:
  name - is string that will identify expression in results
          (can't have = or /) default is "user metric"
  unit - is conversion number that multiplies result

Examples:
  ipc = instructions / cycles
  ipc/1 = instructions / cycles
  instructions / cycles

Currently only one metric can be passed to perf stat command.
The code facilitates the current metric code.

Link: https://lkml.kernel.org/n/tip-oe1ke93t9x9uc1hy0iueksqq@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-stat.txt | 16 ++++++++++
 tools/perf/builtin-stat.c              | 21 +++++++++++++
 tools/perf/util/metricgroup.c          | 43 ++++++++++++++++++++++++++
 tools/perf/util/metricgroup.h          |  2 ++
 4 files changed, 82 insertions(+)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 9431b8066fb4..6a76faec91f1 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -264,6 +264,22 @@ For a group all metrics from the group are added.
 The events from the metrics are automatically measured.
 See perf list output for the possble metrics and metricgroups.
 
+--metric::
+Print metric specified by the argument, where argument is defined as:
+
+  [name[/unit]=]expression
+
+  where:
+    name - is string that will identify expression in results
+           (can't have = or /) default is "user metric"
+    unit - is conversion number that multiplies result
+           default is 1
+
+  Examples:
+      ipc = instructions / cycles
+      ipc/1 = instructions / cycles
+      instructions / cycles
+
 -A::
 --no-aggr::
 Do not aggregate counts across all monitored CPUs.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index a098c2ebf4ea..206df6f1cc8a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -161,6 +161,7 @@ static bool			append_file;
 static bool			interval_count;
 static const char		*output_name;
 static int			output_fd;
+static char			*user_metric;
 
 struct perf_stat {
 	bool			 record;
@@ -841,6 +842,22 @@ static int parse_metric_groups(const struct option *opt,
 	return metricgroup__parse_groups(opt, str, &stat_config.metric_events);
 }
 
+static int parse_metric_expr(const struct option *opt,
+			     const char *str,
+			     int unset __maybe_unused)
+{
+	if (user_metric) {
+		pr_err("Only one user metric is currently supported.\n");
+		return -EINVAL;
+	}
+
+	user_metric = strdup(str);
+	if (!user_metric)
+		return -ENOMEM;
+
+	return metricgroup__parse_expr(opt, user_metric, &stat_config.metric_events);
+}
+
 static struct option stat_options[] = {
 	OPT_BOOLEAN('T', "transaction", &transaction_run,
 		    "hardware transaction statistics"),
@@ -923,6 +940,9 @@ static struct option stat_options[] = {
 	OPT_CALLBACK('M', "metrics", &evsel_list, "metric/metric group list",
 		     "monitor specified metrics or metric groups (separated by ,)",
 		     parse_metric_groups),
+	OPT_CALLBACK('m', "metric", &evsel_list, "metric expression",
+		     "monitor specified metric expression ([name/unit=]expression)",
+		     parse_metric_expr),
 	OPT_BOOLEAN_FLAG(0, "all-kernel", &stat_config.all_kernel,
 			 "Configure all used events to run in kernel space.",
 			 PARSE_OPT_EXCLUSIVE),
@@ -2184,6 +2204,7 @@ int cmd_stat(int argc, const char **argv)
 	perf_evlist__free_stats(evsel_list);
 out:
 	zfree(&stat_config.walltime_run);
+	zfree(&user_metric);
 
 	if (smi_cost && smi_reset)
 		sysfs__write_int(FREEZE_ON_SMI_PATH, 0);
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index abcfa3c1b4d5..c85be0ad8227 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -563,6 +563,49 @@ int metricgroup__parse_groups(const struct option *opt,
 	return ret;
 }
 
+int metricgroup__parse_expr(const struct option *opt, char *str,
+			    struct rblist *metric_events)
+{
+	struct evlist *perf_evlist = *(struct evlist **)opt->value;
+	char *tok, *expr, *unit = NULL;
+	struct strbuf extra_events;
+	LIST_HEAD(group_list);
+	const char *name;
+	int ret;
+
+	/*
+	 * user metric is passed as following argument:
+	 *   [name[/unit]=]expression
+	 */
+	tok = strchr(str, '=');
+	if (tok) {
+		*tok++ = 0;
+		name = str;
+		expr = tok;
+
+		tok = strchr(name, '/');
+		if (tok) {
+			*tok++ = 0;
+			unit = tok;
+		}
+	} else {
+		expr = str;
+		name = "user metric";
+	}
+
+	strbuf_init(&extra_events, 100);
+
+	ret = add_metric(name, expr, unit, &extra_events, &group_list);
+	if (ret)
+		return ret;
+
+	ret = metricgroup__setup(perf_evlist, metric_events, &extra_events,
+				 &group_list);
+	strbuf_release(&extra_events);
+	metricgroup__free_egroups(&group_list);
+	return ret;
+}
+
 bool metricgroup__has_metric(const char *metric)
 {
 	struct pmu_events_map *map = perf_pmu__find_map(NULL);
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..b66546b3ce1c 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -30,6 +30,8 @@ struct metric_event *metricgroup__lookup(struct rblist *metric_events,
 int metricgroup__parse_groups(const struct option *opt,
 			const char *str,
 			struct rblist *metric_events);
+int metricgroup__parse_expr(const struct option *opt, char *str,
+			    struct rblist *metric_events);
 
 void metricgroup__print(bool metrics, bool groups, char *filter,
 			bool raw, bool details);
-- 
2.21.1

Re: [RFC 0/3] perf tools: Add support for used defined metric

[Andi Kleen]

On Wed, Dec 11, 2019 at 11:47:57PM +0100, Jiri Olsa wrote:
> hi,
> Joe asked for possibility to add user defined metrics. Given that
> we already have metrics support, I added --metric option that allows
> to specify metric on the command line, like:
> 
>   # perf stat  --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
>     (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...
> 
> The code facilitates the current metric code, and I was surprised
> how easy it was, so I'm not sure I omitted something ;-)

There are some asserts you can hit, like for too many events.

Also some of the syntax (e.g. using @ instead of / and escapes) are
not super user friendly.

Other than that it should be ok.

Of course it would be better to put it into a file,
and then support comments etc.

I've been considering some extensions to perf to support @file
reading with comment support.

Right now there are some very odd bugs when you do that, lie

-e 'event,<newline>
event,<newline>
event...'

adds the newline to the event name, which breaks the output formats.

-Andi

Re: [PATCH 3/3] perf stat: Add --metric option

[Andi Kleen]

> Currently only one metric can be passed to perf stat command.

It's near certain that users will want more.

-Andi

Re: [RFC 0/3] perf tools: Add support for used defined metric

[Jiri Olsa]

On Wed, Dec 11, 2019 at 03:01:26PM -0800, Andi Kleen wrote:
> On Wed, Dec 11, 2019 at 11:47:57PM +0100, Jiri Olsa wrote:
> > hi,
> > Joe asked for possibility to add user defined metrics. Given that
> > we already have metrics support, I added --metric option that allows
> > to specify metric on the command line, like:
> > 
> >   # perf stat  --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
> >     (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...
> > 
> > The code facilitates the current metric code, and I was surprised
> > how easy it was, so I'm not sure I omitted something ;-)
> 
> There are some asserts you can hit, like for too many events.
> 
> Also some of the syntax (e.g. using @ instead of / and escapes) are
> not super user friendly.
> 
> Other than that it should be ok.
> 
> Of course it would be better to put it into a file,
> and then support comments etc.

should be also reasonable easy, redirecting the input for lex

> 
> I've been considering some extensions to perf to support @file
> reading with comment support.
> 
> Right now there are some very odd bugs when you do that, lie
> 
> -e 'event,<newline>
> event,<newline>
> event...'
> 
> adds the newline to the event name, which breaks the output formats.

should be possible to ignore new lines at some point no?

jirka

Re: [PATCH 3/3] perf stat: Add --metric option

[Jiri Olsa]

On Wed, Dec 11, 2019 at 03:02:23PM -0800, Andi Kleen wrote:
> > Currently only one metric can be passed to perf stat command.
> 
> It's near certain that users will want more.

right, it was just convenient for the rfc ;-)

but I wonder we could keep it that way anyway and have:

current way of using pre-defined metrics:
  # perf stat -M m1,m2,m3

simple/fast way of checking on metric:
  # perf stat -m 'm4=....'

metric from user file:
  # perf stat -M m5,m6 --metric-file=path

thanks,
jirka