lkml.org 
[lkml]   [2022]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 5/9] perf stat: Fix --metric-only --json output
Hi Arnaldo,

On Tue, Nov 8, 2022 at 12:45 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Mon, Nov 07, 2022 at 01:33:10PM -0800, Namhyung Kim escreveu:
> > Currently it prints all metric headers for JSON output. But actually it
> > skips some metrics with valid_only_metric(). So the output looks like:
> >
> > $ perf stat --metric-only --json true
> > {"unit" : "CPUs utilized", "unit" : "/sec", "unit" : "/sec", "unit" : "/sec", "unit" : "GHz", "unit" : "insn per cycle", "unit" : "/sec", "unit" : "branch-misses of all branches"}
> > {"metric-value" : "3.861"}{"metric-value" : "0.79"}{"metric-value" : "3.04"}
> >
> > As you can see there are 8 units in the header but only 3 metric-values
> > are there. It should skip the unused headers as well. Also each unit
> > should be printed as a separate object like metric values.
> >
> > With this patch:
> >
> > $ perf stat --metric-only --json true
> > {"unit" : "GHz"}{"unit" : "insn per cycle"}{"unit" : "branch-misses of all branches"}
> > {"metric-value" : "4.166"}{"metric-value" : "0.73"}{"metric-value" : "2.96"}
>
> Can we get a Fixes tag for this one?

I think this is it:

df936cadfb58 ("perf stat: Add JSON output option")

But this also depends on patch 4. Do you want me to rebase
this not to depend on it?

Thanks,
Namhyung

>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/stat-display.c | 22 +++-------------------
> > 1 file changed, 3 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 115477461224..515fb6db3d67 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -430,12 +430,12 @@ static void print_metric_header(struct perf_stat_config *config,
> > os->evsel->priv != os->evsel->evlist->selected->priv)
> > return;
> >
> > - if (!valid_only_metric(unit) && !config->json_output)
> > + if (!valid_only_metric(unit))
> > return;
> > unit = fixunit(tbuf, os->evsel, unit);
> >
> > if (config->json_output)
> > - fprintf(os->fh, "\"unit\" : \"%s\"", unit);
> > + fprintf(os->fh, "{\"unit\" : \"%s\"}", unit);
> > else if (config->csv_output)
> > fprintf(os->fh, "%s%s", unit, config->csv_sep);
> > else
> > @@ -847,10 +847,6 @@ static void print_metric_headers(struct perf_stat_config *config,
> > .new_line = new_line_metric,
> > .force_header = true,
> > };
> > - bool first = true;
> > -
> > - if (config->json_output && !config->interval)
> > - fprintf(config->output, "{");
> >
> > if (prefix && !config->json_output)
> > fprintf(config->output, "%s", prefix);
> > @@ -871,18 +867,12 @@ static void print_metric_headers(struct perf_stat_config *config,
> > evlist__for_each_entry(evlist, counter) {
> > os.evsel = counter;
> >
> > - if (!first && config->json_output)
> > - fprintf(config->output, ", ");
> > - first = false;
> > -
> > perf_stat__print_shadow_stats(config, counter, 0,
> > 0,
> > &out,
> > &config->metric_events,
> > &rt_stat);
> > }
> > - if (config->json_output)
> > - fprintf(config->output, "}");
> > fputc('\n', config->output);
> > }
> >
> > @@ -954,14 +944,8 @@ static void print_interval(struct perf_stat_config *config,
> > }
> > }
> >
> > - if ((num_print_interval == 0 || config->interval_clear)
> > - && metric_only && !config->json_output)
> > + if ((num_print_interval == 0 || config->interval_clear) && metric_only)
> > print_metric_headers(config, evlist, " ", true);
> > - if ((num_print_interval == 0 || config->interval_clear)
> > - && metric_only && config->json_output) {
> > - fprintf(output, "{");
> > - print_metric_headers(config, evlist, " ", true);
> > - }
> > if (++num_print_interval == 25)
> > num_print_interval = 0;
> > }
> > --
> > 2.38.1.431.g37b22c650d-goog
>
> --
>
> - Arnaldo

\
 
 \ /
  Last update: 2022-11-08 23:09    [W:0.072 / U:0.536 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site