| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fU3Ruy75wRnjzN-GefLnAK8Vg6OEueUbgo2jxfSTe41jA@mail.gmail.com>
Date: Wed, 23 Nov 2022 15:28:39 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Jiri Olsa <jolsa@...nel.org>, Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
LKML <linux-kernel@...r.kernel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
linux-perf-users@...r.kernel.org,
Kan Liang <kan.liang@...ux.intel.com>,
Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
James Clark <james.clark@....com>,
Athira Jajeev <atrajeev@...ux.vnet.ibm.com>
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@...nel.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@...nel.org>
Acked-by: Ian Rogers <irogers@...gle.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
>
Powered by blists - more mailing lists