[PATCHv2 0/3] perf tools: Add support for user defined metric

[PATCHv2 0/3] perf tools: Add support for user defined metric

[Jiri Olsa]

hi,
Joe asked for possibility to add user defined metrics. Given that
we already have metrics support, I added --metrics-file option that
allows to specify custom metrics.

  $ cat metrics
  # IPC
  mine1 = instructions / cycles;
  /* DECODED_ICACHE_UOPS% */
  mine2 = 100 * (idq.dsb_uops / \ (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops));

  $ sudo perf stat --metrics-file ./metrics -M mine1,mine2 --metric-only -a -I 1000
  #           time       insn per cycle                mine1                mine2
       1.000536263                0.71                   0.7                 41.4
       2.002069025                0.31                   0.3                 14.1
       3.003427684                0.27                   0.3                 14.8
       4.004807132                0.25                   0.2                 12.1
  ...

v2 changes:
  - add new --metrics-file option
  - rebased on current perf/core expression bison/flex enhancements

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

thanks,
jirka


---
Jiri Olsa (3):
      perf expr: Add parsing support for multiple expressions
      perf expr: Allow comments in custom metric file
      perf stat: Add --metrics-file option

 tools/perf/Documentation/perf-stat.txt |  3 +++
 tools/perf/builtin-stat.c              |  7 +++++--
 tools/perf/tests/expr.c                | 13 +++++++++++++
 tools/perf/util/expr.c                 |  6 ++++++
 tools/perf/util/expr.h                 | 19 +++++++++++++++++--
 tools/perf/util/expr.l                 | 24 ++++++++++++++++++++++++
 tools/perf/util/expr.y                 | 13 ++++++++++++-
 tools/perf/util/metricgroup.c          | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 tools/perf/util/metricgroup.h          |  3 ++-
 tools/perf/util/stat.h                 |  1 +
 10 files changed, 142 insertions(+), 13 deletions(-)

[PATCH 1/3] perf expr: Add parsing support for multiple expressions

[Jiri Olsa]

Adding support to parse metric difinitions in following form:

  NAME = EXPRESSION ;
  NAME = EXPRESSION ;
  ...

The parsed NAME and EXPRESSION will be used in following
changes to feed the metric code with definitions from
custom file.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/expr.c | 13 +++++++++++++
 tools/perf/util/expr.c  |  6 ++++++
 tools/perf/util/expr.h  | 19 +++++++++++++++++--
 tools/perf/util/expr.l  | 12 ++++++++++++
 tools/perf/util/expr.y  | 13 ++++++++++++-
 5 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index ea10fc4412c4..b220e316048a 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -63,5 +63,18 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
 		zfree(&other[i]);
 	free((void *)other);
 
+	expr__ctx_init(&ctx);
+	ret = expr__parse_custom(&ctx, "IPC=INSTRUCTIONS / CYCLES; CPI=CYCLES / INSTRUCTIONS;");
+	TEST_ASSERT_VAL("parse custom failed", ret == 0);
+	TEST_ASSERT_VAL("parse custom count", ctx.num_custom == 2);
+	TEST_ASSERT_VAL("parse custom name", !strcmp(ctx.custom[0].name, "IPC"));
+	TEST_ASSERT_VAL("parse custom name", !strcmp(ctx.custom[1].name, "CPI"));
+	TEST_ASSERT_VAL("parse custom expr", !strcmp(ctx.custom[0].expr, "INSTRUCTIONS / CYCLES"));
+	TEST_ASSERT_VAL("parse custom expr", !strcmp(ctx.custom[1].expr, "CYCLES / INSTRUCTIONS"));
+
+	for (i = 0; i < ctx.num_custom; i++) {
+		zfree(&ctx.custom[i].name);
+		zfree(&ctx.custom[i].expr);
+	}
 	return 0;
 }
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c3382d58cf40..805fbf6bffc8 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -23,6 +23,7 @@ void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
 void expr__ctx_init(struct expr_parse_ctx *ctx)
 {
 	ctx->num_ids = 0;
+	ctx->num_custom = 0;
 }
 
 static int
@@ -59,6 +60,11 @@ int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
 	return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
 }
 
+int expr__parse_custom(struct expr_parse_ctx *ctx, const char *expr)
+{
+	return __expr__parse(NULL, ctx, expr, EXPR_CUSTOM);
+}
+
 static bool
 already_seen(const char *val, const char *one, const char **other,
 	     int num_other)
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 0938ad166ece..4bbd24f9aaaa 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -4,15 +4,29 @@
 
 #define EXPR_MAX_OTHER 20
 #define MAX_PARSE_ID EXPR_MAX_OTHER
+#define EXPR_MAX 20
 
 struct expr_parse_id {
 	const char *name;
 	double val;
 };
 
+struct expr_parse_custom {
+	const char *name;
+	const char *expr;
+};
+
 struct expr_parse_ctx {
-	int num_ids;
-	struct expr_parse_id ids[MAX_PARSE_ID];
+	union {
+		struct {
+			int			 num_ids;
+			struct expr_parse_id	 ids[MAX_PARSE_ID];
+		};
+		struct {
+			int			 num_custom;
+			struct expr_parse_custom custom[EXPR_MAX];
+		};
+	};
 };
 
 struct expr_scanner_ctx {
@@ -22,6 +36,7 @@ struct expr_scanner_ctx {
 void expr__ctx_init(struct expr_parse_ctx *ctx);
 void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
 int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
+int expr__parse_custom(struct expr_parse_ctx *ctx, const char *expr);
 int expr__find_other(const char *expr, const char *one, const char ***other,
 		int *num_other);
 
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index 2582c2464938..5561c360cd27 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -68,12 +68,15 @@ static int str(yyscan_t scanner, int token)
 }
 %}
 
+%x custom
+
 number		[0-9]+
 
 sch		[-,=]
 spec		\\{sch}
 sym		[0-9a-zA-Z_\.:@]+
 symbol		{spec}*{sym}*{spec}*{sym}*
+all		[^;]+
 
 %%
 	struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
@@ -87,6 +90,12 @@ symbol		{spec}*{sym}*{spec}*{sym}*
 		}
 	}
 
+<custom>{
+
+{all}		{ BEGIN(INITIAL); return str(yyscanner, ALL); }
+
+}
+
 max		{ return MAX; }
 min		{ return MIN; }
 if		{ return IF; }
@@ -105,6 +114,9 @@ else		{ return ELSE; }
 "("		{ return '('; }
 ")"		{ return ')'; }
 ","		{ return ','; }
+";"		{ return ';'; }
+"="		{ BEGIN(custom); return '='; }
+\n		{ }
 .		{ }
 %%
 
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index cd17486c1c5d..a853974e7871 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -24,9 +24,10 @@
 	char	*str;
 }
 
-%token EXPR_PARSE EXPR_OTHER EXPR_ERROR
+%token EXPR_PARSE EXPR_OTHER EXPR_CUSTOM EXPR_ERROR
 %token <num> NUMBER
 %token <str> ID
+%token <str> ALL
 %token MIN MAX IF ELSE SMT_ON
 %left MIN MAX IF
 %left '|'
@@ -66,6 +67,16 @@ start:
 EXPR_PARSE all_expr
 |
 EXPR_OTHER all_other
+|
+EXPR_CUSTOM all_custom
+
+all_custom: all_custom ID '=' ALL ';'
+{
+	ctx->custom[ctx->num_custom].name = $2;
+	ctx->custom[ctx->num_custom].expr = $4;
+	ctx->num_custom++;
+}
+|
 
 all_other: all_other other
 |
-- 
2.25.3

[PATCH 2/3] perf expr: Allow comments in custom metric file

[Jiri Olsa]

Adding support to use comments within the custom metric file
with both the shell '# ... ' and C '/* ... */ styles.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/expr.l | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index 5561c360cd27..99547182ef79 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -69,6 +69,8 @@ static int str(yyscan_t scanner, int token)
 %}
 
 %x custom
+%x comment_C
+%x comment_sh
 
 number		[0-9]+
 
@@ -103,6 +105,16 @@ else		{ return ELSE; }
 #smt_on		{ return SMT_ON; }
 {number}	{ return value(yyscanner, 10); }
 {symbol}	{ return str(yyscanner, ID); }
+
+"/*"		{ BEGIN(comment_C); }
+<comment_C>"*/"	{ BEGIN(INITIAL); }
+<comment_C>\n	{ }
+<comment_C>.	{ }
+
+"#"		{ BEGIN(comment_sh); }
+<comment_sh>\n	{ BEGIN(INITIAL); }
+<comment_sh>.	{ }
+
 "|"		{ return '|'; }
 "^"		{ return '^'; }
 "&"		{ return '&'; }
-- 
2.25.3

[PATCH 3/3] perf stat: Add --metrics-file option

[Jiri Olsa]

Adding --metrics-file option that allows to specify metrics
in the file.

It's now possible to define metrics in file and use them like:

  $ cat metrics
  # IPC
  mine1 = instructions / cycles;
  /* DECODED_ICACHE_UOPS% */
  mine2 = 100 * (idq.dsb_uops / \ (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops));

  $ sudo perf stat --metrics-file ./metrics -M mine1,mine2 --metric-only -a -I 1000
  #           time       insn per cycle                mine1                mine2
       1.000536263                0.71                   0.7                 41.4
       2.002069025                0.31                   0.3                 14.1
       3.003427684                0.27                   0.3                 14.8
       4.004807132                0.25                   0.2                 12.1
  ...

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-stat.txt |  3 ++
 tools/perf/builtin-stat.c              |  7 ++-
 tools/perf/util/metricgroup.c          | 66 +++++++++++++++++++++++---
 tools/perf/util/metricgroup.h          |  3 +-
 tools/perf/util/stat.h                 |  1 +
 5 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 4d56586b2fb9..263b0d2bb835 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -264,6 +264,9 @@ 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.
 
+--metrics-file file::
+Read metrics definitions from file in addition to compiled in metrics.
+
 -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 9207b6c45475..81c2b2f8ec53 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -841,7 +841,8 @@ static int parse_metric_groups(const struct option *opt,
 			       const char *str,
 			       int unset __maybe_unused)
 {
-	return metricgroup__parse_groups(opt, str, &stat_config.metric_events);
+	return metricgroup__parse_groups(opt, str, &stat_config.metric_events,
+					 stat_config.metrics_file);
 }
 
 static struct option stat_options[] = {
@@ -926,6 +927,8 @@ 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_STRING(0, "metrics-file", &stat_config.metrics_file, "file path",
+		   "file with metrics definitions"),
 	OPT_BOOLEAN_FLAG(0, "all-kernel", &stat_config.all_kernel,
 			 "Configure all used events to run in kernel space.",
 			 PARSE_OPT_EXCLUSIVE),
@@ -1443,7 +1446,7 @@ static int add_default_attributes(void)
 			struct option opt = { .value = &evsel_list };
 
 			return metricgroup__parse_groups(&opt, "transaction",
-							 &stat_config.metric_events);
+							 &stat_config.metric_events, NULL);
 		}
 
 		if (pmu_have_event("cpu", "cycles-ct") &&
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 7ad81c8177ea..99e0ef725a6f 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -519,17 +519,17 @@ static int __metricgroup__add_metric(struct strbuf *events,
 }
 
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
-				   struct list_head *group_list)
+				   struct list_head *group_list,
+				   struct expr_parse_ctx *ctx)
 {
 	struct pmu_events_map *map = perf_pmu__find_map(NULL);
-	struct pmu_event *pe;
 	int i, ret = -EINVAL;
 
 	if (!map)
 		return 0;
 
 	for (i = 0; ; i++) {
-		pe = &map->table[i];
+		struct pmu_event *pe = &map->table[i];
 
 		if (!pe->name && !pe->metric_group && !pe->metric_name)
 			break;
@@ -545,15 +545,56 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				break;
 		}
 	}
+
+	if (!ctx->num_custom || ret == -ENOMEM)
+		return ret;
+
+	for (i = 0; i < ctx->num_custom; i++) {
+		struct pmu_event pe = { 0 };
+
+		if (!match_metric(ctx->custom[i].name, metric))
+			continue;
+
+		pe.metric_name = strdup(ctx->custom[i].name);
+		pe.metric_expr = strdup(ctx->custom[i].expr);
+
+		if (!pe.metric_name && !pe.metric_expr)
+			return -ENOMEM;
+
+		ret = __metricgroup__add_metric(events, group_list, &pe);
+		if (ret) {
+			free((char *) pe.metric_name);
+			free((char *) pe.metric_expr);
+			break;
+		}
+	}
+
 	return ret;
 }
 
 static int metricgroup__add_metric_list(const char *list, struct strbuf *events,
-				        struct list_head *group_list)
+					struct list_head *group_list,
+					const char *metrics_file)
 {
+	struct expr_parse_ctx ctx = { 0 };
 	char *llist, *nlist, *p;
 	int ret = -EINVAL;
 
+	if (metrics_file) {
+		size_t size;
+		char *buf;
+
+		if (filename__read_str(metrics_file, &buf, &size)) {
+			pr_err("failed to read metrics file: %s\n", metrics_file);
+			return -1;
+		}
+
+		expr__ctx_init(&ctx);
+		expr__parse_custom(&ctx, buf);
+
+		free(buf);
+	}
+
 	nlist = strdup(list);
 	if (!nlist)
 		return -ENOMEM;
@@ -563,7 +604,7 @@ static int metricgroup__add_metric_list(const char *list, struct strbuf *events,
 	strbuf_addf(events, "%s", "");
 
 	while ((p = strsep(&llist, ",")) != NULL) {
-		ret = metricgroup__add_metric(p, events, group_list);
+		ret = metricgroup__add_metric(p, events, group_list, &ctx);
 		if (ret == -EINVAL) {
 			fprintf(stderr, "Cannot find metric or group `%s'\n",
 					p);
@@ -575,6 +616,15 @@ static int metricgroup__add_metric_list(const char *list, struct strbuf *events,
 	if (!ret)
 		metricgroup___watchdog_constraint_hint(NULL, true);
 
+	if (metrics_file) {
+		int i;
+
+		for (i = 0; i < ctx.num_custom; i++) {
+			zfree(&ctx.custom[i].name);
+			zfree(&ctx.custom[i].expr);
+		}
+	}
+
 	return ret;
 }
 
@@ -594,7 +644,8 @@ static void metricgroup__free_egroups(struct list_head *group_list)
 
 int metricgroup__parse_groups(const struct option *opt,
 			   const char *str,
-			   struct rblist *metric_events)
+			   struct rblist *metric_events,
+			   const char *metrics_file)
 {
 	struct parse_events_error parse_error;
 	struct evlist *perf_evlist = *(struct evlist **)opt->value;
@@ -604,7 +655,8 @@ int metricgroup__parse_groups(const struct option *opt,
 
 	if (metric_events->nr_entries == 0)
 		metricgroup__rblist_init(metric_events);
-	ret = metricgroup__add_metric_list(str, &extra_events, &group_list);
+	ret = metricgroup__add_metric_list(str, &extra_events, &group_list,
+					   metrics_file);
 	if (ret)
 		return ret;
 	pr_debug("adding %s\n", extra_events.buf);
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..bf6e4281915f 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -29,7 +29,8 @@ struct metric_event *metricgroup__lookup(struct rblist *metric_events,
 					 bool create);
 int metricgroup__parse_groups(const struct option *opt,
 			const char *str,
-			struct rblist *metric_events);
+			struct rblist *metric_events,
+			const char *metrics_file);
 
 void metricgroup__print(bool metrics, bool groups, char *filter,
 			bool raw, bool details);
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index b4fdfaa7f2c0..57c4c7695aaf 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -123,6 +123,7 @@ struct perf_stat_config {
 	struct runtime_stat	*stats;
 	int			 stats_num;
 	const char		*csv_sep;
+	const char		*metrics_file;
 	struct stats		*walltime_nsecs_stats;
 	struct rusage		 ru_data;
 	struct perf_cpu_map		*aggr_map;
-- 
2.25.3

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

[Andi Kleen]

> +--metrics-file file::
> +Read metrics definitions from file in addition to compiled in metrics.
> +

You would need to define the syntax and format. Perhaps in a separate
man page.

Also there are some asserts that can be triggered by expressions. I think
you should fix those too and convert them to errors.

-Andi

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

[Jiri Olsa]

On Tue, Apr 21, 2020 at 11:36:15AM -0700, Andi Kleen wrote:
> > +--metrics-file file::
> > +Read metrics definitions from file in addition to compiled in metrics.
> > +
> 
> You would need to define the syntax and format. Perhaps in a separate
> man page.

I'm not sure it's worthy of new man page, but perhaps section in
perf-stat

> 
> Also there are some asserts that can be triggered by expressions. I think
> you should fix those too and convert them to errors.

do you have some details on this? examples of those failures?

thanks,
jirka

> 
> -Andi
> 

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

[Andi Kleen]

> > Also there are some asserts that can be triggered by expressions. I think
> > you should fix those too and convert them to errors.
> 
> do you have some details on this? examples of those failures?

At a minimum 

/* Caller must make sure id is allocated */
void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
{
        int idx;
        assert(ctx->num_ids < MAX_PARSE_ID);


-Andi

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

[Jiri Olsa]

On Tue, Apr 21, 2020 at 01:06:30PM -0700, Andi Kleen wrote:
> > > Also there are some asserts that can be triggered by expressions. I think
> > > you should fix those too and convert them to errors.
> > 
> > do you have some details on this? examples of those failures?
> 
> At a minimum 
> 
> /* Caller must make sure id is allocated */
> void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
> {
>         int idx;
>         assert(ctx->num_ids < MAX_PARSE_ID);
> 
> 
> -Andi
> 

I did it and then I realized this is already caught in the parsing
code (expr.y) with this check:

        if (ctx->num_ids + 1 >= EXPR_MAX_OTHER) {
                pr_err("failed: way too many variables\n");
                YYABORT;
        }

so that assert can stay there and shouldn't be ever hit

jirka

Re: [PATCHv2 0/3] perf tools: Add support for user defined metric

[kajoljain]

On 4/21/20 11:43 PM, Jiri Olsa wrote:
> hi,
> Joe asked for possibility to add user defined metrics. Given that
> we already have metrics support, I added --metrics-file option that
> allows to specify custom metrics.
> 
>   $ cat metrics
>   # IPC
>   mine1 = instructions / cycles;
>   /* DECODED_ICACHE_UOPS% */
>   mine2 = 100 * (idq.dsb_uops / \ (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops));
> 
>   $ sudo perf stat --metrics-file ./metrics -M mine1,mine2 --metric-only -a -I 1000
>   #           time       insn per cycle                mine1                mine2
>        1.000536263                0.71                   0.7                 41.4
>        2.002069025                0.31                   0.3                 14.1
>        3.003427684                0.27                   0.3                 14.8
>        4.004807132                0.25                   0.2                 12.1
>   ...
> 
> v2 changes:
>   - add new --metrics-file option
>   - rebased on current perf/core expression bison/flex enhancements
> 
> Also available in:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   perf/metric
> 
> thanks,
> jirka
> 
> 
> ---
> Jiri Olsa (3):
>       perf expr: Add parsing support for multiple expressions
>       perf expr: Allow comments in custom metric file
>       perf stat: Add --metrics-file option

Hi Jiri,
     I try to look into these patches. As far as I understand we are using
syntax "Name: Expression" for user defined events. It will be great if we mention
this format somewhere for users.

Otherwise it works fine for me. Try by testing it for different metric expressions.
But still curious about reason for adding this support. Isn't json file is there for same purpose?

Thanks,
Kajol Jain
> 
>  tools/perf/Documentation/perf-stat.txt |  3 +++
>  tools/perf/builtin-stat.c              |  7 +++++--
>  tools/perf/tests/expr.c                | 13 +++++++++++++
>  tools/perf/util/expr.c                 |  6 ++++++
>  tools/perf/util/expr.h                 | 19 +++++++++++++++++--
>  tools/perf/util/expr.l                 | 24 ++++++++++++++++++++++++
>  tools/perf/util/expr.y                 | 13 ++++++++++++-
>  tools/perf/util/metricgroup.c          | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  tools/perf/util/metricgroup.h          |  3 ++-
>  tools/perf/util/stat.h                 |  1 +
>  10 files changed, 142 insertions(+), 13 deletions(-)
> 

Re: [PATCHv2 0/3] perf tools: Add support for user defined metric

[Jiri Olsa]

On Thu, Apr 30, 2020 at 04:54:41PM +0530, kajoljain wrote:
> 
> 
> On 4/21/20 11:43 PM, Jiri Olsa wrote:
> > hi,
> > Joe asked for possibility to add user defined metrics. Given that
> > we already have metrics support, I added --metrics-file option that
> > allows to specify custom metrics.
> > 
> >   $ cat metrics
> >   # IPC
> >   mine1 = instructions / cycles;
> >   /* DECODED_ICACHE_UOPS% */
> >   mine2 = 100 * (idq.dsb_uops / \ (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops));
> > 
> >   $ sudo perf stat --metrics-file ./metrics -M mine1,mine2 --metric-only -a -I 1000
> >   #           time       insn per cycle                mine1                mine2
> >        1.000536263                0.71                   0.7                 41.4
> >        2.002069025                0.31                   0.3                 14.1
> >        3.003427684                0.27                   0.3                 14.8
> >        4.004807132                0.25                   0.2                 12.1
> >   ...
> > 
> > v2 changes:
> >   - add new --metrics-file option
> >   - rebased on current perf/core expression bison/flex enhancements
> > 
> > Also available in:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >   perf/metric
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> > Jiri Olsa (3):
> >       perf expr: Add parsing support for multiple expressions
> >       perf expr: Allow comments in custom metric file
> >       perf stat: Add --metrics-file option
> 
> Hi Jiri,
>      I try to look into these patches. As far as I understand we are using
> syntax "Name: Expression" for user defined events. It will be great if we mention
> this format somewhere for users.

right, Andi also asked for that, I'll describe it in a man page

> 
> Otherwise it works fine for me. Try by testing it for different metric expressions.

thanks for testing

> But still curious about reason for adding this support. Isn't json file is there for same purpose?

we've been asked by Joe about the possibility to specify metric by user
through the command line..  with json you have all them compiled in and 
you can't change them or specify your own without recompiling perf

jirka

> 
> Thanks,
> Kajol Jain
> > 
> >  tools/perf/Documentation/perf-stat.txt |  3 +++
> >  tools/perf/builtin-stat.c              |  7 +++++--
> >  tools/perf/tests/expr.c                | 13 +++++++++++++
> >  tools/perf/util/expr.c                 |  6 ++++++
> >  tools/perf/util/expr.h                 | 19 +++++++++++++++++--
> >  tools/perf/util/expr.l                 | 24 ++++++++++++++++++++++++
> >  tools/perf/util/expr.y                 | 13 ++++++++++++-
> >  tools/perf/util/metricgroup.c          | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  tools/perf/util/metricgroup.h          |  3 ++-
> >  tools/perf/util/stat.h                 |  1 +
> >  10 files changed, 142 insertions(+), 13 deletions(-)
> > 
>