From: Michael Petlan <mpetlan@redhat.com>
To: linux-perf-users@vger.kernel.org, acme@kernel.org
Cc: acme@redhat.com, qzhao@redhat.com, cjense@google.com
Subject: [PATCH 0/2] perf test: Fix JSON linter
Date: Fri, 20 Jan 2023 14:40:37 +0100 [thread overview]
Message-ID: <20230120134039.17788-1-mpetlan@redhat.com> (raw)
Hello all.
We have observed issues with the 'perf stat JSON output linter' testcase
on systems that allow topdown metrics.
=========================
1) The topdown metrics handling hardcodes printing more than one metric
per line (refer to util/stat-shadow.c):
if (retiring > 0.7 && heavy_ops > 0.1)
color = PERF_COLOR_GREEN;
print_metric(config, ctxp, color, "%8.1f%%", "Heavy Operations",
heavy_ops * 100.);
if (retiring > 0.7 && light_ops > 0.6)
color = PERF_COLOR_GREEN;
else
color = NULL;
print_metric(config, ctxp, color, "%8.1f%%", "Light Operations",
light_ops * 100.);
Thus, the test cannot rely on the fact that there will be only one
metric per line.
2) Since the JSON printing engine thinks that one and only one metric
is printed, it always closes the line by adding '}'. This is not true,
so I have fixed the comma printing and adjusted the engine to be
capable of printing 1-N metrics and then close the line.
=========================
On machines that don't support topdown, the problem can be shown by
doubling the following line in util/stat-shadow.c, so that the simple
'insn per cycle' metric is printed twice:
print_metric(config, ctxp, NULL, "%7.2f ", "insn per cycle", ratio);
The worst problem is that the JSON line is broken, such as:
... "pcnt-running" : 100.00, "metric-value" : 3.501931, "metric-unit"
: "Heavy Operations"}"metric-value" : 14.007787, "metric-unit" : ...
here ^^^^^
=========================
The first patch solves the JSON output correctness, the second tries
to adjust the testcase to some extent, so that it should work on the
topdown-capable machines.
However, it's highly possible that the testcase will have to be fixed
or maybe rewritten in future. First, it has quite naive evaluation of
what's expected/correct/etc. Second, it does no JSON format validation
at all. As a linter it should do so though.
***
For further discussion: What about removing the check for number of
expected elements in the line and just check for the keywords, such
as "metric-value", "counter-value", "event", etc. and for some values
that should be numeric. And, do proper JSON format sanity check?
Thank you for inputs!
Michael
Michael Petlan (2):
perf stat: Fix JSON metric printout for multiple metrics per line
perf test: Fix JSON format linter test checks
.../tests/shell/lib/perf_json_output_lint.py | 16 +++++------
tools/perf/util/stat-display.c | 28 +++++++++++--------
2 files changed, 24 insertions(+), 20 deletions(-)
--
2.18.4
next reply other threads:[~2023-01-20 13:41 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-20 13:40 Michael Petlan [this message]
2023-01-20 13:40 ` [PATCH 1/2] perf stat: Fix JSON metric printout for multiple metrics per line Michael Petlan
2023-01-23 6:31 ` Namhyung Kim
2023-05-22 12:11 ` Michael Petlan
2023-06-06 11:16 ` Michael Petlan
2023-01-20 13:40 ` [PATCH 2/2] perf test: Fix JSON format linter test checks Michael Petlan
2023-01-23 6:36 ` Namhyung Kim
2023-01-24 16:49 ` Michael Petlan
2023-01-24 17:26 ` Namhyung Kim
2023-01-27 12:26 ` Arnaldo Carvalho de Melo
2023-01-27 12:30 ` Arnaldo Carvalho de Melo
2023-01-31 17:14 ` Michael Petlan
2023-02-02 1:18 ` Arnaldo Carvalho de Melo
2023-01-20 17:37 ` [PATCH 0/2] perf test: Fix JSON linter Ian Rogers
2023-01-23 6:48 ` Namhyung Kim
2023-01-23 13:38 ` Arnaldo Carvalho de Melo
2023-01-24 17:39 ` Michael Petlan
2023-01-25 0:37 ` 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=20230120134039.17788-1-mpetlan@redhat.com \
--to=mpetlan@redhat.com \
--cc=acme@kernel.org \
--cc=acme@redhat.com \
--cc=cjense@google.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=qzhao@redhat.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).