Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 23 Nov 2022 15:28:39 -0800 | Subject | Re: [PATCH 13/15] perf stat: Fix JSON output in metric-only mode |
| |
On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <namhyung@kernel.org> wrote: > > It generated a broken JSON output when aggregation mode or cgroup is > used with --metric-only option. Also get rid of the header line and > make the output single line for each entry. > > It needs to know whether the current metric is the first one or not. > So add 'first' field in the outstate and mark it false after printing. > > Before: > # perf stat -a -j --metric-only true > {"unit" : "GHz"}{"unit" : "insn per cycle"}{"unit" : "branch-misses of all branches"} > {{"metric-value" : "0.797"}{"metric-value" : "1.65"}{"metric-value" : "0.89"} > ^ > > # perf stat -a -j --metric-only --per-socket true > {"unit" : "GHz"}{"unit" : "insn per cycle"}{"unit" : "branch-misses of all branches"} > {"socket" : "S0", "aggregate-number" : 8, {"metric-value" : "0.295"}{"metric-value" : "1.88"}{"metric-value" : "0.64"} > ^ > > After: > # perf stat -a -j --metric-only true > {"GHz" : "0.990", "insn per cycle" : "2.06", "branch-misses of all branches" : "0.59"} > > # perf stat -a -j --metric-only --per-socket true > {"socket" : "S0", "aggregate-number" : 8, "GHz" : "0.439", "insn per cycle" : "2.14", "branch-misses of all branches" : "0.51"} > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks, Ian
> --- > tools/perf/util/stat-display.c | 42 +++++++++++++++++++--------------- > 1 file changed, 24 insertions(+), 18 deletions(-) > > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c > index 335627e8542d..43640115454c 100644 > --- a/tools/perf/util/stat-display.c > +++ b/tools/perf/util/stat-display.c > @@ -279,9 +279,6 @@ static void print_aggr_id_json(struct perf_stat_config *config, > { > FILE *output = config->output; > > - if (!config->interval) > - fputc('{', output); > - > switch (config->aggr_mode) { > case AGGR_CORE: > fprintf(output, "\"core\" : \"S%d-D%d-C%d\", \"aggregate-number\" : %d, ", > @@ -335,6 +332,7 @@ static void aggr_printout(struct perf_stat_config *config, > struct outstate { > FILE *fh; > bool newline; > + bool first; > const char *prefix; > int nfields; > int nr; > @@ -491,6 +489,7 @@ static void print_metric_only(struct perf_stat_config *config, > > color_snprintf(str, sizeof(str), color ?: "", fmt, val); > fprintf(out, "%*s ", mlen, str); > + os->first = false; > } > > static void print_metric_only_csv(struct perf_stat_config *config __maybe_unused, > @@ -512,6 +511,7 @@ static void print_metric_only_csv(struct perf_stat_config *config __maybe_unused > ends++; > *ends = 0; > fprintf(out, "%s%s", vals, config->csv_sep); > + os->first = false; > } > > static void print_metric_only_json(struct perf_stat_config *config __maybe_unused, > @@ -532,7 +532,8 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse > while (isdigit(*ends) || *ends == '.') > ends++; > *ends = 0; > - fprintf(out, "{\"metric-value\" : \"%s\"}", vals); > + fprintf(out, "%s\"%s\" : \"%s\"", os->first ? "" : ", ", unit, vals); > + os->first = false; > } > > static void new_line_metric(struct perf_stat_config *config __maybe_unused, > @@ -561,7 +562,7 @@ static void print_metric_header(struct perf_stat_config *config, > unit = fixunit(tbuf, os->evsel, unit); > > if (config->json_output) > - fprintf(os->fh, "{\"unit\" : \"%s\"}", unit); > + return; > else if (config->csv_output) > fprintf(os->fh, "%s%s", unit, config->csv_sep); > else > @@ -821,6 +822,8 @@ static void print_counter_aggrdata(struct perf_stat_config *config, > run = aggr->counts.run; > > if (!metric_only) { > + if (config->json_output) > + fputc('{', output); > if (os->prefix) > fprintf(output, "%s", os->prefix); > else if (config->summary && config->csv_output && > @@ -844,9 +847,12 @@ static void print_metric_begin(struct perf_stat_config *config, > struct aggr_cpu_id id; > struct evsel *evsel; > > + os->first = true; > if (!config->metric_only) > return; > > + if (config->json_output) > + fputc('{', config->output); > if (os->prefix) > fprintf(config->output, "%s", os->prefix); > > @@ -855,7 +861,7 @@ static void print_metric_begin(struct perf_stat_config *config, > aggr = &evsel->stats->aggr[aggr_idx]; > aggr_printout(config, evsel, id, aggr->nr); > > - print_cgroup(config, os->cgrp); > + print_cgroup(config, os->cgrp ? : evsel->cgrp); > } > > static void print_metric_end(struct perf_stat_config *config) > @@ -863,6 +869,8 @@ static void print_metric_end(struct perf_stat_config *config) > if (!config->metric_only) > return; > > + if (config->json_output) > + fputc('}', config->output); > fputc('\n', config->output); > } > > @@ -1005,11 +1013,9 @@ static void print_metric_headers_csv(struct perf_stat_config *config, > fputs(aggr_header_csv[config->aggr_mode], config->output); > } > > -static void print_metric_headers_json(struct perf_stat_config *config, > +static void print_metric_headers_json(struct perf_stat_config *config __maybe_unused, > bool no_indent __maybe_unused) > { > - if (config->interval) > - fputs("{\"unit\" : \"sec\"}", config->output); > } > > static void print_metric_headers(struct perf_stat_config *config, > @@ -1049,7 +1055,9 @@ static void print_metric_headers(struct perf_stat_config *config, > &config->metric_events, > &rt_stat); > } > - fputc('\n', config->output); > + > + if (!config->json_output) > + fputc('\n', config->output); > } > > static void prepare_interval(struct perf_stat_config *config, > @@ -1058,17 +1066,14 @@ static void prepare_interval(struct perf_stat_config *config, > if (config->iostat_run) > return; > > - if (config->csv_output) > + if (config->json_output) > + scnprintf(prefix, len, "\"interval\" : %lu.%09lu, ", > + (unsigned long) ts->tv_sec, ts->tv_nsec); > + else if (config->csv_output) > scnprintf(prefix, len, "%lu.%09lu%s", > (unsigned long) ts->tv_sec, ts->tv_nsec, config->csv_sep); > - else if (!config->json_output) > - scnprintf(prefix, len, "%6lu.%09lu ", > - (unsigned long) ts->tv_sec, ts->tv_nsec); > - else if (!config->metric_only) > - scnprintf(prefix, len, "{\"interval\" : %lu.%09lu, ", > - (unsigned long) ts->tv_sec, ts->tv_nsec); > else > - scnprintf(prefix, len, "{\"interval\" : %lu.%09lu}", > + scnprintf(prefix, len, "%6lu.%09lu ", > (unsigned long) ts->tv_sec, ts->tv_nsec); > } > > @@ -1365,6 +1370,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf > char buf[64]; > struct outstate os = { > .fh = config->output, > + .first = true, > }; > > if (config->iostat_run) > -- > 2.38.1.584.g0f3c55d4c2-goog >
| |