Re: [PATCH 1/2] perf stat: Use field separator in the metric header

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Jun 27, 2024 at 01:03:52PM -0700, Namhyung Kim wrote:
> It didn't use the passed field separator (using -x option) when it
> prints the metric headers and always put "," between the fields.
> 
> Before:
>   $ sudo ./perf stat -a -x : --per-core -M tma_core_bound --metric-only true
>   core,cpus,%  tma_core_bound:     <<<--- here: "core,cpus," but ":" expected
>   S0-D0-C0:2:10.5:
>   S0-D0-C1:2:14.8:
>   S0-D0-C2:2:9.9:
>   S0-D0-C3:2:13.2:
> 
> After:
>   $ sudo ./perf stat -a -x : --per-core -M tma_core_bound --metric-only true
>   core:cpus:%  tma_core_bound:
>   S0-D0-C0:2:10.5:
>   S0-D0-C1:2:15.0:
>   S0-D0-C2:2:16.5:
>   S0-D0-C3:2:12.5:
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo

> ---
>  tools/perf/util/stat-display.c | 37 ++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 91d2f7f65df7..e8673c9f6b49 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -47,16 +47,27 @@ static int aggr_header_lens[] = {
>  };
>  
>  static const char *aggr_header_csv[] = {
> -	[AGGR_CORE] 	= 	"core,cpus,",
> -	[AGGR_CACHE]	= 	"cache,cpus,",
> -	[AGGR_DIE] 	= 	"die,cpus,",
> -	[AGGR_SOCKET] 	= 	"socket,cpus,",
> -	[AGGR_NONE] 	= 	"cpu,",
> -	[AGGR_THREAD] 	= 	"comm-pid,",
> -	[AGGR_NODE] 	= 	"node,",
> +	[AGGR_CORE] 	= 	"core%scpus%s",
> +	[AGGR_CACHE]	= 	"cache%scpus%s",
> +	[AGGR_DIE] 	= 	"die%scpus%s",
> +	[AGGR_SOCKET] 	= 	"socket%scpus%s",
> +	[AGGR_NONE] 	= 	"cpu%s",
> +	[AGGR_THREAD] 	= 	"comm-pid%s",
> +	[AGGR_NODE] 	= 	"node%s",
>  	[AGGR_GLOBAL] 	=	""
>  };
>  
> +static int aggr_header_num[] = {
> +	[AGGR_CORE] 	= 	2,
> +	[AGGR_CACHE]	= 	2,
> +	[AGGR_DIE] 	= 	2,
> +	[AGGR_SOCKET] 	= 	2,
> +	[AGGR_NONE] 	= 	1,
> +	[AGGR_THREAD] 	= 	1,
> +	[AGGR_NODE] 	= 	1,
> +	[AGGR_GLOBAL] 	=	0,
> +};
> +
>  static const char *aggr_header_std[] = {
>  	[AGGR_CORE] 	= 	"core",
>  	[AGGR_CACHE] 	= 	"cache",
> @@ -1185,8 +1196,18 @@ static void print_metric_headers_csv(struct perf_stat_config *config,
>  {
>  	if (config->interval)
>  		fputs("time,", config->output);
> -	if (!config->iostat_run)
> +	if (config->iostat_run)
> +		return;
> +
> +	if (aggr_header_num[config->aggr_mode] == 1) {
> +		fprintf(config->output, aggr_header_csv[config->aggr_mode],
> +			config->csv_sep);
> +	} else if (aggr_header_num[config->aggr_mode] == 2) {
> +		fprintf(config->output, aggr_header_csv[config->aggr_mode],
> +			config->csv_sep, config->csv_sep);
> +	} else {
>  		fputs(aggr_header_csv[config->aggr_mode], config->output);
> +	}
>  }
>  
>  static void print_metric_headers_json(struct perf_stat_config *config __maybe_unused,
> -- 
> 2.45.2.803.g4e1b14247a-goog