[PING] Re: [PATCH v2] perf stat: Make metric only column line up with header

[Andi Kleen <ak@linux.intel.com>]

Ping for the patch!

On Wed, May 13, 2026 at 07:49:06AM -0700, Andi Kleen wrote:
> Since some time the metric-only output columns are messed up and do not
> line up with the header, which makes it hard to read. I haven't bisected
> it, but presumably it was broken for some time.
> 
> There were multiple problems:
> - The dummy pm invocation at the beginning did print a bogus field
> - The column computation in pm did not agree with the header length
> - The color escape strings from highlighting confuse printf's field
>   length computation
> 
> Fix all those. I simplified the column width computation significantly,
> ignoring EVNAME_LEN, MGROUP_LEN, config->unit_width which don't
> seem to be useful in the metric only context. It now only uses
> the actual unit width as well as config->metric_only_len. The result
> is more code removed than added.
> 
> Before:
> 
> % perf stat --topdown -a -I 1000
> +           time %  tma_backend_bound %  tma_frontend_bound %  tma_bad_speculation      %  tma_retiring
>      1.000190386                                         45.5                   40.0                     5.3                     9.2
>      2.005185654                                         45.3                   40.1                     5.6                     9.0
>      3.009193207                                         45.4                   39.9                     5.6                     9.1
> 
> After:
> 
> % perf stat --topdown -a -I 1000
> +          time %  tma_backend_bound %  tma_frontend_bound %  tma_bad_speculation      %  tma_retiring
>      1.000810024                 46.3                 39.7                   5.3                 8.7
>      2.004800656                 45.8                 39.8                   5.4                 8.9
>      3.008804783                 46.0                 39.6                   5.4                 9.0
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
> 
> v2: Add missing headers removed by git because they started with #
> Remove unrelated hunk.
> ---
>  tools/perf/util/stat-display.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 993f4c4b8f44..2b69d238858c 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -580,16 +580,13 @@ static void print_metricgroup_header_std(struct perf_stat_config *config,
>  					 const char *metricgroup_name)
>  {
>  	struct outstate *os = ctx;
> -	int n;
>  
>  	if (!metricgroup_name) {
>  		__new_line_std(config, os);
>  		return;
>  	}
>  
> -	n = fprintf(config->output, " %*s", EVNAME_LEN, metricgroup_name);
> -
> -	fprintf(config->output, "%*s", MGROUP_LEN + config->unit_width + 2 - n, "");
> +	fprintf(config->output, " %*s", config->metric_only_len, metricgroup_name);
>  }
>  
>  static void print_metric_only(struct perf_stat_config *config,
> @@ -599,19 +596,20 @@ static void print_metric_only(struct perf_stat_config *config,
>  	struct outstate *os = ctx;
>  	FILE *out = os->fh;
>  	char str[1024];
> -	unsigned mlen = config->metric_only_len;
> +	unsigned mlen;
>  	const char *color = metric_threshold_classify__color(thresh);
> +	int olen;
>  
> -	if (!unit)
> -		unit = "";
> -	if (mlen < strlen(unit))
> -		mlen = strlen(unit) + 1;
> +	if (!unit) {
> +		os->first = false;
> +		return;
> +	}
>  
> -	if (color)
> -		mlen += strlen(color) + sizeof(PERF_COLOR_RESET) - 1;
> +	mlen = max_t(unsigned, strlen(unit), config->metric_only_len);
>  
> +	olen = snprintf(str, sizeof(str), fmt ?: "", val);
>  	color_snprintf(str, sizeof(str), color ?: "", fmt ?: "", val);
> -	fprintf(out, "%*s ", mlen, str);
> +	fprintf(out, "%*s%s", max_t(int, mlen - olen, 1), "", str);
>  	os->first = false;
>  }
>  
> -- 
> 2.54.0
>