* [PATCH] perf stat: Align CSV output for summary mode
@ 2021-03-16 7:29 Jin Yao
2021-03-16 13:07 ` Jiri Olsa
0 siblings, 1 reply; 10+ messages in thread
From: Jin Yao @ 2021-03-16 7:29 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
perf-stat has supported the summary mode. But the summary
lines break the CSV output so it's hard for scripts to parse
the result.
Before:
# perf stat -x, -I1000 --interval-count 1 --summary
1.001323097,8013.48,msec,cpu-clock,8013483384,100.00,8.013,CPUs utilized
1.001323097,270,,context-switches,8013513297,100.00,0.034,K/sec
1.001323097,13,,cpu-migrations,8013530032,100.00,0.002,K/sec
1.001323097,184,,page-faults,8013546992,100.00,0.023,K/sec
1.001323097,20574191,,cycles,8013551506,100.00,0.003,GHz
1.001323097,10562267,,instructions,8013564958,100.00,0.51,insn per cycle
1.001323097,2019244,,branches,8013575673,100.00,0.252,M/sec
1.001323097,106152,,branch-misses,8013585776,100.00,5.26,of all branches
8013.48,msec,cpu-clock,8013483384,100.00,7.984,CPUs utilized
270,,context-switches,8013513297,100.00,0.034,K/sec
13,,cpu-migrations,8013530032,100.00,0.002,K/sec
184,,page-faults,8013546992,100.00,0.023,K/sec
20574191,,cycles,8013551506,100.00,0.003,GHz
10562267,,instructions,8013564958,100.00,0.51,insn per cycle
2019244,,branches,8013575673,100.00,0.252,M/sec
106152,,branch-misses,8013585776,100.00,5.26,of all branches
The summary line loses the timestamp column, which breaks the
CVS output.
We add a column at the 'timestamp' position and it just says 'summary'
for the summary line.
After:
# perf stat -x, -I1000 --interval-count 1 --summary
1.001196053,8012.72,msec,cpu-clock,8012722903,100.00,8.013,CPUs utilized
1.001196053,218,,context-switches,8012753271,100.00,0.027,K/sec
1.001196053,9,,cpu-migrations,8012769767,100.00,0.001,K/sec
1.001196053,0,,page-faults,8012786257,100.00,0.000,K/sec
1.001196053,15004518,,cycles,8012790637,100.00,0.002,GHz
1.001196053,7954691,,instructions,8012804027,100.00,0.53,insn per cycle
1.001196053,1590259,,branches,8012814766,100.00,0.198,M/sec
1.001196053,82601,,branch-misses,8012824365,100.00,5.19,of all branches
summary,8012.72,msec,cpu-clock,8012722903,100.00,7.986,CPUs utilized
summary,218,,context-switches,8012753271,100.00,0.027,K/sec
summary,9,,cpu-migrations,8012769767,100.00,0.001,K/sec
summary,0,,page-faults,8012786257,100.00,0.000,K/sec
summary,15004518,,cycles,8012790637,100.00,0.002,GHz
summary,7954691,,instructions,8012804027,100.00,0.53,insn per cycle
summary,1590259,,branches,8012814766,100.00,0.198,M/sec
summary,82601,,branch-misses,8012824365,100.00,5.19,of all branches
Now it's easy for script to analyse the summary lines.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/util/stat-display.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 7f09cdaf5b60..c4183d3e87a4 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -439,6 +439,10 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
if (counter->cgrp)
os.nfields++;
}
+
+ if (config->csv_output && config->summary && !config->interval)
+ fprintf(config->output, "%16s%s", "summary", config->csv_sep);
+
if (run == 0 || ena == 0 || counter->counts->scaled == -1) {
if (config->metric_only) {
pm(config, &os, NULL, "", "", 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 7:29 [PATCH] perf stat: Align CSV output for summary mode Jin Yao
@ 2021-03-16 13:07 ` Jiri Olsa
2021-03-16 13:07 ` Jiri Olsa
2021-03-16 16:34 ` Andi Kleen
0 siblings, 2 replies; 10+ messages in thread
From: Jiri Olsa @ 2021-03-16 13:07 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Tue, Mar 16, 2021 at 03:29:00PM +0800, Jin Yao wrote:
> perf-stat has supported the summary mode. But the summary
> lines break the CSV output so it's hard for scripts to parse
> the result.
>
> Before:
>
> # perf stat -x, -I1000 --interval-count 1 --summary
> 1.001323097,8013.48,msec,cpu-clock,8013483384,100.00,8.013,CPUs utilized
> 1.001323097,270,,context-switches,8013513297,100.00,0.034,K/sec
> 1.001323097,13,,cpu-migrations,8013530032,100.00,0.002,K/sec
> 1.001323097,184,,page-faults,8013546992,100.00,0.023,K/sec
> 1.001323097,20574191,,cycles,8013551506,100.00,0.003,GHz
> 1.001323097,10562267,,instructions,8013564958,100.00,0.51,insn per cycle
> 1.001323097,2019244,,branches,8013575673,100.00,0.252,M/sec
> 1.001323097,106152,,branch-misses,8013585776,100.00,5.26,of all branches
> 8013.48,msec,cpu-clock,8013483384,100.00,7.984,CPUs utilized
> 270,,context-switches,8013513297,100.00,0.034,K/sec
> 13,,cpu-migrations,8013530032,100.00,0.002,K/sec
> 184,,page-faults,8013546992,100.00,0.023,K/sec
> 20574191,,cycles,8013551506,100.00,0.003,GHz
> 10562267,,instructions,8013564958,100.00,0.51,insn per cycle
> 2019244,,branches,8013575673,100.00,0.252,M/sec
> 106152,,branch-misses,8013585776,100.00,5.26,of all branches
>
> The summary line loses the timestamp column, which breaks the
> CVS output.
>
> We add a column at the 'timestamp' position and it just says 'summary'
> for the summary line.
>
> After:
>
> # perf stat -x, -I1000 --interval-count 1 --summary
looks ok, but maybe make the option more related to CVS, like:
--x-summary, --cvs-summary ...?
jirka
> 1.001196053,8012.72,msec,cpu-clock,8012722903,100.00,8.013,CPUs utilized
> 1.001196053,218,,context-switches,8012753271,100.00,0.027,K/sec
> 1.001196053,9,,cpu-migrations,8012769767,100.00,0.001,K/sec
> 1.001196053,0,,page-faults,8012786257,100.00,0.000,K/sec
> 1.001196053,15004518,,cycles,8012790637,100.00,0.002,GHz
> 1.001196053,7954691,,instructions,8012804027,100.00,0.53,insn per cycle
> 1.001196053,1590259,,branches,8012814766,100.00,0.198,M/sec
> 1.001196053,82601,,branch-misses,8012824365,100.00,5.19,of all branches
> summary,8012.72,msec,cpu-clock,8012722903,100.00,7.986,CPUs utilized
> summary,218,,context-switches,8012753271,100.00,0.027,K/sec
> summary,9,,cpu-migrations,8012769767,100.00,0.001,K/sec
> summary,0,,page-faults,8012786257,100.00,0.000,K/sec
> summary,15004518,,cycles,8012790637,100.00,0.002,GHz
> summary,7954691,,instructions,8012804027,100.00,0.53,insn per cycle
> summary,1590259,,branches,8012814766,100.00,0.198,M/sec
> summary,82601,,branch-misses,8012824365,100.00,5.19,of all branches
>
> Now it's easy for script to analyse the summary lines.
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/util/stat-display.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 7f09cdaf5b60..c4183d3e87a4 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -439,6 +439,10 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
> if (counter->cgrp)
> os.nfields++;
> }
> +
> + if (config->csv_output && config->summary && !config->interval)
> + fprintf(config->output, "%16s%s", "summary", config->csv_sep);
> +
> if (run == 0 || ena == 0 || counter->counts->scaled == -1) {
> if (config->metric_only) {
> pm(config, &os, NULL, "", "", 0);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 13:07 ` Jiri Olsa
@ 2021-03-16 13:07 ` Jiri Olsa
2021-03-16 16:34 ` Andi Kleen
1 sibling, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2021-03-16 13:07 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Tue, Mar 16, 2021 at 02:07:12PM +0100, Jiri Olsa wrote:
> On Tue, Mar 16, 2021 at 03:29:00PM +0800, Jin Yao wrote:
> > perf-stat has supported the summary mode. But the summary
> > lines break the CSV output so it's hard for scripts to parse
> > the result.
> >
> > Before:
> >
> > # perf stat -x, -I1000 --interval-count 1 --summary
> > 1.001323097,8013.48,msec,cpu-clock,8013483384,100.00,8.013,CPUs utilized
> > 1.001323097,270,,context-switches,8013513297,100.00,0.034,K/sec
> > 1.001323097,13,,cpu-migrations,8013530032,100.00,0.002,K/sec
> > 1.001323097,184,,page-faults,8013546992,100.00,0.023,K/sec
> > 1.001323097,20574191,,cycles,8013551506,100.00,0.003,GHz
> > 1.001323097,10562267,,instructions,8013564958,100.00,0.51,insn per cycle
> > 1.001323097,2019244,,branches,8013575673,100.00,0.252,M/sec
> > 1.001323097,106152,,branch-misses,8013585776,100.00,5.26,of all branches
> > 8013.48,msec,cpu-clock,8013483384,100.00,7.984,CPUs utilized
> > 270,,context-switches,8013513297,100.00,0.034,K/sec
> > 13,,cpu-migrations,8013530032,100.00,0.002,K/sec
> > 184,,page-faults,8013546992,100.00,0.023,K/sec
> > 20574191,,cycles,8013551506,100.00,0.003,GHz
> > 10562267,,instructions,8013564958,100.00,0.51,insn per cycle
> > 2019244,,branches,8013575673,100.00,0.252,M/sec
> > 106152,,branch-misses,8013585776,100.00,5.26,of all branches
> >
> > The summary line loses the timestamp column, which breaks the
> > CVS output.
> >
> > We add a column at the 'timestamp' position and it just says 'summary'
> > for the summary line.
> >
> > After:
> >
> > # perf stat -x, -I1000 --interval-count 1 --summary
>
> looks ok, but maybe make the option more related to CVS, like:
>
> --x-summary, --cvs-summary ...?
>
and we'll need man page update for that
jirka
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 13:07 ` Jiri Olsa
2021-03-16 13:07 ` Jiri Olsa
@ 2021-03-16 16:34 ` Andi Kleen
2021-03-16 19:05 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 10+ messages in thread
From: Andi Kleen @ 2021-03-16 16:34 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jin Yao, acme, jolsa, peterz, mingo, alexander.shishkin,
Linux-kernel, kan.liang, yao.jin
> looks ok, but maybe make the option more related to CVS, like:
>
> --x-summary, --cvs-summary ...?
Actually I don't think it should be a new option. I doubt
anyone could parse the previous mess. So just make it default
with -x
-Andi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 16:34 ` Andi Kleen
@ 2021-03-16 19:05 ` Arnaldo Carvalho de Melo
2021-03-16 20:02 ` Andi Kleen
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-16 19:05 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, Jin Yao, jolsa, peterz, mingo, alexander.shishkin,
Linux-kernel, kan.liang, yao.jin
Em Tue, Mar 16, 2021 at 09:34:21AM -0700, Andi Kleen escreveu:
> > looks ok, but maybe make the option more related to CVS, like:
> >
> > --x-summary, --cvs-summary ...?
>
> Actually I don't think it should be a new option. I doubt
> anyone could parse the previous mess. So just make it default
> with -x
In these cases I always fear that people are already parsing that mess
by considering the summary lines to be the ones not starting with
spaces, and now we go on and change it to be "better" by prefixing it
with "summary" and... break existing scripts.
Can we do this with a new option?
I.e. like --cvs-summary?
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 19:05 ` Arnaldo Carvalho de Melo
@ 2021-03-16 20:02 ` Andi Kleen
2021-03-16 21:55 ` Jiri Olsa
0 siblings, 1 reply; 10+ messages in thread
From: Andi Kleen @ 2021-03-16 20:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Jin Yao, jolsa, peterz, mingo, alexander.shishkin,
Linux-kernel, kan.liang, yao.jin
On Tue, Mar 16, 2021 at 04:05:13PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 16, 2021 at 09:34:21AM -0700, Andi Kleen escreveu:
> > > looks ok, but maybe make the option more related to CVS, like:
> > >
> > > --x-summary, --cvs-summary ...?
> >
> > Actually I don't think it should be a new option. I doubt
> > anyone could parse the previous mess. So just make it default
> > with -x
>
> In these cases I always fear that people are already parsing that mess
> by considering the summary lines to be the ones not starting with
> spaces, and now we go on and change it to be "better" by prefixing it
> with "summary" and... break existing scripts.
I think it was just one version or so?
FWIW perf has broken CSV output several times, I added workarounds
to toplev every time. Having a broken version for a short time
shouldn't be too bad.
I actually had a workaround for this one, but it can parse either way.
>
> Can we do this with a new option?
>
> I.e. like --cvs-summary?
If you do it I would add an option for the old broken format
--i-want-broken-csv. But not require the option forever
just to get sane output.
Or maybe only a perf config option.
-Andi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 20:02 ` Andi Kleen
@ 2021-03-16 21:55 ` Jiri Olsa
2021-03-17 0:51 ` Jin, Yao
0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2021-03-16 21:55 UTC (permalink / raw)
To: Andi Kleen
Cc: Arnaldo Carvalho de Melo, Jin Yao, jolsa, peterz, mingo,
alexander.shishkin, Linux-kernel, kan.liang, yao.jin
On Tue, Mar 16, 2021 at 01:02:20PM -0700, Andi Kleen wrote:
> On Tue, Mar 16, 2021 at 04:05:13PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 16, 2021 at 09:34:21AM -0700, Andi Kleen escreveu:
> > > > looks ok, but maybe make the option more related to CVS, like:
> > > >
> > > > --x-summary, --cvs-summary ...?
> > >
> > > Actually I don't think it should be a new option. I doubt
> > > anyone could parse the previous mess. So just make it default
> > > with -x
> >
> > In these cases I always fear that people are already parsing that mess
> > by considering the summary lines to be the ones not starting with
> > spaces, and now we go on and change it to be "better" by prefixing it
> > with "summary" and... break existing scripts.
>
> I think it was just one version or so?
>
> FWIW perf has broken CSV output several times, I added workarounds
> to toplev every time. Having a broken version for a short time
> shouldn't be too bad.
>
> I actually had a workaround for this one, but it can parse either way.
>
> >
> > Can we do this with a new option?
> >
> > I.e. like --cvs-summary?
>
> If you do it I would add an option for the old broken format
> --i-want-broken-csv. But not require the option forever
> just to get sane output.
I like that.. also we'll find out how many people are actually parsing that ;-)
jirka
>
> Or maybe only a perf config option.
>
> -Andi
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-16 21:55 ` Jiri Olsa
@ 2021-03-17 0:51 ` Jin, Yao
2021-03-17 1:30 ` Andi Kleen
0 siblings, 1 reply; 10+ messages in thread
From: Jin, Yao @ 2021-03-17 0:51 UTC (permalink / raw)
To: Jiri Olsa, Andi Kleen
Cc: Arnaldo Carvalho de Melo, jolsa, peterz, mingo,
alexander.shishkin, Linux-kernel, kan.liang, yao.jin
On 3/17/2021 5:55 AM, Jiri Olsa wrote:
> On Tue, Mar 16, 2021 at 01:02:20PM -0700, Andi Kleen wrote:
>> On Tue, Mar 16, 2021 at 04:05:13PM -0300, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Mar 16, 2021 at 09:34:21AM -0700, Andi Kleen escreveu:
>>>>> looks ok, but maybe make the option more related to CVS, like:
>>>>>
>>>>> --x-summary, --cvs-summary ...?
>>>>
>>>> Actually I don't think it should be a new option. I doubt
>>>> anyone could parse the previous mess. So just make it default
>>>> with -x
>>>
>>> In these cases I always fear that people are already parsing that mess
>>> by considering the summary lines to be the ones not starting with
>>> spaces, and now we go on and change it to be "better" by prefixing it
>>> with "summary" and... break existing scripts.
>>
>> I think it was just one version or so?
>>
>> FWIW perf has broken CSV output several times, I added workarounds
>> to toplev every time. Having a broken version for a short time
>> shouldn't be too bad.
>>
>> I actually had a workaround for this one, but it can parse either way.
>>
>>>
>>> Can we do this with a new option?
>>>
>>> I.e. like --cvs-summary?
>>
>> If you do it I would add an option for the old broken format
>> --i-want-broken-csv. But not require the option forever
>> just to get sane output.
>
> I like that.. also we'll find out how many people are actually parsing that ;-)
>
> jirka
>
Is it serious or just a joke? :)
Thanks
Jin Yao
>>
>> Or maybe only a perf config option.
>>
>> -Andi
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-17 0:51 ` Jin, Yao
@ 2021-03-17 1:30 ` Andi Kleen
2021-03-17 1:39 ` Jin, Yao
0 siblings, 1 reply; 10+ messages in thread
From: Andi Kleen @ 2021-03-17 1:30 UTC (permalink / raw)
To: Jin, Yao
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, jolsa, peterz, mingo,
alexander.shishkin, Linux-kernel, kan.liang, yao.jin
> Is it serious or just a joke? :)
I would prefer to not be compatible (at least not until someone complains),
but if compatibility is required then yes opting in to the broken
format would be better. Perhaps not with that name.
And the option could be hidden in the perf config file instead
of being on the command line.
-Andi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf stat: Align CSV output for summary mode
2021-03-17 1:30 ` Andi Kleen
@ 2021-03-17 1:39 ` Jin, Yao
0 siblings, 0 replies; 10+ messages in thread
From: Jin, Yao @ 2021-03-17 1:39 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, jolsa, peterz, mingo,
alexander.shishkin, Linux-kernel, kan.liang, yao.jin
On 3/17/2021 9:30 AM, Andi Kleen wrote:
>> Is it serious or just a joke? :)
>
> I would prefer to not be compatible (at least not until someone complains),
> but if compatibility is required then yes opting in to the broken
> format would be better. Perhaps not with that name.
>
> And the option could be hidden in the perf config file instead
> of being on the command line.
>
> -Andi
>
That makes sense, thanks Andi!
Thanks
Jin Yao
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-17 1:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 7:29 [PATCH] perf stat: Align CSV output for summary mode Jin Yao
2021-03-16 13:07 ` Jiri Olsa
2021-03-16 13:07 ` Jiri Olsa
2021-03-16 16:34 ` Andi Kleen
2021-03-16 19:05 ` Arnaldo Carvalho de Melo
2021-03-16 20:02 ` Andi Kleen
2021-03-16 21:55 ` Jiri Olsa
2021-03-17 0:51 ` Jin, Yao
2021-03-17 1:30 ` Andi Kleen
2021-03-17 1:39 ` Jin, Yao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.