From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
linux-kernel@vger.kernel.org, Andi Kleen <ak@linux.intel.com>,
Jin Yao <yao.jin@linux.intel.com>,
John Garry <john.garry@huawei.com>, Paul Clarke <pc@us.ibm.com>,
kajoljain <kjain@linux.ibm.com>,
linux-perf-users@vger.kernel.org
Cc: Stephane Eranian <eranian@google.com>,
Sandeep Dasgupta <sdasgup@google.com>,
Ian Rogers <irogers@google.com>
Subject: [PATCH v9 09/13] perf metric: Allow metrics with no events
Date: Thu, 23 Sep 2021 00:46:12 -0700 [thread overview]
Message-ID: <20210923074616.674826-10-irogers@google.com> (raw)
In-Reply-To: <20210923074616.674826-1-irogers@google.com>
A metric may be a constant value, for example, some SMT metrics are
constant 0 if #smt_on is 0. If we eliminate all the events then there is
no printing. Fix this by forcing metrics like this to have a
duration_time tool event, previously the metric would fail when parsing
the events with a parse error.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/metricgroup.c | 109 ++++++++++++++++++----------------
1 file changed, 59 insertions(+), 50 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 046fb3fe1700..34956977e907 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -198,65 +198,69 @@ 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);
+ int idnum = (int)hashmap__size(pctx->ids);
- /*
- * duration_time is always grouped separately, when events are grouped
- * (ie has_constraint is false) then ignore it in the matching loop and
- * add it to metric_events at the end.
- */
- if (!has_constraint &&
- hashmap__find(pctx->ids, "duration_time", (void **)&val_ptr))
- events_to_match = idnum - 1;
- else
- events_to_match = idnum;
-
- evlist__for_each_entry (perf_evlist, ev) {
+ if (idnum != 0) {
/*
- * Events with a constraint aren't grouped and match the first
- * events available.
+ * duration_time is always grouped separately, when events are
+ * grouped (ie has_constraint is false) then ignore it in the
+ * matching loop and add it to metric_events at the end.
*/
- if (has_constraint && ev->weak_group)
- continue;
- /* Ignore event if already used and merging is disabled. */
- if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
- continue;
- if (!has_constraint && !evsel__has_leader(ev, current_leader)) {
+ events_to_match = idnum;
+ if (!has_constraint && hashmap__find(pctx->ids, "duration_time", (void **)&val_ptr))
+ events_to_match--;
+
+ evlist__for_each_entry(perf_evlist, ev) {
+ /*
+ * Events with a constraint aren't grouped and match the
+ * first events available.
+ */
+ if (has_constraint && ev->weak_group)
+ continue;
+ /* Ignore event if already used and merging is disabled. */
+ if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
+ continue;
+ if (!has_constraint && !evsel__has_leader(ev, current_leader)) {
+ /*
+ * Start of a new group, discard the whole match
+ * and start again.
+ */
+ matched_events = 0;
+ memset(metric_events, 0, sizeof(struct evsel *) * idnum);
+ current_leader = evsel__leader(ev);
+ }
/*
- * Start of a new group, discard the whole match and
- * start again.
+ * Check for duplicate events with the same name. For
+ * example, uncore_imc/cas_count_read/ will turn into 6
+ * events per socket on skylakex. Only the first such
+ * event is placed in metric_events. If events aren't
+ * grouped then this also ensures that the same event in
+ * different sibling groups aren't both added to
+ * metric_events.
*/
- matched_events = 0;
- memset(metric_events, 0,
- sizeof(struct evsel *) * idnum);
- current_leader = evsel__leader(ev);
+ 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))
+ metric_events[matched_events++] = ev;
+
+ if (matched_events == events_to_match)
+ break;
}
+ } else {
/*
- * Check for duplicate events with the same name. For example,
- * uncore_imc/cas_count_read/ will turn into 6 events per socket
- * on skylakex. Only the first such event is placed in
- * metric_events. If events aren't grouped then this also
- * ensures that the same event in different sibling groups
- * aren't both added to metric_events.
+ * There are no events to match, but we need to associate the
+ * metric with an event for printing. A duration_time event was
+ * parsed for this.
*/
- 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))
- metric_events[matched_events++] = ev;
-
- if (matched_events == events_to_match)
- break;
+ idnum = 1;
+ events_to_match = 0;
}
-
if (events_to_match != idnum) {
/* Add the first duration_time. */
- evlist__for_each_entry(perf_evlist, ev) {
- if (!strcmp(ev->name, "duration_time")) {
- metric_events[matched_events++] = ev;
- break;
- }
- }
+ ev = evlist__find_evsel_by_str(perf_evlist, "duration_time");
+ if (ev)
+ metric_events[matched_events++] = ev;
}
if (matched_events != idnum) {
@@ -320,9 +324,10 @@ static int metricgroup__setup_events(struct list_head *groups,
list_for_each_entry (m, groups, nd) {
struct evsel **metric_events;
struct metric_ref *metric_refs = NULL;
+ const size_t ids_size = hashmap__size(m->pctx->ids);
metric_events = calloc(sizeof(void *),
- hashmap__size(m->pctx->ids) + 1);
+ ids_size == 0 ? 2 : ids_size + 1);
if (!metric_events) {
ret = -ENOMEM;
break;
@@ -1240,7 +1245,11 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
goto out;
pr_debug("adding %s\n", extra_events.buf);
bzero(&parse_error, sizeof(parse_error));
- ret = __parse_events(perf_evlist, extra_events.buf, &parse_error, fake_pmu);
+ ret = __parse_events(perf_evlist,
+ extra_events.len > 0
+ ? extra_events.buf
+ : "duration_time",
+ &parse_error, fake_pmu);
if (ret) {
parse_events_print_error(&parse_error, extra_events.buf);
goto out;
--
2.33.0.464.g1972c5931b-goog
next prev parent reply other threads:[~2021-09-23 7:47 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-23 7:46 [PATCH v9 00/13] Don't compute events that won't be used in a metric Ian Rogers
2021-09-23 7:46 ` [PATCH v9 01/13] perf metric: Restructure struct expr_parse_ctx Ian Rogers
2021-09-23 7:46 ` [PATCH v9 02/13] perf metric: Use NAN for missing event IDs Ian Rogers
2021-09-23 7:46 ` [PATCH v9 03/13] perf expr: Remove unused headers and inline d_ratio Ian Rogers
2021-09-23 7:46 ` [PATCH v9 04/13] perf expr: Separate token declataion from type Ian Rogers
2021-09-23 7:46 ` [PATCH v9 05/13] perf expr: Use macros for operators Ian Rogers
2021-09-23 7:46 ` [PATCH v9 06/13] perf expr: Move actions to the left Ian Rogers
2021-09-23 7:46 ` [PATCH v9 07/13] perf metric: Rename expr__find_other Ian Rogers
2021-09-23 7:46 ` [PATCH v9 08/13] perf metric: Add utilities to work on ids map Ian Rogers
2021-09-23 7:46 ` Ian Rogers [this message]
2021-09-23 7:46 ` [PATCH v9 10/13] perf expr: Merge find_ids and regular parsing Ian Rogers
2021-09-28 20:56 ` Jiri Olsa
2021-09-28 21:20 ` Ian Rogers
2021-09-23 7:46 ` [PATCH v9 11/13] perf expr: Propagate constants for binary operations Ian Rogers
2021-09-23 7:46 ` [PATCH v9 12/13] perf metric: Don't compute unused events Ian Rogers
2021-09-23 7:46 ` [PATCH v9 13/13] perf metric: Avoid events for an 'if' constant result Ian Rogers
2021-09-29 5:35 ` [PATCH v9 00/13] Don't compute events that won't be used in a metric Jiri Olsa
2021-09-29 15:19 ` John Garry
2021-09-29 16:09 ` Ian Rogers
2021-09-29 16:51 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210923074616.674826-10-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=eranian@google.com \
--cc=john.garry@huawei.com \
--cc=jolsa@redhat.com \
--cc=kjain@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=pc@us.ibm.com \
--cc=peterz@infradead.org \
--cc=sdasgup@google.com \
--cc=yao.jin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).