* Re: [PATCH 13/15] perf stat: Fix JSON output in metric-only mode [not found] <CO6PR11MB56352D2831C1554FE8C3A872EE179@CO6PR11MB5635.namprd11.prod.outlook.com> @ 2022-12-02 18:03 ` Namhyung Kim 0 siblings, 0 replies; 3+ messages in thread From: Namhyung Kim @ 2022-12-02 18:03 UTC (permalink / raw) To: Wang, Weilin Cc: acme@kernel.org, Hunter, Adrian, atrajeev@linux.vnet.ibm.com, irogers@google.com, james.clark@arm.com, jolsa@kernel.org, kan.liang@linux.intel.com, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, mingo@kernel.org, peterz@infradead.org, zhengjun.xing@linux.intel.com Hello, On Thu, Dec 1, 2022 at 4:56 PM Wang, Weilin <weilin.wang@intel.com> wrote: > > Hi Namhyung, > > > > I encountered an issue with the metric json output, which I think might be related to this commit. > > > > This is one of the commands I tried: > > > > ./perf stat -M memory_bandwidth_total,memory_bandwidth_read,memory_bandwidth_write -a -j -- sleep 1 > > > > Output before this commit: > > ==================================================== > > {"counter-value" : "1117309.000000", "unit" : "", "event" : "UNC_M_CAS_COUNT.RD", "event-runtime" : 8017286783, "pcnt-running" : 100.00, "metric-value" : 124.961911, "metric-unit" : "MB/s memory_bandwidth_read"} {"metric-value" : 237.237504, "metric-unit" : "MB/s memory_bandwidth_total"} {"counter-value" : "1004052.000000", "unit" : "", "event" : "UNC_M_CAS_COUNT.WR", "event-runtime" : 8017286783, "pcnt-running" : 100.00, "metric-value" : 112.275593, "metric-unit" : "MB/s memory_bandwidth_write"} {"counter-value" : "1002140370.000000", "unit" : "ns", "event" : "duration_time", "event-runtime" : 1002140370, "pcnt-running" : 100.00, "metric-value" : 0.000000, "metric-unit" : "(null)"} > > ==================================================== > > > > Output after this commit: > > ==================================================== > > {"counter-value" : "1374098.000000", "unit" : "", "event" : "UNC_M_CAS_COUNT.RD", "event-runtime" : 8016491003, "pcnt-running" : 100.00, "metric-value" : 153.660738, "metric-unit" : "MB/s memory_bandwidth_read"} "metric-value" : 300.338968, "metric-unit" : "MB/s memory_bandwidth_total"} {"counter-value" : "1311710.000000", "unit" : "", "event" : "UNC_M_CAS_COUNT.WR", "event-runtime" : 8016491003, "pcnt-running" : 100.00, "metric-value" : 146.678229, "metric-unit" : "MB/s memory_bandwidth_write"} {"counter-value" : "1002037512.000000", "unit" : "ns", "event" : "duration_time", "event-runtime" : 1002037512, "pcnt-running" : 100.00, "metric-value" : 0.000000, "metric-unit" : "(null)"} > > ==================================================== > > > > The second set of output is missing a "{" on the second line. Looks like it happens when I collect two related metrics together. Another example is: > > > > ./perf stat -M memory_bandwidth_total,memory_bandwidth_read,memory_bandwidth_write,L3_Cache_Fill_BW_1T,L3_Cache_Fill_BW -a -j -- sleep 1 > > > > {"counter-value" : "944204.000000", "unit" : "", "event" : "UNC_M_CAS_COUNT.RD", "event-runtime" : 8033937053, "pcnt-running" : 100.00, "metric-value" : 105.369994, "metric-unit" : "MB/s memory_bandwidth_read"} "metric-value" : 210.592807, "metric-unit" : "MB/s memory_bandwidth_total"} {"counter-value" : "942679.000000", "unit" : "", "event" : "UNC_M_CAS_COUNT.WR", "event-runtime" : 8033937053, "pcnt-running" : 100.00, "metric-value" : 105.222813, "metric-unit" : "MB/s memory_bandwidth_write"} {"counter-value" : "1004471458.000000", "unit" : "ns", "event" : "duration_time", "event-runtime" : 1004471458, "pcnt-running" : 100.00, "metric-value" : 0.000000, "metric-unit" : "(null)"} {"counter-value" : "184675.000000", "unit" : "", "event" : "LONGEST_LAT_CACHE.MISS", "event-runtime" : 88387319917, "pcnt-running" : 100.00, "metric-value" : 0.011767, "metric-unit" : "L3_Cache_Fill_BW_1T"} "metric-value" : 0.011767, "metric-unit" : "L3_Cache_Fill_BW"} {"counter-value" : "1004471458.000000", "unit" : "ns", "event" : "duration_time", "event-runtime" : 1004471458, "pcnt-running" : 100.00, "metric-value" : 0.000000, "metric-unit" : "(null)"} Thanks for your report. It seems it misses the open bracket when the metric produces more than one values. I think new_line_json() should have it. Will send the fix soon. Thanks, Namhyung ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCHSET 00/15] perf stat: Improve perf stat output (v2)
@ 2022-11-23 18:01 Namhyung Kim
2022-11-23 18:02 ` [PATCH 13/15] perf stat: Fix JSON output in metric-only mode Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2022-11-23 18:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
linux-perf-users, Kan Liang, Zhengjun Xing, James Clark,
Athira Jajeev
Hello,
This is a continuation of my perf stat cleanup work focusing on the
metric-only mode with JSON output now. It's based on the previous
patchset which is already merged to acme/perf/core.
The JSON output + metric-only with aggregation has been broken for a
while. The last update fixed the crash but it still produced invalid
JSON objects. Also having a separate header and objects with their own
"metric-value" key is not convenient to process.
# perf stat -aj --metric-only --per-socket sleep 1
{"unit" : "GHz"}{"unit" : "insn per cycle"}{"unit" : "branch-misses of all branches"}
{"socket" : "S0", "aggregate-number" : 8, {"metric-value" : "0.809"}{"metric-value" : "2.10"}{"metric-value" : "0.37"}
So I removed the header and move the metric keys to the object. Then
it doesn't need separate objects anymore. The new output looks like:
# perf stat -aj --metric-only --per-socket sleep 1
{"socket" : "S0", "cpu-count" : 8, "GHz" : "0.817", "insn per cycle" : "2.15", "branch-misses of all branches" : "0.38"}
You can get it from 'perf/stat-display-v2' branch in
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (15):
perf stat: Fix cgroup display in JSON output
perf stat: Move summary prefix printing logic in CSV output
perf stat: Do not align time prefix in CSV output
perf stat: Use scnprintf() in prepare_interval()
perf stat: Remove prefix argument in print_metric_headers()
perf stat: Remove metric_only argument in print_counter_aggrdata()
perf stat: Pass const char *prefix to display routines
perf stat: Use struct outstate in evlist__print_counters()
perf stat: Pass struct outstate to print_metric_begin()
perf stat: Pass struct outstate to printout()
perf stat: Do not pass runtime_stat to printout()
perf stat: Pass through struct outstate
perf stat: Fix JSON output in metric-only mode
perf stat: Rename "aggregate-number" to "cpu-count" in JSON
perf stat: Tidy up JSON metric-only output when no metrics
tools/perf/arch/x86/util/iostat.c | 4 +-
tools/perf/util/iostat.c | 3 +-
tools/perf/util/iostat.h | 4 +-
tools/perf/util/stat-display.c | 257 +++++++++++++++---------------
4 files changed, 133 insertions(+), 135 deletions(-)
base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 13/15] perf stat: Fix JSON output in metric-only mode 2022-11-23 18:01 [PATCHSET 00/15] perf stat: Improve perf stat output (v2) Namhyung Kim @ 2022-11-23 18:02 ` Namhyung Kim 2022-11-23 23:28 ` Ian Rogers 0 siblings, 1 reply; 3+ messages in thread From: Namhyung Kim @ 2022-11-23 18:02 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Jiri Olsa Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter, linux-perf-users, Kan Liang, Zhengjun Xing, James Clark, Athira Jajeev 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> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 13/15] perf stat: Fix JSON output in metric-only mode 2022-11-23 18:02 ` [PATCH 13/15] perf stat: Fix JSON output in metric-only mode Namhyung Kim @ 2022-11-23 23:28 ` Ian Rogers 0 siblings, 0 replies; 3+ messages in thread From: Ian Rogers @ 2022-11-23 23:28 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML, Adrian Hunter, linux-perf-users, Kan Liang, Zhengjun Xing, James Clark, Athira Jajeev 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-02 18:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CO6PR11MB56352D2831C1554FE8C3A872EE179@CO6PR11MB5635.namprd11.prod.outlook.com>
2022-12-02 18:03 ` [PATCH 13/15] perf stat: Fix JSON output in metric-only mode Namhyung Kim
2022-11-23 18:01 [PATCHSET 00/15] perf stat: Improve perf stat output (v2) Namhyung Kim
2022-11-23 18:02 ` [PATCH 13/15] perf stat: Fix JSON output in metric-only mode Namhyung Kim
2022-11-23 23:28 ` Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).