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

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

[Andi Kleen]

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

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

[Ian Rogers]

On Wed, May 13, 2026 at 7:55 AM Andi Kleen <ak@linux.intel.com> 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>

Reviewed-by: Ian Rogers <irogers@google.com>

I dislike the spaghetti code in stat-display.c :-) One day it would be
great to throw an LLM at doing a clean implementation. Having tests
for formatting would be useful :-)

Thanks,
Ian

> ---
>
> 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
>
>

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Cumulative column drift misalignment in --metric-only output.
- [Medium] Metric group header alignment is broken in standard (non-metric-only) perf stat output.
--

commit 1fbd687246bd068cad87b8edb8dcd01fdb03fe25
Author: Andi Kleen <ak@linux.intel.com>

perf stat: Make metric only column line up with header

This commit attempts to fix metric-only output column alignment by simplifying
the column width computation. It ignores EVNAME_LEN, MGROUP_LEN, and
config->unit_width, and relies solely on config->metric_only_len and the
actual unit width.

>     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.

This isn't a bug, but the commit message states these constants aren't useful
in the metric only context. However, print_metricgroup_header_std() is
explicitly used for standard (non-metric-only) output, where these constants
are still necessary for proper alignment.

> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 993f4c4b8f442..2b69d238858cf 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;
>  
>  	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);

Does this change break metric group header alignment in standard perf stat
output?

print_metricgroup_header_std() is used in the standard (non-metric-only)
context. By reducing the padding from EVNAME_LEN + MGROUP_LEN (roughly 56
characters) to config->metric_only_len (typically 20), the metric group header
is printed too early, misaligning it with the subsequent metric columns.

[ ... ]

> @@ -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);

Can this cause a cumulative column drift in the --metric-only output?

The headers are printed with max(metric_only_len, strlen(unit)) characters
plus an unconditional trailing space, making the header column width exactly
mlen + 1 characters.

The modified print_metric_only() outputs exactly mlen characters with no
trailing space. Because each value column is exactly 1 character narrower than
its corresponding header column, the alignment breaks cumulatively across the
line, shifting left by 1 character per successive column.

>  	os->first = false;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513144906.557896-1-ak@linux.intel.com?part=1