All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL 00/11] perf/core improvements and fixes
@ 2014-04-24 15:40 Jiri Olsa
  2014-04-24 15:40 ` [PATCH 01/11] perf report: Count number of entries separately Jiri Olsa
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Jiri Olsa

hi Ingo,
please consider pulling

thanks,
jirka


The following changes since commit a81fef347b32dea2b31275826afe1c93fa0d2d54:

  Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf into perf/core (2014-04-22 20:28:23 +0200)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo

for you to fetch changes up to c3b789527b236873557f53740ceac47747e0e1cb:

  perf hists/tui: Count callchain rows separately (2014-04-24 16:34:27 +0200)

----------------------------------------------------------------
perf/core improvements and fixes:

. Factor hists statistics counts processing which in turn also
  fixes several bugs in TUI report command (Namhyung Kim)

Signed-off-by: Jiri Olsa <jolsa@kernel.org>

----------------------------------------------------------------
Namhyung Kim (11):
      perf report: Count number of entries separately
      perf hists: Rename hists__inc_stats()
      perf hists: Move column length calculation out of hists__inc_stats()
      perf hists: Add a couple of hists stat helper functions
      perf hists: Collapse expanded callchains after filter is applied
      perf tools: Account entry stats when it's added to the output tree
      perf hists: Add missing update on filtered stats in hists__decay_entries()
      perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries()
      perf ui/tui: Rename hist_browser__update_nr_entries()
      perf top/tui: Update nr_entries properly after a filter is applied
      perf hists/tui: Count callchain rows separately

 tools/perf/builtin-annotate.c  |  3 +-
 tools/perf/builtin-diff.c      | 23 ++++++-----
 tools/perf/builtin-report.c    | 64 ++++++++++++++---------------
 tools/perf/ui/browsers/hists.c | 92 +++++++++++++++++++++++++++++-------------
 tools/perf/util/hist.c         | 83 ++++++++++++++++++++++++-------------
 tools/perf/util/hist.h         |  9 ++++-
 6 files changed, 171 insertions(+), 103 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/11] perf report: Count number of entries separately
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 02/11] perf hists: Rename hists__inc_stats() Jiri Olsa
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

The hists->nr_entries is counted in multiple places so that they can
confuse readers of the code.  This is a preparation of later change
and do not intend any functional difference.

Note that report__collapse_hists() now changed to return nothing since
its return value (nr_samples) is only for checking if there's any data
in the input file and this can be acheived by checking ->nr_entries.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-2-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-report.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 76e2bb6..aed5203 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -57,6 +57,7 @@ struct report {
 	const char		*cpu_list;
 	const char		*symbol_filter_str;
 	float			min_percent;
+	u64			nr_entries;
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 };
 
@@ -75,6 +76,17 @@ static int report__config(const char *var, const char *value, void *cb)
 	return perf_default_config(var, value, cb);
 }
 
+static void report__inc_stats(struct report *rep, struct hist_entry *he)
+{
+	/*
+	 * The @he is either of a newly created one or an existing one
+	 * merging current sample.  We only want to count a new one so
+	 * checking ->nr_events being 1.
+	 */
+	if (he->stat.nr_events == 1)
+		rep->nr_entries++;
+}
+
 static int report__add_mem_hist_entry(struct report *rep, struct addr_location *al,
 				      struct perf_sample *sample, struct perf_evsel *evsel)
 {
@@ -121,6 +133,8 @@ static int report__add_mem_hist_entry(struct report *rep, struct addr_location *
 			goto out;
 	}
 
+	report__inc_stats(rep, he);
+
 	evsel->hists.stats.total_period += cost;
 	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
 	if (!he->filtered)
@@ -176,6 +190,8 @@ static int report__add_branch_hist_entry(struct report *rep, struct addr_locatio
 					goto out;
 			}
 
+			report__inc_stats(rep, he);
+
 			evsel->hists.stats.total_period += 1;
 			hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
 			if (!he->filtered)
@@ -212,6 +228,8 @@ static int report__add_hist_entry(struct report *rep, struct perf_evsel *evsel,
 	if (ui__has_annotation())
 		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
 
+	report__inc_stats(rep, he);
+
 	evsel->hists.stats.total_period += sample->period;
 	if (!he->filtered)
 		evsel->hists.stats.nr_non_filtered_samples++;
@@ -486,24 +504,12 @@ static int report__browse_hists(struct report *rep)
 	return ret;
 }
 
-static u64 report__collapse_hists(struct report *rep)
+static void report__collapse_hists(struct report *rep)
 {
 	struct ui_progress prog;
 	struct perf_evsel *pos;
-	u64 nr_samples = 0;
-	/*
- 	 * Count number of histogram entries to use when showing progress,
- 	 * reusing nr_samples variable.
- 	 */
-	evlist__for_each(rep->session->evlist, pos)
-		nr_samples += pos->hists.nr_entries;
 
-	ui_progress__init(&prog, nr_samples, "Merging related events...");
-	/*
-	 * Count total number of samples, will be used to check if this
- 	 * session had any.
- 	 */
-	nr_samples = 0;
+	ui_progress__init(&prog, rep->nr_entries, "Merging related events...");
 
 	evlist__for_each(rep->session->evlist, pos) {
 		struct hists *hists = &pos->hists;
@@ -512,7 +518,6 @@ static u64 report__collapse_hists(struct report *rep)
 			hists->symbol_filter_str = rep->symbol_filter_str;
 
 		hists__collapse_resort(hists, &prog);
-		nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
 
 		/* Non-group events are considered as leader */
 		if (symbol_conf.event_group &&
@@ -525,14 +530,11 @@ static u64 report__collapse_hists(struct report *rep)
 	}
 
 	ui_progress__finish();
-
-	return nr_samples;
 }
 
 static int __cmd_report(struct report *rep)
 {
 	int ret;
-	u64 nr_samples;
 	struct perf_session *session = rep->session;
 	struct perf_evsel *pos;
 	struct perf_data_file *file = session->file;
@@ -572,12 +574,12 @@ static int __cmd_report(struct report *rep)
 		}
 	}
 
-	nr_samples = report__collapse_hists(rep);
+	report__collapse_hists(rep);
 
 	if (session_done())
 		return 0;
 
-	if (nr_samples == 0) {
+	if (rep->nr_entries == 0) {
 		ui__error("The %s file has no samples!\n", file->path);
 		return 0;
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/11] perf hists: Rename hists__inc_stats()
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
  2014-04-24 15:40 ` [PATCH 01/11] perf report: Count number of entries separately Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 03/11] perf hists: Move column length calculation out of hists__inc_stats() Jiri Olsa
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

The existing hists__inc_nr_entries() is a misnomer as it's not only
increasing ->nr_entries but also other stats.  So rename it to more
general hists__inc_stats().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-3-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-diff.c | 2 +-
 tools/perf/util/hist.c    | 6 +++---
 tools/perf/util/hist.h    | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 6ef80f2..0e46fa1 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -586,7 +586,7 @@ static void hists__compute_resort(struct hists *hists)
 		next = rb_next(&he->rb_node_in);
 
 		insert_hist_entry_by_compute(&hists->entries, he, compute);
-		hists__inc_nr_entries(hists, he);
+		hists__inc_stats(hists, he);
 	}
 }
 
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 5a89247..12d6c1b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -317,7 +317,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 	return he;
 }
 
-void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)
+void hists__inc_stats(struct hists *hists, struct hist_entry *h)
 {
 	if (!h->filtered) {
 		hists__calc_col_len(hists, h);
@@ -686,7 +686,7 @@ void hists__output_resort(struct hists *hists)
 		next = rb_next(&n->rb_node_in);
 
 		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits);
-		hists__inc_nr_entries(hists, n);
+		hists__inc_stats(hists, n);
 	}
 }
 
@@ -853,7 +853,7 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists,
 		he->hists = hists;
 		rb_link_node(&he->rb_node_in, parent, p);
 		rb_insert_color(&he->rb_node_in, root);
-		hists__inc_nr_entries(hists, he);
+		hists__inc_stats(hists, he);
 		he->dummy = true;
 	}
 out:
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 5a0343e..51478c9 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -116,7 +116,7 @@ void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
 void hists__output_recalc_col_len(struct hists *hists, int max_rows);
 
 u64 hists__total_period(struct hists *hists);
-void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h);
+void hists__inc_stats(struct hists *hists, struct hist_entry *h);
 void hists__inc_nr_events(struct hists *hists, u32 type);
 void events_stats__inc(struct events_stats *stats, u32 type);
 size_t events_stats__fprintf(struct events_stats *stats, FILE *fp);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/11] perf hists: Move column length calculation out of hists__inc_stats()
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
  2014-04-24 15:40 ` [PATCH 01/11] perf report: Count number of entries separately Jiri Olsa
  2014-04-24 15:40 ` [PATCH 02/11] perf hists: Rename hists__inc_stats() Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 04/11] perf hists: Add a couple of hists stat helper functions Jiri Olsa
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

It's not the part of logic of hists__inc_stats() so it'd be better to
move it out of the function.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-4-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-diff.c | 3 +++
 tools/perf/util/hist.c    | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 0e46fa1..c9cc771 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -587,6 +587,9 @@ static void hists__compute_resort(struct hists *hists)
 
 		insert_hist_entry_by_compute(&hists->entries, he, compute);
 		hists__inc_stats(hists, he);
+
+		if (!he->filtered)
+			hists__calc_col_len(hists, he);
 	}
 }
 
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 12d6c1b..f5b388e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -320,7 +320,6 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 void hists__inc_stats(struct hists *hists, struct hist_entry *h)
 {
 	if (!h->filtered) {
-		hists__calc_col_len(hists, h);
 		hists->nr_non_filtered_entries++;
 		hists->stats.total_non_filtered_period += h->stat.period;
 	}
@@ -687,6 +686,9 @@ void hists__output_resort(struct hists *hists)
 
 		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits);
 		hists__inc_stats(hists, n);
+
+		if (!n->filtered)
+			hists__calc_col_len(hists, n);
 	}
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/11] perf hists: Add a couple of hists stat helper functions
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (2 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 03/11] perf hists: Move column length calculation out of hists__inc_stats() Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 05/11] perf hists: Collapse expanded callchains after filter is applied Jiri Olsa
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

Add hists__{reset,inc}_[filter_]stats() functions to cleanup accesses
to hist stats (for output).  Note that number of samples in the stat
is not handled here since it belongs to the input stage.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-5-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-diff.c |  5 +---
 tools/perf/util/hist.c    | 59 ++++++++++++++++++++++++++++++-----------------
 tools/perf/util/hist.h    |  1 +
 3 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index c9cc771..52d91ac 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -573,10 +573,7 @@ static void hists__compute_resort(struct hists *hists)
 	hists->entries = RB_ROOT;
 	next = rb_first(root);
 
-	hists->nr_entries = 0;
-	hists->nr_non_filtered_entries = 0;
-	hists->stats.total_period = 0;
-	hists->stats.total_non_filtered_period = 0;
+	hists__reset_stats(hists);
 	hists__reset_col_len(hists);
 
 	while (next != NULL) {
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f5b388e..b675857 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -317,16 +317,6 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 	return he;
 }
 
-void hists__inc_stats(struct hists *hists, struct hist_entry *h)
-{
-	if (!h->filtered) {
-		hists->nr_non_filtered_entries++;
-		hists->stats.total_non_filtered_period += h->stat.period;
-	}
-	hists->nr_entries++;
-	hists->stats.total_period += h->stat.period;
-}
-
 static u8 symbol__parent_filter(const struct symbol *parent)
 {
 	if (symbol_conf.exclude_other && parent == NULL)
@@ -632,6 +622,35 @@ out:
 	return ret;
 }
 
+static void hists__reset_filter_stats(struct hists *hists)
+{
+	hists->nr_non_filtered_entries = 0;
+	hists->stats.total_non_filtered_period = 0;
+}
+
+void hists__reset_stats(struct hists *hists)
+{
+	hists->nr_entries = 0;
+	hists->stats.total_period = 0;
+
+	hists__reset_filter_stats(hists);
+}
+
+static void hists__inc_filter_stats(struct hists *hists, struct hist_entry *h)
+{
+	hists->nr_non_filtered_entries++;
+	hists->stats.total_non_filtered_period += h->stat.period;
+}
+
+void hists__inc_stats(struct hists *hists, struct hist_entry *h)
+{
+	if (!h->filtered)
+		hists__inc_filter_stats(hists, h);
+
+	hists->nr_entries++;
+	hists->stats.total_period += h->stat.period;
+}
+
 static void __hists__insert_output_entry(struct rb_root *entries,
 					 struct hist_entry *he,
 					 u64 min_callchain_hits)
@@ -675,9 +694,7 @@ void hists__output_resort(struct hists *hists)
 	next = rb_first(root);
 	hists->entries = RB_ROOT;
 
-	hists->nr_non_filtered_entries = 0;
-	hists->stats.total_period = 0;
-	hists->stats.total_non_filtered_period = 0;
+	hists__reset_stats(hists);
 	hists__reset_col_len(hists);
 
 	while (next) {
@@ -699,13 +716,13 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	if (h->filtered)
 		return;
 
-	++hists->nr_non_filtered_entries;
 	if (h->ms.unfolded)
 		hists->nr_non_filtered_entries += h->nr_rows;
 	h->row_offset = 0;
-	hists->stats.total_non_filtered_period += h->stat.period;
+
 	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
 
+	hists__inc_filter_stats(hists, h);
 	hists__calc_col_len(hists, h);
 }
 
@@ -726,9 +743,9 @@ void hists__filter_by_dso(struct hists *hists)
 {
 	struct rb_node *nd;
 
-	hists->nr_non_filtered_entries = 0;
-	hists->stats.total_non_filtered_period = 0;
 	hists->stats.nr_non_filtered_samples = 0;
+
+	hists__reset_filter_stats(hists);
 	hists__reset_col_len(hists);
 
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
@@ -760,9 +777,9 @@ void hists__filter_by_thread(struct hists *hists)
 {
 	struct rb_node *nd;
 
-	hists->nr_non_filtered_entries = 0;
-	hists->stats.total_non_filtered_period = 0;
 	hists->stats.nr_non_filtered_samples = 0;
+
+	hists__reset_filter_stats(hists);
 	hists__reset_col_len(hists);
 
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
@@ -792,9 +809,9 @@ void hists__filter_by_symbol(struct hists *hists)
 {
 	struct rb_node *nd;
 
-	hists->nr_non_filtered_entries = 0;
-	hists->stats.total_non_filtered_period = 0;
 	hists->stats.nr_non_filtered_samples = 0;
+
+	hists__reset_filter_stats(hists);
 	hists__reset_col_len(hists);
 
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 51478c9..ef1ad7a 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -116,6 +116,7 @@ void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
 void hists__output_recalc_col_len(struct hists *hists, int max_rows);
 
 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 events_stats__inc(struct events_stats *stats, u32 type);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/11] perf hists: Collapse expanded callchains after filter is applied
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (3 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 04/11] perf hists: Add a couple of hists stat helper functions Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 06/11] perf tools: Account entry stats when it's added to the output tree Jiri Olsa
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

When a filter is applied a hist entry checks whether its callchain was
folded and account it to the output stat.  But this is rather hacky
and only TUI-specific.  Simply fold the callchains for the entry looks
like a simpler and more generic solution IMHO.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-6-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/hist.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b675857..8d5cfcc 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -716,8 +716,8 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	if (h->filtered)
 		return;
 
-	if (h->ms.unfolded)
-		hists->nr_non_filtered_entries += h->nr_rows;
+	/* force fold unfiltered entry for simplicity */
+	h->ms.unfolded = false;
 	h->row_offset = 0;
 
 	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/11] perf tools: Account entry stats when it's added to the output tree
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (4 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 05/11] perf hists: Collapse expanded callchains after filter is applied Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 07/11] perf hists: Add missing update on filtered stats in hists__decay_entries() Jiri Olsa
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

Currently, accounting each sample is done in multiple places - once
when adding them to the input tree, other when adding them to the
output tree.  It's not only confusing but also can cause a subtle
problem since concurrent processing like in perf top might see the
updated stats before adding entries into the output tree - like seeing
more (blank) lines at the end and/or slight inaccurate percentage.

To fix this, only account the entries when it's moved into the output
tree so that they cannot be seen prematurely.  There're some
exceptional cases here and there - they should be addressed separately
with comments.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-7-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-annotate.c |  3 +--
 tools/perf/builtin-diff.c     | 13 +++++++++----
 tools/perf/builtin-report.c   | 24 ++++++++++--------------
 tools/perf/util/hist.c        |  1 -
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 0da603b..d30d2c2 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -46,7 +46,7 @@ struct perf_annotate {
 };
 
 static int perf_evsel__add_sample(struct perf_evsel *evsel,
-				  struct perf_sample *sample,
+				  struct perf_sample *sample __maybe_unused,
 				  struct addr_location *al,
 				  struct perf_annotate *ann)
 {
@@ -70,7 +70,6 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
 		return -ENOMEM;
 
 	ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
-	evsel->hists.stats.total_period += sample->period;
 	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
 	return ret;
 }
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 52d91ac..f3b10dc 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -341,11 +341,16 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,
 		return -1;
 	}
 
-	if (al.filtered == 0) {
-		evsel->hists.stats.total_non_filtered_period += sample->period;
-		evsel->hists.nr_non_filtered_entries++;
-	}
+	/*
+	 * The total_period is updated here before going to the output
+	 * tree since normally only the baseline hists will call
+	 * hists__output_resort() and precompute needs the total
+	 * period in order to sort entries by percentage delta.
+	 */
 	evsel->hists.stats.total_period += sample->period;
+	if (!al.filtered)
+		evsel->hists.stats.total_non_filtered_period += sample->period;
+
 	return 0;
 }
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index aed5203..89c9528 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -85,6 +85,16 @@ static void report__inc_stats(struct report *rep, struct hist_entry *he)
 	 */
 	if (he->stat.nr_events == 1)
 		rep->nr_entries++;
+
+	/*
+	 * Only counts number of samples at this stage as it's more
+	 * natural to do it here and non-sample events are also
+	 * counted in perf_session_deliver_event().  The dump_trace
+	 * requires this info is ready before going to the output tree.
+	 */
+	hists__inc_nr_events(he->hists, PERF_RECORD_SAMPLE);
+	if (!he->filtered)
+		he->hists->stats.nr_non_filtered_samples++;
 }
 
 static int report__add_mem_hist_entry(struct report *rep, struct addr_location *al,
@@ -135,10 +145,6 @@ static int report__add_mem_hist_entry(struct report *rep, struct addr_location *
 
 	report__inc_stats(rep, he);
 
-	evsel->hists.stats.total_period += cost;
-	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
-	if (!he->filtered)
-		evsel->hists.stats.nr_non_filtered_samples++;
 	err = hist_entry__append_callchain(he, sample);
 out:
 	return err;
@@ -189,13 +195,7 @@ static int report__add_branch_hist_entry(struct report *rep, struct addr_locatio
 				if (err)
 					goto out;
 			}
-
 			report__inc_stats(rep, he);
-
-			evsel->hists.stats.total_period += 1;
-			hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
-			if (!he->filtered)
-				evsel->hists.stats.nr_non_filtered_samples++;
 		} else
 			goto out;
 	}
@@ -230,10 +230,6 @@ static int report__add_hist_entry(struct report *rep, struct perf_evsel *evsel,
 
 	report__inc_stats(rep, he);
 
-	evsel->hists.stats.total_period += sample->period;
-	if (!he->filtered)
-		evsel->hists.stats.nr_non_filtered_samples++;
-	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
 out:
 	return err;
 }
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 8d5cfcc..6d0d2d7 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -382,7 +382,6 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
-	hists->nr_entries++;
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);
 out:
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/11] perf hists: Add missing update on filtered stats in hists__decay_entries()
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (5 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 06/11] perf tools: Account entry stats when it's added to the output tree Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 08/11] perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries() Jiri Olsa
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

When a filter is used for perf top, its hists->nr_non_filtered_entries
was not updated after it removed an entry in hists__decay_entries().
Also hists->stats.total_non_filtered_period was missed too.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-8-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/hist.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6d0d2d7..7f0236c 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -225,14 +225,18 @@ static void he_stat__decay(struct he_stat *he_stat)
 static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 {
 	u64 prev_period = he->stat.period;
+	u64 diff;
 
 	if (prev_period == 0)
 		return true;
 
 	he_stat__decay(&he->stat);
 
+	diff = prev_period - he->stat.period;
+
+	hists->stats.total_period -= diff;
 	if (!he->filtered)
-		hists->stats.total_period -= prev_period - he->stat.period;
+		hists->stats.total_non_filtered_period -= diff;
 
 	return he->stat.period == 0;
 }
@@ -259,8 +263,11 @@ void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel)
 			if (sort__need_collapse)
 				rb_erase(&n->rb_node_in, &hists->entries_collapsed);
 
-			hist_entry__free(n);
 			--hists->nr_entries;
+			if (!n->filtered)
+				--hists->nr_non_filtered_entries;
+
+			hist_entry__free(n);
 		}
 	}
 }
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/11] perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries()
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (6 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 07/11] perf hists: Add missing update on filtered stats in hists__decay_entries() Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 09/11] perf ui/tui: Rename hist_browser__update_nr_entries() Jiri Olsa
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

The nr_entries variable is increased inside the loop in the function
but it always count the first entry regardless of it's filtered or
not; caused an off-by-one error.

It'd become a problem especially there's no entry at all - it'd get a
segfault during referencing a NULL pointer.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-9-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 4d41698..311226e 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1348,10 +1348,10 @@ static void hist_browser__update_pcnt_entries(struct hist_browser *hb)
 	u64 nr_entries = 0;
 	struct rb_node *nd = rb_first(&hb->hists->entries);
 
-	while (nd) {
+	while ((nd = hists__filter_entries(nd, hb->hists,
+					   hb->min_pcnt)) != NULL) {
 		nr_entries++;
-		nd = hists__filter_entries(rb_next(nd), hb->hists,
-					   hb->min_pcnt);
+		nd = rb_next(nd);
 	}
 
 	hb->nr_pcnt_entries = nr_entries;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/11] perf ui/tui: Rename hist_browser__update_nr_entries()
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (7 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 08/11] perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries() Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 10/11] perf top/tui: Update nr_entries properly after a filter is applied Jiri Olsa
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

Rename ->nr_pcnt_entries and hist_browser__update_pcnt_entries() to
->nr_non_filtered_entries and hist_browser__update_nr_entries() since
it's now used for filtering as well.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-10-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 311226e..769295b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -26,13 +26,14 @@ struct hist_browser {
 	int		     print_seq;
 	bool		     show_dso;
 	float		     min_pcnt;
-	u64		     nr_pcnt_entries;
+	u64		     nr_non_filtered_entries;
 };
 
 extern void hist_browser__init_hpp(void);
 
 static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);
+static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
 static void hist_browser__refresh_dimensions(struct hist_browser *browser)
 {
@@ -310,8 +311,6 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
 		"Or reduce the sampling frequency.");
 }
 
-static void hist_browser__update_pcnt_entries(struct hist_browser *hb);
-
 static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 			     struct hist_browser_timer *hbt)
 {
@@ -322,7 +321,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 	browser->b.entries = &browser->hists->entries;
 	browser->b.nr_entries = browser->hists->nr_entries;
 	if (browser->min_pcnt)
-		browser->b.nr_entries = browser->nr_pcnt_entries;
+		browser->b.nr_entries = browser->nr_non_filtered_entries;
 
 	hist_browser__refresh_dimensions(browser);
 	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
@@ -340,8 +339,8 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 			hbt->timer(hbt->arg);
 
 			if (browser->min_pcnt) {
-				hist_browser__update_pcnt_entries(browser);
-				nr_entries = browser->nr_pcnt_entries;
+				hist_browser__update_nr_entries(browser);
+				nr_entries = browser->nr_non_filtered_entries;
 			} else {
 				nr_entries = browser->hists->nr_entries;
 			}
@@ -1343,7 +1342,7 @@ close_file_and_continue:
 	return ret;
 }
 
-static void hist_browser__update_pcnt_entries(struct hist_browser *hb)
+static void hist_browser__update_nr_entries(struct hist_browser *hb)
 {
 	u64 nr_entries = 0;
 	struct rb_node *nd = rb_first(&hb->hists->entries);
@@ -1354,7 +1353,7 @@ static void hist_browser__update_pcnt_entries(struct hist_browser *hb)
 		nd = rb_next(nd);
 	}
 
-	hb->nr_pcnt_entries = nr_entries;
+	hb->nr_non_filtered_entries = nr_entries;
 }
 
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
@@ -1411,7 +1410,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	if (min_pcnt) {
 		browser->min_pcnt = min_pcnt;
-		hist_browser__update_pcnt_entries(browser);
+		hist_browser__update_nr_entries(browser);
 	}
 
 	fstack = pstack__new(2);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/11] perf top/tui: Update nr_entries properly after a filter is applied
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (8 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 09/11] perf ui/tui: Rename hist_browser__update_nr_entries() Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-24 15:40 ` [PATCH 11/11] perf hists/tui: Count callchain rows separately Jiri Olsa
  2014-04-25  8:05 ` [GIT PULL 00/11] perf/core improvements and fixes Ingo Molnar
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

The hist_browser__reset() is only called right after a filter is
applied so it needs to udpate browser->nr_entries properly.  We cannot
use hists->nr_non_filtered_entreis directly since it's possible that
such entries are also filtered out by minimum percentage limit.

In addition when a filter is used for perf top, hist browser's
nr_entries field was not updated after applying the filter.  But it
needs to be updated as new samples are coming.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-11-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++----
 tools/perf/util/hist.h         |  6 ++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 769295b..886eee0 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -35,6 +35,11 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);
 static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
+static bool hist_browser__has_filter(struct hist_browser *hb)
+{
+	return hists__has_filter(hb->hists) || hb->min_pcnt;
+}
+
 static void hist_browser__refresh_dimensions(struct hist_browser *browser)
 {
 	/* 3 == +/- toggle symbol before actual hist_entry rendering */
@@ -44,7 +49,8 @@ static void hist_browser__refresh_dimensions(struct hist_browser *browser)
 
 static void hist_browser__reset(struct hist_browser *browser)
 {
-	browser->b.nr_entries = browser->hists->nr_entries;
+	hist_browser__update_nr_entries(browser);
+	browser->b.nr_entries = browser->nr_non_filtered_entries;
 	hist_browser__refresh_dimensions(browser);
 	ui_browser__reset_index(&browser->b);
 }
@@ -319,9 +325,10 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 	int delay_secs = hbt ? hbt->refresh : 0;
 
 	browser->b.entries = &browser->hists->entries;
-	browser->b.nr_entries = browser->hists->nr_entries;
-	if (browser->min_pcnt)
+	if (hist_browser__has_filter(browser))
 		browser->b.nr_entries = browser->nr_non_filtered_entries;
+	else
+		browser->b.nr_entries = browser->hists->nr_entries;
 
 	hist_browser__refresh_dimensions(browser);
 	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
@@ -338,7 +345,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 			u64 nr_entries;
 			hbt->timer(hbt->arg);
 
-			if (browser->min_pcnt) {
+			if (hist_browser__has_filter(browser)) {
 				hist_browser__update_nr_entries(browser);
 				nr_entries = browser->nr_non_filtered_entries;
 			} else {
@@ -1347,6 +1354,11 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb)
 	u64 nr_entries = 0;
 	struct rb_node *nd = rb_first(&hb->hists->entries);
 
+	if (hb->min_pcnt == 0) {
+		hb->nr_non_filtered_entries = hb->hists->nr_non_filtered_entries;
+		return;
+	}
+
 	while ((nd = hists__filter_entries(nd, hb->hists,
 					   hb->min_pcnt)) != NULL) {
 		nr_entries++;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index ef1ad7a..38c3e87 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -129,6 +129,12 @@ void hists__filter_by_dso(struct hists *hists);
 void hists__filter_by_thread(struct hists *hists);
 void hists__filter_by_symbol(struct hists *hists);
 
+static inline bool hists__has_filter(struct hists *hists)
+{
+	return hists->thread_filter || hists->dso_filter ||
+		hists->symbol_filter_str;
+}
+
 u16 hists__col_len(struct hists *hists, enum hist_column col);
 void hists__set_col_len(struct hists *hists, enum hist_column col, u16 len);
 bool hists__new_col_len(struct hists *hists, enum hist_column col, u16 len);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/11] perf hists/tui: Count callchain rows separately
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (9 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 10/11] perf top/tui: Update nr_entries properly after a filter is applied Jiri Olsa
@ 2014-04-24 15:40 ` Jiri Olsa
  2014-04-25  8:05 ` [GIT PULL 00/11] perf/core improvements and fixes Ingo Molnar
  11 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2014-04-24 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Namhyung Kim, Jiri Olsa

From: Namhyung Kim <namhyung@kernel.org>

When TUI hist browser expands/collapses callchains it accounted number
of callchain nodes into total entries to show.  However this code
ignores filtering so that it can make the cursor go to out of screen.

Thanks to Jiri Olsa for pointing out a bug (and a fix) in the code.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-12-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 63 ++++++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 886eee0..b0861e3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -27,6 +27,7 @@ struct hist_browser {
 	bool		     show_dso;
 	float		     min_pcnt;
 	u64		     nr_non_filtered_entries;
+	u64		     nr_callchain_rows;
 };
 
 extern void hist_browser__init_hpp(void);
@@ -35,11 +36,27 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);
 static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
+static struct rb_node *hists__filter_entries(struct rb_node *nd,
+					     struct hists *hists,
+					     float min_pcnt);
+
 static bool hist_browser__has_filter(struct hist_browser *hb)
 {
 	return hists__has_filter(hb->hists) || hb->min_pcnt;
 }
 
+static u32 hist_browser__nr_entries(struct hist_browser *hb)
+{
+	u32 nr_entries;
+
+	if (hist_browser__has_filter(hb))
+		nr_entries = hb->nr_non_filtered_entries;
+	else
+		nr_entries = hb->hists->nr_entries;
+
+	return nr_entries + hb->nr_callchain_rows;
+}
+
 static void hist_browser__refresh_dimensions(struct hist_browser *browser)
 {
 	/* 3 == +/- toggle symbol before actual hist_entry rendering */
@@ -49,8 +66,14 @@ static void hist_browser__refresh_dimensions(struct hist_browser *browser)
 
 static void hist_browser__reset(struct hist_browser *browser)
 {
+	/*
+	 * The hists__remove_entry_filter() already folds non-filtered
+	 * entries so we can assume it has 0 callchain rows.
+	 */
+	browser->nr_callchain_rows = 0;
+
 	hist_browser__update_nr_entries(browser);
-	browser->b.nr_entries = browser->nr_non_filtered_entries;
+	browser->b.nr_entries = hist_browser__nr_entries(browser);
 	hist_browser__refresh_dimensions(browser);
 	ui_browser__reset_index(&browser->b);
 }
@@ -205,14 +228,16 @@ static bool hist_browser__toggle_fold(struct hist_browser *browser)
 		struct hist_entry *he = browser->he_selection;
 
 		hist_entry__init_have_children(he);
-		browser->hists->nr_entries -= he->nr_rows;
+		browser->b.nr_entries -= he->nr_rows;
+		browser->nr_callchain_rows -= he->nr_rows;
 
 		if (he->ms.unfolded)
 			he->nr_rows = callchain__count_rows(&he->sorted_chain);
 		else
 			he->nr_rows = 0;
-		browser->hists->nr_entries += he->nr_rows;
-		browser->b.nr_entries = browser->hists->nr_entries;
+
+		browser->b.nr_entries += he->nr_rows;
+		browser->nr_callchain_rows += he->nr_rows;
 
 		return true;
 	}
@@ -287,23 +312,27 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
 		he->nr_rows = 0;
 }
 
-static void hists__set_folding(struct hists *hists, bool unfold)
+static void
+__hist_browser__set_folding(struct hist_browser *browser, bool unfold)
 {
 	struct rb_node *nd;
+	struct hists *hists = browser->hists;
 
-	hists->nr_entries = 0;
-
-	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
+	for (nd = rb_first(&hists->entries);
+	     (nd = hists__filter_entries(nd, hists, browser->min_pcnt)) != NULL;
+	     nd = rb_next(nd)) {
 		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
 		hist_entry__set_folding(he, unfold);
-		hists->nr_entries += 1 + he->nr_rows;
+		browser->nr_callchain_rows += he->nr_rows;
 	}
 }
 
 static void hist_browser__set_folding(struct hist_browser *browser, bool unfold)
 {
-	hists__set_folding(browser->hists, unfold);
-	browser->b.nr_entries = browser->hists->nr_entries;
+	browser->nr_callchain_rows = 0;
+	__hist_browser__set_folding(browser, unfold);
+
+	browser->b.nr_entries = hist_browser__nr_entries(browser);
 	/* Go to the start, we may be way after valid entries after a collapse */
 	ui_browser__reset_index(&browser->b);
 }
@@ -325,10 +354,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 	int delay_secs = hbt ? hbt->refresh : 0;
 
 	browser->b.entries = &browser->hists->entries;
-	if (hist_browser__has_filter(browser))
-		browser->b.nr_entries = browser->nr_non_filtered_entries;
-	else
-		browser->b.nr_entries = browser->hists->nr_entries;
+	browser->b.nr_entries = hist_browser__nr_entries(browser);
 
 	hist_browser__refresh_dimensions(browser);
 	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
@@ -345,13 +371,10 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 			u64 nr_entries;
 			hbt->timer(hbt->arg);
 
-			if (hist_browser__has_filter(browser)) {
+			if (hist_browser__has_filter(browser))
 				hist_browser__update_nr_entries(browser);
-				nr_entries = browser->nr_non_filtered_entries;
-			} else {
-				nr_entries = browser->hists->nr_entries;
-			}
 
+			nr_entries = hist_browser__nr_entries(browser);
 			ui_browser__update_nr_entries(&browser->b, nr_entries);
 
 			if (browser->hists->stats.nr_lost_warned !=
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [GIT PULL 00/11] perf/core improvements and fixes
  2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
                   ` (10 preceding siblings ...)
  2014-04-24 15:40 ` [PATCH 11/11] perf hists/tui: Count callchain rows separately Jiri Olsa
@ 2014-04-25  8:05 ` Ingo Molnar
  11 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2014-04-25  8:05 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra


* Jiri Olsa <jolsa@kernel.org> wrote:

> hi Ingo,
> please consider pulling
> 
> thanks,
> jirka
> 
> 
> The following changes since commit a81fef347b32dea2b31275826afe1c93fa0d2d54:
> 
>   Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf into perf/core (2014-04-22 20:28:23 +0200)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo
> 
> for you to fetch changes up to c3b789527b236873557f53740ceac47747e0e1cb:
> 
>   perf hists/tui: Count callchain rows separately (2014-04-24 16:34:27 +0200)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> . Factor hists statistics counts processing which in turn also
>   fixes several bugs in TUI report command (Namhyung Kim)
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> ----------------------------------------------------------------
> Namhyung Kim (11):
>       perf report: Count number of entries separately
>       perf hists: Rename hists__inc_stats()
>       perf hists: Move column length calculation out of hists__inc_stats()
>       perf hists: Add a couple of hists stat helper functions
>       perf hists: Collapse expanded callchains after filter is applied
>       perf tools: Account entry stats when it's added to the output tree
>       perf hists: Add missing update on filtered stats in hists__decay_entries()
>       perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries()
>       perf ui/tui: Rename hist_browser__update_nr_entries()
>       perf top/tui: Update nr_entries properly after a filter is applied
>       perf hists/tui: Count callchain rows separately
> 
>  tools/perf/builtin-annotate.c  |  3 +-
>  tools/perf/builtin-diff.c      | 23 ++++++-----
>  tools/perf/builtin-report.c    | 64 ++++++++++++++---------------
>  tools/perf/ui/browsers/hists.c | 92 +++++++++++++++++++++++++++++-------------
>  tools/perf/util/hist.c         | 83 ++++++++++++++++++++++++-------------
>  tools/perf/util/hist.h         |  9 ++++-
>  6 files changed, 171 insertions(+), 103 deletions(-)

Pulled, thanks a lot Jiri!

	Ingo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 02/11] perf hists: Rename hists__inc_stats()
  2014-04-24  8:23 [PATCHSET 00/11] perf tools: Fixup for the --percentage change (v3) Namhyung Kim
@ 2014-04-24  8:23 ` Namhyung Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Namhyung Kim @ 2014-04-24  8:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, David Ahern, Andi Kleen

The existing hists__inc_nr_entries() is a misnomer as it's not only
increasing ->nr_entries but also other stats.  So rename it to more
general hists__inc_stats().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 2 +-
 tools/perf/util/hist.c    | 6 +++---
 tools/perf/util/hist.h    | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 6ef80f22c1e2..0e46fa1b5ca0 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -586,7 +586,7 @@ static void hists__compute_resort(struct hists *hists)
 		next = rb_next(&he->rb_node_in);
 
 		insert_hist_entry_by_compute(&hists->entries, he, compute);
-		hists__inc_nr_entries(hists, he);
+		hists__inc_stats(hists, he);
 	}
 }
 
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 5a892477aa50..12d6c1bd761d 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -317,7 +317,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 	return he;
 }
 
-void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)
+void hists__inc_stats(struct hists *hists, struct hist_entry *h)
 {
 	if (!h->filtered) {
 		hists__calc_col_len(hists, h);
@@ -686,7 +686,7 @@ void hists__output_resort(struct hists *hists)
 		next = rb_next(&n->rb_node_in);
 
 		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits);
-		hists__inc_nr_entries(hists, n);
+		hists__inc_stats(hists, n);
 	}
 }
 
@@ -853,7 +853,7 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists,
 		he->hists = hists;
 		rb_link_node(&he->rb_node_in, parent, p);
 		rb_insert_color(&he->rb_node_in, root);
-		hists__inc_nr_entries(hists, he);
+		hists__inc_stats(hists, he);
 		he->dummy = true;
 	}
 out:
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 5a0343eb22e2..51478c94d976 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -116,7 +116,7 @@ void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
 void hists__output_recalc_col_len(struct hists *hists, int max_rows);
 
 u64 hists__total_period(struct hists *hists);
-void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h);
+void hists__inc_stats(struct hists *hists, struct hist_entry *h);
 void hists__inc_nr_events(struct hists *hists, u32 type);
 void events_stats__inc(struct events_stats *stats, u32 type);
 size_t events_stats__fprintf(struct events_stats *stats, FILE *fp);
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-04-25  8:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-24 15:40 [GIT PULL 00/11] perf/core improvements and fixes Jiri Olsa
2014-04-24 15:40 ` [PATCH 01/11] perf report: Count number of entries separately Jiri Olsa
2014-04-24 15:40 ` [PATCH 02/11] perf hists: Rename hists__inc_stats() Jiri Olsa
2014-04-24 15:40 ` [PATCH 03/11] perf hists: Move column length calculation out of hists__inc_stats() Jiri Olsa
2014-04-24 15:40 ` [PATCH 04/11] perf hists: Add a couple of hists stat helper functions Jiri Olsa
2014-04-24 15:40 ` [PATCH 05/11] perf hists: Collapse expanded callchains after filter is applied Jiri Olsa
2014-04-24 15:40 ` [PATCH 06/11] perf tools: Account entry stats when it's added to the output tree Jiri Olsa
2014-04-24 15:40 ` [PATCH 07/11] perf hists: Add missing update on filtered stats in hists__decay_entries() Jiri Olsa
2014-04-24 15:40 ` [PATCH 08/11] perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries() Jiri Olsa
2014-04-24 15:40 ` [PATCH 09/11] perf ui/tui: Rename hist_browser__update_nr_entries() Jiri Olsa
2014-04-24 15:40 ` [PATCH 10/11] perf top/tui: Update nr_entries properly after a filter is applied Jiri Olsa
2014-04-24 15:40 ` [PATCH 11/11] perf hists/tui: Count callchain rows separately Jiri Olsa
2014-04-25  8:05 ` [GIT PULL 00/11] perf/core improvements and fixes Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2014-04-24  8:23 [PATCHSET 00/11] perf tools: Fixup for the --percentage change (v3) Namhyung Kim
2014-04-24  8:23 ` [PATCH 02/11] perf hists: Rename hists__inc_stats() Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.