Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 14 Nov 2022 10:18:30 -0800 | Subject | Re: [PATCH 03/11] perf stat: Fix --metric-only --json output |
| |
On Fri, Nov 11, 2022 at 7:22 PM Namhyung Kim <namhyung@kernel.org> wrote: > > 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"} > > Fixes: df936cadfb58 ("perf stat: Add JSON output option") > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks, Ian
> --- > 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 c7b3a1e10263..96ad0c71adc2 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.493.g58b659f92b-goog >
| |