[PATCH 0/4] perf report: Omit dummy events in the output (v3)

[PATCH 0/4] perf report: Omit dummy events in the output (v3)

[Namhyung Kim]

Hello,

This work is to make the output compact by removing dummy events in
the output.  The dummy events are used to save side-band information
like task creation or memory address space change using mmap(2).  But
after collecting these, it's not used because it won't have any
samples.

 v2 changes)
 * just hide the (dummy) event instead of removing it from evlist

 v3 changes)
 * rename to struct hpp_fmt_value  (Arnaldo)
 * add Tested-by from Arnaldo
 
Sometimes users want to run perf report --group to show all recorded
events together but they are not interested in those dummy events.
This just wastes the precious screen space so we want to get rid of
them in the output.

perf report already has --skip-empty option to skip 0 result in the
stat output.  I think we can extend it to skip empty events that have
no samples.

Example output:

  $ sudo perf mem record -a sleep 1
  $ sudo perf report --group

Before)
  #
  # Samples: 232  of events 'cpu/mem-loads,ldlat=30/P, cpu/mem-stores/P, dummy:u'
  # Event count (approx.): 3089861
  #
  #                 Overhead  Command      Shared Object      Symbol                               
  # ........................  ...........  .................  .....................................
  #
       9.29%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] update_blocked_averages
       5.26%   0.15%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_se
       4.15%   0.00%   0.00%  perf-exec    [kernel.kallsyms]  [k] slab_update_freelist.isra.0
       3.87%   0.00%   0.00%  perf-exec    [kernel.kallsyms]  [k] memcg_slab_post_alloc_hook
       3.79%   0.17%   0.00%  swapper      [kernel.kallsyms]  [k] enqueue_task_fair
       3.63%   0.00%   0.00%  sleep        [kernel.kallsyms]  [k] next_uptodate_page
       2.86%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_cfs_rq
       2.78%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] __schedule
       2.34%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] intel_idle
       2.32%   0.97%   0.00%  swapper      [kernel.kallsyms]  [k] psi_group_change

After)
  #
  # Samples: 232  of events 'cpu/mem-loads,ldlat=30/P, cpu/mem-stores/P'
  # Event count (approx.): 3089861
  #
  #         Overhead  Command      Shared Object      Symbol                               
  # ................  ...........  .................  .....................................
  #
       9.29%   0.00%  swapper      [kernel.kallsyms]  [k] update_blocked_averages
       5.26%   0.15%  swapper      [kernel.kallsyms]  [k] __update_load_avg_se
       4.15%   0.00%  perf-exec    [kernel.kallsyms]  [k] slab_update_freelist.isra.0
       3.87%   0.00%  perf-exec    [kernel.kallsyms]  [k] memcg_slab_post_alloc_hook
       3.79%   0.17%  swapper      [kernel.kallsyms]  [k] enqueue_task_fair
       3.63%   0.00%  sleep        [kernel.kallsyms]  [k] next_uptodate_page
       2.86%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_cfs_rq
       2.78%   0.00%  swapper      [kernel.kallsyms]  [k] __schedule
       2.34%   0.00%  swapper      [kernel.kallsyms]  [k] intel_idle
       2.32%   0.97%  swapper      [kernel.kallsyms]  [k] psi_group_change

Now 'Overhead' column only has two values for mem-loads and mem-stores.

Thanks,
Namhyung


Namhyung Kim (4):
  perf hist: Factor out __hpp__fmt_print()
  perf hist: Simplify __hpp_fmt() using hpp_fmt_data
  perf hist: Add symbol_conf.skip_empty
  perf hist: Honor symbol_conf.skip_empty

 tools/perf/builtin-annotate.c  |   4 +-
 tools/perf/builtin-report.c    |  12 +--
 tools/perf/ui/hist.c           | 144 ++++++++++++++++-----------------
 tools/perf/ui/stdio/hist.c     |   5 +-
 tools/perf/util/events_stats.h |   3 +-
 tools/perf/util/evsel.c        |  13 ++-
 tools/perf/util/hist.c         |   6 +-
 tools/perf/util/hist.h         |   3 +-
 tools/perf/util/python.c       |   3 +
 tools/perf/util/session.c      |   5 +-
 tools/perf/util/session.h      |   3 +-
 tools/perf/util/symbol_conf.h  |   3 +-
 12 files changed, 105 insertions(+), 99 deletions(-)

-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH 1/4] perf hist: Factor out __hpp__fmt_print()

[Namhyung Kim]

Split the logic to print the histogram values according to the format
string.  This was used in 3 different places so it's better to move out
the logic into a function.

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c | 83 +++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 47 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 685ba2a54fd8..e30fcb1e87e7 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -23,35 +23,42 @@
 	__ret;							\
 })
 
-static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-		      hpp_field_fn get_field, const char *fmt, int len,
-		      hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
+static int __hpp__fmt_print(struct perf_hpp *hpp, struct hists *hists, u64 val,
+			    int nr_samples, const char *fmt, int len,
+			    hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
 {
-	int ret;
-	struct hists *hists = he->hists;
-	struct evsel *evsel = hists_to_evsel(hists);
-	char *buf = hpp->buf;
-	size_t size = hpp->size;
-
 	if (fmtype == PERF_HPP_FMT_TYPE__PERCENT) {
 		double percent = 0.0;
 		u64 total = hists__total_period(hists);
 
 		if (total)
-			percent = 100.0 * get_field(he) / total;
+			percent = 100.0 * val / total;
 
-		ret = hpp__call_print_fn(hpp, print_fn, fmt, len, percent);
-	} else if (fmtype == PERF_HPP_FMT_TYPE__AVERAGE) {
-		double average = 0;
+		return hpp__call_print_fn(hpp, print_fn, fmt, len, percent);
+	}
 
-		if (he->stat.nr_events)
-			average = 1.0 * get_field(he) / he->stat.nr_events;
+	if (fmtype == PERF_HPP_FMT_TYPE__AVERAGE) {
+		double avg = nr_samples ? (1.0 * val / nr_samples) : 0;
 
-		ret = hpp__call_print_fn(hpp, print_fn, fmt, len, average);
-	} else {
-		ret = hpp__call_print_fn(hpp, print_fn, fmt, len, get_field(he));
+		return hpp__call_print_fn(hpp, print_fn, fmt, len, avg);
 	}
 
+	return hpp__call_print_fn(hpp, print_fn, fmt, len, val);
+}
+
+static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
+		      hpp_field_fn get_field, const char *fmt, int len,
+		      hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
+{
+	int ret;
+	struct hists *hists = he->hists;
+	struct evsel *evsel = hists_to_evsel(hists);
+	char *buf = hpp->buf;
+	size_t size = hpp->size;
+
+	ret = __hpp__fmt_print(hpp, hists, get_field(he), he->stat.nr_events,
+			       fmt, len, print_fn, fmtype);
+
 	if (evsel__is_group_event(evsel)) {
 		int prev_idx, idx_delta;
 		struct hist_entry *pair;
@@ -72,30 +79,16 @@ static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 
 			while (idx_delta--) {
 				/*
-				 * zero-fill group members in the middle which
-				 * have no sample
+				 * zero-fill group members in the middle which have
+				 * no samples, pair->hists is not correct but it's
+				 * fine since the value is 0.
 				 */
-				if (fmtype != PERF_HPP_FMT_TYPE__RAW) {
-					ret += hpp__call_print_fn(hpp, print_fn,
-								  fmt, len, 0.0);
-				} else {
-					ret += hpp__call_print_fn(hpp, print_fn,
-								  fmt, len, 0ULL);
-				}
+				ret += __hpp__fmt_print(hpp, pair->hists, 0, 0,
+							fmt, len, print_fn, fmtype);
 			}
 
-			if (fmtype == PERF_HPP_FMT_TYPE__PERCENT) {
-				ret += hpp__call_print_fn(hpp, print_fn, fmt, len,
-							  100.0 * period / total);
-			} else if (fmtype == PERF_HPP_FMT_TYPE__AVERAGE) {
-				double avg = nr_samples ? (period / nr_samples) : 0;
-
-				ret += hpp__call_print_fn(hpp, print_fn, fmt,
-							  len, avg);
-			} else {
-				ret += hpp__call_print_fn(hpp, print_fn, fmt,
-							  len, period);
-			}
+			ret += __hpp__fmt_print(hpp, pair->hists, period, nr_samples,
+						fmt, len, print_fn, fmtype);
 
 			prev_idx = evsel__group_idx(evsel);
 		}
@@ -104,15 +97,11 @@ static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 
 		while (idx_delta--) {
 			/*
-			 * zero-fill group members at last which have no sample
+			 * zero-fill group members at last which have no sample.
+			 * the hists is not correct but it's fine like above.
 			 */
-			if (fmtype != PERF_HPP_FMT_TYPE__RAW) {
-				ret += hpp__call_print_fn(hpp, print_fn,
-							  fmt, len, 0.0);
-			} else {
-				ret += hpp__call_print_fn(hpp, print_fn,
-							  fmt, len, 0ULL);
-			}
+			ret += __hpp__fmt_print(hpp, evsel__hists(evsel), 0, 0,
+						fmt, len, print_fn, fmtype);
 		}
 	}
 
-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH 2/4] perf hist: Simplify __hpp_fmt() using hpp_fmt_data

[Namhyung Kim]

The struct hpp_fmt_data is to keep the values for each group members so
it doesn't need to check the event index in the group.

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c | 75 +++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 39 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index e30fcb1e87e7..7e863cd92781 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -46,65 +46,62 @@ static int __hpp__fmt_print(struct perf_hpp *hpp, struct hists *hists, u64 val,
 	return hpp__call_print_fn(hpp, print_fn, fmt, len, val);
 }
 
+struct hpp_fmt_value {
+	struct hists *hists;
+	u64 val;
+	int samples;
+};
+
 static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 		      hpp_field_fn get_field, const char *fmt, int len,
 		      hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
 {
-	int ret;
+	int ret = 0;
 	struct hists *hists = he->hists;
 	struct evsel *evsel = hists_to_evsel(hists);
+	struct evsel *pos;
 	char *buf = hpp->buf;
 	size_t size = hpp->size;
+	int i, nr_members = 1;
+	struct hpp_fmt_value *values;
+
+	if (evsel__is_group_event(evsel))
+		nr_members = evsel->core.nr_members;
+
+	values = calloc(nr_members, sizeof(*values));
+	if (values == NULL)
+		return 0;
+
+	i = 0;
+	for_each_group_evsel(pos, evsel)
+		values[i++].hists = evsel__hists(pos);
 
-	ret = __hpp__fmt_print(hpp, hists, get_field(he), he->stat.nr_events,
-			       fmt, len, print_fn, fmtype);
+	values[0].val = get_field(he);
+	values[0].samples = he->stat.nr_events;
 
 	if (evsel__is_group_event(evsel)) {
-		int prev_idx, idx_delta;
 		struct hist_entry *pair;
-		int nr_members = evsel->core.nr_members;
-
-		prev_idx = evsel__group_idx(evsel);
 
 		list_for_each_entry(pair, &he->pairs.head, pairs.node) {
-			u64 period = get_field(pair);
-			u64 total = hists__total_period(pair->hists);
-			int nr_samples = pair->stat.nr_events;
-
-			if (!total)
-				continue;
+			for (i = 0; i < nr_members; i++) {
+				if (values[i].hists != pair->hists)
+					continue;
 
-			evsel = hists_to_evsel(pair->hists);
-			idx_delta = evsel__group_idx(evsel) - prev_idx - 1;
-
-			while (idx_delta--) {
-				/*
-				 * zero-fill group members in the middle which have
-				 * no samples, pair->hists is not correct but it's
-				 * fine since the value is 0.
-				 */
-				ret += __hpp__fmt_print(hpp, pair->hists, 0, 0,
-							fmt, len, print_fn, fmtype);
+				values[i].val = get_field(pair);
+				values[i].samples = pair->stat.nr_events;
+				break;
 			}
-
-			ret += __hpp__fmt_print(hpp, pair->hists, period, nr_samples,
-						fmt, len, print_fn, fmtype);
-
-			prev_idx = evsel__group_idx(evsel);
 		}
+	}
 
-		idx_delta = nr_members - prev_idx - 1;
-
-		while (idx_delta--) {
-			/*
-			 * zero-fill group members at last which have no sample.
-			 * the hists is not correct but it's fine like above.
-			 */
-			ret += __hpp__fmt_print(hpp, evsel__hists(evsel), 0, 0,
-						fmt, len, print_fn, fmtype);
-		}
+	for (i = 0; i < nr_members; i++) {
+		ret += __hpp__fmt_print(hpp, values[i].hists, values[i].val,
+					values[i].samples, fmt, len,
+					print_fn, fmtype);
 	}
 
+	free(values);
+
 	/*
 	 * Restore original buf and size as it's where caller expects
 	 * the result will be saved.
-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH 3/4] perf hist: Add symbol_conf.skip_empty

[Namhyung Kim]

Add the skip_empty flag to symbol_conf and set the value from the report
command to preserve the existing behavior.  This makes the code simpler
and will be needed other code which is hard to add a new argument.

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c  |  4 ++--
 tools/perf/builtin-report.c    | 12 ++++++------
 tools/perf/ui/stdio/hist.c     |  5 ++---
 tools/perf/util/events_stats.h |  3 +--
 tools/perf/util/hist.c         |  6 +++---
 tools/perf/util/hist.h         |  3 +--
 tools/perf/util/session.c      |  5 ++---
 tools/perf/util/session.h      |  3 +--
 tools/perf/util/symbol_conf.h  |  3 ++-
 9 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 50d2fb222d48..b10b7f005658 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -571,8 +571,8 @@ static int __cmd_annotate(struct perf_annotate *ann)
 		goto out;
 
 	if (dump_trace) {
-		perf_session__fprintf_nr_events(session, stdout, false);
-		evlist__fprintf_nr_events(session->evlist, stdout, false);
+		perf_session__fprintf_nr_events(session, stdout);
+		evlist__fprintf_nr_events(session->evlist, stdout);
 		goto out;
 	}
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 69618fb0110b..9718770facb5 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -810,8 +810,8 @@ static int stats_print(struct report *rep)
 {
 	struct perf_session *session = rep->session;
 
-	perf_session__fprintf_nr_events(session, stdout, rep->skip_empty);
-	evlist__fprintf_nr_events(session->evlist, stdout, rep->skip_empty);
+	perf_session__fprintf_nr_events(session, stdout);
+	evlist__fprintf_nr_events(session->evlist, stdout);
 	return 0;
 }
 
@@ -1089,10 +1089,8 @@ static int __cmd_report(struct report *rep)
 			perf_session__fprintf_dsos(session, stdout);
 
 		if (dump_trace) {
-			perf_session__fprintf_nr_events(session, stdout,
-							rep->skip_empty);
-			evlist__fprintf_nr_events(session->evlist, stdout,
-						  rep->skip_empty);
+			perf_session__fprintf_nr_events(session, stdout);
+			evlist__fprintf_nr_events(session->evlist, stdout);
 			return 0;
 		}
 	}
@@ -1562,6 +1560,8 @@ int cmd_report(int argc, const char **argv)
 	data.path  = input_name;
 	data.force = symbol_conf.force;
 
+	symbol_conf.skip_empty = report.skip_empty;
+
 repeat:
 	session = perf_session__new(&data, &report.tool);
 	if (IS_ERR(session)) {
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index b849caace398..9372e8904d22 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -897,8 +897,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 	return ret;
 }
 
-size_t events_stats__fprintf(struct events_stats *stats, FILE *fp,
-			     bool skip_empty)
+size_t events_stats__fprintf(struct events_stats *stats, FILE *fp)
 {
 	int i;
 	size_t ret = 0;
@@ -910,7 +909,7 @@ size_t events_stats__fprintf(struct events_stats *stats, FILE *fp,
 		name = perf_event__name(i);
 		if (!strcmp(name, "UNKNOWN"))
 			continue;
-		if (skip_empty && !stats->nr_events[i])
+		if (symbol_conf.skip_empty && !stats->nr_events[i])
 			continue;
 
 		if (i && total) {
diff --git a/tools/perf/util/events_stats.h b/tools/perf/util/events_stats.h
index 8fecc9fbaecc..f43e5b1a366a 100644
--- a/tools/perf/util/events_stats.h
+++ b/tools/perf/util/events_stats.h
@@ -52,7 +52,6 @@ struct hists_stats {
 
 void events_stats__inc(struct events_stats *stats, u32 type);
 
-size_t events_stats__fprintf(struct events_stats *stats, FILE *fp,
-			     bool skip_empty);
+size_t events_stats__fprintf(struct events_stats *stats, FILE *fp);
 
 #endif /* __PERF_EVENTS_STATS_ */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 2e9e193179dd..f028f113c4fd 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2706,8 +2706,7 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al,
 	}
 }
 
-size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp,
-				 bool skip_empty)
+size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp)
 {
 	struct evsel *pos;
 	size_t ret = 0;
@@ -2715,7 +2714,8 @@ size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp,
 	evlist__for_each_entry(evlist, pos) {
 		struct hists *hists = evsel__hists(pos);
 
-		if (skip_empty && !hists->stats.nr_samples && !hists->stats.nr_lost_samples)
+		if (symbol_conf.skip_empty && !hists->stats.nr_samples &&
+		    !hists->stats.nr_lost_samples)
 			continue;
 
 		ret += fprintf(fp, "%s stats:\n", evsel__name(pos));
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 8fb3bdd29188..5273f5c37050 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -375,8 +375,7 @@ void hists__inc_nr_lost_samples(struct hists *hists, u32 lost);
 size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		      int max_cols, float min_pcnt, FILE *fp,
 		      bool ignore_callchains);
-size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp,
-				 bool skip_empty);
+size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp);
 
 void hists__filter_by_dso(struct hists *hists);
 void hists__filter_by_thread(struct hists *hists);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index a10343b9dcd4..0ec92d47373c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2696,8 +2696,7 @@ size_t perf_session__fprintf_dsos_buildid(struct perf_session *session, FILE *fp
 	return machines__fprintf_dsos_buildid(&session->machines, fp, skip, parm);
 }
 
-size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp,
-				       bool skip_empty)
+size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp)
 {
 	size_t ret;
 	const char *msg = "";
@@ -2707,7 +2706,7 @@ size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp,
 
 	ret = fprintf(fp, "\nAggregated stats:%s\n", msg);
 
-	ret += events_stats__fprintf(&session->evlist->stats, fp, skip_empty);
+	ret += events_stats__fprintf(&session->evlist->stats, fp);
 	return ret;
 }
 
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 3b0256e977a6..4c29dc86956f 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -130,8 +130,7 @@ size_t perf_session__fprintf_dsos(struct perf_session *session, FILE *fp);
 size_t perf_session__fprintf_dsos_buildid(struct perf_session *session, FILE *fp,
 					  bool (fn)(struct dso *dso, int parm), int parm);
 
-size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp,
-				       bool skip_empty);
+size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp);
 
 void perf_session__dump_kmaps(struct perf_session *session);
 
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index c114bbceef40..657cfa5af43c 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -46,7 +46,8 @@ struct symbol_conf {
 			lazy_load_kernel_maps,
 			keep_exited_threads,
 			annotate_data_member,
-			annotate_data_sample;
+			annotate_data_sample,
+			skip_empty;
 	const char	*vmlinux_name,
 			*kallsyms_name,
 			*source_prefix,
-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH 4/4] perf hist: Honor symbol_conf.skip_empty

[Namhyung Kim]

So that it can skip events with no sample according to the config value.
This can omit the dummy event in the output of perf report --group.

An example output:

  $ sudo perf mem record -a sleep 1
  $ sudo perf report --group

Before)
  #
  # Samples: 232  of events 'cpu/mem-loads,ldlat=30/P, cpu/mem-stores/P, dummy:u'
  # Event count (approx.): 3089861
  #
  #                 Overhead  Command      Shared Object      Symbol
  # ........................  ...........  .................  .....................................
  #
       9.29%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] update_blocked_averages
       5.26%   0.15%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_se
       4.15%   0.00%   0.00%  perf-exec    [kernel.kallsyms]  [k] slab_update_freelist.isra.0
       3.87%   0.00%   0.00%  perf-exec    [kernel.kallsyms]  [k] memcg_slab_post_alloc_hook
       3.79%   0.17%   0.00%  swapper      [kernel.kallsyms]  [k] enqueue_task_fair
       3.63%   0.00%   0.00%  sleep        [kernel.kallsyms]  [k] next_uptodate_page
       2.86%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_cfs_rq
       2.78%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] __schedule
       2.34%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] intel_idle
       2.32%   0.97%   0.00%  swapper      [kernel.kallsyms]  [k] psi_group_change

After)
  #
  # Samples: 232  of events 'cpu/mem-loads,ldlat=30/P, cpu/mem-stores/P'
  # Event count (approx.): 3089861
  #
  #         Overhead  Command      Shared Object      Symbol
  # ................  ...........  .................  .....................................
  #
       9.29%   0.00%  swapper      [kernel.kallsyms]  [k] update_blocked_averages
       5.26%   0.15%  swapper      [kernel.kallsyms]  [k] __update_load_avg_se
       4.15%   0.00%  perf-exec    [kernel.kallsyms]  [k] slab_update_freelist.isra.0
       3.87%   0.00%  perf-exec    [kernel.kallsyms]  [k] memcg_slab_post_alloc_hook
       3.79%   0.17%  swapper      [kernel.kallsyms]  [k] enqueue_task_fair
       3.63%   0.00%  sleep        [kernel.kallsyms]  [k] next_uptodate_page
       2.86%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_cfs_rq
       2.78%   0.00%  swapper      [kernel.kallsyms]  [k] __schedule
       2.34%   0.00%  swapper      [kernel.kallsyms]  [k] intel_idle
       2.32%   0.97%  swapper      [kernel.kallsyms]  [k] psi_group_change

Now it doesn't have a column for the dummy event.

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c     | 18 ++++++++++++++++--
 tools/perf/util/evsel.c  | 13 ++++++++++---
 tools/perf/util/python.c |  3 +++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 7e863cd92781..5d1f04f66a5a 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -95,6 +95,10 @@ static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 	}
 
 	for (i = 0; i < nr_members; i++) {
+		if (symbol_conf.skip_empty &&
+		    values[i].hists->stats.nr_samples == 0)
+			continue;
+
 		ret += __hpp__fmt_print(hpp, values[i].hists, values[i].val,
 					values[i].samples, fmt, len,
 					print_fn, fmtype);
@@ -296,8 +300,18 @@ static int hpp__width_fn(struct perf_hpp_fmt *fmt,
 	int len = fmt->user_len ?: fmt->len;
 	struct evsel *evsel = hists_to_evsel(hists);
 
-	if (symbol_conf.event_group)
-		len = max(len, evsel->core.nr_members * fmt->len);
+	if (symbol_conf.event_group) {
+		int nr = 0;
+		struct evsel *pos;
+
+		for_each_group_evsel(pos, evsel) {
+			if (!symbol_conf.skip_empty ||
+			    evsel__hists(pos)->stats.nr_samples)
+				nr++;
+		}
+
+		len = max(len, nr * fmt->len);
+	}
 
 	if (len < (int)strlen(fmt->name))
 		len = strlen(fmt->name);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4f818ab6b662..befb80c272d2 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -53,6 +53,7 @@
 #include "../perf-sys.h"
 #include "util/parse-branch-options.h"
 #include "util/bpf-filter.h"
+#include "util/hist.h"
 #include <internal/xyarray.h>
 #include <internal/lib.h>
 #include <internal/threadmap.h>
@@ -830,16 +831,22 @@ const char *evsel__group_name(struct evsel *evsel)
 int evsel__group_desc(struct evsel *evsel, char *buf, size_t size)
 {
 	int ret = 0;
+	bool first = true;
 	struct evsel *pos;
 	const char *group_name = evsel__group_name(evsel);
 
 	if (!evsel->forced_leader)
 		ret = scnprintf(buf, size, "%s { ", group_name);
 
-	ret += scnprintf(buf + ret, size - ret, "%s", evsel__name(evsel));
+	for_each_group_evsel(pos, evsel) {
+		if (symbol_conf.skip_empty &&
+		    evsel__hists(pos)->stats.nr_samples == 0)
+			continue;
 
-	for_each_group_member(pos, evsel)
-		ret += scnprintf(buf + ret, size - ret, ", %s", evsel__name(pos));
+		ret += scnprintf(buf + ret, size - ret, "%s%s",
+				 first ? "" : ", ", evsel__name(pos));
+		first = false;
+	}
 
 	if (!evsel->forced_leader)
 		ret += scnprintf(buf + ret, size - ret, " }");
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 0aeb97c11c03..88f98f2772fb 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -23,6 +23,7 @@
 #include "util/env.h"
 #include "util/pmu.h"
 #include "util/pmus.h"
+#include "util/symbol_conf.h"
 #include <internal/lib.h>
 #include "util.h"
 
@@ -50,6 +51,8 @@
 #define Py_TYPE(ob) (((PyObject*)(ob))->ob_type)
 #endif
 
+struct symbol_conf symbol_conf;
+
 /*
  * Avoid bringing in event parsing.
  */
-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH 0/4] perf report: Omit dummy events in the output (v3)

[Namhyung Kim]

On Fri, 07 Jun 2024 13:29:14 -0700, Namhyung Kim wrote:

> This work is to make the output compact by removing dummy events in
> the output.  The dummy events are used to save side-band information
> like task creation or memory address space change using mmap(2).  But
> after collecting these, it's not used because it won't have any
> samples.
> 
>  v2 changes)
>  * just hide the (dummy) event instead of removing it from evlist
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
Namhyung

[PATCH 0/4] perf report: Omit dummy events in the output (v2)

[Namhyung Kim]

Hello,

This work is to make the output compact by removing dummy events in
the output.  The dummy events are used to save side-band information
like task creation or memory address space change using mmap(2).  But
after collecting these, it's not used because it won't have any
samples.

 v2 changes)
 * just hide the (dummy) event instead of removing it from evlist

Sometimes users want to run perf report --group to show all recorded
events together but they are not interested in those dummy events.
This just wastes the precious screen space so we want to get rid of
them after use.

perf report already has --skip-empty option to skip 0 result in the
stat output.  I think we can extend it to skip empty events that have
no samples.

Example output:

Before)
  #
  # Samples: 232  of events 'cpu/mem-loads,ldlat=30/P, cpu/mem-stores/P, dummy:u'
  # Event count (approx.): 3089861
  #
  #                 Overhead  Command      Shared Object      Symbol                               
  # ........................  ...........  .................  .....................................
  #
       9.29%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] update_blocked_averages
       5.26%   0.15%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_se
       4.15%   0.00%   0.00%  perf-exec    [kernel.kallsyms]  [k] slab_update_freelist.isra.0
       3.87%   0.00%   0.00%  perf-exec    [kernel.kallsyms]  [k] memcg_slab_post_alloc_hook
       3.79%   0.17%   0.00%  swapper      [kernel.kallsyms]  [k] enqueue_task_fair
       3.63%   0.00%   0.00%  sleep        [kernel.kallsyms]  [k] next_uptodate_page
       2.86%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_cfs_rq
       2.78%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] __schedule
       2.34%   0.00%   0.00%  swapper      [kernel.kallsyms]  [k] intel_idle
       2.32%   0.97%   0.00%  swapper      [kernel.kallsyms]  [k] psi_group_change

After)
  #
  # Samples: 232  of events 'cpu/mem-loads,ldlat=30/P, cpu/mem-stores/P'
  # Event count (approx.): 3089861
  #
  #         Overhead  Command      Shared Object      Symbol                               
  # ................  ...........  .................  .....................................
  #
       9.29%   0.00%  swapper      [kernel.kallsyms]  [k] update_blocked_averages
       5.26%   0.15%  swapper      [kernel.kallsyms]  [k] __update_load_avg_se
       4.15%   0.00%  perf-exec    [kernel.kallsyms]  [k] slab_update_freelist.isra.0
       3.87%   0.00%  perf-exec    [kernel.kallsyms]  [k] memcg_slab_post_alloc_hook
       3.79%   0.17%  swapper      [kernel.kallsyms]  [k] enqueue_task_fair
       3.63%   0.00%  sleep        [kernel.kallsyms]  [k] next_uptodate_page
       2.86%   0.00%  swapper      [kernel.kallsyms]  [k] __update_load_avg_cfs_rq
       2.78%   0.00%  swapper      [kernel.kallsyms]  [k] __schedule
       2.34%   0.00%  swapper      [kernel.kallsyms]  [k] intel_idle
       2.32%   0.97%  swapper      [kernel.kallsyms]  [k] psi_group_change

Now 'Overhead' column only has two values for mem-loads and mem-stores.

Thanks,
Namhyung


Namhyung Kim (4):
  perf hist: Factor out __hpp__fmt_print()
  perf hist: Simplify __hpp_fmt() using hpp_fmt_data
  perf hist: Add symbol_conf.skip_empty
  perf hist: Honor symbol_conf.skip_empty

 tools/perf/builtin-annotate.c  |   4 +-
 tools/perf/builtin-report.c    |  12 +--
 tools/perf/ui/hist.c           | 144 ++++++++++++++++-----------------
 tools/perf/ui/stdio/hist.c     |   5 +-
 tools/perf/util/events_stats.h |   3 +-
 tools/perf/util/evsel.c        |  13 ++-
 tools/perf/util/hist.c         |   6 +-
 tools/perf/util/hist.h         |   3 +-
 tools/perf/util/python.c       |   3 +
 tools/perf/util/session.c      |   5 +-
 tools/perf/util/session.h      |   3 +-
 tools/perf/util/symbol_conf.h  |   3 +-
 12 files changed, 105 insertions(+), 99 deletions(-)

-- 
2.45.1.288.g0e0cd299f1-goog

[PATCH 2/4] perf hist: Simplify __hpp_fmt() using hpp_fmt_data

[Namhyung Kim]

The struct hpp_fmt_data is to keep the values for each group members so
it doesn't need to check the event index in the group.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c | 75 +++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 39 deletions(-)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index e30fcb1e87e7..539978c95cfd 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -46,65 +46,62 @@ static int __hpp__fmt_print(struct perf_hpp *hpp, struct hists *hists, u64 val,
 	return hpp__call_print_fn(hpp, print_fn, fmt, len, val);
 }
 
+struct hpp_fmt_data {
+	struct hists *hists;
+	u64 val;
+	int samples;
+};
+
 static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 		      hpp_field_fn get_field, const char *fmt, int len,
 		      hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
 {
-	int ret;
+	int ret = 0;
 	struct hists *hists = he->hists;
 	struct evsel *evsel = hists_to_evsel(hists);
+	struct evsel *pos;
 	char *buf = hpp->buf;
 	size_t size = hpp->size;
+	int i, nr_members = 1;
+	struct hpp_fmt_data *data;
+
+	if (evsel__is_group_event(evsel))
+		nr_members = evsel->core.nr_members;
+
+	data = calloc(nr_members, sizeof(*data));
+	if (data == NULL)
+		return 0;
+
+	i = 0;
+	for_each_group_evsel(pos, evsel)
+		data[i++].hists = evsel__hists(pos);
 
-	ret = __hpp__fmt_print(hpp, hists, get_field(he), he->stat.nr_events,
-			       fmt, len, print_fn, fmtype);
+	data[0].val = get_field(he);
+	data[0].samples = he->stat.nr_events;
 
 	if (evsel__is_group_event(evsel)) {
-		int prev_idx, idx_delta;
 		struct hist_entry *pair;
-		int nr_members = evsel->core.nr_members;
-
-		prev_idx = evsel__group_idx(evsel);
 
 		list_for_each_entry(pair, &he->pairs.head, pairs.node) {
-			u64 period = get_field(pair);
-			u64 total = hists__total_period(pair->hists);
-			int nr_samples = pair->stat.nr_events;
-
-			if (!total)
-				continue;
+			for (i = 0; i < nr_members; i++) {
+				if (data[i].hists != pair->hists)
+					continue;
 
-			evsel = hists_to_evsel(pair->hists);
-			idx_delta = evsel__group_idx(evsel) - prev_idx - 1;
-
-			while (idx_delta--) {
-				/*
-				 * zero-fill group members in the middle which have
-				 * no samples, pair->hists is not correct but it's
-				 * fine since the value is 0.
-				 */
-				ret += __hpp__fmt_print(hpp, pair->hists, 0, 0,
-							fmt, len, print_fn, fmtype);
+				data[i].val = get_field(pair);
+				data[i].samples = pair->stat.nr_events;
+				break;
 			}
-
-			ret += __hpp__fmt_print(hpp, pair->hists, period, nr_samples,
-						fmt, len, print_fn, fmtype);
-
-			prev_idx = evsel__group_idx(evsel);
 		}
+	}
 
-		idx_delta = nr_members - prev_idx - 1;
-
-		while (idx_delta--) {
-			/*
-			 * zero-fill group members at last which have no sample.
-			 * the hists is not correct but it's fine like above.
-			 */
-			ret += __hpp__fmt_print(hpp, evsel__hists(evsel), 0, 0,
-						fmt, len, print_fn, fmtype);
-		}
+	for (i = 0; i < nr_members; i++) {
+		ret += __hpp__fmt_print(hpp, data[i].hists, data[i].val,
+					data[i].samples, fmt, len,
+					print_fn, fmtype);
 	}
 
+	free(data);
+
 	/*
 	 * Restore original buf and size as it's where caller expects
 	 * the result will be saved.
-- 
2.45.1.288.g0e0cd299f1-goog

Re: [PATCH 2/4] perf hist: Simplify __hpp_fmt() using hpp_fmt_data

[Arnaldo Carvalho de Melo]

On Mon, Jun 03, 2024 at 03:44:10PM -0700, Namhyung Kim wrote:
> The struct hpp_fmt_data is to keep the values for each group members so
> it doesn't need to check the event index in the group.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/hist.c | 75 +++++++++++++++++++++-----------------------
>  1 file changed, 36 insertions(+), 39 deletions(-)
> 
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index e30fcb1e87e7..539978c95cfd 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -46,65 +46,62 @@ static int __hpp__fmt_print(struct perf_hpp *hpp, struct hists *hists, u64 val,
>  	return hpp__call_print_fn(hpp, print_fn, fmt, len, val);
>  }
>  
> +struct hpp_fmt_data {
> +	struct hists *hists;
> +	u64 val;
> +	int samples;
> +};

Can we try to avoid vague terms like 'data' and use a hopefully more
clear 'hpp_fmt_value' instead?

>  static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
>  		      hpp_field_fn get_field, const char *fmt, int len,
>  		      hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
>  {
> -	int ret;
> +	int ret = 0;
>  	struct hists *hists = he->hists;
>  	struct evsel *evsel = hists_to_evsel(hists);
> +	struct evsel *pos;
>  	char *buf = hpp->buf;
>  	size_t size = hpp->size;
> +	int i, nr_members = 1;
> +	struct hpp_fmt_data *data;

Here we then use:

	struct hpp_fmt_value *values;

> +
> +	if (evsel__is_group_event(evsel))
> +		nr_members = evsel->core.nr_members;
> +
> +	data = calloc(nr_members, sizeof(*data));
> +	if (data == NULL)
> +		return 0;


> +
> +	i = 0;
> +	for_each_group_evsel(pos, evsel)
> +		data[i++].hists = evsel__hists(pos);
>  
> -	ret = __hpp__fmt_print(hpp, hists, get_field(he), he->stat.nr_events,
> -			       fmt, len, print_fn, fmtype);
> +	data[0].val = get_field(he);
> +	data[0].samples = he->stat.nr_events;
>  
>  	if (evsel__is_group_event(evsel)) {
> -		int prev_idx, idx_delta;
>  		struct hist_entry *pair;
> -		int nr_members = evsel->core.nr_members;
> -
> -		prev_idx = evsel__group_idx(evsel);
>  
>  		list_for_each_entry(pair, &he->pairs.head, pairs.node) {
> -			u64 period = get_field(pair);
> -			u64 total = hists__total_period(pair->hists);
> -			int nr_samples = pair->stat.nr_events;
> -
> -			if (!total)
> -				continue;
> +			for (i = 0; i < nr_members; i++) {
> +				if (data[i].hists != pair->hists)
> +					continue;
>  
> -			evsel = hists_to_evsel(pair->hists);
> -			idx_delta = evsel__group_idx(evsel) - prev_idx - 1;
> -
> -			while (idx_delta--) {
> -				/*
> -				 * zero-fill group members in the middle which have
> -				 * no samples, pair->hists is not correct but it's
> -				 * fine since the value is 0.
> -				 */
> -				ret += __hpp__fmt_print(hpp, pair->hists, 0, 0,
> -							fmt, len, print_fn, fmtype);
> +				data[i].val = get_field(pair);
> +				data[i].samples = pair->stat.nr_events;
> +				break;
>  			}
> -
> -			ret += __hpp__fmt_print(hpp, pair->hists, period, nr_samples,
> -						fmt, len, print_fn, fmtype);
> -
> -			prev_idx = evsel__group_idx(evsel);
>  		}
> +	}
>  
> -		idx_delta = nr_members - prev_idx - 1;
> -
> -		while (idx_delta--) {
> -			/*
> -			 * zero-fill group members at last which have no sample.
> -			 * the hists is not correct but it's fine like above.
> -			 */
> -			ret += __hpp__fmt_print(hpp, evsel__hists(evsel), 0, 0,
> -						fmt, len, print_fn, fmtype);
> -		}
> +	for (i = 0; i < nr_members; i++) {
> +		ret += __hpp__fmt_print(hpp, data[i].hists, data[i].val,
> +					data[i].samples, fmt, len,
> +					print_fn, fmtype);
>  	}
>  
> +	free(data);
> +
>  	/*
>  	 * Restore original buf and size as it's where caller expects
>  	 * the result will be saved.
> -- 
> 2.45.1.288.g0e0cd299f1-goog

Re: [PATCH 2/4] perf hist: Simplify __hpp_fmt() using hpp_fmt_data

[Namhyung Kim]

On Tue, Jun 04, 2024 at 12:18:47PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Jun 03, 2024 at 03:44:10PM -0700, Namhyung Kim wrote:
> > The struct hpp_fmt_data is to keep the values for each group members so
> > it doesn't need to check the event index in the group.
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/ui/hist.c | 75 +++++++++++++++++++++-----------------------
> >  1 file changed, 36 insertions(+), 39 deletions(-)
> > 
> > diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> > index e30fcb1e87e7..539978c95cfd 100644
> > --- a/tools/perf/ui/hist.c
> > +++ b/tools/perf/ui/hist.c
> > @@ -46,65 +46,62 @@ static int __hpp__fmt_print(struct perf_hpp *hpp, struct hists *hists, u64 val,
> >  	return hpp__call_print_fn(hpp, print_fn, fmt, len, val);
> >  }
> >  
> > +struct hpp_fmt_data {
> > +	struct hists *hists;
> > +	u64 val;
> > +	int samples;
> > +};
> 
> Can we try to avoid vague terms like 'data' and use a hopefully more
> clear 'hpp_fmt_value' instead?

Sure, I can do that.  I thought 'data' was better since it contains more
than a value like a pointer to hist and number of samples.  But it's not
a big deal, and I can make the change.

> 
> >  static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
> >  		      hpp_field_fn get_field, const char *fmt, int len,
> >  		      hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
> >  {
> > -	int ret;
> > +	int ret = 0;
> >  	struct hists *hists = he->hists;
> >  	struct evsel *evsel = hists_to_evsel(hists);
> > +	struct evsel *pos;
> >  	char *buf = hpp->buf;
> >  	size_t size = hpp->size;
> > +	int i, nr_members = 1;
> > +	struct hpp_fmt_data *data;
> 
> Here we then use:
> 
> 	struct hpp_fmt_value *values;

Yep, will change in v3.

Thanks,
Namhyung

> 
> > +
> > +	if (evsel__is_group_event(evsel))
> > +		nr_members = evsel->core.nr_members;
> > +
> > +	data = calloc(nr_members, sizeof(*data));
> > +	if (data == NULL)
> > +		return 0;
> 
> 
> > +
> > +	i = 0;
> > +	for_each_group_evsel(pos, evsel)
> > +		data[i++].hists = evsel__hists(pos);
> >  
> > -	ret = __hpp__fmt_print(hpp, hists, get_field(he), he->stat.nr_events,
> > -			       fmt, len, print_fn, fmtype);
> > +	data[0].val = get_field(he);
> > +	data[0].samples = he->stat.nr_events;
> >  
> >  	if (evsel__is_group_event(evsel)) {
> > -		int prev_idx, idx_delta;
> >  		struct hist_entry *pair;
> > -		int nr_members = evsel->core.nr_members;
> > -
> > -		prev_idx = evsel__group_idx(evsel);
> >  
> >  		list_for_each_entry(pair, &he->pairs.head, pairs.node) {
> > -			u64 period = get_field(pair);
> > -			u64 total = hists__total_period(pair->hists);
> > -			int nr_samples = pair->stat.nr_events;
> > -
> > -			if (!total)
> > -				continue;
> > +			for (i = 0; i < nr_members; i++) {
> > +				if (data[i].hists != pair->hists)
> > +					continue;
> >  
> > -			evsel = hists_to_evsel(pair->hists);
> > -			idx_delta = evsel__group_idx(evsel) - prev_idx - 1;
> > -
> > -			while (idx_delta--) {
> > -				/*
> > -				 * zero-fill group members in the middle which have
> > -				 * no samples, pair->hists is not correct but it's
> > -				 * fine since the value is 0.
> > -				 */
> > -				ret += __hpp__fmt_print(hpp, pair->hists, 0, 0,
> > -							fmt, len, print_fn, fmtype);
> > +				data[i].val = get_field(pair);
> > +				data[i].samples = pair->stat.nr_events;
> > +				break;
> >  			}
> > -
> > -			ret += __hpp__fmt_print(hpp, pair->hists, period, nr_samples,
> > -						fmt, len, print_fn, fmtype);
> > -
> > -			prev_idx = evsel__group_idx(evsel);
> >  		}
> > +	}
> >  
> > -		idx_delta = nr_members - prev_idx - 1;
> > -
> > -		while (idx_delta--) {
> > -			/*
> > -			 * zero-fill group members at last which have no sample.
> > -			 * the hists is not correct but it's fine like above.
> > -			 */
> > -			ret += __hpp__fmt_print(hpp, evsel__hists(evsel), 0, 0,
> > -						fmt, len, print_fn, fmtype);
> > -		}
> > +	for (i = 0; i < nr_members; i++) {
> > +		ret += __hpp__fmt_print(hpp, data[i].hists, data[i].val,
> > +					data[i].samples, fmt, len,
> > +					print_fn, fmtype);
> >  	}
> >  
> > +	free(data);
> > +
> >  	/*
> >  	 * Restore original buf and size as it's where caller expects
> >  	 * the result will be saved.
> > -- 
> > 2.45.1.288.g0e0cd299f1-goog