2022-11-14 23:05:47

by Namhyung Kim

[permalink] [raw]
Subject: [PATCHSET 00/19] perf stat: Improve perf stat output (v1)

Hello,

I'm working on cleanup up the perf stat code. The main focus this time
is the display logic which has various combinations of options.

I split the code for each output mode - std, csv and json. And then
organize them according to the purpose like header, prefix, value,
metric and footer. I hope this would help maintaining the code a bit
more.

Also I found that cgroup support is missing or insufficient.
Specifically when --for-each-cgroup option is given, it'd have multiple
copies of the events for those cgroups. Then the output should group
the result. This is true for the normal output mode, but the metric-
only mode didn't support it well.

With this change, I can see the per-cgroup topdown metrics like below:

$ sudo ./perf stat -a --topdown --for-each-cgroup user.slice,system.slice sleep 3
nmi_watchdog enabled with topdown. May give wrong results.
Disable with echo 0 > /proc/sys/kernel/nmi_watchdog

Performance counter stats for 'system wide':

retiring bad speculation frontend bound backend bound
S0-D0-C0 2 user.slice 117.3% 3.9% 47.8% -69.1%
S0-D0-C1 2 user.slice 119.8% 4.1% 49.3% -73.2%
S0-D0-C2 2 user.slice 24.4% 7.9% 68.4% 0.0%
S0-D0-C3 2 user.slice 24.0% 9.2% 91.2% -24.4%
S0-D0-C0 2 system.slice 73.5% 4.0% 19.4% 3.1%
S0-D0-C1 2 system.slice 90.0% 5.8% 19.7% -15.5%
S0-D0-C2 2 system.slice 101.2% 6.6% 33.4% -41.1%
S0-D0-C3 2 system.slice 90.7% 4.9% 22.3% -18.0%

3.001678216 seconds time elapsed

You can get it from 'perf/stat-display-v1' branch in

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung

Namhyung Kim (19):
perf stat: Clear screen only if output file is a tty
perf stat: Split print_running() function
perf stat: Split print_noise_pct() function
perf stat: Split print_cgroup() function
perf stat: Split aggr_printout() function
perf stat: Factor out print_counter_value() function
perf stat: Handle bad events in abs_printout()
perf stat: Add before_metric argument
perf stat: Align cgroup names
perf stat: Split print_metric_headers() function
perf stat: Factor out prepare_interval()
perf stat: Cleanup interval print alignment
perf stat: Remove impossible condition
perf stat: Rework header display
perf stat: Move condition to print_footer()
perf stat: Factor out prefix display
perf stat: Factor out print_metric_{begin,end}()
perf stat: Support --for-each-cgroup and --metric-only
perf stat: Add print_aggr_cgroup() for --for-each-cgroup and --topdown

tools/perf/builtin-stat.c | 8 +
tools/perf/util/stat-display.c | 996 ++++++++++++++++++++-------------
2 files changed, 624 insertions(+), 380 deletions(-)


base-commit: 7565f9617efac0c0c8e2dbd08dbe0695d56684f5
--
2.38.1.493.g58b659f92b-goog



2022-11-14 23:06:22

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 06/19] perf stat: Factor out print_counter_value() function

And split it for each output mode like others. I believe it makes the
code simpler and more intuitive. Now abs_printout() becomes just to
call sub-functions.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 81 ++++++++++++++++++++++------------
1 file changed, 53 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index ed421f6d512f..a72c7442ff3d 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -517,46 +517,71 @@ static void print_metric_header(struct perf_stat_config *config,
fprintf(os->fh, "%*s ", config->metric_only_len, unit);
}

-static void abs_printout(struct perf_stat_config *config,
- struct aggr_cpu_id id, int nr, struct evsel *evsel, double avg)
+static void print_counter_value_std(struct perf_stat_config *config,
+ struct evsel *evsel, double avg)
{
FILE *output = config->output;
double sc = evsel->scale;
const char *fmt;

- if (config->csv_output) {
- fmt = floor(sc) != sc ? "%.2f%s" : "%.0f%s";
- } else {
- if (config->big_num)
- fmt = floor(sc) != sc ? "%'18.2f%s" : "%'18.0f%s";
- else
- fmt = floor(sc) != sc ? "%18.2f%s" : "%18.0f%s";
- }
+ if (config->big_num)
+ fmt = floor(sc) != sc ? "%'18.2f " : "%'18.0f ";
+ else
+ fmt = floor(sc) != sc ? "%18.2f " : "%18.0f ";

- aggr_printout(config, evsel, id, nr);
+ fprintf(output, fmt, avg);

- if (config->json_output)
- fprintf(output, "\"counter-value\" : \"%f\", ", avg);
- else
- fprintf(output, fmt, avg, config->csv_sep);
+ if (evsel->unit)
+ fprintf(output, "%-*s ", config->unit_width, evsel->unit);

- if (config->json_output) {
- if (evsel->unit) {
- fprintf(output, "\"unit\" : \"%s\", ",
- evsel->unit);
- }
- } else {
- if (evsel->unit)
- fprintf(output, "%-*s%s",
- config->csv_output ? 0 : config->unit_width,
- evsel->unit, config->csv_sep);
- }
+ fprintf(output, "%-*s", 32, evsel__name(evsel));
+}

+static void print_counter_value_csv(struct perf_stat_config *config,
+ struct evsel *evsel, double avg)
+{
+ FILE *output = config->output;
+ double sc = evsel->scale;
+ const char *sep = config->csv_sep;
+ const char *fmt = floor(sc) != sc ? "%.2f%s" : "%.0f%s";
+
+ fprintf(output, fmt, avg, sep);
+
+ if (evsel->unit)
+ fprintf(output, "%s%s", evsel->unit, sep);
+
+ fprintf(output, "%s", evsel__name(evsel));
+}
+
+static void print_counter_value_json(struct perf_stat_config *config,
+ struct evsel *evsel, double avg)
+{
+ FILE *output = config->output;
+
+ fprintf(output, "\"counter-value\" : \"%f\", ", avg);
+
+ if (evsel->unit)
+ fprintf(output, "\"unit\" : \"%s\", ", evsel->unit);
+
+ fprintf(output, "\"event\" : \"%s\", ", evsel__name(evsel));
+}
+
+static void print_counter_value(struct perf_stat_config *config,
+ struct evsel *evsel, double avg)
+{
if (config->json_output)
- fprintf(output, "\"event\" : \"%s\", ", evsel__name(evsel));
+ print_counter_value_json(config, evsel, avg);
+ else if (config->csv_output)
+ print_counter_value_csv(config, evsel, avg);
else
- fprintf(output, "%-*s", config->csv_output ? 0 : 32, evsel__name(evsel));
+ print_counter_value_std(config, evsel, avg);
+}

+static void abs_printout(struct perf_stat_config *config,
+ struct aggr_cpu_id id, int nr, struct evsel *evsel, double avg)
+{
+ aggr_printout(config, evsel, id, nr);
+ print_counter_value(config, evsel, avg);
print_cgroup(config, evsel);
}

--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:09:01

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 17/19] perf stat: Factor out print_metric_{begin,end}()

For the metric-only case, add new functions to handle the start and the
end of each metric display.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 56 +++++++++++++++++++++-------------
1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index bb40ed29300d..7a0673be720b 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -836,12 +836,39 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
fputc('\n', output);
}

+static void print_metric_begin(struct perf_stat_config *config,
+ struct evlist *evlist,
+ char *prefix, int aggr_idx)
+{
+ struct perf_stat_aggr *aggr;
+ struct aggr_cpu_id id;
+ struct evsel *evsel;
+
+ if (!config->metric_only)
+ return;
+
+ if (prefix)
+ fprintf(config->output, "%s", prefix);
+
+ evsel = evlist__first(evlist);
+ id = config->aggr_map->map[aggr_idx];
+ aggr = &evsel->stats->aggr[aggr_idx];
+ aggr_printout(config, evsel, id, aggr->nr);
+}
+
+static void print_metric_end(struct perf_stat_config *config)
+{
+ if (!config->metric_only)
+ return;
+
+ fputc('\n', config->output);
+}
+
static void print_aggr(struct perf_stat_config *config,
struct evlist *evlist,
char *prefix)
{
bool metric_only = config->metric_only;
- FILE *output = config->output;
struct evsel *counter;
int s;

@@ -853,17 +880,7 @@ static void print_aggr(struct perf_stat_config *config,
* Without each counter has its own line.
*/
for (s = 0; s < config->aggr_map->nr; s++) {
- if (metric_only) {
- struct perf_stat_aggr *aggr;
- struct aggr_cpu_id id = config->aggr_map->map[s];
-
- if (prefix)
- fprintf(output, "%s", prefix);
-
- counter = evlist__first(evlist);
- aggr = &counter->stats->aggr[s];
- aggr_printout(config, counter, id, aggr->nr);
- }
+ print_metric_begin(config, evlist, prefix, s);

evlist__for_each_entry(evlist, counter) {
if (counter->merged_stat)
@@ -872,8 +889,7 @@ static void print_aggr(struct perf_stat_config *config,
print_counter_aggrdata(config, counter, s, prefix,
metric_only);
}
- if (metric_only)
- fputc('\n', output);
+ print_metric_end(config);
}
}

@@ -919,9 +935,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,

id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
if (first) {
- if (prefix)
- fputs(prefix, config->output);
- aggr_printout(config, counter, id, 0);
+ print_metric_begin(config, evlist, prefix, counter_idx);
first = false;
}
val = ps->aggr[counter_idx].counts.val;
@@ -933,7 +947,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
run, ena, 1.0, &rt_stat, counter_idx);
}
if (!first)
- fputc('\n', config->output);
+ print_metric_end(config);
}
}

@@ -1322,13 +1336,11 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
iostat_print_counters(evlist, config, ts, prefix = buf,
print_counter);
else {
- if (prefix && metric_only)
- fprintf(config->output, "%s", prefix);
+ print_metric_begin(config, evlist, prefix, /*aggr_idx=*/0);
evlist__for_each_entry(evlist, counter) {
print_counter(config, counter, prefix);
}
- if (metric_only)
- fputc('\n', config->output);
+ print_metric_end(config);
}
break;
case AGGR_NONE:
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:10:46

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 15/19] perf stat: Move condition to print_footer()

Likewise, I think it'd better to have the control inside the function, and keep
the higher level function clearer.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index f97817628478..73cf898060c0 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -1205,6 +1205,9 @@ static void print_footer(struct perf_stat_config *config)
double avg = avg_stats(config->walltime_nsecs_stats) / NSEC_PER_SEC;
FILE *output = config->output;

+ if (config->interval || config->csv_output || config->json_output)
+ return;
+
if (!config->null_run)
fprintf(output, "\n");

@@ -1359,8 +1362,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
break;
}

- if (!interval && !config->csv_output && !config->json_output)
- print_footer(config);
+ print_footer(config);

fflush(config->output);
}
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:18:22

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 13/19] perf stat: Remove impossible condition

The print would run only if metric_only is not set, but it's already in a
block that says it's in metric_only case. And there's no place to change
the setting.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index f983432aaddd..cc8bb6d07dcb 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -1292,9 +1292,6 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
num_print_iv = 0;
if (config->aggr_mode == AGGR_GLOBAL && prefix && !config->iostat_run)
fprintf(config->output, "%s", prefix);
-
- if (config->json_output && !config->metric_only)
- fprintf(config->output, "}");
}

switch (config->aggr_mode) {
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:19:31

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 01/19] perf stat: Clear screen only if output file is a tty

The --interval-clear option makes perf stat to clear the terminal at
each interval. But it doesn't need to clear the screen when it saves
to a file. Make it fail when it's enabled with the output options.

$ perf stat -I 1 --interval-clear -o myfile true
--interval-clear does not work with output

Usage: perf stat [<options>] [<command>]

-o, --output <file> output file name
--log-fd <n> log output to fd, instead of stderr
--interval-clear clear screen in between new interval

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-stat.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d5e1670bca20..1d79801f4e84 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2403,6 +2403,14 @@ int cmd_stat(int argc, const char **argv)
}
}

+ if (stat_config.interval_clear && !isatty(fileno(output))) {
+ fprintf(stderr, "--interval-clear does not work with output\n");
+ parse_options_usage(stat_usage, stat_options, "o", 1);
+ parse_options_usage(NULL, stat_options, "log-fd", 0);
+ parse_options_usage(NULL, stat_options, "interval-clear", 0);
+ return -1;
+ }
+
stat_config.output = output;

/*
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:20:54

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 19/19] perf stat: Add print_aggr_cgroup() for --for-each-cgroup and --topdown

Normally, --for-each-cgroup only works with AGGR_GLOBAL. However
the --topdown on some cpu (e.g. Intel Skylake) converts it to the
AGGR_CORE internally.

To support those machines, add print_aggr_cgroup and handle the events
like in print_cgroup_events().

$ perf stat -a --for-each-cgroup system.slice,user.slice --topdown sleep 1
nmi_watchdog enabled with topdown. May give wrong results.
Disable with echo 0 > /proc/sys/kernel/nmi_watchdog

Performance counter stats for 'system wide':

retiring bad speculation frontend bound backend bound
S0-D0-C0 2 system.slice 49.0% -46.6% 31.4%
S0-D0-C1 2 system.slice 55.5% 8.0% 45.5% -9.0%
S0-D0-C2 2 system.slice 87.8% 22.1% 30.3% -40.3%
S0-D0-C3 2 system.slice 53.3% -11.9% 45.2% 13.4%
S0-D0-C0 2 user.slice 123.5% 4.0% 48.5% -75.9%
S0-D0-C1 2 user.slice 19.9% 6.5% 89.9% -16.3%
S0-D0-C2 2 user.slice 29.9% 7.9% 71.3% -9.1%
S0-D0-C3 2 user.slice 28.0% 7.2% 43.3% 21.5%

1.004136937 seconds time elapsed

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 41 +++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index cf25ed99b5df..f5501760ff2e 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -900,6 +900,42 @@ static void print_aggr(struct perf_stat_config *config,
}
}

+static void print_aggr_cgroup(struct perf_stat_config *config,
+ struct evlist *evlist,
+ char *prefix)
+{
+ bool metric_only = config->metric_only;
+ struct evsel *counter, *evsel;
+ struct cgroup *cgrp = NULL;
+ int s;
+
+ if (!config->aggr_map || !config->aggr_get_id)
+ return;
+
+ evlist__for_each_entry(evlist, evsel) {
+ if (cgrp == evsel->cgrp)
+ continue;
+
+ cgrp = evsel->cgrp;
+
+ for (s = 0; s < config->aggr_map->nr; s++) {
+ print_metric_begin(config, evlist, prefix, s, cgrp);
+
+ evlist__for_each_entry(evlist, counter) {
+ if (counter->merged_stat)
+ continue;
+
+ if (counter->cgrp != cgrp)
+ continue;
+
+ print_counter_aggrdata(config, counter, s, prefix,
+ metric_only);
+ }
+ print_metric_end(config);
+ }
+ }
+}
+
static void print_counter(struct perf_stat_config *config,
struct evsel *counter, char *prefix)
{
@@ -1361,7 +1397,10 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
case AGGR_DIE:
case AGGR_SOCKET:
case AGGR_NODE:
- print_aggr(config, evlist, prefix);
+ if (config->cgroup_list)
+ print_aggr_cgroup(config, evlist, prefix);
+ else
+ print_aggr(config, evlist, prefix);
break;
case AGGR_THREAD:
case AGGR_GLOBAL:
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:21:33

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 16/19] perf stat: Factor out prefix display

The prefix is needed for interval mode to print timestamp at the
beginning of each line. But the it's tricky for the metric only
mode since it doesn't print every evsel and combines the metrics
into a single line.

So it needed to pass 'first' argument to print_counter_aggrdata()
to determine if the current event is being printed at first. This
makes the code hard to read.

Let's move the logic out of the function and do it in the outer
print loop. This would enable further cleanups later.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 43 ++++++++++++----------------------
1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 73cf898060c0..bb40ed29300d 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -805,8 +805,7 @@ static void uniquify_counter(struct perf_stat_config *config, struct evsel *coun

static void print_counter_aggrdata(struct perf_stat_config *config,
struct evsel *counter, int s,
- char *prefix, bool metric_only,
- bool *first)
+ char *prefix, bool metric_only)
{
FILE *output = config->output;
u64 ena, run, val;
@@ -825,10 +824,6 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
ena = aggr->counts.ena;
run = aggr->counts.run;

- if (*first && metric_only) {
- *first = false;
- aggr_printout(config, counter, id, aggr->nr);
- }
if (prefix && !metric_only)
fprintf(output, "%s", prefix);

@@ -849,7 +844,6 @@ static void print_aggr(struct perf_stat_config *config,
FILE *output = config->output;
struct evsel *counter;
int s;
- bool first;

if (!config->aggr_map || !config->aggr_get_id)
return;
@@ -860,21 +854,23 @@ static void print_aggr(struct perf_stat_config *config,
*/
for (s = 0; s < config->aggr_map->nr; s++) {
if (metric_only) {
+ struct perf_stat_aggr *aggr;
+ struct aggr_cpu_id id = config->aggr_map->map[s];
+
if (prefix)
fprintf(output, "%s", prefix);
- else if (config->summary && !config->no_csv_summary &&
- config->csv_output && !config->interval)
- fprintf(output, "%16s%s", "summary", config->csv_sep);
+
+ counter = evlist__first(evlist);
+ aggr = &counter->stats->aggr[s];
+ aggr_printout(config, counter, id, aggr->nr);
}

- first = true;
evlist__for_each_entry(evlist, counter) {
if (counter->merged_stat)
continue;

- print_counter_aggrdata(config, counter, s,
- prefix, metric_only,
- &first);
+ print_counter_aggrdata(config, counter, s, prefix,
+ metric_only);
}
if (metric_only)
fputc('\n', output);
@@ -885,7 +881,6 @@ static void print_counter(struct perf_stat_config *config,
struct evsel *counter, char *prefix)
{
bool metric_only = config->metric_only;
- bool first = false;
int s;

/* AGGR_THREAD doesn't have config->aggr_get_id */
@@ -896,9 +891,8 @@ static void print_counter(struct perf_stat_config *config,
return;

for (s = 0; s < config->aggr_map->nr; s++) {
- print_counter_aggrdata(config, counter, s,
- prefix, metric_only,
- &first);
+ print_counter_aggrdata(config, counter, s, prefix,
+ metric_only);
}
}

@@ -1260,7 +1254,6 @@ static void print_percore(struct perf_stat_config *config,
FILE *output = config->output;
struct cpu_aggr_map *core_map;
int s, c, i;
- bool first = true;

if (!config->aggr_map || !config->aggr_get_id)
return;
@@ -1288,11 +1281,7 @@ static void print_percore(struct perf_stat_config *config,
if (found)
continue;

- if (prefix && metric_only)
- fprintf(output, "%s", prefix);
-
- print_counter_aggrdata(config, counter, s,
- prefix, metric_only, &first);
+ print_counter_aggrdata(config, counter, s, prefix, metric_only);

core_map->map[c++] = core_id;
}
@@ -1319,10 +1308,6 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
}

print_header(config, _target, evlist, argc, argv);
- if (metric_only) {
- if (config->aggr_mode == AGGR_GLOBAL && prefix && !config->iostat_run)
- fprintf(config->output, "%s", prefix);
- }

switch (config->aggr_mode) {
case AGGR_CORE:
@@ -1337,6 +1322,8 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
iostat_print_counters(evlist, config, ts, prefix = buf,
print_counter);
else {
+ if (prefix && metric_only)
+ fprintf(config->output, "%s", prefix);
evlist__for_each_entry(evlist, counter) {
print_counter(config, counter, prefix);
}
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:25:54

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 12/19] perf stat: Cleanup interval print alignment

Instead of using magic values, define symbolic constants and use them.
Also add aggr_header_std[] array to simplify aggr_mode handling.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 165 ++++++++++++++++++---------------
1 file changed, 91 insertions(+), 74 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index c234be656db9..f983432aaddd 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -25,6 +25,45 @@
#define CNTR_NOT_SUPPORTED "<not supported>"
#define CNTR_NOT_COUNTED "<not counted>"

+#define METRIC_LEN 38
+#define EVNAME_LEN 32
+#define COUNTS_LEN 18
+#define INTERVAL_LEN 16
+#define CGROUP_LEN 16
+#define COMM_LEN 16
+#define PID_LEN 7
+#define CPUS_LEN 4
+
+static int aggr_header_lens[] = {
+ [AGGR_CORE] = 18,
+ [AGGR_DIE] = 12,
+ [AGGR_SOCKET] = 6,
+ [AGGR_NODE] = 6,
+ [AGGR_NONE] = 6,
+ [AGGR_THREAD] = 16,
+ [AGGR_GLOBAL] = 0,
+};
+
+static const char *aggr_header_csv[] = {
+ [AGGR_CORE] = "core,cpus,",
+ [AGGR_DIE] = "die,cpus,",
+ [AGGR_SOCKET] = "socket,cpus,",
+ [AGGR_NONE] = "cpu,",
+ [AGGR_THREAD] = "comm-pid,",
+ [AGGR_NODE] = "node,",
+ [AGGR_GLOBAL] = ""
+};
+
+static const char *aggr_header_std[] = {
+ [AGGR_CORE] = "core",
+ [AGGR_DIE] = "die",
+ [AGGR_SOCKET] = "socket",
+ [AGGR_NONE] = "cpu",
+ [AGGR_THREAD] = "comm-pid",
+ [AGGR_NODE] = "node",
+ [AGGR_GLOBAL] = ""
+};
+
static void print_running_std(struct perf_stat_config *config, u64 run, u64 ena)
{
if (run != ena)
@@ -116,7 +155,7 @@ static void print_noise(struct perf_stat_config *config,

static void print_cgroup_std(struct perf_stat_config *config, const char *cgrp_name)
{
- fprintf(config->output, " %-16s", cgrp_name);
+ fprintf(config->output, " %-*s", CGROUP_LEN, cgrp_name);
}

static void print_cgroup_csv(struct perf_stat_config *config, const char *cgrp_name)
@@ -147,44 +186,46 @@ static void print_aggr_id_std(struct perf_stat_config *config,
struct evsel *evsel, struct aggr_cpu_id id, int nr)
{
FILE *output = config->output;
+ int idx = config->aggr_mode;
+ char buf[128];

switch (config->aggr_mode) {
case AGGR_CORE:
- fprintf(output, "S%d-D%d-C%*d %*d ",
- id.socket, id.die, -8, id.core, 4, nr);
+ snprintf(buf, sizeof(buf), "S%d-D%d-C%d", id.socket, id.die, id.core);
break;
case AGGR_DIE:
- fprintf(output, "S%d-D%*d %*d ",
- id.socket, -8, id.die, 4, nr);
+ snprintf(buf, sizeof(buf), "S%d-D%d", id.socket, id.die);
break;
case AGGR_SOCKET:
- fprintf(output, "S%*d %*d ",
- -5, id.socket, 4, nr);
+ snprintf(buf, sizeof(buf), "S%d", id.socket);
break;
case AGGR_NODE:
- fprintf(output, "N%*d %*d ",
- -5, id.node, 4, nr);
+ snprintf(buf, sizeof(buf), "N%d", id.node);
break;
case AGGR_NONE:
if (evsel->percore && !config->percore_show_thread) {
- fprintf(output, "S%d-D%d-C%*d ",
- id.socket, id.die, -3, id.core);
+ snprintf(buf, sizeof(buf), "S%d-D%d-C%d ",
+ id.socket, id.die, id.core);
+ fprintf(output, "%-*s ",
+ aggr_header_lens[AGGR_CORE], buf);
} else if (id.cpu.cpu > -1) {
- fprintf(output, "CPU%*d ",
- -7, id.cpu.cpu);
+ fprintf(output, "CPU%-*d ",
+ aggr_header_lens[AGGR_NONE] - 3, id.cpu.cpu);
}
- break;
+ return;
case AGGR_THREAD:
- fprintf(output, "%*s-%*d ",
- 16, perf_thread_map__comm(evsel->core.threads, id.thread_idx),
- -8, perf_thread_map__pid(evsel->core.threads, id.thread_idx));
- break;
+ fprintf(output, "%*s-%-*d ",
+ COMM_LEN, perf_thread_map__comm(evsel->core.threads, id.thread_idx),
+ PID_LEN, perf_thread_map__pid(evsel->core.threads, id.thread_idx));
+ return;
case AGGR_GLOBAL:
case AGGR_UNSET:
case AGGR_MAX:
default:
- break;
+ return;
}
+
+ fprintf(output, "%-*s %*d ", aggr_header_lens[idx], buf, 4, nr);
}

static void print_aggr_id_csv(struct perf_stat_config *config,
@@ -301,8 +342,6 @@ struct outstate {
struct evsel *evsel;
};

-#define METRIC_LEN 38
-
static void new_line_std(struct perf_stat_config *config __maybe_unused,
void *ctx)
{
@@ -534,19 +573,19 @@ static void print_counter_value_std(struct perf_stat_config *config,
const char *bad_count = evsel->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED;

if (config->big_num)
- fmt = floor(sc) != sc ? "%'18.2f " : "%'18.0f ";
+ fmt = floor(sc) != sc ? "%'*.2f " : "%'*.0f ";
else
- fmt = floor(sc) != sc ? "%18.2f " : "%18.0f ";
+ fmt = floor(sc) != sc ? "%*.2f " : "%*.0f ";

if (ok)
- fprintf(output, fmt, avg);
+ fprintf(output, fmt, COUNTS_LEN, avg);
else
- fprintf(output, "%18s ", bad_count);
+ fprintf(output, "%*s ", COUNTS_LEN, bad_count);

if (evsel->unit)
fprintf(output, "%-*s ", config->unit_width, evsel->unit);

- fprintf(output, "%-*s", 32, evsel__name(evsel));
+ fprintf(output, "%-*s", EVNAME_LEN, evsel__name(evsel));
}

static void print_counter_value_csv(struct perf_stat_config *config,
@@ -904,34 +943,19 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
}
}

-static int aggr_header_lens[] = {
- [AGGR_CORE] = 24,
- [AGGR_DIE] = 18,
- [AGGR_SOCKET] = 12,
- [AGGR_NONE] = 6,
- [AGGR_THREAD] = 24,
- [AGGR_NODE] = 6,
- [AGGR_GLOBAL] = 0,
-};
-
-static const char *aggr_header_csv[] = {
- [AGGR_CORE] = "core,cpus,",
- [AGGR_DIE] = "die,cpus,",
- [AGGR_SOCKET] = "socket,cpus,",
- [AGGR_NONE] = "cpu,",
- [AGGR_THREAD] = "comm-pid,",
- [AGGR_NODE] = "node,",
- [AGGR_GLOBAL] = ""
-};
-
static void print_metric_headers_std(struct perf_stat_config *config,
const char *prefix, bool no_indent)
{
if (prefix)
fprintf(config->output, "%s", prefix);
+
if (!no_indent) {
- fprintf(config->output, "%*s",
- aggr_header_lens[config->aggr_mode], "");
+ int len = aggr_header_lens[config->aggr_mode];
+
+ if (nr_cgroups)
+ len += CGROUP_LEN + 1;
+
+ fprintf(config->output, "%*s", len, "");
}
}

@@ -1025,46 +1049,39 @@ static void print_interval(struct perf_stat_config *config,
!config->csv_output && !config->json_output) {
switch (config->aggr_mode) {
case AGGR_NODE:
- fprintf(output, "# time node cpus");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
- break;
case AGGR_SOCKET:
- fprintf(output, "# time socket cpus");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
- break;
case AGGR_DIE:
- fprintf(output, "# time die cpus");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
- break;
case AGGR_CORE:
- fprintf(output, "# time core cpus");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
+ fprintf(output, "#%*s %-*s cpus",
+ INTERVAL_LEN - 1, "time",
+ aggr_header_lens[config->aggr_mode],
+ aggr_header_std[config->aggr_mode]);
break;
case AGGR_NONE:
- fprintf(output, "# time CPU ");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
+ fprintf(output, "#%*s %-*s",
+ INTERVAL_LEN - 1, "time",
+ aggr_header_lens[config->aggr_mode],
+ aggr_header_std[config->aggr_mode]);
break;
case AGGR_THREAD:
- fprintf(output, "# time comm-pid");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
+ fprintf(output, "#%*s %*s-%-*s",
+ INTERVAL_LEN - 1, "time",
+ COMM_LEN, "comm", PID_LEN, "pid");
break;
case AGGR_GLOBAL:
default:
- if (!config->iostat_run) {
- fprintf(output, "# time");
- if (!metric_only)
- fprintf(output, " counts %*s events\n", unit_width, "unit");
- }
+ if (!config->iostat_run)
+ fprintf(output, "#%*s",
+ INTERVAL_LEN - 1, "time");
case AGGR_UNSET:
case AGGR_MAX:
break;
}
+
+ if (!metric_only) {
+ fprintf(output, " %*s %*s events\n",
+ COUNTS_LEN, "counts", unit_width, "unit");
+ }
}

if ((num_print_interval == 0 || config->interval_clear) && metric_only)
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:28:44

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 11/19] perf stat: Factor out prepare_interval()

This logic does not print the time directly, but it just puts the
timestamp in the buffer as a prefix. To reduce the confusion, factor
out the code into a separate function.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 39 +++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index bb2791459f5f..c234be656db9 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -993,9 +993,25 @@ static void print_metric_headers(struct perf_stat_config *config,
fputc('\n', config->output);
}

+static void prepare_interval(struct perf_stat_config *config,
+ char *prefix, struct timespec *ts)
+{
+ if (config->iostat_run)
+ return;
+
+ if (!config->json_output)
+ sprintf(prefix, "%6lu.%09lu%s", (unsigned long) ts->tv_sec,
+ ts->tv_nsec, config->csv_sep);
+ else if (!config->metric_only)
+ sprintf(prefix, "{\"interval\" : %lu.%09lu, ", (unsigned long)
+ ts->tv_sec, ts->tv_nsec);
+ else
+ sprintf(prefix, "{\"interval\" : %lu.%09lu}", (unsigned long)
+ ts->tv_sec, ts->tv_nsec);
+}
+
static void print_interval(struct perf_stat_config *config,
- struct evlist *evlist,
- char *prefix, struct timespec *ts)
+ struct evlist *evlist)
{
bool metric_only = config->metric_only;
unsigned int unit_width = config->unit_width;
@@ -1005,16 +1021,6 @@ static void print_interval(struct perf_stat_config *config,
if (config->interval_clear && isatty(fileno(output)))
puts(CONSOLE_CLEAR);

- if (!config->iostat_run && !config->json_output)
- sprintf(prefix, "%6lu.%09lu%s", (unsigned long) ts->tv_sec,
- ts->tv_nsec, config->csv_sep);
- if (!config->iostat_run && config->json_output && !config->metric_only)
- sprintf(prefix, "{\"interval\" : %lu.%09lu, ", (unsigned long)
- ts->tv_sec, ts->tv_nsec);
- if (!config->iostat_run && config->json_output && config->metric_only)
- sprintf(prefix, "{\"interval\" : %lu.%09lu}", (unsigned long)
- ts->tv_sec, ts->tv_nsec);
-
if ((num_print_interval == 0 || config->interval_clear) &&
!config->csv_output && !config->json_output) {
switch (config->aggr_mode) {
@@ -1252,10 +1258,13 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
if (config->iostat_run)
evlist->selected = evlist__first(evlist);

- if (interval)
- print_interval(config, evlist, prefix = buf, ts);
- else
+ if (interval) {
+ prefix = buf;
+ prepare_interval(config, prefix, ts);
+ print_interval(config, evlist);
+ } else {
print_header(config, _target, argc, argv);
+ }

if (metric_only) {
static int num_print_iv;
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:31:03

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 09/19] perf stat: Align cgroup names

We don't know how long cgroup name is, but at least we can align short
ones like below.

$ perf stat -a --for-each-cgroup system.slice,user.slice true

Performance counter stats for 'system wide':

0.13 msec cpu-clock system.slice # 0.010 CPUs utilized
4 context-switches system.slice # 31.989 K/sec
1 cpu-migrations system.slice # 7.997 K/sec
0 page-faults system.slice # 0.000 /sec
450,673 cycles system.slice # 3.604 GHz (92.41%)
161,216 instructions system.slice # 0.36 insn per cycle (92.41%)
32,678 branches system.slice # 261.332 M/sec (92.41%)
2,628 branch-misses system.slice # 8.04% of all branches (92.41%)
14.29 msec cpu-clock user.slice # 1.163 CPUs utilized
35 context-switches user.slice # 2.449 K/sec
12 cpu-migrations user.slice # 839.691 /sec
57 page-faults user.slice # 3.989 K/sec
49,683,026 cycles user.slice # 3.477 GHz (99.38%)
110,790,266 instructions user.slice # 2.23 insn per cycle (99.38%)
24,552,255 branches user.slice # 1.718 G/sec (99.38%)
127,779 branch-misses user.slice # 0.52% of all branches (99.38%)

0.012289431 seconds time elapsed

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index bf3f2f9d5dee..e66f766a3d78 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -116,7 +116,7 @@ static void print_noise(struct perf_stat_config *config,

static void print_cgroup_std(struct perf_stat_config *config, const char *cgrp_name)
{
- fprintf(config->output, " %s", cgrp_name);
+ fprintf(config->output, " %-16s", cgrp_name);
}

static void print_cgroup_csv(struct perf_stat_config *config, const char *cgrp_name)
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:36:19

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 18/19] perf stat: Support --for-each-cgroup and --metric-only

When we have events for each cgroup, the metric should be printed for
each cgroup separately. Add print_cgroup_counter() to handle that
situation properly.

Also change print_metric_headers() not to print duplicate headers
by checking cgroups.

$ perf stat -a --for-each-cgroup system.slice,user.slice --metric-only sleep 1

Performance counter stats for 'system wide':

GHz insn per cycle branch-misses of all branches
system.slice 3.792 0.61 3.24%
user.slice 3.661 2.32 0.37%

1.016111516 seconds time elapsed

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 58 +++++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 7a0673be720b..cf25ed99b5df 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -168,10 +168,10 @@ static void print_cgroup_json(struct perf_stat_config *config, const char *cgrp_
fprintf(config->output, "\"cgroup\" : \"%s\", ", cgrp_name);
}

-static void print_cgroup(struct perf_stat_config *config, struct evsel *evsel)
+static void print_cgroup(struct perf_stat_config *config, struct cgroup *cgrp)
{
- if (nr_cgroups) {
- const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name : "";
+ if (nr_cgroups || config->cgroup_list) {
+ const char *cgrp_name = cgrp ? cgrp->name : "";

if (config->json_output)
print_cgroup_json(config, cgrp_name);
@@ -340,6 +340,7 @@ struct outstate {
int nr;
struct aggr_cpu_id id;
struct evsel *evsel;
+ struct cgroup *cgrp;
};

static void new_line_std(struct perf_stat_config *config __maybe_unused,
@@ -552,6 +553,9 @@ static void print_metric_header(struct perf_stat_config *config,
os->evsel->priv != os->evsel->evlist->selected->priv)
return;

+ if (os->evsel->cgrp != os->cgrp)
+ return;
+
if (!valid_only_metric(unit))
return;
unit = fixunit(tbuf, os->evsel, unit);
@@ -642,7 +646,7 @@ static void abs_printout(struct perf_stat_config *config,
{
aggr_printout(config, evsel, id, nr);
print_counter_value(config, evsel, avg, ok);
- print_cgroup(config, evsel);
+ print_cgroup(config, evsel->cgrp);
}

static bool is_mixed_hw_group(struct evsel *counter)
@@ -838,7 +842,8 @@ static void print_counter_aggrdata(struct perf_stat_config *config,

static void print_metric_begin(struct perf_stat_config *config,
struct evlist *evlist,
- char *prefix, int aggr_idx)
+ char *prefix, int aggr_idx,
+ struct cgroup *cgrp)
{
struct perf_stat_aggr *aggr;
struct aggr_cpu_id id;
@@ -854,6 +859,8 @@ static void print_metric_begin(struct perf_stat_config *config,
id = config->aggr_map->map[aggr_idx];
aggr = &evsel->stats->aggr[aggr_idx];
aggr_printout(config, evsel, id, aggr->nr);
+
+ print_cgroup(config, cgrp);
}

static void print_metric_end(struct perf_stat_config *config)
@@ -880,7 +887,7 @@ static void print_aggr(struct perf_stat_config *config,
* Without each counter has its own line.
*/
for (s = 0; s < config->aggr_map->nr; s++) {
- print_metric_begin(config, evlist, prefix, s);
+ print_metric_begin(config, evlist, prefix, s, /*cgrp=*/NULL);

evlist__for_each_entry(evlist, counter) {
if (counter->merged_stat)
@@ -935,7 +942,8 @@ static void print_no_aggr_metric(struct perf_stat_config *config,

id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
if (first) {
- print_metric_begin(config, evlist, prefix, counter_idx);
+ print_metric_begin(config, evlist, prefix,
+ counter_idx, /*cgrp=*/NULL);
first = false;
}
val = ps->aggr[counter_idx].counts.val;
@@ -960,7 +968,7 @@ static void print_metric_headers_std(struct perf_stat_config *config,
if (!no_indent) {
int len = aggr_header_lens[config->aggr_mode];

- if (nr_cgroups)
+ if (nr_cgroups || config->cgroup_list)
len += CGROUP_LEN + 1;

fprintf(config->output, "%*s", len, "");
@@ -1012,6 +1020,9 @@ static void print_metric_headers(struct perf_stat_config *config,
if (config->iostat_run)
iostat_print_header_prefix(config);

+ if (config->cgroup_list)
+ os.cgrp = evlist__first(evlist)->cgrp;
+
/* Print metrics headers only */
evlist__for_each_entry(evlist, counter) {
os.evsel = counter;
@@ -1305,6 +1316,28 @@ static void print_percore(struct perf_stat_config *config,
fputc('\n', output);
}

+static void print_cgroup_counter(struct perf_stat_config *config, struct evlist *evlist,
+ char *prefix)
+{
+ struct cgroup *cgrp = NULL;
+ struct evsel *counter;
+
+ evlist__for_each_entry(evlist, counter) {
+ if (cgrp != counter->cgrp) {
+ if (cgrp != NULL)
+ print_metric_end(config);
+
+ cgrp = counter->cgrp;
+ print_metric_begin(config, evlist, prefix,
+ /*aggr_idx=*/0, cgrp);
+ }
+
+ print_counter(config, counter, prefix);
+ }
+ if (cgrp)
+ print_metric_end(config);
+}
+
void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *config,
struct target *_target, struct timespec *ts, int argc, const char **argv)
{
@@ -1332,11 +1365,14 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
break;
case AGGR_THREAD:
case AGGR_GLOBAL:
- if (config->iostat_run)
+ if (config->iostat_run) {
iostat_print_counters(evlist, config, ts, prefix = buf,
print_counter);
- else {
- print_metric_begin(config, evlist, prefix, /*aggr_idx=*/0);
+ } else if (config->cgroup_list) {
+ print_cgroup_counter(config, evlist, prefix);
+ } else {
+ print_metric_begin(config, evlist, prefix,
+ /*aggr_idx=*/0, /*cgrp=*/NULL);
evlist__for_each_entry(evlist, counter) {
print_counter(config, counter, prefix);
}
--
2.38.1.493.g58b659f92b-goog


2022-11-14 23:59:21

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 08/19] perf stat: Add before_metric argument

Unfortunately, event running time, percentage and noise data are printed
in different positions in normal output than CSV/JSON. I think it's
better to put such details in where it actually prints.

So add before_metric argument to print_noise() and print_running() and
call them twice before and after the metric.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 82 +++++++++++++++++-----------------
1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index fe5483893289..bf3f2f9d5dee 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -52,14 +52,18 @@ static void print_running_json(struct perf_stat_config *config, u64 run, u64 ena
}

static void print_running(struct perf_stat_config *config,
- u64 run, u64 ena)
+ u64 run, u64 ena, bool before_metric)
{
- if (config->json_output)
- print_running_json(config, run, ena);
- else if (config->csv_output)
- print_running_csv(config, run, ena);
- else
- print_running_std(config, run, ena);
+ if (config->json_output) {
+ if (before_metric)
+ print_running_json(config, run, ena);
+ } else if (config->csv_output) {
+ if (before_metric)
+ print_running_csv(config, run, ena);
+ } else {
+ if (!before_metric)
+ print_running_std(config, run, ena);
+ }
}

static void print_noise_pct_std(struct perf_stat_config *config,
@@ -82,20 +86,24 @@ static void print_noise_pct_json(struct perf_stat_config *config,
}

static void print_noise_pct(struct perf_stat_config *config,
- double total, double avg)
+ double total, double avg, bool before_metric)
{
double pct = rel_stddev_stats(total, avg);

- if (config->json_output)
- print_noise_pct_json(config, pct);
- else if (config->csv_output)
- print_noise_pct_csv(config, pct);
- else
- print_noise_pct_std(config, pct);
+ if (config->json_output) {
+ if (before_metric)
+ print_noise_pct_json(config, pct);
+ } else if (config->csv_output) {
+ if (before_metric)
+ print_noise_pct_csv(config, pct);
+ } else {
+ if (!before_metric)
+ print_noise_pct_std(config, pct);
+ }
}

static void print_noise(struct perf_stat_config *config,
- struct evsel *evsel, double avg)
+ struct evsel *evsel, double avg, bool before_metric)
{
struct perf_stat_evsel *ps;

@@ -103,7 +111,7 @@ static void print_noise(struct perf_stat_config *config,
return;

ps = evsel->stats;
- print_noise_pct(config, stddev_stats(&ps->res_stats), avg);
+ print_noise_pct(config, stddev_stats(&ps->res_stats), avg, before_metric);
}

static void print_cgroup_std(struct perf_stat_config *config, const char *cgrp_name)
@@ -637,6 +645,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
};
print_metric_t pm;
new_line_t nl;
+ bool ok = true;

if (config->csv_output) {
static const int aggr_fields[AGGR_MAX] = {
@@ -672,7 +681,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
return;
}

- abs_printout(config, id, nr, counter, uval, /*ok=*/false);
+ ok = false;

if (counter->supported) {
if (!evlist__has_hybrid(counter->evlist)) {
@@ -681,37 +690,30 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
config->print_mixed_hw_group_error = 1;
}
}
-
- if (!config->csv_output && !config->json_output)
- pm(config, &os, NULL, NULL, "", 0);
- print_noise(config, counter, noise);
- print_running(config, run, ena);
- if (config->csv_output || config->json_output)
- pm(config, &os, NULL, NULL, "", 0);
- return;
}

- if (!config->metric_only)
- abs_printout(config, id, nr, counter, uval, /*ok=*/true);
-
out.print_metric = pm;
out.new_line = nl;
out.ctx = &os;
out.force_header = false;

- if (config->csv_output && !config->metric_only) {
- print_noise(config, counter, noise);
- print_running(config, run, ena);
- } else if (config->json_output && !config->metric_only) {
- print_noise(config, counter, noise);
- print_running(config, run, ena);
+ if (!config->metric_only) {
+ abs_printout(config, id, nr, counter, uval, ok);
+
+ print_noise(config, counter, noise, /*before_metric=*/true);
+ print_running(config, run, ena, /*before_metric=*/true);
+ }
+
+ if (ok) {
+ perf_stat__print_shadow_stats(config, counter, uval, map_idx,
+ &out, &config->metric_events, st);
+ } else {
+ pm(config, &os, /*color=*/NULL, /*format=*/NULL, /*unit=*/"", /*val=*/0);
}

- perf_stat__print_shadow_stats(config, counter, uval, map_idx,
- &out, &config->metric_events, st);
- if (!config->csv_output && !config->metric_only && !config->json_output) {
- print_noise(config, counter, noise);
- print_running(config, run, ena);
+ if (!config->metric_only) {
+ print_noise(config, counter, noise, /*before_metric=*/false);
+ print_running(config, run, ena, /*before_metric=*/false);
}
}

@@ -1151,7 +1153,7 @@ static void print_footer(struct perf_stat_config *config)
fprintf(output, " %17.*f +- %.*f seconds time elapsed",
precision, avg, precision, sd);

- print_noise_pct(config, sd, avg);
+ print_noise_pct(config, sd, avg, /*before_metric=*/false);
}
fprintf(output, "\n\n");

--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:02:32

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 03/19] perf stat: Split print_noise_pct() function

Likewise, split print_noise_pct() for each output mode. Although it's
a tiny function, more logic will be added soon so it'd be better split
it and treat it in the same way.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 281b811f8574..a230f65efa62 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -62,17 +62,36 @@ static void print_running(struct perf_stat_config *config,
print_running_std(config, run, ena);
}

+static void print_noise_pct_std(struct perf_stat_config *config,
+ double pct)
+{
+ if (pct)
+ fprintf(config->output, " ( +-%6.2f%% )", pct);
+}
+
+static void print_noise_pct_csv(struct perf_stat_config *config,
+ double pct)
+{
+ fprintf(config->output, "%s%.2f%%", config->csv_sep, pct);
+}
+
+static void print_noise_pct_json(struct perf_stat_config *config,
+ double pct)
+{
+ fprintf(config->output, "\"variance\" : %.2f, ", pct);
+}
+
static void print_noise_pct(struct perf_stat_config *config,
double total, double avg)
{
double pct = rel_stddev_stats(total, avg);

if (config->json_output)
- fprintf(config->output, "\"variance\" : %.2f, ", pct);
+ print_noise_pct_json(config, pct);
else if (config->csv_output)
- fprintf(config->output, "%s%.2f%%", config->csv_sep, pct);
- else if (pct)
- fprintf(config->output, " ( +-%6.2f%% )", pct);
+ print_noise_pct_csv(config, pct);
+ else
+ print_noise_pct_std(config, pct);
}

static void print_noise(struct perf_stat_config *config,
--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:03:44

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 14/19] perf stat: Rework header display

There are print_header() and print_interval() to print header lines before
actual counter values. Also print_metric_headers() needs to be called for
the metric-only case.

Let's move all these logics to a single place including num_print_iv to
refresh the headers for interval mode.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 185 +++++++++++++++++++--------------
1 file changed, 106 insertions(+), 79 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index cc8bb6d07dcb..f97817628478 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -1034,94 +1034,129 @@ static void prepare_interval(struct perf_stat_config *config,
ts->tv_sec, ts->tv_nsec);
}

-static void print_interval(struct perf_stat_config *config,
- struct evlist *evlist)
+static void print_header_interval_std(struct perf_stat_config *config,
+ struct target *_target __maybe_unused,
+ struct evlist *evlist,
+ int argc __maybe_unused,
+ const char **argv __maybe_unused)
{
- bool metric_only = config->metric_only;
- unsigned int unit_width = config->unit_width;
FILE *output = config->output;
- static int num_print_interval;

- if (config->interval_clear && isatty(fileno(output)))
- puts(CONSOLE_CLEAR);
+ switch (config->aggr_mode) {
+ case AGGR_NODE:
+ case AGGR_SOCKET:
+ case AGGR_DIE:
+ case AGGR_CORE:
+ fprintf(output, "#%*s %-*s cpus",
+ INTERVAL_LEN - 1, "time",
+ aggr_header_lens[config->aggr_mode],
+ aggr_header_std[config->aggr_mode]);
+ break;
+ case AGGR_NONE:
+ fprintf(output, "#%*s %-*s",
+ INTERVAL_LEN - 1, "time",
+ aggr_header_lens[config->aggr_mode],
+ aggr_header_std[config->aggr_mode]);
+ break;
+ case AGGR_THREAD:
+ fprintf(output, "#%*s %*s-%-*s",
+ INTERVAL_LEN - 1, "time",
+ COMM_LEN, "comm", PID_LEN, "pid");
+ break;
+ case AGGR_GLOBAL:
+ default:
+ if (!config->iostat_run)
+ fprintf(output, "#%*s",
+ INTERVAL_LEN - 1, "time");
+ case AGGR_UNSET:
+ case AGGR_MAX:
+ break;
+ }

- if ((num_print_interval == 0 || config->interval_clear) &&
- !config->csv_output && !config->json_output) {
- switch (config->aggr_mode) {
- case AGGR_NODE:
- case AGGR_SOCKET:
- case AGGR_DIE:
- case AGGR_CORE:
- fprintf(output, "#%*s %-*s cpus",
- INTERVAL_LEN - 1, "time",
- aggr_header_lens[config->aggr_mode],
- aggr_header_std[config->aggr_mode]);
- break;
- case AGGR_NONE:
- fprintf(output, "#%*s %-*s",
- INTERVAL_LEN - 1, "time",
- aggr_header_lens[config->aggr_mode],
- aggr_header_std[config->aggr_mode]);
- break;
- case AGGR_THREAD:
- fprintf(output, "#%*s %*s-%-*s",
- INTERVAL_LEN - 1, "time",
- COMM_LEN, "comm", PID_LEN, "pid");
- break;
- case AGGR_GLOBAL:
- default:
- if (!config->iostat_run)
- fprintf(output, "#%*s",
- INTERVAL_LEN - 1, "time");
- case AGGR_UNSET:
- case AGGR_MAX:
- break;
- }
+ if (config->metric_only)
+ print_metric_headers(config, evlist, " ", true);
+ else
+ fprintf(output, " %*s %*s events\n",
+ COUNTS_LEN, "counts", config->unit_width, "unit");
+}

- if (!metric_only) {
- fprintf(output, " %*s %*s events\n",
- COUNTS_LEN, "counts", unit_width, "unit");
- }
- }
+static void print_header_std(struct perf_stat_config *config,
+ struct target *_target, struct evlist *evlist,
+ int argc, const char **argv)
+{
+ FILE *output = config->output;
+ int i;
+
+ fprintf(output, "\n");
+ fprintf(output, " Performance counter stats for ");
+ if (_target->bpf_str)
+ fprintf(output, "\'BPF program(s) %s", _target->bpf_str);
+ else if (_target->system_wide)
+ fprintf(output, "\'system wide");
+ else if (_target->cpu_list)
+ fprintf(output, "\'CPU(s) %s", _target->cpu_list);
+ else if (!target__has_task(_target)) {
+ fprintf(output, "\'%s", argv ? argv[0] : "pipe");
+ for (i = 1; argv && (i < argc); i++)
+ fprintf(output, " %s", argv[i]);
+ } else if (_target->pid)
+ fprintf(output, "process id \'%s", _target->pid);
+ else
+ fprintf(output, "thread id \'%s", _target->tid);

- if ((num_print_interval == 0 || config->interval_clear) && metric_only)
+ fprintf(output, "\'");
+ if (config->run_count > 1)
+ fprintf(output, " (%d runs)", config->run_count);
+ fprintf(output, ":\n\n");
+
+ if (config->metric_only)
+ print_metric_headers(config, evlist, " ", false);
+}
+
+static void print_header_csv(struct perf_stat_config *config,
+ struct target *_target __maybe_unused,
+ struct evlist *evlist,
+ int argc __maybe_unused,
+ const char **argv __maybe_unused)
+{
+ if (config->metric_only)
+ print_metric_headers(config, evlist, " ", true);
+}
+static void print_header_json(struct perf_stat_config *config,
+ struct target *_target __maybe_unused,
+ struct evlist *evlist,
+ int argc __maybe_unused,
+ const char **argv __maybe_unused)
+{
+ if (config->metric_only)
print_metric_headers(config, evlist, " ", true);
- if (++num_print_interval == 25)
- num_print_interval = 0;
}

static void print_header(struct perf_stat_config *config,
struct target *_target,
+ struct evlist *evlist,
int argc, const char **argv)
{
- FILE *output = config->output;
- int i;
+ static int num_print_iv;

fflush(stdout);

- if (!config->csv_output && !config->json_output) {
- fprintf(output, "\n");
- fprintf(output, " Performance counter stats for ");
- if (_target->bpf_str)
- fprintf(output, "\'BPF program(s) %s", _target->bpf_str);
- else if (_target->system_wide)
- fprintf(output, "\'system wide");
- else if (_target->cpu_list)
- fprintf(output, "\'CPU(s) %s", _target->cpu_list);
- else if (!target__has_task(_target)) {
- fprintf(output, "\'%s", argv ? argv[0] : "pipe");
- for (i = 1; argv && (i < argc); i++)
- fprintf(output, " %s", argv[i]);
- } else if (_target->pid)
- fprintf(output, "process id \'%s", _target->pid);
- else
- fprintf(output, "thread id \'%s", _target->tid);
+ if (config->interval_clear)
+ puts(CONSOLE_CLEAR);

- fprintf(output, "\'");
- if (config->run_count > 1)
- fprintf(output, " (%d runs)", config->run_count);
- fprintf(output, ":\n\n");
+ if (num_print_iv == 0 || config->interval_clear) {
+ if (config->json_output)
+ print_header_json(config, _target, evlist, argc, argv);
+ else if (config->csv_output)
+ print_header_csv(config, _target, evlist, argc, argv);
+ else if (config->interval)
+ print_header_interval_std(config, _target, evlist, argc, argv);
+ else
+ print_header_std(config, _target, evlist, argc, argv);
}
+
+ if (num_print_iv++ == 25)
+ num_print_iv = 0;
}

static int get_precision(double num)
@@ -1278,18 +1313,10 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
if (interval) {
prefix = buf;
prepare_interval(config, prefix, ts);
- print_interval(config, evlist);
- } else {
- print_header(config, _target, argc, argv);
}

+ print_header(config, _target, evlist, argc, argv);
if (metric_only) {
- static int num_print_iv;
-
- if (num_print_iv == 0 && !interval)
- print_metric_headers(config, evlist, prefix, false);
- if (num_print_iv++ == 25)
- num_print_iv = 0;
if (config->aggr_mode == AGGR_GLOBAL && prefix && !config->iostat_run)
fprintf(config->output, "%s", prefix);
}
--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:05:42

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 07/19] perf stat: Handle bad events in abs_printout()

In the printout() function, it checks if the event is bad (i.e. not
counted or not supported) and print the result. But it does the same
what abs_printout() is doing. So add an argument to indicate the value
is ok or not and use the same function in both cases.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 68 ++++++++++++++--------------------
1 file changed, 27 insertions(+), 41 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index a72c7442ff3d..fe5483893289 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -518,18 +518,22 @@ static void print_metric_header(struct perf_stat_config *config,
}

static void print_counter_value_std(struct perf_stat_config *config,
- struct evsel *evsel, double avg)
+ struct evsel *evsel, double avg, bool ok)
{
FILE *output = config->output;
double sc = evsel->scale;
const char *fmt;
+ const char *bad_count = evsel->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED;

if (config->big_num)
fmt = floor(sc) != sc ? "%'18.2f " : "%'18.0f ";
else
fmt = floor(sc) != sc ? "%18.2f " : "%18.0f ";

- fprintf(output, fmt, avg);
+ if (ok)
+ fprintf(output, fmt, avg);
+ else
+ fprintf(output, "%18s ", bad_count);

if (evsel->unit)
fprintf(output, "%-*s ", config->unit_width, evsel->unit);
@@ -538,14 +542,18 @@ static void print_counter_value_std(struct perf_stat_config *config,
}

static void print_counter_value_csv(struct perf_stat_config *config,
- struct evsel *evsel, double avg)
+ struct evsel *evsel, double avg, bool ok)
{
FILE *output = config->output;
double sc = evsel->scale;
const char *sep = config->csv_sep;
const char *fmt = floor(sc) != sc ? "%.2f%s" : "%.0f%s";
+ const char *bad_count = evsel->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED;

- fprintf(output, fmt, avg, sep);
+ if (ok)
+ fprintf(output, fmt, avg, sep);
+ else
+ fprintf(output, "%s%s", bad_count, sep);

if (evsel->unit)
fprintf(output, "%s%s", evsel->unit, sep);
@@ -554,11 +562,15 @@ static void print_counter_value_csv(struct perf_stat_config *config,
}

static void print_counter_value_json(struct perf_stat_config *config,
- struct evsel *evsel, double avg)
+ struct evsel *evsel, double avg, bool ok)
{
FILE *output = config->output;
+ const char *bad_count = evsel->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED;

- fprintf(output, "\"counter-value\" : \"%f\", ", avg);
+ if (ok)
+ fprintf(output, "\"counter-value\" : \"%f\", ", avg);
+ else
+ fprintf(output, "\"counter-value\" : \"%s\", ", bad_count);

if (evsel->unit)
fprintf(output, "\"unit\" : \"%s\", ", evsel->unit);
@@ -567,21 +579,22 @@ static void print_counter_value_json(struct perf_stat_config *config,
}

static void print_counter_value(struct perf_stat_config *config,
- struct evsel *evsel, double avg)
+ struct evsel *evsel, double avg, bool ok)
{
if (config->json_output)
- print_counter_value_json(config, evsel, avg);
+ print_counter_value_json(config, evsel, avg, ok);
else if (config->csv_output)
- print_counter_value_csv(config, evsel, avg);
+ print_counter_value_csv(config, evsel, avg, ok);
else
- print_counter_value_std(config, evsel, avg);
+ print_counter_value_std(config, evsel, avg, ok);
}

static void abs_printout(struct perf_stat_config *config,
- struct aggr_cpu_id id, int nr, struct evsel *evsel, double avg)
+ struct aggr_cpu_id id, int nr,
+ struct evsel *evsel, double avg, bool ok)
{
aggr_printout(config, evsel, id, nr);
- print_counter_value(config, evsel, avg);
+ print_counter_value(config, evsel, avg, ok);
print_cgroup(config, evsel);
}

@@ -658,17 +671,8 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
pm(config, &os, NULL, "", "", 0);
return;
}
- aggr_printout(config, counter, id, nr);

- if (config->json_output) {
- fprintf(config->output, "\"counter-value\" : \"%s\", ",
- counter->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED);
- } else {
- fprintf(config->output, "%*s%s",
- config->csv_output ? 0 : 18,
- counter->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED,
- config->csv_sep);
- }
+ abs_printout(config, id, nr, counter, uval, /*ok=*/false);

if (counter->supported) {
if (!evlist__has_hybrid(counter->evlist)) {
@@ -678,24 +682,6 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
}
}

- if (config->json_output) {
- fprintf(config->output, "\"unit\" : \"%s\", ", counter->unit);
- } else {
- fprintf(config->output, "%-*s%s",
- config->csv_output ? 0 : config->unit_width,
- counter->unit, config->csv_sep);
- }
-
- if (config->json_output) {
- fprintf(config->output, "\"event\" : \"%s\", ",
- evsel__name(counter));
- } else {
- fprintf(config->output, "%*s",
- config->csv_output ? 0 : -25, evsel__name(counter));
- }
-
- print_cgroup(config, counter);
-
if (!config->csv_output && !config->json_output)
pm(config, &os, NULL, NULL, "", 0);
print_noise(config, counter, noise);
@@ -706,7 +692,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
}

if (!config->metric_only)
- abs_printout(config, id, nr, counter, uval);
+ abs_printout(config, id, nr, counter, uval, /*ok=*/true);

out.print_metric = pm;
out.new_line = nl;
--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:06:29

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 02/19] perf stat: Split print_running() function

To make the code more obvious and hopefully simpler, factor out the
code for each output mode - stdio, CSV, JSON.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 37 +++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 2a3c1e0098b9..281b811f8574 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -25,24 +25,41 @@
#define CNTR_NOT_SUPPORTED "<not supported>"
#define CNTR_NOT_COUNTED "<not counted>"

-static void print_running(struct perf_stat_config *config,
- u64 run, u64 ena)
+static void print_running_std(struct perf_stat_config *config, u64 run, u64 ena)
+{
+ if (run != ena)
+ fprintf(config->output, " (%.2f%%)", 100.0 * run / ena);
+}
+
+static void print_running_csv(struct perf_stat_config *config, u64 run, u64 ena)
{
+ double enabled_percent = 100;
+
+ if (run != ena)
+ enabled_percent = 100 * run / ena;
+ fprintf(config->output, "%s%" PRIu64 "%s%.2f",
+ config->csv_sep, run, config->csv_sep, enabled_percent);
+}

+static void print_running_json(struct perf_stat_config *config, u64 run, u64 ena)
+{
double enabled_percent = 100;

if (run != ena)
enabled_percent = 100 * run / ena;
+ fprintf(config->output, "\"event-runtime\" : %" PRIu64 ", \"pcnt-running\" : %.2f, ",
+ run, enabled_percent);
+}
+
+static void print_running(struct perf_stat_config *config,
+ u64 run, u64 ena)
+{
if (config->json_output)
- fprintf(config->output,
- "\"event-runtime\" : %" PRIu64 ", \"pcnt-running\" : %.2f, ",
- run, enabled_percent);
+ print_running_json(config, run, ena);
else if (config->csv_output)
- fprintf(config->output,
- "%s%" PRIu64 "%s%.2f", config->csv_sep,
- run, config->csv_sep, enabled_percent);
- else if (run != ena)
- fprintf(config->output, " (%.2f%%)", 100.0 * run / ena);
+ print_running_csv(config, run, ena);
+ else
+ print_running_std(config, run, ena);
}

static void print_noise_pct(struct perf_stat_config *config,
--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:08:04

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 04/19] perf stat: Split print_cgroup() function

Likewise, split print_cgroup() for each output mode.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index a230f65efa62..af2a561eb20c 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -106,15 +106,32 @@ static void print_noise(struct perf_stat_config *config,
print_noise_pct(config, stddev_stats(&ps->res_stats), avg);
}

+static void print_cgroup_std(struct perf_stat_config *config, const char *cgrp_name)
+{
+ fprintf(config->output, " %s", cgrp_name);
+}
+
+static void print_cgroup_csv(struct perf_stat_config *config, const char *cgrp_name)
+{
+ fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
+}
+
+static void print_cgroup_json(struct perf_stat_config *config, const char *cgrp_name)
+{
+ fprintf(config->output, "\"cgroup\" : \"%s\", ", cgrp_name);
+}
+
static void print_cgroup(struct perf_stat_config *config, struct evsel *evsel)
{
if (nr_cgroups) {
const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name : "";

if (config->json_output)
- fprintf(config->output, "\"cgroup\" : \"%s\", ", cgrp_name);
+ print_cgroup_json(config, cgrp_name);
+ if (config->csv_output)
+ print_cgroup_csv(config, cgrp_name);
else
- fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
+ print_cgroup_std(config, cgrp_name);
}
}

--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:29:33

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 10/19] perf stat: Split print_metric_headers() function

The print_metric_headers() shows metric headers a little bit for each
mode. Split it out to make the code clearer.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 52 ++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e66f766a3d78..bb2791459f5f 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -924,6 +924,37 @@ static const char *aggr_header_csv[] = {
[AGGR_GLOBAL] = ""
};

+static void print_metric_headers_std(struct perf_stat_config *config,
+ const char *prefix, bool no_indent)
+{
+ if (prefix)
+ fprintf(config->output, "%s", prefix);
+ if (!no_indent) {
+ fprintf(config->output, "%*s",
+ aggr_header_lens[config->aggr_mode], "");
+ }
+}
+
+static void print_metric_headers_csv(struct perf_stat_config *config,
+ const char *prefix,
+ bool no_indent __maybe_unused)
+{
+ if (prefix)
+ fprintf(config->output, "%s", prefix);
+ if (config->interval)
+ fputs("time,", config->output);
+ if (!config->iostat_run)
+ fputs(aggr_header_csv[config->aggr_mode], config->output);
+}
+
+static void print_metric_headers_json(struct perf_stat_config *config,
+ const char *prefix __maybe_unused,
+ bool no_indent __maybe_unused)
+{
+ if (config->interval)
+ fputs("{\"unit\" : \"sec\"}", config->output);
+}
+
static void print_metric_headers(struct perf_stat_config *config,
struct evlist *evlist,
const char *prefix, bool no_indent)
@@ -939,22 +970,13 @@ static void print_metric_headers(struct perf_stat_config *config,
.force_header = true,
};

- if (prefix && !config->json_output)
- fprintf(config->output, "%s", prefix);
+ if (config->json_output)
+ print_metric_headers_json(config, prefix, no_indent);
+ else if (config->csv_output)
+ print_metric_headers_csv(config, prefix, no_indent);
+ else
+ print_metric_headers_std(config, prefix, no_indent);

- if (!config->csv_output && !config->json_output && !no_indent)
- fprintf(config->output, "%*s",
- aggr_header_lens[config->aggr_mode], "");
- if (config->csv_output) {
- if (config->interval)
- fputs("time,", config->output);
- if (!config->iostat_run)
- fputs(aggr_header_csv[config->aggr_mode], config->output);
- }
- if (config->json_output) {
- if (config->interval)
- fputs("{\"unit\" : \"sec\"}", config->output);
- }
if (config->iostat_run)
iostat_print_header_prefix(config);

--
2.38.1.493.g58b659f92b-goog


2022-11-15 00:36:33

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 05/19] perf stat: Split aggr_printout() function

The aggr_printout() function is to print aggr_id and count (nr).
Split it for each output mode to simplify the code.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 220 ++++++++++++++++++---------------
1 file changed, 121 insertions(+), 99 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index af2a561eb20c..ed421f6d512f 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -135,124 +135,135 @@ static void print_cgroup(struct perf_stat_config *config, struct evsel *evsel)
}
}

-
-static void aggr_printout(struct perf_stat_config *config,
- struct evsel *evsel, struct aggr_cpu_id id, int nr)
+static void print_aggr_id_std(struct perf_stat_config *config,
+ struct evsel *evsel, struct aggr_cpu_id id, int nr)
{
+ FILE *output = config->output;

+ switch (config->aggr_mode) {
+ case AGGR_CORE:
+ fprintf(output, "S%d-D%d-C%*d %*d ",
+ id.socket, id.die, -8, id.core, 4, nr);
+ break;
+ case AGGR_DIE:
+ fprintf(output, "S%d-D%*d %*d ",
+ id.socket, -8, id.die, 4, nr);
+ break;
+ case AGGR_SOCKET:
+ fprintf(output, "S%*d %*d ",
+ -5, id.socket, 4, nr);
+ break;
+ case AGGR_NODE:
+ fprintf(output, "N%*d %*d ",
+ -5, id.node, 4, nr);
+ break;
+ case AGGR_NONE:
+ if (evsel->percore && !config->percore_show_thread) {
+ fprintf(output, "S%d-D%d-C%*d ",
+ id.socket, id.die, -3, id.core);
+ } else if (id.cpu.cpu > -1) {
+ fprintf(output, "CPU%*d ",
+ -7, id.cpu.cpu);
+ }
+ break;
+ case AGGR_THREAD:
+ fprintf(output, "%*s-%*d ",
+ 16, perf_thread_map__comm(evsel->core.threads, id.thread_idx),
+ -8, perf_thread_map__pid(evsel->core.threads, id.thread_idx));
+ break;
+ case AGGR_GLOBAL:
+ case AGGR_UNSET:
+ case AGGR_MAX:
+ default:
+ break;
+ }
+}

- if (config->json_output && !config->interval)
- fprintf(config->output, "{");
+static void print_aggr_id_csv(struct perf_stat_config *config,
+ struct evsel *evsel, struct aggr_cpu_id id, int nr)
+{
+ FILE *output = config->output;
+ const char *sep = config->csv_sep;

switch (config->aggr_mode) {
case AGGR_CORE:
- if (config->json_output) {
- fprintf(config->output,
- "\"core\" : \"S%d-D%d-C%d\", \"aggregate-number\" : %d, ",
- id.socket,
- id.die,
- id.core,
- nr);
- } else {
- fprintf(config->output, "S%d-D%d-C%*d%s%*d%s",
- id.socket,
- id.die,
- config->csv_output ? 0 : -8,
- id.core,
- config->csv_sep,
- config->csv_output ? 0 : 4,
- nr,
- config->csv_sep);
- }
+ fprintf(output, "S%d-D%d-C%d%s%d%s",
+ id.socket, id.die, id.core, sep, nr, sep);
break;
case AGGR_DIE:
- if (config->json_output) {
- fprintf(config->output,
- "\"die\" : \"S%d-D%d\", \"aggregate-number\" : %d, ",
- id.socket,
- id.die,
- nr);
- } else {
- fprintf(config->output, "S%d-D%*d%s%*d%s",
- id.socket,
- config->csv_output ? 0 : -8,
- id.die,
- config->csv_sep,
- config->csv_output ? 0 : 4,
- nr,
- config->csv_sep);
- }
+ fprintf(output, "S%d-D%d%s%d%s",
+ id.socket, id.die, sep, nr, sep);
break;
case AGGR_SOCKET:
- if (config->json_output) {
- fprintf(config->output,
- "\"socket\" : \"S%d\", \"aggregate-number\" : %d, ",
- id.socket,
- nr);
- } else {
- fprintf(config->output, "S%*d%s%*d%s",
- config->csv_output ? 0 : -5,
- id.socket,
- config->csv_sep,
- config->csv_output ? 0 : 4,
- nr,
- config->csv_sep);
- }
+ fprintf(output, "S%d%s%d%s",
+ id.socket, sep, nr, sep);
break;
case AGGR_NODE:
- if (config->json_output) {
- fprintf(config->output, "\"node\" : \"N%d\", \"aggregate-number\" : %d, ",
- id.node,
- nr);
- } else {
- fprintf(config->output, "N%*d%s%*d%s",
- config->csv_output ? 0 : -5,
- id.node,
- config->csv_sep,
- config->csv_output ? 0 : 4,
- nr,
- config->csv_sep);
- }
+ fprintf(output, "N%d%s%d%s",
+ id.node, sep, nr, sep);
break;
case AGGR_NONE:
- if (config->json_output) {
- if (evsel->percore && !config->percore_show_thread) {
- fprintf(config->output, "\"core\" : \"S%d-D%d-C%d\"",
- id.socket,
- id.die,
- id.core);
- } else if (id.cpu.cpu > -1) {
- fprintf(config->output, "\"cpu\" : \"%d\", ",
- id.cpu.cpu);
- }
- } else {
- if (evsel->percore && !config->percore_show_thread) {
- fprintf(config->output, "S%d-D%d-C%*d%s",
- id.socket,
- id.die,
- config->csv_output ? 0 : -3,
- id.core, config->csv_sep);
- } else if (id.cpu.cpu > -1) {
- fprintf(config->output, "CPU%*d%s",
- config->csv_output ? 0 : -7,
- id.cpu.cpu, config->csv_sep);
- }
+ if (evsel->percore && !config->percore_show_thread) {
+ fprintf(output, "S%d-D%d-C%d%s",
+ id.socket, id.die, id.core, sep);
+ } else if (id.cpu.cpu > -1) {
+ fprintf(output, "CPU%d%s",
+ id.cpu.cpu, sep);
}
break;
case AGGR_THREAD:
- if (config->json_output) {
- fprintf(config->output, "\"thread\" : \"%s-%d\", ",
- perf_thread_map__comm(evsel->core.threads, id.thread_idx),
- perf_thread_map__pid(evsel->core.threads, id.thread_idx));
- } else {
- fprintf(config->output, "%*s-%*d%s",
- config->csv_output ? 0 : 16,
- perf_thread_map__comm(evsel->core.threads, id.thread_idx),
- config->csv_output ? 0 : -8,
- perf_thread_map__pid(evsel->core.threads, id.thread_idx),
- config->csv_sep);
+ fprintf(output, "%s-%d%s",
+ perf_thread_map__comm(evsel->core.threads, id.thread_idx),
+ perf_thread_map__pid(evsel->core.threads, id.thread_idx),
+ sep);
+ break;
+ case AGGR_GLOBAL:
+ case AGGR_UNSET:
+ case AGGR_MAX:
+ default:
+ break;
+ }
+}
+
+static void print_aggr_id_json(struct perf_stat_config *config,
+ struct evsel *evsel, struct aggr_cpu_id id, int nr)
+{
+ FILE *output = config->output;
+
+ if (!config->interval)
+ fputc('{', output);
+
+ switch (config->aggr_mode) {
+ case AGGR_CORE:
+ fprintf(output, "\"core\" : \"S%d-D%d-C%d\", \"aggregate-number\" : %d, ",
+ id.socket, id.die, id.core, nr);
+ break;
+ case AGGR_DIE:
+ fprintf(output, "\"die\" : \"S%d-D%d\", \"aggregate-number\" : %d, ",
+ id.socket, id.die, nr);
+ break;
+ case AGGR_SOCKET:
+ fprintf(output, "\"socket\" : \"S%d\", \"aggregate-number\" : %d, ",
+ id.socket, nr);
+ break;
+ case AGGR_NODE:
+ fprintf(output, "\"node\" : \"N%d\", \"aggregate-number\" : %d, ",
+ id.node, nr);
+ break;
+ case AGGR_NONE:
+ if (evsel->percore && !config->percore_show_thread) {
+ fprintf(output, "\"core\" : \"S%d-D%d-C%d\"",
+ id.socket, id.die, id.core);
+ } else if (id.cpu.cpu > -1) {
+ fprintf(output, "\"cpu\" : \"%d\", ",
+ id.cpu.cpu);
}
break;
+ case AGGR_THREAD:
+ fprintf(output, "\"thread\" : \"%s-%d\", ",
+ perf_thread_map__comm(evsel->core.threads, id.thread_idx),
+ perf_thread_map__pid(evsel->core.threads, id.thread_idx));
+ break;
case AGGR_GLOBAL:
case AGGR_UNSET:
case AGGR_MAX:
@@ -261,6 +272,17 @@ static void aggr_printout(struct perf_stat_config *config,
}
}

+static void aggr_printout(struct perf_stat_config *config,
+ struct evsel *evsel, struct aggr_cpu_id id, int nr)
+{
+ if (config->json_output)
+ print_aggr_id_json(config, evsel, id, nr);
+ else if (config->csv_output)
+ print_aggr_id_csv(config, evsel, id, nr);
+ else
+ print_aggr_id_std(config, evsel, id, nr);
+}
+
struct outstate {
FILE *fh;
bool newline;
--
2.38.1.493.g58b659f92b-goog


2022-11-15 14:12:51

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCHSET 00/19] perf stat: Improve perf stat output (v1)

Em Mon, Nov 14, 2022 at 03:02:08PM -0800, Namhyung Kim escreveu:
> Hello,
>
> I'm working on cleanup up the perf stat code. The main focus this time
> is the display logic which has various combinations of options.
>
> I split the code for each output mode - std, csv and json. And then
> organize them according to the purpose like header, prefix, value,
> metric and footer. I hope this would help maintaining the code a bit
> more.
>
> Also I found that cgroup support is missing or insufficient.
> Specifically when --for-each-cgroup option is given, it'd have multiple
> copies of the events for those cgroups. Then the output should group
> the result. This is true for the normal output mode, but the metric-
> only mode didn't support it well.
>
> With this change, I can see the per-cgroup topdown metrics like below:
>
> $ sudo ./perf stat -a --topdown --for-each-cgroup user.slice,system.slice sleep 3
> nmi_watchdog enabled with topdown. May give wrong results.
> Disable with echo 0 > /proc/sys/kernel/nmi_watchdog
>
> Performance counter stats for 'system wide':
>
> retiring bad speculation frontend bound backend bound
> S0-D0-C0 2 user.slice 117.3% 3.9% 47.8% -69.1%
> S0-D0-C1 2 user.slice 119.8% 4.1% 49.3% -73.2%
> S0-D0-C2 2 user.slice 24.4% 7.9% 68.4% 0.0%
> S0-D0-C3 2 user.slice 24.0% 9.2% 91.2% -24.4%
> S0-D0-C0 2 system.slice 73.5% 4.0% 19.4% 3.1%
> S0-D0-C1 2 system.slice 90.0% 5.8% 19.7% -15.5%
> S0-D0-C2 2 system.slice 101.2% 6.6% 33.4% -41.1%
> S0-D0-C3 2 system.slice 90.7% 4.9% 22.3% -18.0%
>
> 3.001678216 seconds time elapsed
>
> You can get it from 'perf/stat-display-v1' branch in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

applied locally, build testing.

- Arnaldo

> Thanks,
> Namhyung
>
> Namhyung Kim (19):
> perf stat: Clear screen only if output file is a tty
> perf stat: Split print_running() function
> perf stat: Split print_noise_pct() function
> perf stat: Split print_cgroup() function
> perf stat: Split aggr_printout() function
> perf stat: Factor out print_counter_value() function
> perf stat: Handle bad events in abs_printout()
> perf stat: Add before_metric argument
> perf stat: Align cgroup names
> perf stat: Split print_metric_headers() function
> perf stat: Factor out prepare_interval()
> perf stat: Cleanup interval print alignment
> perf stat: Remove impossible condition
> perf stat: Rework header display
> perf stat: Move condition to print_footer()
> perf stat: Factor out prefix display
> perf stat: Factor out print_metric_{begin,end}()
> perf stat: Support --for-each-cgroup and --metric-only
> perf stat: Add print_aggr_cgroup() for --for-each-cgroup and --topdown
>
> tools/perf/builtin-stat.c | 8 +
> tools/perf/util/stat-display.c | 996 ++++++++++++++++++++-------------
> 2 files changed, 624 insertions(+), 380 deletions(-)
>
>
> base-commit: 7565f9617efac0c0c8e2dbd08dbe0695d56684f5
> --
> 2.38.1.493.g58b659f92b-goog

--

- Arnaldo