* [PATCH v2] perf stat: Make metric only column line up with header
@ 2026-05-13 14:49 Andi Kleen
2026-05-13 16:23 ` Ian Rogers
0 siblings, 1 reply; 2+ messages in thread
From: Andi Kleen @ 2026-05-13 14:49 UTC (permalink / raw)
To: namhyung; +Cc: acme, linux-perf-users, 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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] perf stat: Make metric only column line up with header
2026-05-13 14:49 [PATCH v2] perf stat: Make metric only column line up with header Andi Kleen
@ 2026-05-13 16:23 ` Ian Rogers
0 siblings, 0 replies; 2+ messages in thread
From: Ian Rogers @ 2026-05-13 16:23 UTC (permalink / raw)
To: Andi Kleen; +Cc: namhyung, acme, linux-perf-users
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
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-13 16:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 14:49 [PATCH v2] perf stat: Make metric only column line up with header Andi Kleen
2026-05-13 16:23 ` Ian Rogers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox