Messages in this thread Patch in this message | | | From | Cong Wang <> | Date | Tue, 6 Mar 2018 11:03:42 -0800 | Subject | Re: [PATCH] perf stat: fix cvs output format |
| |
On Tue, Mar 6, 2018 at 9:53 AM, Andi Kleen <ak@linux.intel.com> wrote: >> Here is the output from your own commit: >> >> 423470,,stalled-cycles-frontend,509102,100.00,65.69,frontend cycles idle >> <not supported>,,stalled-cycles-backend,0,100.00,,,, >> >> so line 1 has 7 fields, line 2 has 9 fields, and this is expected? > > If you had metrics on line 1 it would be correct. > > So you just shifted it to break that case. > > If you always want to have the same number of fields > you need to add two empty fields to the normal output > when there are no metrics.
The number of separators is the only way to learn the number of fields, therefore it must be a fixed number.
Yeah, it could be the other way that supported ones have less separators than it should. If we look at print_metric_csv() alone, it should produce a same number of separators for all cases, otherwise hard to count.
So I believe we need an additional patch, like the one attached, to make it complete? Note, I only spot the cgroup field here. diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 54a4c152edb3..6896e739ae4e 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -1044,8 +1044,7 @@ static void nsec_printout(int id, int nr, struct perf_evsel *evsel, double avg) fprintf(output, fmt_n, name); - if (evsel->cgrp) - fprintf(output, "%s%s", csv_sep, evsel->cgrp->name); + fprintf(output, "%s%s", csv_sep, evsel->cgrp ? evsel->cgrp->name : ""); } static int first_shadow_cpu(struct perf_evsel *evsel, int id) @@ -1092,12 +1091,13 @@ static void abs_printout(int id, int nr, struct perf_evsel *evsel, double avg) if (evsel->unit) fprintf(output, "%-*s%s", csv_output ? 0 : unit_width, - evsel->unit, csv_sep); + evsel->unit ? evsel->unit : "", csv_sep); + else + fprintf(output, "%s", csv_sep); fprintf(output, "%-*s", csv_output ? 0 : 25, perf_evsel__name(evsel)); - if (evsel->cgrp) - fprintf(output, "%s%s", csv_sep, evsel->cgrp->name); + fprintf(output, "%s%s", csv_sep, evsel->cgrp ? evsel->cgrp->name : ""); } static void printout(int id, int nr, struct perf_evsel *counter, double uval, @@ -1163,9 +1163,8 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval, csv_output ? 0 : -25, perf_evsel__name(counter)); - if (counter->cgrp) - fprintf(stat_config.output, "%s%s", - csv_sep, counter->cgrp->name); + fprintf(stat_config.output, "%s%s", csv_sep, + counter->cgrp ? counter->cgrp->name : ""); if (!csv_output) pm(&os, NULL, NULL, "", 0); | |