[PATCHSET 0/5] perf report: Make --stat output more compact

[PATCHSET 0/5] perf report: Make --stat output more compact

[Namhyung Kim]

Hello,

The perf report --stat shows event statistics like below:

  $ perf report --stat

  Aggregated stats:
             TOTAL events:      20064
              MMAP events:        239
              LOST events:          0
              COMM events:       1518
              EXIT events:          1
          THROTTLE events:          0
        UNTHROTTLE events:          0
              FORK events:       1517
              READ events:          0
            SAMPLE events:       4015
             MMAP2 events:      12769
               AUX events:          0
      ITRACE_START events:          0
      LOST_SAMPLES events:          0
            SWITCH events:          0
   SWITCH_CPU_WIDE events:          0
        NAMESPACES events:          0
           KSYMBOL events:          0
         BPF_EVENT events:          0
            CGROUP events:          0
         TEXT_POKE events:          0
              ATTR events:          0
        EVENT_TYPE events:          0
      TRACING_DATA events:          0
          BUILD_ID events:          0
    FINISHED_ROUND events:          2
          ID_INDEX events:          0
     AUXTRACE_INFO events:          0
          AUXTRACE events:          0
    AUXTRACE_ERROR events:          0
        THREAD_MAP events:          1
           CPU_MAP events:          1
       STAT_CONFIG events:          0
              STAT events:          0
        STAT_ROUND events:          0
      EVENT_UPDATE events:          0
         TIME_CONV events:          1
           FEATURE events:          0
        COMPRESSED events:          0

But it's too long and mostly 0 so we can make it more compact.  Also
perf report -D has similar output at the end with each event's sample
count.  It'd be better if we can have the same output in both case.

So I added --skip-empty (and --no-skip-empty automatically) to suppres
the 0 output and add the event stats like below.

  $ perf report --stat --skip-empty
  
  Aggregated stats:
             TOTAL events:      20064
              MMAP events:        239
              COMM events:       1518
              EXIT events:          1
              FORK events:       1517
            SAMPLE events:       4015
             MMAP2 events:      12769
    FINISHED_ROUND events:          2
        THREAD_MAP events:          1
           CPU_MAP events:          1
         TIME_CONV events:          1
  cycles stats:
            SAMPLE events:       2475
  instructions stats:
            SAMPLE events:       1540


And I make it enabled by default with a new config option
report.skip-empty to change the behavior if needed.

Thanks,
Namhyung


Namhyung Kim (5):
  perf top: Use evlist->events_stat to count events
  perf hists: Split hists_stats from events_stats
  perf report: Show event sample counts in --stat output
  perf report: Add --skip-empty option to suppress 0 event stat
  perf report: Make --skip-empty as default

 tools/perf/Documentation/perf-config.txt |  5 ++++
 tools/perf/Documentation/perf-report.txt |  3 ++
 tools/perf/builtin-annotate.c            |  6 ++--
 tools/perf/builtin-report.c              | 38 ++++++++++++++++++++----
 tools/perf/builtin-top.c                 | 20 +++++--------
 tools/perf/tests/hists_filter.c          | 14 ++++-----
 tools/perf/ui/stdio/hist.c               |  5 +++-
 tools/perf/util/events_stats.h           | 13 +++++---
 tools/perf/util/hist.c                   | 26 +++++++++++-----
 tools/perf/util/hist.h                   |  7 +++--
 tools/perf/util/session.c                |  5 ++--
 tools/perf/util/session.h                |  3 +-
 12 files changed, 100 insertions(+), 45 deletions(-)


base-commit: 4c391ea001cb2e7bd9a691a886c0dcb030c1791c
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH 1/5] perf top: Use evlist->events_stat to count events

[Namhyung Kim]

It's mainly to count lost events for the warning so it should be ok
to use the evlist->stats instead.  This is needed for changes in the
next commit.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-top.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 173ace43f845..69cb3635f5ef 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -328,13 +328,13 @@ static void perf_top__print_sym_table(struct perf_top *top)
 	printf("%-*.*s\n", win_width, win_width, graph_dotted_line);
 
 	if (!top->record_opts.overwrite &&
-	    (hists->stats.nr_lost_warned !=
-	    hists->stats.nr_events[PERF_RECORD_LOST])) {
-		hists->stats.nr_lost_warned =
-			      hists->stats.nr_events[PERF_RECORD_LOST];
+	    (top->evlist->stats.nr_lost_warned !=
+	     top->evlist->stats.nr_events[PERF_RECORD_LOST])) {
+		top->evlist->stats.nr_lost_warned =
+			      top->evlist->stats.nr_events[PERF_RECORD_LOST];
 		color_fprintf(stdout, PERF_COLOR_RED,
 			      "WARNING: LOST %d chunks, Check IO/CPU overload",
-			      hists->stats.nr_lost_warned);
+			      top->evlist->stats.nr_lost_warned);
 		++printed;
 	}
 
@@ -852,11 +852,9 @@ static void
 perf_top__process_lost(struct perf_top *top, union perf_event *event,
 		       struct evsel *evsel)
 {
-	struct hists *hists = evsel__hists(evsel);
-
 	top->lost += event->lost.lost;
 	top->lost_total += event->lost.lost;
-	hists->stats.total_lost += event->lost.lost;
+	evsel->evlist->stats.total_lost += event->lost.lost;
 }
 
 static void
@@ -864,11 +862,9 @@ perf_top__process_lost_samples(struct perf_top *top,
 			       union perf_event *event,
 			       struct evsel *evsel)
 {
-	struct hists *hists = evsel__hists(evsel);
-
 	top->lost += event->lost_samples.lost;
 	top->lost_total += event->lost_samples.lost;
-	hists->stats.total_lost_samples += event->lost_samples.lost;
+	evsel->evlist->stats.total_lost_samples += event->lost_samples.lost;
 }
 
 static u64 last_timestamp;
@@ -1205,7 +1201,7 @@ static int deliver_event(struct ordered_events *qe,
 	} else if (event->header.type == PERF_RECORD_LOST_SAMPLES) {
 		perf_top__process_lost_samples(top, event, evsel);
 	} else if (event->header.type < PERF_RECORD_MAX) {
-		hists__inc_nr_events(evsel__hists(evsel), event->header.type);
+		events_stats__inc(&session->evlist->stats, event->header.type);
 		machine__process_event(machine, event, &sample);
 	} else
 		++session->evlist->stats.nr_unknown_events;
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH 2/5] perf hists: Split hists_stats from events_stats

[Namhyung Kim]

Each struct hists have events_stats but most of the fields were not
used.  It's to count number of samples and periods whether filtered or
not.  And other fields are used only by evlist.

So it'd be better to split hists_stats and events_stats to reduce
wasted memory in the struct hists.  This makes the output of event
statistics in the perf report compact by skipping 0 events in each
evsel/hists.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c   |  2 +-
 tools/perf/builtin-report.c     |  4 ++--
 tools/perf/tests/hists_filter.c | 14 +++++++-------
 tools/perf/util/events_stats.h  | 10 +++++++---
 tools/perf/util/hist.c          | 20 ++++++++++++++------
 tools/perf/util/hist.h          |  4 ++--
 6 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 524e6f0dff22..717efd78eee6 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -418,7 +418,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 	total_nr_samples = 0;
 	evlist__for_each_entry(session->evlist, pos) {
 		struct hists *hists = evsel__hists(pos);
-		u32 nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
+		u32 nr_samples = hists->stats.nr_samples;
 
 		if (nr_samples > 0) {
 			total_nr_samples += nr_samples;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 0d65c98794a8..b0b9b60f74e5 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -436,7 +436,7 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 {
 	size_t ret;
 	char unit;
-	unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
+	unsigned long nr_samples = hists->stats.nr_samples;
 	u64 nr_events = hists->stats.total_period;
 	struct evsel *evsel = hists_to_evsel(hists);
 	char buf[512];
@@ -464,7 +464,7 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 				nr_samples += pos_hists->stats.nr_non_filtered_samples;
 				nr_events += pos_hists->stats.total_non_filtered_period;
 			} else {
-				nr_samples += pos_hists->stats.nr_events[PERF_RECORD_SAMPLE];
+				nr_samples += pos_hists->stats.nr_samples;
 				nr_events += pos_hists->stats.total_period;
 			}
 		}
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 123e07d35b55..ca6120cd1d90 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -150,13 +150,13 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
 		}
 
 		TEST_ASSERT_VAL("Invalid nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] == 10);
+				hists->stats.nr_samples == 10);
 		TEST_ASSERT_VAL("Invalid nr hist entries",
 				hists->nr_entries == 9);
 		TEST_ASSERT_VAL("Invalid total period",
 				hists->stats.total_period == 1000);
 		TEST_ASSERT_VAL("Unmatched nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] ==
+				hists->stats.nr_samples ==
 				hists->stats.nr_non_filtered_samples);
 		TEST_ASSERT_VAL("Unmatched nr hist entries",
 				hists->nr_entries == hists->nr_non_filtered_entries);
@@ -175,7 +175,7 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
 
 		/* normal stats should be invariant */
 		TEST_ASSERT_VAL("Invalid nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] == 10);
+				hists->stats.nr_samples == 10);
 		TEST_ASSERT_VAL("Invalid nr hist entries",
 				hists->nr_entries == 9);
 		TEST_ASSERT_VAL("Invalid total period",
@@ -204,7 +204,7 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
 
 		/* normal stats should be invariant */
 		TEST_ASSERT_VAL("Invalid nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] == 10);
+				hists->stats.nr_samples == 10);
 		TEST_ASSERT_VAL("Invalid nr hist entries",
 				hists->nr_entries == 9);
 		TEST_ASSERT_VAL("Invalid total period",
@@ -239,7 +239,7 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
 
 		/* normal stats should be invariant */
 		TEST_ASSERT_VAL("Invalid nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] == 10);
+				hists->stats.nr_samples == 10);
 		TEST_ASSERT_VAL("Invalid nr hist entries",
 				hists->nr_entries == 9);
 		TEST_ASSERT_VAL("Invalid total period",
@@ -268,7 +268,7 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
 
 		/* normal stats should be invariant */
 		TEST_ASSERT_VAL("Invalid nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] == 10);
+				hists->stats.nr_samples == 10);
 		TEST_ASSERT_VAL("Invalid nr hist entries",
 				hists->nr_entries == 9);
 		TEST_ASSERT_VAL("Invalid total period",
@@ -299,7 +299,7 @@ int test__hists_filter(struct test *test __maybe_unused, int subtest __maybe_unu
 
 		/* normal stats should be invariant */
 		TEST_ASSERT_VAL("Invalid nr samples",
-				hists->stats.nr_events[PERF_RECORD_SAMPLE] == 10);
+				hists->stats.nr_samples == 10);
 		TEST_ASSERT_VAL("Invalid nr hist entries",
 				hists->nr_entries == 9);
 		TEST_ASSERT_VAL("Invalid total period",
diff --git a/tools/perf/util/events_stats.h b/tools/perf/util/events_stats.h
index 631a4af2ed86..e271c8004c89 100644
--- a/tools/perf/util/events_stats.h
+++ b/tools/perf/util/events_stats.h
@@ -26,15 +26,12 @@
  * perf_record_sample.period and stash the result in total_period.
  */
 struct events_stats {
-	u64 total_period;
-	u64 total_non_filtered_period;
 	u64 total_lost;
 	u64 total_lost_samples;
 	u64 total_aux_lost;
 	u64 total_aux_partial;
 	u64 total_invalid_chains;
 	u32 nr_events[PERF_RECORD_HEADER_MAX];
-	u32 nr_non_filtered_samples;
 	u32 nr_lost_warned;
 	u32 nr_unknown_events;
 	u32 nr_invalid_chains;
@@ -44,6 +41,13 @@ struct events_stats {
 	u32 nr_proc_map_timeout;
 };
 
+struct hists_stats {
+	u64 total_period;
+	u64 total_non_filtered_period;
+	u32 nr_samples;
+	u32 nr_non_filtered_samples;
+};
+
 void events_stats__inc(struct events_stats *stats, u32 type);
 
 size_t events_stats__fprintf(struct events_stats *stats, FILE *fp);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 9299ee535518..691a6a777d14 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2325,14 +2325,19 @@ void events_stats__inc(struct events_stats *stats, u32 type)
 	++stats->nr_events[type];
 }
 
-void hists__inc_nr_events(struct hists *hists, u32 type)
+static void hists_stats__inc(struct hists_stats *stats)
 {
-	events_stats__inc(&hists->stats, type);
+	++stats->nr_samples;
+}
+
+void hists__inc_nr_events(struct hists *hists)
+{
+	hists_stats__inc(&hists->stats);
 }
 
 void hists__inc_nr_samples(struct hists *hists, bool filtered)
 {
-	events_stats__inc(&hists->stats, PERF_RECORD_SAMPLE);
+	hists_stats__inc(&hists->stats);
 	if (!filtered)
 		hists->stats.nr_non_filtered_samples++;
 }
@@ -2677,8 +2682,11 @@ size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp)
 	size_t ret = 0;
 
 	evlist__for_each_entry(evlist, pos) {
+		struct hists *hists = evsel__hists(pos);
+
 		ret += fprintf(fp, "%s stats:\n", evsel__name(pos));
-		ret += events_stats__fprintf(&evsel__hists(pos)->stats, fp);
+		ret += fprintf(fp, "%16s events: %10d\n",
+			       "SAMPLE", hists->stats.nr_samples);
 	}
 
 	return ret;
@@ -2698,7 +2706,7 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh
 	const struct dso *dso = hists->dso_filter;
 	struct thread *thread = hists->thread_filter;
 	int socket_id = hists->socket_filter;
-	unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
+	unsigned long nr_samples = hists->stats.nr_samples;
 	u64 nr_events = hists->stats.total_period;
 	struct evsel *evsel = hists_to_evsel(hists);
 	const char *ev_name = evsel__name(evsel);
@@ -2725,7 +2733,7 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh
 				nr_samples += pos_hists->stats.nr_non_filtered_samples;
 				nr_events += pos_hists->stats.total_non_filtered_period;
 			} else {
-				nr_samples += pos_hists->stats.nr_events[PERF_RECORD_SAMPLE];
+				nr_samples += pos_hists->stats.nr_samples;
 				nr_events += pos_hists->stats.total_period;
 			}
 		}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index e2faa745c8d6..6b0f708f08ac 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -96,7 +96,7 @@ struct hists {
 	const char		*uid_filter_str;
 	const char		*symbol_filter_str;
 	pthread_mutex_t		lock;
-	struct events_stats	stats;
+	struct hists_stats	stats;
 	u64			event_stream;
 	u16			col_len[HISTC_NR_COLS];
 	bool			has_callchains;
@@ -196,7 +196,7 @@ struct hist_entry *hists__get_entry(struct hists *hists, int idx);
 u64 hists__total_period(struct hists *hists);
 void hists__reset_stats(struct hists *hists);
 void hists__inc_stats(struct hists *hists, struct hist_entry *h);
-void hists__inc_nr_events(struct hists *hists, u32 type);
+void hists__inc_nr_events(struct hists *hists);
 void hists__inc_nr_samples(struct hists *hists, bool filtered);
 
 size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH 3/5] perf report: Show event sample counts in --stat output

[Namhyung Kim]

To make the output identical with perf report -D, it needs to show
per-event sample counts along with the aggregated stat  at the end.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index b0b9b60f74e5..be56f3efa413 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -708,9 +708,22 @@ static void report__output_resort(struct report *rep)
 	ui_progress__finish();
 }
 
+static int count_sample_event(struct perf_tool *tool __maybe_unused,
+			      union perf_event *event __maybe_unused,
+			      struct perf_sample *sample __maybe_unused,
+			      struct evsel *evsel,
+			      struct machine *machine __maybe_unused)
+{
+	struct hists *hists = evsel__hists(evsel);
+
+	hists__inc_nr_events(hists);
+	return 0;
+}
+
 static void stats_setup(struct report *rep)
 {
 	memset(&rep->tool, 0, sizeof(rep->tool));
+	rep->tool.sample = count_sample_event;
 	rep->tool.no_warn = true;
 }
 
@@ -719,6 +732,7 @@ static int stats_print(struct report *rep)
 	struct perf_session *session = rep->session;
 
 	perf_session__fprintf_nr_events(session, stdout);
+	perf_evlist__fprintf_nr_events(session->evlist, stdout);
 	return 0;
 }
 
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH 4/5] perf report: Add --skip-empty option to suppress 0 event stat

[Namhyung Kim]

To make the output more readable, I think it's better to remove 0's in
the output.  Also the dummy event has no event stats so it just wasts
the space.  Let's use the --skip-empty option to suppress it.

  $ perf report --stat --skip-empty

  Aggregated stats:
             TOTAL events:      16530
              MMAP events:        226
              COMM events:       1596
              EXIT events:          2
          THROTTLE events:        121
        UNTHROTTLE events:        117
              FORK events:       1595
            SAMPLE events:        719
             MMAP2 events:      12147
            CGROUP events:          2
    FINISHED_ROUND events:          2
        THREAD_MAP events:          1
           CPU_MAP events:          1
         TIME_CONV events:          1
  cycles stats:
            SAMPLE events:        719

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt |  3 +++
 tools/perf/builtin-annotate.c            |  4 ++--
 tools/perf/builtin-report.c              | 16 ++++++++++++----
 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 ++-
 9 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index f51f0000676e..24efc0583c93 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -571,6 +571,9 @@ include::itrace.txt[]
 			    sampled cycles
 	'Avg Cycles'      - block average sampled cycles
 
+--skip-empty::
+	Do not print 0 results in the --stat output.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 717efd78eee6..49627a7bed7c 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -404,8 +404,8 @@ static int __cmd_annotate(struct perf_annotate *ann)
 		goto out;
 
 	if (dump_trace) {
-		perf_session__fprintf_nr_events(session, stdout);
-		evlist__fprintf_nr_events(session->evlist, stdout);
+		perf_session__fprintf_nr_events(session, stdout, false);
+		evlist__fprintf_nr_events(session->evlist, stdout, false);
 		goto out;
 	}
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index be56f3efa413..4910194acaa6 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -85,6 +85,7 @@ struct report {
 	bool			group_set;
 	bool			stitch_lbr;
 	bool			disable_order;
+	bool			skip_empty;
 	int			max_stack;
 	struct perf_read_values	show_threads_values;
 	struct annotation_options annotation_opts;
@@ -530,6 +531,9 @@ static int evlist__tty_browse_hists(struct evlist *evlist, struct report *rep, c
 		if (symbol_conf.event_group && !evsel__is_group_leader(pos))
 			continue;
 
+		if (rep->skip_empty && !hists->stats.nr_samples)
+			continue;
+
 		hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
 
 		if (rep->total_cycles_mode) {
@@ -731,8 +735,8 @@ static int stats_print(struct report *rep)
 {
 	struct perf_session *session = rep->session;
 
-	perf_session__fprintf_nr_events(session, stdout);
-	perf_evlist__fprintf_nr_events(session->evlist, stdout);
+	perf_session__fprintf_nr_events(session, stdout, rep->skip_empty);
+	evlist__fprintf_nr_events(session->evlist, stdout, rep->skip_empty);
 	return 0;
 }
 
@@ -944,8 +948,10 @@ static int __cmd_report(struct report *rep)
 			perf_session__fprintf_dsos(session, stdout);
 
 		if (dump_trace) {
-			perf_session__fprintf_nr_events(session, stdout);
-			evlist__fprintf_nr_events(session->evlist, stdout);
+			perf_session__fprintf_nr_events(session, stdout,
+							rep->skip_empty);
+			evlist__fprintf_nr_events(session->evlist, stdout,
+						  rep->skip_empty);
 			return 0;
 		}
 	}
@@ -1313,6 +1319,8 @@ int cmd_report(int argc, const char **argv)
 		    "Sort all blocks by 'Sampled Cycles%'"),
 	OPT_BOOLEAN(0, "disable-order", &report.disable_order,
 		    "Disable raw trace ordering"),
+	OPT_BOOLEAN(0, "skip-empty", &report.skip_empty,
+		    "Do not display empty (or dummy) events in the output"),
 	OPT_END()
 	};
 	struct perf_data data = {
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 2ab2af4d4849..d9e634406175 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -897,7 +897,8 @@ 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)
+size_t events_stats__fprintf(struct events_stats *stats, FILE *fp,
+			     bool skip_empty)
 {
 	int i;
 	size_t ret = 0;
@@ -908,6 +909,8 @@ 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])
+			continue;
 
 		ret += fprintf(fp, "%16s events: %10d\n", name, stats->nr_events[i]);
 	}
diff --git a/tools/perf/util/events_stats.h b/tools/perf/util/events_stats.h
index e271c8004c89..3480bafd414b 100644
--- a/tools/perf/util/events_stats.h
+++ b/tools/perf/util/events_stats.h
@@ -50,6 +50,7 @@ struct hists_stats {
 
 void events_stats__inc(struct events_stats *stats, u32 type);
 
-size_t events_stats__fprintf(struct events_stats *stats, FILE *fp);
+size_t events_stats__fprintf(struct events_stats *stats, FILE *fp,
+			     bool skip_empty);
 
 #endif /* __PERF_EVENTS_STATS_ */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 691a6a777d14..65fe65ba03c2 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2676,7 +2676,8 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al,
 	}
 }
 
-size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp)
+size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp,
+				 bool skip_empty)
 {
 	struct evsel *pos;
 	size_t ret = 0;
@@ -2684,6 +2685,9 @@ 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)
+			continue;
+
 		ret += fprintf(fp, "%s stats:\n", evsel__name(pos));
 		ret += fprintf(fp, "%16s events: %10d\n",
 			       "SAMPLE", hists->stats.nr_samples);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 6b0f708f08ac..5343b62476e6 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -202,7 +202,8 @@ void hists__inc_nr_samples(struct hists *hists, bool filtered);
 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);
+size_t evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp,
+				 bool skip_empty);
 
 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 eba3769be3f1..a6659b616e6d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2352,7 +2352,8 @@ 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)
+size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp,
+				       bool skip_empty)
 {
 	size_t ret;
 	const char *msg = "";
@@ -2362,7 +2363,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);
+	ret += events_stats__fprintf(&session->evlist->stats, fp, skip_empty);
 	return ret;
 }
 
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index f76480166d38..e31ba4c92a6c 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -113,7 +113,8 @@ 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);
+size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp,
+				       bool skip_empty);
 
 struct evsel *perf_session__find_first_evtype(struct perf_session *session,
 					    unsigned int type);
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH 5/5] perf report: Make --skip-empty as default

[Namhyung Kim]

so that the compact output is shown by default.  Also add
'report.skip-empty' config option to override the default.
Users can also use --no-skip-empty command line option to
change the behavior anytime.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-config.txt | 5 +++++
 tools/perf/builtin-report.c              | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 154a1ced72b2..b0872c801866 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -123,6 +123,7 @@ Example
 		queue-size = 0
 		children = true
 		group = true
+		skip-empty = true
 
 	[llvm]
 		dump-obj = true
@@ -531,6 +532,10 @@ Variables
 		     0.07%   0.00%  noploop  ld-2.15.so         [.] strcmp
 		     0.03%   0.00%  noploop  [kernel.kallsyms]  [k] timerqueue_del
 
+	report.skip-empty::
+		This option can change default stat behavior with empty results.
+		If it's set true, 'perf report --stat' will not show 0 stats.
+
 top.*::
 	top.children::
 		Same as 'report.children'. So if it is enabled, the output of 'top'
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4910194acaa6..36f9ccfeb38a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -136,6 +136,11 @@ static int report__config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "report.skip-empty")) {
+		rep->skip_empty = perf_config_bool(var, value);
+		return 0;
+	}
+
 	return 0;
 }
 
@@ -1160,6 +1165,7 @@ int cmd_report(int argc, const char **argv)
 		.pretty_printing_style	 = "normal",
 		.socket_filter		 = -1,
 		.annotation_opts	 = annotation__default_options,
+		.skip_empty		 = true,
 	};
 	const struct option options[] = {
 	OPT_STRING('i', "input", &input_name, "file",
-- 
2.31.1.498.g6c1eba8ee3d-goog

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Andi Kleen]

> So I added --skip-empty (and --no-skip-empty automatically) to suppres
> the 0 output and add the event stats like below.

I doubt we need the option for this.

But if you change it I would add the percentages after the absolute values.

-Andi

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Namhyung Kim]

Hi Andi,

On Sat, Apr 24, 2021 at 3:46 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> > So I added --skip-empty (and --no-skip-empty automatically) to suppres
> > the 0 output and add the event stats like below.
>
> I doubt we need the option for this.
>
> But if you change it I would add the percentages after the absolute values.

What kind of percentages are you talking about?

Thanks,
Namhyung

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Andi Kleen]

On Sat, Apr 24, 2021 at 04:00:55AM +0900, Namhyung Kim wrote:
> Hi Andi,
> 
> On Sat, Apr 24, 2021 at 3:46 AM Andi Kleen <ak@linux.intel.com> wrote:
> >
> > > So I added --skip-empty (and --no-skip-empty automatically) to suppres
> > > the 0 output and add the event stats like below.
> >
> > I doubt we need the option for this.
> >
> > But if you change it I would add the percentages after the absolute values.
> 
> What kind of percentages are you talking about?

The percentage of that value to the total sum of all the counts.

-Andi

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Namhyung Kim]

On Sat, Apr 24, 2021 at 5:20 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> On Sat, Apr 24, 2021 at 04:00:55AM +0900, Namhyung Kim wrote:
> > Hi Andi,
> >
> > On Sat, Apr 24, 2021 at 3:46 AM Andi Kleen <ak@linux.intel.com> wrote:
> > >
> > > > So I added --skip-empty (and --no-skip-empty automatically) to suppres
> > > > the 0 output and add the event stats like below.
> > >
> > > I doubt we need the option for this.
> > >
> > > But if you change it I would add the percentages after the absolute values.
> >
> > What kind of percentages are you talking about?
>
> The percentage of that value to the total sum of all the counts.

Hmm.. do you want something like this?

             TOTAL events:      20064
              MMAP events:        239   ( 1.2%)
              COMM events:       1518   ( 7.6%)
              EXIT events:          1   (0.0%)
              FORK events:       1517   (7.6%)
            SAMPLE events:       4015   (20.0%)
             MMAP2 events:      12769   (63.6%)
    ...

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Andi Kleen]

> Hmm.. do you want something like this?
> 
>              TOTAL events:      20064
>               MMAP events:        239   ( 1.2%)
>               COMM events:       1518   ( 7.6%)
>               EXIT events:          1   (0.0%)
>               FORK events:       1517   (7.6%)
>             SAMPLE events:       4015   (20.0%)
>              MMAP2 events:      12769   (63.6%)

Yes that's it.

Really shows how inefficient perf is for short measurement
periods.

-Andi

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Arnaldo Carvalho de Melo]

Em Sun, Apr 25, 2021 at 09:31:47PM -0700, Andi Kleen escreveu:
> > Hmm.. do you want something like this?

> >              TOTAL events:      20064
> >               MMAP events:        239   ( 1.2%)
> >               COMM events:       1518   ( 7.6%)
> >               EXIT events:          1   (0.0%)
> >               FORK events:       1517   (7.6%)
> >             SAMPLE events:       4015   (20.0%)
> >              MMAP2 events:      12769   (63.6%)
 
> Yes that's it.
 
> Really shows how inefficient perf is for short measurement
> periods.

Brainstorming a bit:

Yeah, I wonder if we could have a new mode where 'perf daemon' collects
the !SAMPLE records and then a 'perf record' would collect just
PERF_RECORD_SAMPLEs, and then 'perf report' would merge things up.

A perf.data file cap for the 'perf daemon' would mean that when a 'perf
report' result looks interesting, one could press a hotkey and generate
a complete perf.data file with the !SAMPLE records needed to have it
self sufficient.

Additionally maybe we could have 'perf daemon' providing a interface to
resolve samples, returning unresolved ones for older stuff.

wdyt?

- Arnaldo

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Jiri Olsa]

On Fri, Apr 23, 2021 at 11:46:47AM -0700, Andi Kleen wrote:
> > So I added --skip-empty (and --no-skip-empty automatically) to suppres
> > the 0 output and add the event stats like below.
> 
> I doubt we need the option for this.

I agree this could be default, but I'm getting compilation fail:

  CC       ui/browsers/hists.o
ui/browsers/hists.c: In function ‘hist_browser__handle_hotkey’:
ui/browsers/hists.c:699:29: error: ‘struct hists_stats’ has no member named ‘nr_lost_warned’
  699 |       (browser->hists->stats.nr_lost_warned !=
      |                             ^
ui/browsers/hists.c:700:28: error: ‘struct hists_stats’ has no member named ‘nr_events’
  700 |       browser->hists->stats.nr_events[PERF_RECORD_LOST])) {
      |                            ^
ui/browsers/hists.c:701:25: error: ‘struct hists_stats’ has no member named ‘nr_lost_warned’
  701 |    browser->hists->stats.nr_lost_warned =
      |                         ^
ui/browsers/hists.c:702:26: error: ‘struct hists_stats’ has no member named ‘nr_events’
  702 |     browser->hists->stats.nr_events[PERF_RECORD_LOST];
      |                          ^
ui/browsers/hists.c: In function ‘perf_evsel_menu__write’:
ui/browsers/hists.c:3419:40: error: ‘struct hists_stats’ has no member named ‘nr_events’
 3419 |  unsigned long nr_events = hists->stats.nr_events[PERF_RECORD_SAMPLE];
      |                                        ^
ui/browsers/hists.c:3435:33: error: ‘struct hists_stats’ has no member named ‘nr_events’
 3435 |    nr_events += pos_hists->stats.nr_events[PERF_RECORD_SAMPLE];
      |                                 ^
ui/browsers/hists.c:3444:26: error: ‘struct hists_stats’ has no member named ‘nr_events’
 3444 |  nr_events = hists->stats.nr_events[PERF_RECORD_LOST];
      |                          ^
ui/browsers/hists.c: In function ‘block_hists_browser__title’:
ui/browsers/hists.c:3650:41: error: ‘struct hists_stats’ has no member named ‘nr_events’
 3650 |  unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
      |                                         ^

jirka


> 
> But if you change it I would add the percentages after the absolute values.
> 
> -Andi
> 

Re: [PATCHSET 0/5] perf report: Make --stat output more compact

[Namhyung Kim]

On Sat, Apr 24, 2021 at 9:23 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Apr 23, 2021 at 11:46:47AM -0700, Andi Kleen wrote:
> > > So I added --skip-empty (and --no-skip-empty automatically) to suppres
> > > the 0 output and add the event stats like below.
> >
> > I doubt we need the option for this.
>
> I agree this could be default, but I'm getting compilation fail:

Actually the patch 5/5 makes it as default.  I just wrote the example
command line to see the difference explicitly.

Anyway, will fix the build error.

Thanks,
Namhyung

>
>   CC       ui/browsers/hists.o
> ui/browsers/hists.c: In function ‘hist_browser__handle_hotkey’:
> ui/browsers/hists.c:699:29: error: ‘struct hists_stats’ has no member named ‘nr_lost_warned’
>   699 |       (browser->hists->stats.nr_lost_warned !=
>       |                             ^
> ui/browsers/hists.c:700:28: error: ‘struct hists_stats’ has no member named ‘nr_events’
>   700 |       browser->hists->stats.nr_events[PERF_RECORD_LOST])) {
>       |                            ^
> ui/browsers/hists.c:701:25: error: ‘struct hists_stats’ has no member named ‘nr_lost_warned’
>   701 |    browser->hists->stats.nr_lost_warned =
>       |                         ^
> ui/browsers/hists.c:702:26: error: ‘struct hists_stats’ has no member named ‘nr_events’
>   702 |     browser->hists->stats.nr_events[PERF_RECORD_LOST];
>       |                          ^
> ui/browsers/hists.c: In function ‘perf_evsel_menu__write’:
> ui/browsers/hists.c:3419:40: error: ‘struct hists_stats’ has no member named ‘nr_events’
>  3419 |  unsigned long nr_events = hists->stats.nr_events[PERF_RECORD_SAMPLE];
>       |                                        ^
> ui/browsers/hists.c:3435:33: error: ‘struct hists_stats’ has no member named ‘nr_events’
>  3435 |    nr_events += pos_hists->stats.nr_events[PERF_RECORD_SAMPLE];
>       |                                 ^
> ui/browsers/hists.c:3444:26: error: ‘struct hists_stats’ has no member named ‘nr_events’
>  3444 |  nr_events = hists->stats.nr_events[PERF_RECORD_LOST];
>       |                          ^
> ui/browsers/hists.c: In function ‘block_hists_browser__title’:
> ui/browsers/hists.c:3650:41: error: ‘struct hists_stats’ has no member named ‘nr_events’
>  3650 |  unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
>       |                                         ^
>
> jirka
>
>
> >
> > But if you change it I would add the percentages after the absolute values.
> >
> > -Andi
> >
>