[PATCH 0/2] perf test: Fix JSON linter

* [PATCH 0/2] perf test: Fix JSON linter
@ 2023-01-20 13:40 Michael Petlan
  2023-01-20 13:40 ` [PATCH 1/2] perf stat: Fix JSON metric printout for multiple metrics per line Michael Petlan
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Michael Petlan @ 2023-01-20 13:40 UTC (permalink / raw)
  To: linux-perf-users, acme; +Cc: acme, qzhao, cjense

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

^ permalink raw reply	[flat|nested] 18+ messages in thread