Hello,
This is a continuation of my perf stat cleanup work focusing on the
metric-only mode with JSON output now. It's based on the previous
patchset which is already merged to acme/perf/core.
The JSON output + metric-only with aggregation has been broken for a
while. The last update fixed the crash but it still produced invalid
JSON objects. Also having a separate header and objects with their own
"metric-value" key is not convenient to process.
# perf stat -aj --metric-only --per-socket sleep 1
{"unit" : "GHz"}{"unit" : "insn per cycle"}{"unit" : "branch-misses of all branches"}
{"socket" : "S0", "aggregate-number" : 8, {"metric-value" : "0.809"}{"metric-value" : "2.10"}{"metric-value" : "0.37"}
So I removed the header and move the metric keys to the object. Then
it doesn't need separate objects anymore. The new output looks like:
# perf stat -aj --metric-only --per-socket sleep 1
{"socket" : "S0", "cpu-count" : 8, "GHz" : "0.817", "insn per cycle" : "2.15", "branch-misses of all branches" : "0.38"}
You can get it from 'perf/stat-display-v2' branch in
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (15):
perf stat: Fix cgroup display in JSON output
perf stat: Move summary prefix printing logic in CSV output
perf stat: Do not align time prefix in CSV output
perf stat: Use scnprintf() in prepare_interval()
perf stat: Remove prefix argument in print_metric_headers()
perf stat: Remove metric_only argument in print_counter_aggrdata()
perf stat: Pass const char *prefix to display routines
perf stat: Use struct outstate in evlist__print_counters()
perf stat: Pass struct outstate to print_metric_begin()
perf stat: Pass struct outstate to printout()
perf stat: Do not pass runtime_stat to printout()
perf stat: Pass through struct outstate
perf stat: Fix JSON output in metric-only mode
perf stat: Rename "aggregate-number" to "cpu-count" in JSON
perf stat: Tidy up JSON metric-only output when no metrics
tools/perf/arch/x86/util/iostat.c | 4 +-
tools/perf/util/iostat.c | 3 +-
tools/perf/util/iostat.h | 4 +-
tools/perf/util/stat-display.c | 257 +++++++++++++++---------------
4 files changed, 133 insertions(+), 135 deletions(-)
base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
--
2.38.1.584.g0f3c55d4c2-goog
The printout() takes a lot of arguments and sets an outstate with the
value. Instead, we can fill the outstate first and then pass it to
reduce the number of arguments.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 38 ++++++++++++++++------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 3ed63061d6f8..dd190f71e933 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -673,22 +673,15 @@ static bool is_mixed_hw_group(struct evsel *counter)
return false;
}
-static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int nr,
- struct evsel *counter, double uval,
- const char *prefix, u64 run, u64 ena, double noise,
+static void printout(struct perf_stat_config *config, struct outstate *os,
+ double uval, u64 run, u64 ena, double noise,
struct runtime_stat *st, int map_idx)
{
struct perf_stat_output_ctx out;
- struct outstate os = {
- .fh = config->output,
- .prefix = prefix ? prefix : "",
- .id = id,
- .nr = nr,
- .evsel = counter,
- };
print_metric_t pm;
new_line_t nl;
bool ok = true;
+ struct evsel *counter = os->evsel;
if (config->csv_output) {
static const int aggr_fields[AGGR_MAX] = {
@@ -704,7 +697,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
pm = config->metric_only ? print_metric_only_csv : print_metric_csv;
nl = config->metric_only ? new_line_metric : new_line_csv;
- os.nfields = 3 + aggr_fields[config->aggr_mode] + (counter->cgrp ? 1 : 0);
+ os->nfields = 3 + aggr_fields[config->aggr_mode] + (counter->cgrp ? 1 : 0);
} else if (config->json_output) {
pm = config->metric_only ? print_metric_only_json : print_metric_json;
nl = config->metric_only ? new_line_metric : new_line_json;
@@ -715,7 +708,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
if (run == 0 || ena == 0 || counter->counts->scaled == -1) {
if (config->metric_only) {
- pm(config, &os, NULL, "", "", 0);
+ pm(config, os, NULL, "", "", 0);
return;
}
@@ -732,11 +725,11 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
out.print_metric = pm;
out.new_line = nl;
- out.ctx = &os;
+ out.ctx = os;
out.force_header = false;
if (!config->metric_only) {
- abs_printout(config, id, nr, counter, uval, ok);
+ abs_printout(config, os->id, os->nr, counter, uval, ok);
print_noise(config, counter, noise, /*before_metric=*/true);
print_running(config, run, ena, /*before_metric=*/true);
@@ -814,6 +807,13 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
struct aggr_cpu_id id = config->aggr_map->map[s];
double avg = aggr->counts.val;
bool metric_only = config->metric_only;
+ struct outstate os = {
+ .fh = config->output,
+ .prefix = prefix ? prefix : "",
+ .id = id,
+ .nr = aggr->nr,
+ .evsel = counter,
+ };
if (counter->supported && aggr->nr == 0)
return;
@@ -834,8 +834,7 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
uval = val * counter->scale;
- printout(config, id, aggr->nr, counter, uval,
- prefix, run, ena, avg, &rt_stat, s);
+ printout(config, &os, uval, run, ena, avg, &rt_stat, s);
if (!metric_only)
fputc('\n', output);
@@ -971,14 +970,14 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
evlist__for_each_entry(evlist, counter) {
u64 ena, run, val;
double uval;
- struct aggr_cpu_id id;
struct perf_stat_evsel *ps = counter->stats;
int counter_idx = perf_cpu_map__idx(evsel__cpus(counter), cpu);
if (counter_idx < 0)
continue;
- id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
+ os.evsel = counter;
+ os.id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
if (first) {
print_metric_begin(config, evlist, &os, counter_idx);
first = false;
@@ -988,8 +987,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
run = ps->aggr[counter_idx].counts.run;
uval = val * counter->scale;
- printout(config, id, 0, counter, uval, prefix,
- run, ena, 1.0, &rt_stat, counter_idx);
+ printout(config, &os, uval, run, ena, 1.0, &rt_stat, counter_idx);
}
if (!first)
print_metric_end(config);
--
2.38.1.584.g0f3c55d4c2-goog
It missed the 'else' keyword after checking json output mode.
Fixes: 41cb875242e7 ("perf stat: Split print_cgroup() function")
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 f5501760ff2e..46e90f0bb423 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -175,7 +175,7 @@ static void print_cgroup(struct perf_stat_config *config, struct cgroup *cgrp)
if (config->json_output)
print_cgroup_json(config, cgrp_name);
- if (config->csv_output)
+ else if (config->csv_output)
print_cgroup_csv(config, cgrp_name);
else
print_cgroup_std(config, cgrp_name);
--
2.38.1.584.g0f3c55d4c2-goog
It always passes a pointer to rt_stat as it's the only one. Let's not
pass it and directly refer it in the printout().
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/stat-display.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index dd190f71e933..cdf4ca7f6e3a 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -674,8 +674,7 @@ static bool is_mixed_hw_group(struct evsel *counter)
}
static void printout(struct perf_stat_config *config, struct outstate *os,
- double uval, u64 run, u64 ena, double noise,
- struct runtime_stat *st, int map_idx)
+ double uval, u64 run, u64 ena, double noise, int map_idx)
{
struct perf_stat_output_ctx out;
print_metric_t pm;
@@ -737,7 +736,7 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
if (ok) {
perf_stat__print_shadow_stats(config, counter, uval, map_idx,
- &out, &config->metric_events, st);
+ &out, &config->metric_events, &rt_stat);
} else {
pm(config, &os, /*color=*/NULL, /*format=*/NULL, /*unit=*/"", /*val=*/0);
}
@@ -834,7 +833,7 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
uval = val * counter->scale;
- printout(config, &os, uval, run, ena, avg, &rt_stat, s);
+ printout(config, &os, uval, run, ena, avg, s);
if (!metric_only)
fputc('\n', output);
@@ -987,7 +986,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
run = ps->aggr[counter_idx].counts.run;
uval = val * counter->scale;
- printout(config, &os, uval, run, ena, 1.0, &rt_stat, counter_idx);
+ printout(config, &os, uval, run, ena, 1.0, counter_idx);
}
if (!first)
print_metric_end(config);
--
2.38.1.584.g0f3c55d4c2-goog
On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <[email protected]> wrote:
>
> The printout() takes a lot of arguments and sets an outstate with the
> value. Instead, we can fill the outstate first and then pass it to
> reduce the number of arguments.
>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> tools/perf/util/stat-display.c | 38 ++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 3ed63061d6f8..dd190f71e933 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -673,22 +673,15 @@ static bool is_mixed_hw_group(struct evsel *counter)
> return false;
> }
>
> -static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int nr,
> - struct evsel *counter, double uval,
> - const char *prefix, u64 run, u64 ena, double noise,
> +static void printout(struct perf_stat_config *config, struct outstate *os,
> + double uval, u64 run, u64 ena, double noise,
> struct runtime_stat *st, int map_idx)
> {
> struct perf_stat_output_ctx out;
> - struct outstate os = {
> - .fh = config->output,
> - .prefix = prefix ? prefix : "",
> - .id = id,
> - .nr = nr,
> - .evsel = counter,
> - };
> print_metric_t pm;
> new_line_t nl;
> bool ok = true;
> + struct evsel *counter = os->evsel;
>
> if (config->csv_output) {
> static const int aggr_fields[AGGR_MAX] = {
> @@ -704,7 +697,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
>
> pm = config->metric_only ? print_metric_only_csv : print_metric_csv;
> nl = config->metric_only ? new_line_metric : new_line_csv;
> - os.nfields = 3 + aggr_fields[config->aggr_mode] + (counter->cgrp ? 1 : 0);
> + os->nfields = 3 + aggr_fields[config->aggr_mode] + (counter->cgrp ? 1 : 0);
> } else if (config->json_output) {
> pm = config->metric_only ? print_metric_only_json : print_metric_json;
> nl = config->metric_only ? new_line_metric : new_line_json;
> @@ -715,7 +708,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
>
> if (run == 0 || ena == 0 || counter->counts->scaled == -1) {
> if (config->metric_only) {
> - pm(config, &os, NULL, "", "", 0);
> + pm(config, os, NULL, "", "", 0);
> return;
> }
>
> @@ -732,11 +725,11 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
>
> out.print_metric = pm;
> out.new_line = nl;
> - out.ctx = &os;
> + out.ctx = os;
> out.force_header = false;
>
> if (!config->metric_only) {
> - abs_printout(config, id, nr, counter, uval, ok);
> + abs_printout(config, os->id, os->nr, counter, uval, ok);
>
> print_noise(config, counter, noise, /*before_metric=*/true);
> print_running(config, run, ena, /*before_metric=*/true);
> @@ -814,6 +807,13 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> struct aggr_cpu_id id = config->aggr_map->map[s];
> double avg = aggr->counts.val;
> bool metric_only = config->metric_only;
> + struct outstate os = {
> + .fh = config->output,
> + .prefix = prefix ? prefix : "",
> + .id = id,
> + .nr = aggr->nr,
> + .evsel = counter,
> + };
>
> if (counter->supported && aggr->nr == 0)
> return;
> @@ -834,8 +834,7 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
>
> uval = val * counter->scale;
>
> - printout(config, id, aggr->nr, counter, uval,
> - prefix, run, ena, avg, &rt_stat, s);
> + printout(config, &os, uval, run, ena, avg, &rt_stat, s);
>
> if (!metric_only)
> fputc('\n', output);
> @@ -971,14 +970,14 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> evlist__for_each_entry(evlist, counter) {
> u64 ena, run, val;
> double uval;
> - struct aggr_cpu_id id;
> struct perf_stat_evsel *ps = counter->stats;
> int counter_idx = perf_cpu_map__idx(evsel__cpus(counter), cpu);
>
> if (counter_idx < 0)
> continue;
>
> - id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
> + os.evsel = counter;
> + os.id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
> if (first) {
> print_metric_begin(config, evlist, &os, counter_idx);
> first = false;
> @@ -988,8 +987,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> run = ps->aggr[counter_idx].counts.run;
>
> uval = val * counter->scale;
> - printout(config, id, 0, counter, uval, prefix,
> - run, ena, 1.0, &rt_stat, counter_idx);
> + printout(config, &os, uval, run, ena, 1.0, &rt_stat, counter_idx);
> }
> if (!first)
> print_metric_end(config);
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <[email protected]> wrote:
>
> It missed the 'else' keyword after checking json output mode.
>
> Fixes: 41cb875242e7 ("perf stat: Split print_cgroup() function")
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> 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 f5501760ff2e..46e90f0bb423 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -175,7 +175,7 @@ static void print_cgroup(struct perf_stat_config *config, struct cgroup *cgrp)
>
> if (config->json_output)
> print_cgroup_json(config, cgrp_name);
> - if (config->csv_output)
> + else if (config->csv_output)
> print_cgroup_csv(config, cgrp_name);
> else
> print_cgroup_std(config, cgrp_name);
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <[email protected]> wrote:
>
> It always passes a pointer to rt_stat as it's the only one. Let's not
> pass it and directly refer it in the printout().
>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> tools/perf/util/stat-display.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index dd190f71e933..cdf4ca7f6e3a 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -674,8 +674,7 @@ static bool is_mixed_hw_group(struct evsel *counter)
> }
>
> static void printout(struct perf_stat_config *config, struct outstate *os,
> - double uval, u64 run, u64 ena, double noise,
> - struct runtime_stat *st, int map_idx)
> + double uval, u64 run, u64 ena, double noise, int map_idx)
> {
> struct perf_stat_output_ctx out;
> print_metric_t pm;
> @@ -737,7 +736,7 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
>
> if (ok) {
> perf_stat__print_shadow_stats(config, counter, uval, map_idx,
> - &out, &config->metric_events, st);
> + &out, &config->metric_events, &rt_stat);
> } else {
> pm(config, &os, /*color=*/NULL, /*format=*/NULL, /*unit=*/"", /*val=*/0);
> }
> @@ -834,7 +833,7 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
>
> uval = val * counter->scale;
>
> - printout(config, &os, uval, run, ena, avg, &rt_stat, s);
> + printout(config, &os, uval, run, ena, avg, s);
>
> if (!metric_only)
> fputc('\n', output);
> @@ -987,7 +986,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> run = ps->aggr[counter_idx].counts.run;
>
> uval = val * counter->scale;
> - printout(config, &os, uval, run, ena, 1.0, &rt_stat, counter_idx);
> + printout(config, &os, uval, run, ena, 1.0, counter_idx);
> }
> if (!first)
> print_metric_end(config);
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
Em Wed, Nov 23, 2022 at 03:20:12PM -0800, Ian Rogers escreveu:
> On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <[email protected]> wrote:
> >
> > It missed the 'else' keyword after checking json output mode.
> >
> > Fixes: 41cb875242e7 ("perf stat: Split print_cgroup() function")
> > Signed-off-by: Namhyung Kim <[email protected]>
>
> Acked-by: Ian Rogers <[email protected]>
Thanks, applied the series.
- Arnaldo