Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Mar 2021 14:07:09 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Jin Yao <yao.jin@linux.intel.com>
Cc:     acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
        mingo@redhat.com, alexander.shishkin@linux.intel.com,
        Linux-kernel@vger.kernel.org, ak@linux.intel.com,
        kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH] perf stat: Align CSV output for summary mode
Message-ID: <YFCtfXlaKbIeEMwk@krava>
References: <20210316072900.1739-1-yao.jin@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210316072900.1739-1-yao.jin@linux.intel.com>
Precedence: bulk

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
>