From: Ian Rogers <irogers@google.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
Kan Liang <kan.liang@linux.intel.com>,
Ahmad Yasin <ahmad.yasin@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Stephane Eranian <eranian@google.com>,
Andi Kleen <ak@linux.intel.com>,
Perry Taylor <perry.taylor@intel.com>,
Samantha Alt <samantha.alt@intel.com>,
Caleb Biggers <caleb.biggers@intel.com>,
Weilin Wang <weilin.wang@intel.com>,
Edward Baker <edward.baker@intel.com>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Florian Fischer <florian.fischer@muhq.space>,
Rob Herring <robh@kernel.org>,
Zhengjun Xing <zhengjun.xing@linux.intel.com>,
John Garry <john.g.garry@oracle.com>,
Kajol Jain <kjain@linux.ibm.com>,
Sumanth Korikkar <sumanthk@linux.ibm.com>,
Thomas Richter <tmricht@linux.ibm.com>,
Tiezhu Yang <yangtiezhu@loongson.cn>,
Ravi Bangoria <ravi.bangoria@amd.com>,
Leo Yan <leo.yan@linaro.org>,
Yang Jihong <yangjihong1@huawei.com>,
James Clark <james.clark@arm.com>,
Suzuki Poulouse <suzuki.poulose@arm.com>,
Kang Minchul <tegongkang@gmail.com>,
Athira Rajeev <atrajeev@linux.vnet.ibm.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Ian Rogers <irogers@google.com>
Subject: [PATCH v3 02/46] perf metric: Change divide by zero and !support events behavior
Date: Fri, 28 Apr 2023 22:34:22 -0700 [thread overview]
Message-ID: <20230429053506.1962559-3-irogers@google.com> (raw)
In-Reply-To: <20230429053506.1962559-1-irogers@google.com>
Division by zero causes expression parsing to fail and no metric to be
generated. This can mean for short running benchmarks metrics are not
shown. Change the behavior to make the value nan, which gets shown like:
'''
$ perf stat -M TopdownL2 true
Performance counter stats for 'true':
1,031,492 INST_RETIRED.ANY # nan % tma_fetch_bandwidth
# nan % tma_heavy_operations
# nan % tma_light_operations
29,304 CPU_CLK_UNHALTED.REF_XCLK # nan % tma_fetch_latency
# nan % tma_branch_mispredicts
# nan % tma_machine_clears
# nan % tma_core_bound
# nan % tma_memory_bound
2,658,319 IDQ_UOPS_NOT_DELIVERED.CORE
11,167 EXE_ACTIVITY.BOUND_ON_STORES
262,058 EXE_ACTIVITY.1_PORTS_UTIL
<not counted> BR_MISP_RETIRED.ALL_BRANCHES (0.00%)
<not counted> INT_MISC.RECOVERY_CYCLES_ANY (0.00%)
<not counted> CPU_CLK_UNHALTED.ONE_THREAD_ACTIVE (0.00%)
<not counted> CPU_CLK_UNHALTED.THREAD (0.00%)
<not counted> UOPS_RETIRED.RETIRE_SLOTS (0.00%)
<not counted> CYCLE_ACTIVITY.STALLS_MEM_ANY (0.00%)
<not counted> UOPS_RETIRED.MACRO_FUSED (0.00%)
<not counted> IDQ_UOPS_NOT_DELIVERED.CYCLES_0_UOPS_DELIV.CORE (0.00%)
<not counted> EXE_ACTIVITY.2_PORTS_UTIL (0.00%)
<not counted> CYCLE_ACTIVITY.STALLS_TOTAL (0.00%)
<not counted> MACHINE_CLEARS.COUNT (0.00%)
<not counted> UOPS_ISSUED.ANY (0.00%)
0.002864879 seconds time elapsed
0.003012000 seconds user
0.000000000 seconds sys
'''
When events aren't supported a count of 0 can be confusing and make
metrics look meaningful. Change these to be nan also which, with the
next change, gets shown like:
'''
$ perf stat true
Performance counter stats for 'true':
1.25 msec task-clock:u # 0.387 CPUs utilized
0 context-switches:u # 0.000 /sec
0 cpu-migrations:u # 0.000 /sec
46 page-faults:u # 36.702 K/sec
255,942 cycles:u # 0.204 GHz (88.66%)
123,046 instructions:u # 0.48 insn per cycle
28,301 branches:u # 22.580 M/sec
2,489 branch-misses:u # 8.79% of all branches
4,719 CPU_CLK_UNHALTED.REF_XCLK:u # 3.765 M/sec
# nan % tma_frontend_bound
# nan % tma_retiring
# nan % tma_backend_bound
# nan % tma_bad_speculation
344,855 IDQ_UOPS_NOT_DELIVERED.CORE:u # 275.147 M/sec
<not supported> INT_MISC.RECOVERY_CYCLES_ANY:u
<not counted> CPU_CLK_UNHALTED.ONE_THREAD_ACTIVE:u (0.00%)
<not counted> CPU_CLK_UNHALTED.THREAD:u (0.00%)
<not counted> UOPS_RETIRED.RETIRE_SLOTS:u (0.00%)
<not counted> UOPS_ISSUED.ANY:u (0.00%)
0.003238142 seconds time elapsed
0.000000000 seconds user
0.003434000 seconds sys
'''
Ensure that nan metric values are quoted as nan isn't a valid number
in json.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/expr.c | 3 ++-
tools/perf/tests/parse-metric.c | 1 +
tools/perf/util/expr.y | 6 +++++-
tools/perf/util/stat-display.c | 2 +-
tools/perf/util/stat-shadow.c | 25 +++++++++++++++++++------
5 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index cbf0e0c74906..733ead151c63 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -120,7 +120,8 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
p = "FOO/0";
ret = expr__parse(&val, ctx, p);
- TEST_ASSERT_VAL("division by zero", ret == -1);
+ TEST_ASSERT_VAL("division by zero", ret == 0);
+ TEST_ASSERT_VAL("division by zero", isnan(val));
p = "BAR/";
ret = expr__parse(&val, ctx, p);
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index 1185b79e6274..c05148ea400c 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -38,6 +38,7 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals)
evlist__alloc_aggr_stats(evlist, 1);
evlist__for_each_entry(evlist, evsel) {
count = find_value(evsel->name, vals);
+ evsel->supported = true;
evsel->stats->aggr->counts.val = count;
if (evsel__name_is(evsel, "duration_time"))
update_stats(&walltime_nsecs_stats, count);
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 250e444bf032..4ce931cccb63 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -225,7 +225,11 @@ expr: NUMBER
{
if (fpclassify($3.val) == FP_ZERO) {
pr_debug("division by zero\n");
- YYABORT;
+ assert($3.ids == NULL);
+ if (compute_ids)
+ ids__free($1.ids);
+ $$.val = NAN;
+ $$.ids = NULL;
} else if (!compute_ids || (is_const($1.val) && is_const($3.val))) {
assert($1.ids == NULL);
assert($3.ids == NULL);
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e6035ecbeee8..d87fb5328543 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -431,7 +431,7 @@ static void print_metric_json(struct perf_stat_config *config __maybe_unused,
struct outstate *os = ctx;
FILE *out = os->fh;
- fprintf(out, "\"metric-value\" : %f, ", val);
+ fprintf(out, "\"metric-value\" : \"%f\", ", val);
fprintf(out, "\"metric-unit\" : \"%s\"", unit);
if (!config->metric_only)
fprintf(out, "}");
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index eeccab6751d7..1566a206ba42 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -403,12 +403,25 @@ static int prepare_metric(struct evsel **metric_events,
if (!aggr)
break;
- /*
- * If an event was scaled during stat gathering, reverse
- * the scale before computing the metric.
- */
- val = aggr->counts.val * (1.0 / metric_events[i]->scale);
- source_count = evsel__source_count(metric_events[i]);
+ if (!metric_events[i]->supported) {
+ /*
+ * Not supported events will have a count of 0,
+ * which can be confusing in a
+ * metric. Explicitly set the value to NAN. Not
+ * counted events (enable time of 0) are read as
+ * 0.
+ */
+ val = NAN;
+ source_count = 0;
+ } else {
+ /*
+ * If an event was scaled during stat gathering,
+ * reverse the scale before computing the
+ * metric.
+ */
+ val = aggr->counts.val * (1.0 / metric_events[i]->scale);
+ source_count = evsel__source_count(metric_events[i]);
+ }
}
n = strdup(evsel__metric_id(metric_events[i]));
if (!n)
--
2.40.1.495.gc816e09b53d-goog
next prev parent reply other threads:[~2023-04-29 5:35 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-29 5:34 [PATCH v3 00/46] Fix perf on Intel hybrid CPUs Ian Rogers
2023-04-29 5:34 ` [PATCH v3 01/46] perf stat: Disable TopdownL1 on hybrid Ian Rogers
2023-04-29 5:34 ` Ian Rogers [this message]
2023-04-29 5:34 ` [PATCH v3 03/46] perf stat: Introduce skippable evsels Ian Rogers
2023-05-01 14:56 ` Liang, Kan
2023-05-01 15:29 ` Ian Rogers
2023-05-01 20:25 ` Liang, Kan
2023-05-01 20:48 ` Ian Rogers
2023-05-01 23:34 ` Liang, Kan
2023-04-29 5:34 ` [PATCH v3 05/46] perf parse-events: Don't reorder ungrouped events by pmu Ian Rogers
2023-04-29 5:34 ` [PATCH v3 06/46] perf vendor events intel: Add alderlake metric constraints Ian Rogers
2023-04-29 5:34 ` [PATCH v3 07/46] perf vendor events intel: Add icelake " Ian Rogers
2023-04-29 5:34 ` [PATCH v3 08/46] perf vendor events intel: Add icelakex " Ian Rogers
2023-04-29 5:34 ` [PATCH v3 09/46] perf vendor events intel: Add sapphirerapids " Ian Rogers
2023-04-29 5:34 ` [PATCH v3 10/46] perf vendor events intel: Add tigerlake " Ian Rogers
2023-04-29 5:34 ` [PATCH v3 11/46] perf stat: Avoid segv on counter->name Ian Rogers
2023-04-29 5:34 ` [PATCH v3 12/46] perf test: Test more sysfs events Ian Rogers
2023-05-02 10:27 ` Ravi Bangoria
2023-05-02 15:16 ` Ian Rogers
2023-05-02 15:29 ` Ian Rogers
2023-04-29 5:34 ` [PATCH v3 13/46] perf test: Use valid for PMU tests Ian Rogers
2023-04-29 5:34 ` [PATCH v3 14/46] perf test: Mask config then test Ian Rogers
2023-05-02 10:44 ` Ravi Bangoria
2023-05-02 16:19 ` Ian Rogers
2023-04-29 5:34 ` [PATCH v3 15/46] perf test: Test more with config_cache Ian Rogers
2023-04-29 5:34 ` [PATCH v3 16/46] perf test: Roundtrip name, don't assume 1 event per name Ian Rogers
2023-04-29 5:34 ` [PATCH v3 17/46] perf parse-events: Set attr.type to PMU type early Ian Rogers
2023-04-29 5:34 ` [PATCH v3 18/46] perf parse-events: Set pmu_name whenever a pmu is given Ian Rogers
2023-04-29 5:34 ` [PATCH v3 19/46] perf print-events: Avoid unnecessary strlist Ian Rogers
2023-04-29 5:34 ` [PATCH v3 20/46] perf parse-events: Avoid scanning PMUs before parsing Ian Rogers
2023-04-29 5:34 ` [PATCH v3 21/46] perf evsel: Modify group pmu name for software events Ian Rogers
2023-04-29 5:34 ` [PATCH v3 22/46] perf test: Move x86 hybrid tests to arch/x86 Ian Rogers
2023-04-29 5:34 ` [PATCH v3 23/46] perf test x86 hybrid: Update test expectations Ian Rogers
2023-04-29 5:34 ` [PATCH v3 24/46] perf test x86 hybrid: Add hybrid extended type checks Ian Rogers
2023-04-29 5:34 ` [PATCH v3 25/46] perf parse-events: Support PMUs for legacy cache events Ian Rogers
2023-04-29 5:34 ` [PATCH v3 26/46] perf parse-events: Wildcard " Ian Rogers
2023-04-29 5:34 ` [PATCH v3 27/46] perf print-events: Print legacy cache events for each PMU Ian Rogers
2023-05-02 10:48 ` Ravi Bangoria
2023-05-02 17:40 ` Ian Rogers
2023-04-29 5:34 ` [PATCH v3 28/46] perf parse-events: Support wildcards on raw events Ian Rogers
2023-04-29 5:34 ` [PATCH v3 29/46] perf parse-events: Remove now unused hybrid logic Ian Rogers
2023-04-29 5:34 ` [PATCH v3 30/46] perf parse-events: Minor type safety cleanup Ian Rogers
2023-04-29 5:34 ` [PATCH v3 31/46] perf parse-events: Add pmu filter Ian Rogers
2023-04-29 5:34 ` [PATCH v3 32/46] perf stat: Make cputype filter generic Ian Rogers
2023-05-02 10:51 ` Ravi Bangoria
2023-05-02 20:09 ` Ian Rogers
2023-05-02 20:16 ` Ian Rogers
2023-04-29 5:34 ` [PATCH v3 33/46] perf test: Add cputype testing to perf stat Ian Rogers
2023-04-29 5:34 ` [PATCH v3 34/46] perf test: Fix parse-events tests for >1 core PMU Ian Rogers
2023-04-29 5:34 ` [PATCH v3 35/46] perf parse-events: Support hardware events as terms Ian Rogers
2023-05-02 10:55 ` Ravi Bangoria
2023-05-02 17:57 ` Ian Rogers
2023-04-29 5:34 ` [PATCH v3 36/46] perf parse-events: Avoid error when assigning a term Ian Rogers
2023-04-29 5:34 ` [PATCH v3 37/46] perf parse-events: Avoid error when assigning a legacy cache term Ian Rogers
2023-04-29 5:34 ` [PATCH v3 38/46] perf parse-events: Don't auto merge hybrid wildcard events Ian Rogers
2023-04-29 5:34 ` [PATCH v3 39/46] perf parse-events: Don't reorder atom cpu events Ian Rogers
2023-04-29 5:35 ` [PATCH v3 40/46] perf metrics: Be PMU specific for referenced metrics Ian Rogers
2023-04-29 5:35 ` [PATCH v3 41/46] perf stat: Command line PMU metric filtering Ian Rogers
2023-04-29 5:35 ` [PATCH v3 42/46] perf vendor events intel: Correct alderlake metrics Ian Rogers
2023-04-29 5:35 ` [PATCH v3 43/46] perf jevents: Don't rewrite metrics across PMUs Ian Rogers
2023-04-29 5:35 ` [PATCH v3 44/46] perf metrics: Be PMU specific in event match Ian Rogers
2023-04-29 5:35 ` [PATCH v3 45/46] perf stat: Don't disable TopdownL1 metric on hybrid Ian Rogers
2023-04-29 5:35 ` [PATCH v3 46/46] perf parse-events: Reduce scope of is_event_supported Ian Rogers
2023-05-01 20:34 ` [PATCH v3 00/46] Fix perf on Intel hybrid CPUs Liang, Kan
2023-05-01 20:51 ` Ian Rogers
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=20230429053506.1962559-3-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ahmad.yasin@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=atrajeev@linux.vnet.ibm.com \
--cc=caleb.biggers@intel.com \
--cc=edward.baker@intel.com \
--cc=eranian@google.com \
--cc=florian.fischer@muhq.space \
--cc=james.clark@arm.com \
--cc=john.g.garry@oracle.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=kjain@linux.ibm.com \
--cc=leo.yan@linaro.org \
--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=perry.taylor@intel.com \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=robh@kernel.org \
--cc=samantha.alt@intel.com \
--cc=sumanthk@linux.ibm.com \
--cc=suzuki.poulose@arm.com \
--cc=tegongkang@gmail.com \
--cc=tmricht@linux.ibm.com \
--cc=weilin.wang@intel.com \
--cc=yangjihong1@huawei.com \
--cc=yangtiezhu@loongson.cn \
--cc=zhengjun.xing@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).