* [PATCH v2 1/5] perf metric: Restructure struct expr_parse_ctx.
2020-11-18 5:03 [PATCH v2 0/5] Don't compute events that won't be used in a metric Ian Rogers
@ 2020-11-18 5:03 ` Ian Rogers
2020-11-18 5:03 ` [PATCH v2 2/5] perf metric: Use NAN for missing event IDs Ian Rogers
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2020-11-18 5:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel, Andi Kleen, Jin Yao, John Garry, Paul Clarke,
kajoljain
Cc: Stephane Eranian, Sandeep Dasgupta, linux-perf-users, Ian Rogers
A later change to parsing the ids out (in expr__find_other) will
potentially drop hashmaps and so it is more convenient to move
expr_parse_ctx to have a hashmap pointer rather than a struct value. As
this pointer must be freed, rather than just going out of scope,
add expr__ctx_new and expr__ctx_free to manage expr_parse_ctx memory.
Adjust use of struct expr_parse_ctx accordingly.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/expr.c | 81 ++++++++++++++++++-----------------
tools/perf/tests/pmu-events.c | 37 +++++++++-------
tools/perf/util/expr.c | 38 ++++++++++++----
tools/perf/util/expr.h | 5 ++-
tools/perf/util/metricgroup.c | 44 ++++++++++---------
tools/perf/util/stat-shadow.c | 50 +++++++++++++--------
6 files changed, 151 insertions(+), 104 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 4d01051951cd..b0a3b5fd0c00 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -22,67 +22,70 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
const char *p;
double val;
int ret;
- struct expr_parse_ctx ctx;
+ struct expr_parse_ctx *ctx;
- expr__ctx_init(&ctx);
- expr__add_id_val(&ctx, strdup("FOO"), 1);
- expr__add_id_val(&ctx, strdup("BAR"), 2);
+ ctx = expr__ctx_new();
+ TEST_ASSERT_VAL("expr__ctx_new", ctx);
+ expr__add_id_val(ctx, strdup("FOO"), 1);
+ expr__add_id_val(ctx, strdup("BAR"), 2);
- ret = test(&ctx, "1+1", 2);
- ret |= test(&ctx, "FOO+BAR", 3);
- ret |= test(&ctx, "(BAR/2)%2", 1);
- ret |= test(&ctx, "1 - -4", 5);
- ret |= test(&ctx, "(FOO-1)*2 + (BAR/2)%2 - -4", 5);
- ret |= test(&ctx, "1-1 | 1", 1);
- ret |= test(&ctx, "1-1 & 1", 0);
- ret |= test(&ctx, "min(1,2) + 1", 2);
- ret |= test(&ctx, "max(1,2) + 1", 3);
- ret |= test(&ctx, "1+1 if 3*4 else 0", 2);
- ret |= test(&ctx, "1.1 + 2.1", 3.2);
- ret |= test(&ctx, ".1 + 2.", 2.1);
- ret |= test(&ctx, "d_ratio(1, 2)", 0.5);
- ret |= test(&ctx, "d_ratio(2.5, 0)", 0);
- ret |= test(&ctx, "1.1 < 2.2", 1);
- ret |= test(&ctx, "2.2 > 1.1", 1);
- ret |= test(&ctx, "1.1 < 1.1", 0);
- ret |= test(&ctx, "2.2 > 2.2", 0);
- ret |= test(&ctx, "2.2 < 1.1", 0);
- ret |= test(&ctx, "1.1 > 2.2", 0);
+ ret = test(ctx, "1+1", 2);
+ ret |= test(ctx, "FOO+BAR", 3);
+ ret |= test(ctx, "(BAR/2)%2", 1);
+ ret |= test(ctx, "1 - -4", 5);
+ ret |= test(ctx, "(FOO-1)*2 + (BAR/2)%2 - -4", 5);
+ ret |= test(ctx, "1-1 | 1", 1);
+ ret |= test(ctx, "1-1 & 1", 0);
+ ret |= test(ctx, "min(1,2) + 1", 2);
+ ret |= test(ctx, "max(1,2) + 1", 3);
+ ret |= test(ctx, "1+1 if 3*4 else 0", 2);
+ ret |= test(ctx, "1.1 + 2.1", 3.2);
+ ret |= test(ctx, ".1 + 2.", 2.1);
+ ret |= test(ctx, "d_ratio(1, 2)", 0.5);
+ ret |= test(ctx, "d_ratio(2.5, 0)", 0);
+ ret |= test(ctx, "1.1 < 2.2", 1);
+ ret |= test(ctx, "2.2 > 1.1", 1);
+ ret |= test(ctx, "1.1 < 1.1", 0);
+ ret |= test(ctx, "2.2 > 2.2", 0);
+ ret |= test(ctx, "2.2 < 1.1", 0);
+ ret |= test(ctx, "1.1 > 2.2", 0);
- if (ret)
+ if (ret) {
+ expr__ctx_free(ctx);
return ret;
+ }
p = "FOO/0";
- ret = expr__parse(&val, &ctx, p, 1);
+ ret = expr__parse(&val, ctx, p, 1);
TEST_ASSERT_VAL("division by zero", ret == -1);
p = "BAR/";
- ret = expr__parse(&val, &ctx, p, 1);
+ ret = expr__parse(&val, ctx, p, 1);
TEST_ASSERT_VAL("missing operand", ret == -1);
- expr__ctx_clear(&ctx);
+ expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find other",
expr__find_other("FOO + BAR + BAZ + BOZO", "FOO",
- &ctx, 1) == 0);
- TEST_ASSERT_VAL("find other", hashmap__size(&ctx.ids) == 3);
- TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "BAR",
+ ctx, 1) == 0);
+ TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 3);
+ TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAR",
(void **)&val_ptr));
- TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "BAZ",
+ TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAZ",
(void **)&val_ptr));
- TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "BOZO",
+ TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BOZO",
(void **)&val_ptr));
- expr__ctx_clear(&ctx);
+ expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find other",
expr__find_other("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@",
- NULL, &ctx, 3) == 0);
- TEST_ASSERT_VAL("find other", hashmap__size(&ctx.ids) == 2);
- TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "EVENT1,param=3/",
+ NULL, ctx, 3) == 0);
+ TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 2);
+ TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT1,param=3/",
(void **)&val_ptr));
- TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "EVENT2,param=3/",
+ TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT2,param=3/",
(void **)&val_ptr));
- expr__ctx_clear(&ctx);
+ expr__ctx_free(ctx);
return 0;
}
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index ad2b21591275..294daf568bb6 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -478,9 +478,14 @@ static int test_parsing(void)
struct pmu_event *pe;
int i, j, k;
int ret = 0;
- struct expr_parse_ctx ctx;
+ struct expr_parse_ctx *ctx;
double result;
+ ctx = expr__ctx_new();
+ if (!ctx) {
+ pr_debug("expr__ctx_new failed");
+ return TEST_FAIL;
+ }
i = 0;
for (;;) {
map = &pmu_events_map[i++];
@@ -496,8 +501,8 @@ static int test_parsing(void)
break;
if (!pe->metric_expr)
continue;
- expr__ctx_init(&ctx);
- if (expr__find_other(pe->metric_expr, NULL, &ctx, 0)
+ expr__ctx_clear(ctx);
+ if (expr__find_other(pe->metric_expr, NULL, ctx, 0)
< 0) {
expr_failure("Parse other failed", map, pe);
ret++;
@@ -510,22 +515,22 @@ static int test_parsing(void)
* make them unique.
*/
k = 1;
- hashmap__for_each_entry((&ctx.ids), cur, bkt)
- expr__add_id_val(&ctx, strdup(cur->key), k++);
+ hashmap__for_each_entry(ctx->ids, cur, bkt)
+ expr__add_id_val(ctx, strdup(cur->key), k++);
- hashmap__for_each_entry((&ctx.ids), cur, bkt) {
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
if (check_parse_cpu(cur->key, map == cpus_map,
pe))
ret++;
}
- if (expr__parse(&result, &ctx, pe->metric_expr, 0)) {
+ if (expr__parse(&result, ctx, pe->metric_expr, 0)) {
expr_failure("Parse failed", map, pe);
ret++;
}
- expr__ctx_clear(&ctx);
}
}
+ expr__ctx_free(ctx);
/* TODO: fail when not ok */
return ret == 0 ? TEST_OK : TEST_SKIP;
}
@@ -544,7 +549,7 @@ static struct test_metric metrics[] = {
static int metric_parse_fake(const char *str)
{
- struct expr_parse_ctx ctx;
+ struct expr_parse_ctx *ctx;
struct hashmap_entry *cur;
double result;
int ret = -1;
@@ -553,8 +558,8 @@ static int metric_parse_fake(const char *str)
pr_debug("parsing '%s'\n", str);
- expr__ctx_init(&ctx);
- if (expr__find_other(str, NULL, &ctx, 0) < 0) {
+ ctx = expr__ctx_new();
+ if (expr__find_other(str, NULL, ctx, 0) < 0) {
pr_err("expr__find_other failed\n");
return -1;
}
@@ -565,23 +570,23 @@ static int metric_parse_fake(const char *str)
* make them unique.
*/
i = 1;
- hashmap__for_each_entry((&ctx.ids), cur, bkt)
- expr__add_id_val(&ctx, strdup(cur->key), i++);
+ hashmap__for_each_entry(ctx->ids, cur, bkt)
+ expr__add_id_val(ctx, strdup(cur->key), i++);
- hashmap__for_each_entry((&ctx.ids), cur, bkt) {
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
if (check_parse_fake(cur->key)) {
pr_err("check_parse_fake failed\n");
goto out;
}
}
- if (expr__parse(&result, &ctx, str, 1))
+ if (expr__parse(&result, ctx, str, 1))
pr_err("expr__parse failed\n");
else
ret = 0;
out:
- expr__ctx_clear(&ctx);
+ expr__ctx_free(ctx);
return ret;
}
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index a850fd0be3ee..e0623d38e6ee 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -73,7 +73,7 @@ int expr__add_id(struct expr_parse_ctx *ctx, const char *id)
data_ptr->parent = ctx->parent;
data_ptr->kind = EXPR_ID_DATA__PARENT;
- ret = hashmap__set(&ctx->ids, id, data_ptr,
+ ret = hashmap__set(ctx->ids, id, data_ptr,
(const void **)&old_key, (void **)&old_data);
if (ret)
free(data_ptr);
@@ -95,7 +95,7 @@ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val)
data_ptr->val = val;
data_ptr->kind = EXPR_ID_DATA__VALUE;
- ret = hashmap__set(&ctx->ids, id, data_ptr,
+ ret = hashmap__set(ctx->ids, id, data_ptr,
(const void **)&old_key, (void **)&old_data);
if (ret)
free(data_ptr);
@@ -140,7 +140,7 @@ int expr__add_ref(struct expr_parse_ctx *ctx, struct metric_ref *ref)
data_ptr->ref.metric_expr = ref->metric_expr;
data_ptr->kind = EXPR_ID_DATA__REF;
- ret = hashmap__set(&ctx->ids, name, data_ptr,
+ ret = hashmap__set(ctx->ids, name, data_ptr,
(const void **)&old_key, (void **)&old_data);
if (ret)
free(data_ptr);
@@ -156,7 +156,7 @@ int expr__add_ref(struct expr_parse_ctx *ctx, struct metric_ref *ref)
int expr__get_id(struct expr_parse_ctx *ctx, const char *id,
struct expr_id_data **data)
{
- return hashmap__find(&ctx->ids, id, (void **)data) ? 0 : -1;
+ return hashmap__find(ctx->ids, id, (void **)data) ? 0 : -1;
}
int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id,
@@ -205,15 +205,23 @@ void expr__del_id(struct expr_parse_ctx *ctx, const char *id)
struct expr_id_data *old_val = NULL;
char *old_key = NULL;
- hashmap__delete(&ctx->ids, id,
+ hashmap__delete(ctx->ids, id,
(const void **)&old_key, (void **)&old_val);
free(old_key);
free(old_val);
}
-void expr__ctx_init(struct expr_parse_ctx *ctx)
+struct expr_parse_ctx *expr__ctx_new(void)
{
- hashmap__init(&ctx->ids, key_hash, key_equal, NULL);
+ struct expr_parse_ctx *ctx;
+
+ ctx = malloc(sizeof(struct expr_parse_ctx));
+ if (!ctx)
+ return NULL;
+
+ ctx->ids = hashmap__new(key_hash, key_equal, NULL);
+ ctx->parent = NULL;
+ return ctx;
}
void expr__ctx_clear(struct expr_parse_ctx *ctx)
@@ -221,11 +229,23 @@ void expr__ctx_clear(struct expr_parse_ctx *ctx)
struct hashmap_entry *cur;
size_t bkt;
- hashmap__for_each_entry((&ctx->ids), cur, bkt) {
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
+ free((char *)cur->key);
+ free(cur->value);
+ }
+ hashmap__clear(ctx->ids);
+}
+
+void expr__ctx_free(struct expr_parse_ctx *ctx)
+{
+ struct hashmap_entry *cur;
+ size_t bkt;
+
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
free((char *)cur->key);
free(cur->value);
}
- hashmap__clear(&ctx->ids);
+ hashmap__free(ctx->ids);
}
static int
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index dcf8d19b83c8..00b941cfe6a6 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -19,7 +19,7 @@ struct expr_id {
};
struct expr_parse_ctx {
- struct hashmap ids;
+ struct hashmap *ids;
struct expr_id *parent;
};
@@ -30,8 +30,9 @@ struct expr_scanner_ctx {
int runtime;
};
-void expr__ctx_init(struct expr_parse_ctx *ctx);
+struct expr_parse_ctx *expr__ctx_new(void);
void expr__ctx_clear(struct expr_parse_ctx *ctx);
+void expr__ctx_free(struct expr_parse_ctx *ctx);
void expr__del_id(struct expr_parse_ctx *ctx, const char *id);
int expr__add_id(struct expr_parse_ctx *ctx, const char *id);
int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val);
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 81d201c8b833..342dcccb860f 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -118,7 +118,7 @@ struct metric_ref_node {
struct metric {
struct list_head nd;
- struct expr_parse_ctx pctx;
+ struct expr_parse_ctx *pctx;
const char *metric_name;
const char *metric_expr;
const char *metric_unit;
@@ -190,7 +190,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
struct evsel *ev, *current_leader = NULL;
struct expr_id_data *val_ptr;
int i = 0, matched_events = 0, events_to_match;
- const int idnum = (int)hashmap__size(&pctx->ids);
+ const int idnum = (int)hashmap__size(pctx->ids);
/*
* duration_time is always grouped separately, when events are grouped
@@ -198,7 +198,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
* add it to metric_events at the end.
*/
if (!has_constraint &&
- hashmap__find(&pctx->ids, "duration_time", (void **)&val_ptr))
+ hashmap__find(pctx->ids, "duration_time", (void **)&val_ptr))
events_to_match = idnum - 1;
else
events_to_match = idnum;
@@ -234,7 +234,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
if (contains_event(metric_events, matched_events, ev->name))
continue;
/* Does this event belong to the parse context? */
- if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr))
+ if (hashmap__find(pctx->ids, ev->name, (void **)&val_ptr))
metric_events[matched_events++] = ev;
if (matched_events == events_to_match)
@@ -313,12 +313,12 @@ static int metricgroup__setup_events(struct list_head *groups,
struct metric_ref *metric_refs = NULL;
metric_events = calloc(sizeof(void *),
- hashmap__size(&m->pctx.ids) + 1);
+ hashmap__size(m->pctx->ids) + 1);
if (!metric_events) {
ret = -ENOMEM;
break;
}
- evsel = find_evsel_group(perf_evlist, &m->pctx,
+ evsel = find_evsel_group(perf_evlist, m->pctx,
metric_no_merge,
m->has_constraint, metric_events,
evlist_used);
@@ -606,7 +606,7 @@ static void metricgroup__add_metric_weak_group(struct strbuf *events,
size_t bkt;
bool no_group = true, has_duration = false;
- hashmap__for_each_entry((&ctx->ids), cur, bkt) {
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
pr_debug("found event %s\n", (const char *)cur->key);
/*
* Duration time maps to a software event and can make
@@ -637,7 +637,7 @@ static void metricgroup__add_metric_non_group(struct strbuf *events,
size_t bkt;
bool first = true;
- hashmap__for_each_entry((&ctx->ids), cur, bkt) {
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
if (!first)
strbuf_addf(events, ",");
strbuf_addf(events, "%s", (const char *)cur->key);
@@ -703,7 +703,11 @@ static int __add_metric(struct list_head *metric_list,
if (!m)
return -ENOMEM;
- expr__ctx_init(&m->pctx);
+ m->pctx = expr__ctx_new();
+ if (!m->pctx) {
+ free(m);
+ return -ENOMEM;
+ }
m->metric_name = pe->metric_name;
m->metric_expr = pe->metric_expr;
m->metric_unit = pe->unit;
@@ -751,15 +755,15 @@ static int __add_metric(struct list_head *metric_list,
/* Force all found IDs in metric to have us as parent ID. */
WARN_ON_ONCE(!parent);
- m->pctx.parent = parent;
+ m->pctx->parent = parent;
/*
* For both the parent and referenced metrics, we parse
* all the metric's IDs and add it to the parent context.
*/
- if (expr__find_other(pe->metric_expr, NULL, &m->pctx, runtime) < 0) {
+ if (expr__find_other(pe->metric_expr, NULL, m->pctx, runtime) < 0) {
if (m->metric_refs_cnt == 0) {
- expr__ctx_clear(&m->pctx);
+ expr__ctx_free(m->pctx);
free(m);
*mp = NULL;
}
@@ -782,8 +786,8 @@ static int __add_metric(struct list_head *metric_list,
list_for_each_prev(pos, metric_list) {
struct metric *old = list_entry(pos, struct metric, nd);
- if (hashmap__size(&m->pctx.ids) <=
- hashmap__size(&old->pctx.ids))
+ if (hashmap__size(m->pctx->ids) <=
+ hashmap__size(old->pctx->ids))
break;
}
list_add(&m->nd, pos);
@@ -829,7 +833,7 @@ static int recursion_check(struct metric *m, const char *id, struct expr_id **pa
* if we already processed 'id', if we did, it's recursion
* and we fail.
*/
- ret = expr__get_id(&m->pctx, id, &data);
+ ret = expr__get_id(m->pctx, id, &data);
if (ret)
return ret;
@@ -884,7 +888,7 @@ static int __resolve_metric(struct metric *m,
*/
do {
all = true;
- hashmap__for_each_entry((&m->pctx.ids), cur, bkt) {
+ hashmap__for_each_entry(m->pctx->ids, cur, bkt) {
struct expr_id *parent;
struct pmu_event *pe;
@@ -898,7 +902,7 @@ static int __resolve_metric(struct metric *m,
all = false;
/* The metric key itself needs to go out.. */
- expr__del_id(&m->pctx, cur->key);
+ expr__del_id(m->pctx, cur->key);
/* ... and it gets resolved to the parent context. */
ret = add_metric(metric_list, pe, metric_no_group, &m, parent, ids);
@@ -1005,10 +1009,10 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
if (m->has_constraint) {
metricgroup__add_metric_non_group(events,
- &m->pctx);
+ m->pctx);
} else {
metricgroup__add_metric_weak_group(events,
- &m->pctx);
+ m->pctx);
}
}
@@ -1071,7 +1075,7 @@ static void metricgroup__free_metrics(struct list_head *metric_list)
list_for_each_entry_safe (m, tmp, metric_list, nd) {
metric__free_refs(m);
- expr__ctx_clear(&m->pctx);
+ expr__ctx_free(m->pctx);
list_del_init(&m->nd);
free(m);
}
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 901265127e36..bea7b5c6b1c0 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -1,8 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
+#include <math.h>
#include <stdio.h>
#include "evsel.h"
#include "stat.h"
#include "color.h"
+#include "debug.h"
#include "pmu.h"
#include "rblist.h"
#include "evlist.h"
@@ -335,12 +337,16 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
{
struct evsel *counter, *leader, **metric_events, *oc;
bool found;
- struct expr_parse_ctx ctx;
+ struct expr_parse_ctx *ctx;
struct hashmap_entry *cur;
size_t bkt;
int i;
- expr__ctx_init(&ctx);
+ ctx = expr__ctx_new();
+ if (!ctx) {
+ pr_debug("expr__ctx_new failed");
+ return;
+ }
evlist__for_each_entry(evsel_list, counter) {
bool invalid = false;
@@ -348,25 +354,25 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
if (!counter->metric_expr)
continue;
- expr__ctx_clear(&ctx);
+ expr__ctx_clear(ctx);
metric_events = counter->metric_events;
if (!metric_events) {
if (expr__find_other(counter->metric_expr,
counter->name,
- &ctx, 1) < 0)
+ ctx, 1) < 0)
continue;
metric_events = calloc(sizeof(struct evsel *),
- hashmap__size(&ctx.ids) + 1);
+ hashmap__size(ctx->ids) + 1);
if (!metric_events) {
- expr__ctx_clear(&ctx);
+ expr__ctx_free(ctx);
return;
}
counter->metric_events = metric_events;
}
i = 0;
- hashmap__for_each_entry((&ctx.ids), cur, bkt) {
+ hashmap__for_each_entry(ctx->ids, cur, bkt) {
const char *metric_name = (const char *)cur->key;
found = false;
@@ -418,7 +424,7 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
counter->metric_expr = NULL;
}
}
- expr__ctx_clear(&ctx);
+ expr__ctx_free(ctx);
}
static double runtime_stat_avg(struct runtime_stat *st,
@@ -793,7 +799,6 @@ static int prepare_metric(struct evsel **metric_events,
char *n, *pn;
int i, j, ret;
- expr__ctx_init(pctx);
for (i = 0; metric_events[i]; i++) {
struct saved_value *v;
struct stats *stats;
@@ -854,17 +859,22 @@ static void generic_metric(struct perf_stat_config *config,
struct runtime_stat *st)
{
print_metric_t print_metric = out->print_metric;
- struct expr_parse_ctx pctx;
+ struct expr_parse_ctx *pctx;
double ratio, scale;
int i;
void *ctxp = out->ctx;
- i = prepare_metric(metric_events, metric_refs, &pctx, cpu, st);
- if (i < 0)
+ pctx = expr__ctx_new();
+ if (!pctx)
return;
+ i = prepare_metric(metric_events, metric_refs, pctx, cpu, st);
+ if (i < 0) {
+ expr__ctx_free(pctx);
+ return;
+ }
if (!metric_events[i]) {
- if (expr__parse(&ratio, &pctx, metric_expr, runtime) == 0) {
+ if (expr__parse(&ratio, pctx, metric_expr, runtime) == 0) {
char *unit;
char metric_bf[64];
@@ -900,22 +910,26 @@ static void generic_metric(struct perf_stat_config *config,
(metric_name ? metric_name : name) : "", 0);
}
- expr__ctx_clear(&pctx);
+ expr__ctx_free(pctx);
}
double test_generic_metric(struct metric_expr *mexp, int cpu, struct runtime_stat *st)
{
- struct expr_parse_ctx pctx;
+ struct expr_parse_ctx *pctx;
double ratio = 0.0;
- if (prepare_metric(mexp->metric_events, mexp->metric_refs, &pctx, cpu, st) < 0)
+ pctx = expr__ctx_new();
+ if (!pctx)
+ return NAN;
+
+ if (prepare_metric(mexp->metric_events, mexp->metric_refs, pctx, cpu, st) < 0)
goto out;
- if (expr__parse(&ratio, &pctx, mexp->metric_expr, 1))
+ if (expr__parse(&ratio, pctx, mexp->metric_expr, 1))
ratio = 0.0;
out:
- expr__ctx_clear(&pctx);
+ expr__ctx_free(pctx);
return ratio;
}
--
2.29.2.299.gdc1121823c-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 3/5] perf metric: Rename expr__find_other.
2020-11-18 5:03 [PATCH v2 0/5] Don't compute events that won't be used in a metric Ian Rogers
2020-11-18 5:03 ` [PATCH v2 1/5] perf metric: Restructure struct expr_parse_ctx Ian Rogers
2020-11-18 5:03 ` [PATCH v2 2/5] perf metric: Use NAN for missing event IDs Ian Rogers
@ 2020-11-18 5:03 ` Ian Rogers
2020-11-18 5:03 ` [PATCH v2 4/5] perf metric: Add utilities to work on ids map Ian Rogers
2020-11-18 5:03 ` [PATCH v2 5/5] perf metric: Don't compute unused events Ian Rogers
4 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2020-11-18 5:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel, Andi Kleen, Jin Yao, John Garry, Paul Clarke,
kajoljain
Cc: Stephane Eranian, Sandeep Dasgupta, linux-perf-users, Ian Rogers
A later change will remove the notion of other, rename the function to
expr__find_ids as this is what it populated.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/expr.c | 8 ++++----
tools/perf/tests/pmu-events.c | 9 ++++-----
tools/perf/util/expr.c | 4 ++--
tools/perf/util/expr.h | 2 +-
tools/perf/util/metricgroup.c | 2 +-
tools/perf/util/stat-shadow.c | 6 +++---
6 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index b0a3b5fd0c00..9d7032041318 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -65,8 +65,8 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find other",
- expr__find_other("FOO + BAR + BAZ + BOZO", "FOO",
- ctx, 1) == 0);
+ expr__find_ids("FOO + BAR + BAZ + BOZO", "FOO",
+ ctx, 1) == 0);
TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 3);
TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAR",
(void **)&val_ptr));
@@ -77,8 +77,8 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find other",
- expr__find_other("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@",
- NULL, ctx, 3) == 0);
+ expr__find_ids("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@",
+ NULL, ctx, 3) == 0);
TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 2);
TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT1,param=3/",
(void **)&val_ptr));
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 294daf568bb6..3ac70fa31379 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -502,9 +502,8 @@ static int test_parsing(void)
if (!pe->metric_expr)
continue;
expr__ctx_clear(ctx);
- if (expr__find_other(pe->metric_expr, NULL, ctx, 0)
- < 0) {
- expr_failure("Parse other failed", map, pe);
+ if (expr__find_ids(pe->metric_expr, NULL, ctx, 0) < 0) {
+ expr_failure("Parse find ids failed", map, pe);
ret++;
continue;
}
@@ -559,8 +558,8 @@ static int metric_parse_fake(const char *str)
pr_debug("parsing '%s'\n", str);
ctx = expr__ctx_new();
- if (expr__find_other(str, NULL, ctx, 0) < 0) {
- pr_err("expr__find_other failed\n");
+ if (expr__find_ids(str, NULL, ctx, 0) < 0) {
+ pr_err("expr__find_ids failed\n");
return -1;
}
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index e0623d38e6ee..a248d14882cc 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -287,8 +287,8 @@ int expr__parse(double *final_val, struct expr_parse_ctx *ctx,
return __expr__parse(final_val, ctx, expr, EXPR_PARSE, runtime) ? -1 : 0;
}
-int expr__find_other(const char *expr, const char *one,
- struct expr_parse_ctx *ctx, int runtime)
+int expr__find_ids(const char *expr, const char *one,
+ struct expr_parse_ctx *ctx, int runtime)
{
int ret = __expr__parse(NULL, ctx, expr, EXPR_OTHER, runtime);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 00b941cfe6a6..955d5adb7ca4 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -43,7 +43,7 @@ int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id,
struct expr_id_data **datap);
int expr__parse(double *final_val, struct expr_parse_ctx *ctx,
const char *expr, int runtime);
-int expr__find_other(const char *expr, const char *one,
+int expr__find_ids(const char *expr, const char *one,
struct expr_parse_ctx *ids, int runtime);
double expr_id_data__value(const struct expr_id_data *data);
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 342dcccb860f..0be684bb020f 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -761,7 +761,7 @@ static int __add_metric(struct list_head *metric_list,
* For both the parent and referenced metrics, we parse
* all the metric's IDs and add it to the parent context.
*/
- if (expr__find_other(pe->metric_expr, NULL, m->pctx, runtime) < 0) {
+ if (expr__find_ids(pe->metric_expr, NULL, m->pctx, runtime) < 0) {
if (m->metric_refs_cnt == 0) {
expr__ctx_free(m->pctx);
free(m);
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index bea7b5c6b1c0..91bb7245f8b1 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -357,9 +357,9 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
expr__ctx_clear(ctx);
metric_events = counter->metric_events;
if (!metric_events) {
- if (expr__find_other(counter->metric_expr,
- counter->name,
- ctx, 1) < 0)
+ if (expr__find_ids(counter->metric_expr,
+ counter->name,
+ ctx, 1) < 0)
continue;
metric_events = calloc(sizeof(struct evsel *),
--
2.29.2.299.gdc1121823c-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 4/5] perf metric: Add utilities to work on ids map.
2020-11-18 5:03 [PATCH v2 0/5] Don't compute events that won't be used in a metric Ian Rogers
` (2 preceding siblings ...)
2020-11-18 5:03 ` [PATCH v2 3/5] perf metric: Rename expr__find_other Ian Rogers
@ 2020-11-18 5:03 ` Ian Rogers
2020-11-18 5:03 ` [PATCH v2 5/5] perf metric: Don't compute unused events Ian Rogers
4 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2020-11-18 5:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel, Andi Kleen, Jin Yao, John Garry, Paul Clarke,
kajoljain
Cc: Stephane Eranian, Sandeep Dasgupta, linux-perf-users, Ian Rogers
Add utilities to new/free an ids hashmap, as well as to union. Add
testing of the union. Unioning hashmaps will be used when parsing the
metric, if a value is known then the hashmap is unnecessary, otherwise
we need to union together all the event ids to compute their values for
reporting.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/expr.c | 47 ++++++++++++++++++++++
tools/perf/util/expr.c | 87 +++++++++++++++++++++++++++++++++++++++--
tools/perf/util/expr.h | 9 +++++
3 files changed, 139 insertions(+), 4 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 9d7032041318..7c2a01cf0650 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -6,6 +6,51 @@
#include <string.h>
#include <linux/zalloc.h>
+static int test_ids_union(void)
+{
+ struct hashmap *ids1, *ids2;
+
+ /* Empty union. */
+ ids1 = ids__new();
+ TEST_ASSERT_VAL("ids__new", ids1);
+ ids2 = ids__new();
+ TEST_ASSERT_VAL("ids__new", ids2);
+
+ ids1 = ids__union(ids1, ids2);
+ TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 0);
+
+ /* Union {foo, bar} against {}. */
+ ids2 = ids__new();
+ TEST_ASSERT_VAL("ids__new", ids2);
+
+ TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("foo"), NULL), 0);
+ TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("bar"), NULL), 0);
+
+ ids1 = ids__union(ids1, ids2);
+ TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 2);
+
+ /* Union {foo, bar} against {foo}. */
+ ids2 = ids__new();
+ TEST_ASSERT_VAL("ids__new", ids2);
+ TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("foo"), NULL), 0);
+
+ ids1 = ids__union(ids1, ids2);
+ TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 2);
+
+ /* Union {foo, bar} against {bar,baz}. */
+ ids2 = ids__new();
+ TEST_ASSERT_VAL("ids__new", ids2);
+ TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("bar"), NULL), 0);
+ TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("baz"), NULL), 0);
+
+ ids1 = ids__union(ids1, ids2);
+ TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 3);
+
+ ids__free(ids1);
+
+ return 0;
+}
+
static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
@@ -24,6 +69,8 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
int ret;
struct expr_parse_ctx *ctx;
+ TEST_ASSERT_EQUAL("ids_union", test_ids_union(), 0);
+
ctx = expr__ctx_new();
TEST_ASSERT_VAL("expr__ctx_new", ctx);
expr__add_id_val(ctx, strdup("FOO"), 1);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index a248d14882cc..1adb6cd202e0 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -59,8 +59,48 @@ static bool key_equal(const void *key1, const void *key2,
return !strcmp((const char *)key1, (const char *)key2);
}
-/* Caller must make sure id is allocated */
-int expr__add_id(struct expr_parse_ctx *ctx, const char *id)
+struct hashmap *ids__new(void)
+{
+ return hashmap__new(key_hash, key_equal, NULL);
+}
+
+void ids__free(struct hashmap *ids)
+{
+ struct hashmap_entry *cur;
+ size_t bkt;
+
+ if (ids == NULL)
+ return;
+
+#ifdef PARSER_DEBUG
+ fprintf(stderr, "freeing ids: ");
+ ids__print(ids);
+ fprintf(stderr, "\n");
+#endif
+
+ hashmap__for_each_entry(ids, cur, bkt) {
+ free((char *)cur->key);
+ free(cur->value);
+ }
+
+ hashmap__free(ids);
+}
+
+void ids__print(struct hashmap *ids)
+{
+ size_t bkt;
+ struct hashmap_entry *cur;
+
+ if (!ids)
+ return;
+
+ hashmap__for_each_entry(ids, cur, bkt) {
+ fprintf(stderr, "key:%s, ", (const char *)cur->key);
+ }
+}
+
+int ids__insert(struct hashmap *ids, const char *id,
+ struct expr_id *parent)
{
struct expr_id_data *data_ptr = NULL, *old_data = NULL;
char *old_key = NULL;
@@ -70,10 +110,10 @@ int expr__add_id(struct expr_parse_ctx *ctx, const char *id)
if (!data_ptr)
return -ENOMEM;
- data_ptr->parent = ctx->parent;
+ data_ptr->parent = parent;
data_ptr->kind = EXPR_ID_DATA__PARENT;
- ret = hashmap__set(ctx->ids, id, data_ptr,
+ ret = hashmap__set(ids, id, data_ptr,
(const void **)&old_key, (void **)&old_data);
if (ret)
free(data_ptr);
@@ -82,6 +122,45 @@ int expr__add_id(struct expr_parse_ctx *ctx, const char *id)
return ret;
}
+struct hashmap *ids__union(struct hashmap *ids1, struct hashmap *ids2)
+{
+ size_t bkt;
+ struct hashmap_entry *cur;
+ int ret;
+ struct expr_id_data *old_data = NULL;
+ char *old_key = NULL;
+
+ if (!ids1)
+ return ids2;
+
+ if (!ids2)
+ return ids1;
+
+ if (hashmap__size(ids1) < hashmap__size(ids2)) {
+ struct hashmap *tmp = ids1;
+
+ ids1 = ids2;
+ ids2 = tmp;
+ }
+ hashmap__for_each_entry(ids2, cur, bkt) {
+ ret = hashmap__set(ids1, cur->key, cur->value,
+ (const void **)&old_key, (void **)&old_data);
+ free(old_key);
+ free(old_data);
+
+ if (ret)
+ break;
+ }
+ hashmap__free(ids2);
+ return ids1;
+}
+
+/* Caller must make sure id is allocated */
+int expr__add_id(struct expr_parse_ctx *ctx, const char *id)
+{
+ return ids__insert(ctx->ids, id, ctx->parent);
+}
+
/* Caller must make sure id is allocated */
int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val)
{
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 955d5adb7ca4..62d3ae5ddfba 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -30,9 +30,16 @@ struct expr_scanner_ctx {
int runtime;
};
+struct hashmap *ids__new(void);
+void ids__free(struct hashmap *ids);
+void ids__print(struct hashmap *ids);
+int ids__insert(struct hashmap *ids, const char *id, struct expr_id *parent);
+struct hashmap *ids__union(struct hashmap *ids1, struct hashmap *ids2);
+
struct expr_parse_ctx *expr__ctx_new(void);
void expr__ctx_clear(struct expr_parse_ctx *ctx);
void expr__ctx_free(struct expr_parse_ctx *ctx);
+
void expr__del_id(struct expr_parse_ctx *ctx, const char *id);
int expr__add_id(struct expr_parse_ctx *ctx, const char *id);
int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val);
@@ -41,8 +48,10 @@ int expr__get_id(struct expr_parse_ctx *ctx, const char *id,
struct expr_id_data **data);
int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id,
struct expr_id_data **datap);
+
int expr__parse(double *final_val, struct expr_parse_ctx *ctx,
const char *expr, int runtime);
+
int expr__find_ids(const char *expr, const char *one,
struct expr_parse_ctx *ids, int runtime);
--
2.29.2.299.gdc1121823c-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 5/5] perf metric: Don't compute unused events.
2020-11-18 5:03 [PATCH v2 0/5] Don't compute events that won't be used in a metric Ian Rogers
` (3 preceding siblings ...)
2020-11-18 5:03 ` [PATCH v2 4/5] perf metric: Add utilities to work on ids map Ian Rogers
@ 2020-11-18 5:03 ` Ian Rogers
2020-11-19 20:59 ` Jiri Olsa
4 siblings, 1 reply; 9+ messages in thread
From: Ian Rogers @ 2020-11-18 5:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel, Andi Kleen, Jin Yao, John Garry, Paul Clarke,
kajoljain
Cc: Stephane Eranian, Sandeep Dasgupta, linux-perf-users, Ian Rogers
For a metric like:
EVENT1 if #smt_on else EVENT2
currently EVENT1 and EVENT2 will be measured and then when the metric is
reported EVENT1 or EVENT2 will be printed depending on the value from
smt_on() during the expr parsing. Computing both events is unnecessary and
can lead to multiplexing as discussed in this thread:
https://lore.kernel.org/lkml/20201110100346.2527031-1-irogers@google.com/
This change modifies the expression parsing code by:
- getting rid of the "other" parsing and introducing a boolean argument
to say whether ids should be computed or not.
- expressions are changed so that a pair of value and ids are returned.
- when computing the metric value the ids are unused.
- when computing the ids, constant values and smt_on are assigned to
the value.
- If the value is from an event ID then the event is added to the ids
hashmap and the value set to NAN.
- Typically operators union IDs for their inputs and set the value to
NAN, however, if the inputs are constant then these are computed and
propagated as the value.
- If the input is constant to certain operators like:
IDS1 if CONST else IDS2
then the result will be either IDS1 or IDS2 depending on CONST which
may be evaluated from an entire expression.
- The ids at the end of parsing are added to the context.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/expr.c | 10 ++
tools/perf/util/expr.c | 9 +-
tools/perf/util/expr.h | 1 -
tools/perf/util/expr.l | 9 --
tools/perf/util/expr.y | 341 +++++++++++++++++++++++++++++++---------
5 files changed, 284 insertions(+), 86 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 7c2a01cf0650..94ddd01b29fc 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include "util/debug.h"
#include "util/expr.h"
+#include "util/smt.h"
#include "tests.h"
#include <stdlib.h>
#include <string.h>
@@ -132,6 +133,15 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT2,param=3/",
(void **)&val_ptr));
+ expr__ctx_clear(ctx);
+ TEST_ASSERT_VAL("find ids",
+ expr__find_ids("EVENT1 if #smt_on else EVENT2",
+ NULL, ctx, 0) == 0);
+ TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
+ TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids,
+ smt_on() ? "EVENT1" : "EVENT2",
+ (void **)&val_ptr));
+
expr__ctx_free(ctx);
return 0;
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index 1adb6cd202e0..28aaa50c6c68 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -329,10 +329,9 @@ void expr__ctx_free(struct expr_parse_ctx *ctx)
static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
- int start, int runtime)
+ bool compute_ids, int runtime)
{
struct expr_scanner_ctx scanner_ctx = {
- .start_token = start,
.runtime = runtime,
};
YY_BUFFER_STATE buffer;
@@ -352,7 +351,7 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
expr_set_debug(1, scanner);
#endif
- ret = expr_parse(val, ctx, scanner);
+ ret = expr_parse(val, ctx, compute_ids, scanner);
expr__flush_buffer(buffer, scanner);
expr__delete_buffer(buffer, scanner);
@@ -363,13 +362,13 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int expr__parse(double *final_val, struct expr_parse_ctx *ctx,
const char *expr, int runtime)
{
- return __expr__parse(final_val, ctx, expr, EXPR_PARSE, runtime) ? -1 : 0;
+ return __expr__parse(final_val, ctx, expr, /*compute_ids=*/false, runtime) ? -1 : 0;
}
int expr__find_ids(const char *expr, const char *one,
struct expr_parse_ctx *ctx, int runtime)
{
- int ret = __expr__parse(NULL, ctx, expr, EXPR_OTHER, runtime);
+ int ret = __expr__parse(NULL, ctx, expr, /*compute_ids=*/true, runtime);
if (one)
expr__del_id(ctx, one);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 62d3ae5ddfba..cefeb2c8d85e 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -26,7 +26,6 @@ struct expr_parse_ctx {
struct expr_id_data;
struct expr_scanner_ctx {
- int start_token;
int runtime;
};
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index 13e5e3c75f56..702fdf6456ca 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -91,15 +91,6 @@ symbol ({spec}|{sym})+
%%
struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
- {
- int start_token = sctx->start_token;
-
- if (sctx->start_token) {
- sctx->start_token = 0;
- return start_token;
- }
- }
-
d_ratio { return D_RATIO; }
max { return MAX; }
min { return MIN; }
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index c22e3500a40f..931c1dc2bd26 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -10,34 +10,29 @@
#include "expr.h"
#include "smt.h"
#include <string.h>
-
-static double d_ratio(double val0, double val1)
-{
- if (val1 == 0) {
- return 0;
- }
- return val0 / val1;
-}
-
%}
%define api.pure full
%parse-param { double *final_val }
%parse-param { struct expr_parse_ctx *ctx }
+%parse-param { bool compute_ids }
%parse-param {void *scanner}
%lex-param {void* scanner}
%union {
double num;
char *str;
+ struct ids {
+ /* When creating ids, holds the working set of event ids. */
+ struct hashmap *ids;
+ /* The metric value. When computing ids NAN is used to indicate
+ * that all event ids are necessary. */
+ double val;
+ } ids;
}
-%token EXPR_PARSE EXPR_OTHER EXPR_ERROR
-%token <num> NUMBER
-%token <str> ID
-%destructor { free ($$); } <str>
-%token MIN MAX IF ELSE SMT_ON D_RATIO
+%token ID NUMBER MIN MAX IF ELSE SMT_ON D_RATIO EXPR_ERROR
%left MIN MAX IF
%left '|'
%left '^'
@@ -46,11 +41,16 @@ static double d_ratio(double val0, double val1)
%left '-' '+'
%left '*' '/' '%'
%left NEG NOT
-%type <num> expr if_expr
+%type <num> NUMBER
+%type <str> ID
+%destructor { free ($$); } <str>
+%type <ids> expr if_expr
+%destructor { ids__free($$.ids); } <ids>
%{
static void expr_error(double *final_val __maybe_unused,
struct expr_parse_ctx *ctx __maybe_unused,
+ bool compute_ids __maybe_unused,
void *scanner,
const char *s)
{
@@ -60,68 +60,267 @@ static void expr_error(double *final_val __maybe_unused,
%}
%%
-start:
-EXPR_PARSE all_expr
-|
-EXPR_OTHER all_other
+start: if_expr
+{
+ if (compute_ids)
+ ctx->ids = ids__union($1.ids, ctx->ids);
-all_other: all_other other
-|
+ if (final_val)
+ *final_val = $1.val;
+}
+;
-other: ID
+if_expr: expr IF expr ELSE expr
{
- expr__add_id(ctx, $1);
+ if (fpclassify($3.val) == FP_ZERO) {
+ $$.val = $5.val;
+ $$.ids = $5.ids;
+ ids__free($1.ids);
+ ids__free($3.ids);
+ } else if (isfinite($3.val)) {
+ $$.val = $1.val;
+ $$.ids = $1.ids;
+ ids__free($3.ids);
+ ids__free($5.ids);
+ } else if (!compute_ids) {
+ $$.val = $5.val;
+ $$.ids = NULL;
+ } else {
+ $$.ids = ids__union(ids__union($1.ids, $3.ids), $5.ids);
+ $$.val = NAN;
+ }
}
-|
-MIN | MAX | IF | ELSE | SMT_ON | NUMBER | '|' | '^' | '&' | '-' | '+' | '*' | '/' | '%' | '(' | ')' | ','
-|
-'<' | '>' | D_RATIO
-
-all_expr: if_expr { *final_val = $1; }
- ;
-
-if_expr:
- expr IF expr ELSE expr { $$ = $3 ? $1 : $5; }
- | expr
- ;
+| expr
+;
-expr: NUMBER
- | ID {
- struct expr_id_data *data;
+expr: NUMBER
+{
+ $$.val = $1;
+ $$.ids = NULL;
+}
+| ID
+{
+ if (!compute_ids) {
+ struct expr_id_data *data;
- if (expr__resolve_id(ctx, $1, &data)) {
- $$ = NAN;
- }
+ $$.val = NAN;
+ if (expr__resolve_id(ctx, $1, &data) == 0)
+ $$.val = expr_id_data__value(data);
- $$ = expr_id_data__value(data);
- free($1);
- }
- | expr '|' expr { $$ = (long)$1 | (long)$3; }
- | expr '&' expr { $$ = (long)$1 & (long)$3; }
- | expr '^' expr { $$ = (long)$1 ^ (long)$3; }
- | expr '<' expr { $$ = $1 < $3; }
- | expr '>' expr { $$ = $1 > $3; }
- | expr '+' expr { $$ = $1 + $3; }
- | expr '-' expr { $$ = $1 - $3; }
- | expr '*' expr { $$ = $1 * $3; }
- | expr '/' expr { if ($3 == 0) {
- pr_debug("division by zero\n");
- YYABORT;
- }
- $$ = $1 / $3;
- }
- | expr '%' expr { if ((long)$3 == 0) {
- pr_debug("division by zero\n");
- YYABORT;
- }
- $$ = (long)$1 % (long)$3;
- }
- | '-' expr %prec NEG { $$ = -$2; }
- | '(' if_expr ')' { $$ = $2; }
- | MIN '(' expr ',' expr ')' { $$ = $3 < $5 ? $3 : $5; }
- | MAX '(' expr ',' expr ')' { $$ = $3 > $5 ? $3 : $5; }
- | SMT_ON { $$ = smt_on() > 0; }
- | D_RATIO '(' expr ',' expr ')' { $$ = d_ratio($3,$5); }
- ;
+ $$.ids = NULL;
+ free($1);
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__new();
+ if (!$$.ids || ids__insert($$.ids, $1, ctx->parent))
+ YYABORT;
+ }
+}
+| expr '|' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = (long)$1.val | (long)$3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '&' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = (long)$1.val & (long)$3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '^' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = (long)$1.val ^ (long)$3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '<' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = $1.val < $3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '>' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = $1.val > $3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '+' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = $1.val + $3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '-' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = $1.val - $3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '*' expr
+{
+ if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = $1.val * $3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| expr '/' expr
+{
+ if (fpclassify($3.val) == FP_ZERO) {
+ pr_debug("division by zero\n");
+ YYABORT;
+ } else if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = $1.val / $3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| D_RATIO '(' expr ',' expr ')'
+{
+ if (fpclassify($5.val) == FP_ZERO) {
+ $$.val = 0.0;
+ $$.ids = NULL;
+ ids__free($3.ids);
+ ids__free($5.ids);
+ } else if (!compute_ids || (isfinite($3.val) && isfinite($5.val))) {
+ $$.val = $3.val / $5.val;
+ $$.ids = NULL;
+ ids__free($3.ids);
+ ids__free($5.ids);
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($3.ids, $5.ids);
+ }
+}
+| expr '%' expr
+{
+ if (fpclassify($3.val) == FP_ZERO) {
+ pr_debug("division by zero\n");
+ YYABORT;
+ } else if (!compute_ids || (isfinite($1.val) && isfinite($3.val))) {
+ $$.val = (long)$1.val % (long)$3.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($1.ids);
+ ids__free($3.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($1.ids, $3.ids);
+ }
+}
+| '-' expr %prec NEG
+{
+ $$.val = -$2.val;
+ $$.ids = $2.ids;
+}
+| '(' if_expr ')'
+{
+ $$ = $2;
+}
+| MIN '(' expr ',' expr ')'
+{
+ if (!compute_ids || (isfinite($3.val) && isfinite($5.val))) {
+ $$.val = $3.val < $5.val ? $3.val : $5.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($3.ids);
+ ids__free($5.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($3.ids, $5.ids);
+ }
+}
+| MAX '(' expr ',' expr ')'
+{
+ if (!compute_ids || (isfinite($3.val) && isfinite($5.val))) {
+ $$.val = $3.val > $5.val ? $3.val : $5.val;
+ $$.ids = NULL;
+ if (compute_ids) {
+ ids__free($3.ids);
+ ids__free($5.ids);
+ }
+ } else {
+ $$.val = NAN;
+ $$.ids = ids__union($3.ids, $5.ids);
+ }
+}
+| SMT_ON
+{
+ $$.val = smt_on() > 0 ? 1.0 : 0.0;
+ $$.ids = NULL;
+}
+;
%%
--
2.29.2.299.gdc1121823c-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread