All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
@ 2016-01-27 15:40 Namhyung Kim
  2016-01-27 15:40 ` [PATCH 01/10] perf hists: Fix min callchain hits calculation Namhyung Kim
                   ` (11 more replies)
  0 siblings, 12 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Hello,

This patchset tries to implement percent limit to callchains which was
requested by Andi Kleen.  For some reason, limiting callchains by
(overhead) percentage didn't work well.  This patch fixes it and make
--percent-limit also works for callchains as well as hist entries.

 * Changes from v1)
  - fix insertion path instead of changing all UI code
  - show percent value even on single path (if needed)
  - change default callchain percent limit
  
This is available on 'perf/callchain-limit-v2' branch in my tree:

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

Any comments are welcome,

Thanks,
Namhyung
*** BLURB HERE ***

Namhyung Kim (10):
  perf hists: Fix min callchain hits calculation
  perf hists: Update hists' total period when adding entries
  perf report: Apply --percent-limit to callchains also
  perf report: Get rid of hist_entry__callchain_fprintf()
  perf tools: Pass parent_samples to __callchain__fprintf_graph()
  perf report: Fix percent display in callchains on --stdio
  perf hists browser: Fix dump to show correct callchain style
  perf hists browser: Pass parent_total to callchain print functions
  perf hists browser: Fix percent display in callchains
  perf tools: Change default calchain percent limit to 0.005%

 tools/perf/Documentation/perf-report.txt |   2 +-
 tools/perf/builtin-report.c              |  11 ++-
 tools/perf/builtin-top.c                 |   2 +-
 tools/perf/ui/browsers/hists.c           | 113 +++++++++++++++++++------------
 tools/perf/ui/stdio/hist.c               |  72 ++++++++++----------
 tools/perf/util/hist.c                   |  24 +++++--
 tools/perf/util/util.c                   |   2 +-
 7 files changed, 136 insertions(+), 90 deletions(-)

-- 
2.6.4

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

* [PATCH 01/10] perf hists: Fix min callchain hits calculation
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:21   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 02/10] perf hists: Update hists' total period when adding entries Namhyung Kim
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

The total period should be get using hists__total_period() since it
takes filtered entries into account.  In addition, if callchain mode is
'fractal', the total period should be the entry's period.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 81ce0aff69d1..b96194676c91 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1163,9 +1163,18 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
 
-	if (use_callchain)
+	if (use_callchain) {
+		if (callchain_param.mode == CHAIN_GRAPH_REL) {
+			u64 total = he->stat.period;
+
+			if (symbol_conf.cumulate_callchain)
+				total = he->stat_acc->period;
+
+			min_callchain_hits = total * (callchain_param.min_percent / 100);
+		}
 		callchain_param.sort(&he->sorted_chain, he->callchain,
 				      min_callchain_hits, &callchain_param);
+	}
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1195,7 +1204,7 @@ void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 	else
 		use_callchain = symbol_conf.use_callchain;
 
-	min_callchain_hits = hists->stats.total_period * (callchain_param.min_percent / 100);
+	min_callchain_hits = hists__total_period(hists) * (callchain_param.min_percent / 100);
 
 	if (sort__need_collapse)
 		root = &hists->entries_collapsed;
-- 
2.6.4

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

* [PATCH 02/10] perf hists: Update hists' total period when adding entries
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
  2016-01-27 15:40 ` [PATCH 01/10] perf hists: Fix min callchain hits calculation Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:21   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 03/10] perf report: Apply --percent-limit to callchains also Namhyung Kim
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Currently the hist entry addition path doesn't update total_period of
hists and it's calculated during 'resort' path.  But the resort path
needs to know the total period before doing its job because it's used
for calculating percent limit of callchains in hist entries.

So this patch update the total period during the addition path.  It
makes the percent limit of callchains working (again).

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Namhyung Kim <namhyung@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 b96194676c91..098310bc4489 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -432,8 +432,12 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 		cmp = hist_entry__cmp(he, entry);
 
 		if (!cmp) {
-			if (sample_self)
+			if (sample_self) {
 				he_stat__add_period(&he->stat, period, weight);
+				hists->stats.total_period += period;
+				if (!he->filtered)
+					hists->stats.total_non_filtered_period += period;
+			}
 			if (symbol_conf.cumulate_callchain)
 				he_stat__add_period(he->stat_acc, period, weight);
 
@@ -466,7 +470,10 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
-	hists->nr_entries++;
+	if (sample_self)
+		hists__inc_stats(hists, he);
+	else
+		hists->nr_entries++;
 
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);
-- 
2.6.4

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

* [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
  2016-01-27 15:40 ` [PATCH 01/10] perf hists: Fix min callchain hits calculation Namhyung Kim
  2016-01-27 15:40 ` [PATCH 02/10] perf hists: Update hists' total period when adding entries Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-01 20:19   ` Arnaldo Carvalho de Melo
  2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 04/10] perf report: Get rid of hist_entry__callchain_fprintf() Namhyung Kim
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Currently --percent-limit option only works for hist entries.  However
it'd be better to have same effect to callchains as well

Requested-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2bf537f190a0..72ed0b46d5a1 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "report.percent-limit")) {
-		rep->min_percent = strtof(value, NULL);
+		double pcnt = strtof(value, NULL);
+
+		rep->min_percent = pcnt;
+		callchain_param.min_percent = pcnt;
 		return 0;
 	}
 	if (!strcmp(var, "report.children")) {
@@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
 		    int unset __maybe_unused)
 {
 	struct report *rep = opt->value;
+	double pcnt = strtof(str, NULL);
 
-	rep->min_percent = strtof(str, NULL);
+	rep->min_percent = pcnt;
+	callchain_param.min_percent = pcnt;
 	return 0;
 }
 
-- 
2.6.4

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

* [PATCH 04/10] perf report: Get rid of hist_entry__callchain_fprintf()
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (2 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 03/10] perf report: Apply --percent-limit to callchains also Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 05/10] perf tools: Pass parent_samples to __callchain__fprintf_graph() Namhyung Kim
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

It's just a wrapper function to align the start position ofcallchains to
'comm' of each thread if it's a first sort key.  But it doesn't not work
with tracepoint events and also with upcoming hierarchy view.

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

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 387110d50b00..8e25f7dd6e84 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -349,30 +349,6 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
-					    struct hists *hists,
-					    FILE *fp)
-{
-	int left_margin = 0;
-	u64 total_period = hists->stats.total_period;
-
-	if (field_order == NULL && (sort_order == NULL ||
-				    !prefixcmp(sort_order, "comm"))) {
-		struct perf_hpp_fmt *fmt;
-
-		perf_hpp__for_each_format(fmt) {
-			if (!perf_hpp__is_sort_entry(fmt))
-				continue;
-
-			/* must be 'comm' sort entry */
-			left_margin = fmt->width(fmt, NULL, hists_to_evsel(hists));
-			left_margin -= thread__comm_len(he->thread);
-			break;
-		}
-	}
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
-}
-
 static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -418,6 +394,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.buf		= bf,
 		.size		= size,
 	};
+	u64 total_period = hists->stats.total_period;
 
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
@@ -427,7 +404,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	ret = fprintf(fp, "%s\n", bf);
 
 	if (symbol_conf.use_callchain)
-		ret += hist_entry__callchain_fprintf(he, hists, fp);
+		ret += hist_entry_callchain__fprintf(he, total_period, 0, fp);
 
 	return ret;
 }
-- 
2.6.4

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

* [PATCH 05/10] perf tools: Pass parent_samples to __callchain__fprintf_graph()
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (3 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 04/10] perf report: Get rid of hist_entry__callchain_fprintf() Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:22   ` [tip:perf/core] perf callchain: " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 06/10] perf report: Fix percent display in callchains on --stdio Namhyung Kim
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Pass hist entry's period to graph callchain print function.  This info
is needed by later patch to determine whether it can omit percentage of
top-level node or not.

No functional change intended.

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

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8e25f7dd6e84..96188ea12771 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -166,7 +166,8 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 }
 
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-				       u64 total_samples, int left_margin)
+				       u64 total_samples, u64 parent_samples,
+				       int left_margin)
 {
 	struct callchain_node *cnode;
 	struct callchain_list *chain;
@@ -213,6 +214,9 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 		root = &cnode->rb_root;
 	}
 
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		total_samples = parent_samples;
+
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
 	ret += fprintf(fp, "\n");
@@ -323,16 +327,19 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 					    u64 total_samples, int left_margin,
 					    FILE *fp)
 {
+	u64 parent_samples = he->stat.period;
+
+	if (symbol_conf.cumulate_callchain)
+		parent_samples = he->stat_acc->period;
+
 	switch (callchain_param.mode) {
 	case CHAIN_GRAPH_REL:
-		return callchain__fprintf_graph(fp, &he->sorted_chain,
-						symbol_conf.cumulate_callchain ?
-						he->stat_acc->period : he->stat.period,
-						left_margin);
+		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
+						parent_samples, left_margin);
 		break;
 	case CHAIN_GRAPH_ABS:
 		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
-						left_margin);
+						parent_samples, left_margin);
 		break;
 	case CHAIN_FLAT:
 		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);
-- 
2.6.4

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

* [PATCH 06/10] perf report: Fix percent display in callchains on --stdio
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (4 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 05/10] perf tools: Pass parent_samples to __callchain__fprintf_graph() Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 07/10] perf hists browser: Fix dump to show correct callchain style Namhyung Kim
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report -s comm --percent-limit 0.01 --stdio
  ...
     9.95%  swapper
            |
            |--7.57%--intel_idle
            |          cpuidle_enter_state
            |          cpuidle_enter
            |          call_cpuidle
            |          cpu_startup_entry
            |          |
            |          |--4.89%--start_secondary
            |          |
            |           --2.68%--rest_init
            |                     start_kernel
            |                     x86_64_start_reservations
            |                     x86_64_start_kernel
	    |
	    |--0.15%--__schedule
	    |          |
	    |          |--0.13%--schedule
	    |          |          schedule_preempt_disable
	    |          |          cpu_startup_entry
            |          |          |
            |          |          |--0.09%--start_secondary
            |          |          |
            |          |           --0.04%--rest_init
            |          |                     start_kernel
            |          |                     x86_64_start_reservations
            |          |                     x86_64_start_kernel
            |          |
            |           --0.01%--schedule_preempt_disabled
            |                     cpu_startup_entry
  ...

Current code omits the percent if 'intel_idle' becomes the only node
when percent limit is set to 0.5%, its percent is not 9.95% but users
will assume it incorrectly.

Before:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
            ---intel_idle
               cpuidle_enter_state
               cpuidle_enter
               call_cpuidle
               cpu_startup_entry
               |
               |--4.89%--start_secondary
               |
                --2.68%--rest_init
                          start_kernel
                          x86_64_start_reservations
                          x86_64_start_kernel

After:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
             --7.57%--intel_idle
                       cpuidle_enter_state
                       cpuidle_enter
                       call_cpuidle
                       cpu_startup_entry
                       |
                       |--4.89%--start_secondary
                       |
                        --2.68%--rest_init
                                  start_kernel
                                  x86_64_start_reservations
                                  x86_64_start_kernel

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

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 96188ea12771..76ff46becac8 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -165,6 +165,25 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	return ret;
 }
 
+/*
+ * If have one single callchain root, don't bother printing
+ * its percentage (100 % in fractal mode and the same percentage
+ * than the hist in graph mode). This also avoid one level of column.
+ *
+ * However when percent-limit applied, it's possible that single callchain
+ * node have different (non-100% in fractal mode) percentage.
+ */
+static bool need_percent_display(struct rb_node *node, u64 parent_samples)
+{
+	struct callchain_node *cnode;
+
+	if (rb_next(node))
+		return true;
+
+	cnode = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(cnode) != parent_samples;
+}
+
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 				       u64 total_samples, u64 parent_samples,
 				       int left_margin)
@@ -178,13 +197,8 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	int ret = 0;
 	char bf[1024];
 
-	/*
-	 * If have one single callchain root, don't bother printing
-	 * its percentage (100 % in fractal mode and the same percentage
-	 * than the hist in graph mode). This also avoid one level of column.
-	 */
 	node = rb_first(root);
-	if (node && !rb_next(node)) {
+	if (node && !need_percent_display(node, parent_samples)) {
 		cnode = rb_entry(node, struct callchain_node, rb_node);
 		list_for_each_entry(chain, &cnode->val, list) {
 			/*
-- 
2.6.4

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

* [PATCH 07/10] perf hists browser: Fix dump to show correct callchain style
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (5 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 06/10] perf report: Fix percent display in callchains on --stdio Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:23   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 08/10] perf hists browser: Pass parent_total to callchain print functions Namhyung Kim
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

The commit 8c430a348699 ("perf hists browser: Support folded
callchains") missed to update hist_browser__dump() so it always shows
graph-style callchains regardless of current setting.

To fix that, factor out callchain printing code and rename the existing
function which prints graph-style callchain.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 73 ++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d07e6be42ab1..91804c5b0130 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -844,7 +844,7 @@ next:
 	return row - first_row;
 }
 
-static int hist_browser__show_callchain(struct hist_browser *browser,
+static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
 					print_callchain_entry_fn print,
@@ -898,7 +898,7 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 			else
 				new_total = total;
 
-			row += hist_browser__show_callchain(browser, &child->rb_root,
+			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
 							    new_level, row, new_total,
 							    print, arg, is_output_full);
 		}
@@ -910,6 +910,43 @@ out:
 	return row - first_row;
 }
 
+static int hist_browser__show_callchain(struct hist_browser *browser,
+					struct hist_entry *entry, int level,
+					unsigned short row,
+					print_callchain_entry_fn print,
+					struct callchain_print_arg *arg,
+					check_output_full_fn is_output_full)
+{
+	u64 total = hists__total_period(entry->hists);
+	int printed;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL) {
+		if (symbol_conf.cumulate_callchain)
+			total = entry->stat_acc->period;
+		else
+			total = entry->stat.period;
+	}
+
+	if (callchain_param.mode == CHAIN_FLAT) {
+		printed = hist_browser__show_callchain_flat(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else if (callchain_param.mode == CHAIN_FOLDED) {
+		printed = hist_browser__show_callchain_folded(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else {
+		printed = hist_browser__show_callchain_graph(browser,
+						&entry->sorted_chain, level, row, total,
+						print, arg, is_output_full);
+	}
+
+	if (arg->is_current_entry)
+		browser->he_selection = entry;
+
+	return printed;
+}
+
 struct hpp_arg {
 	struct ui_browser *b;
 	char folded_sign;
@@ -1084,38 +1121,14 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		--row_offset;
 
 	if (folded_sign == '-' && row != browser->b.rows) {
-		u64 total = hists__total_period(entry->hists);
 		struct callchain_print_arg arg = {
 			.row_offset = row_offset,
 			.is_current_entry = current_entry,
 		};
 
-		if (callchain_param.mode == CHAIN_GRAPH_REL) {
-			if (symbol_conf.cumulate_callchain)
-				total = entry->stat_acc->period;
-			else
-				total = entry->stat.period;
-		}
-
-		if (callchain_param.mode == CHAIN_FLAT) {
-			printed += hist_browser__show_callchain_flat(browser,
-					&entry->sorted_chain, row, total,
+		printed += hist_browser__show_callchain(browser, entry, 1, row,
 					hist_browser__show_callchain_entry, &arg,
 					hist_browser__check_output_full);
-		} else if (callchain_param.mode == CHAIN_FOLDED) {
-			printed += hist_browser__show_callchain_folded(browser,
-					&entry->sorted_chain, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		} else {
-			printed += hist_browser__show_callchain(browser,
-					&entry->sorted_chain, 1, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		}
-
-		if (arg.is_current_entry)
-			browser->he_selection = entry;
 	}
 
 	return printed;
@@ -1380,15 +1393,11 @@ do_offset:
 static int hist_browser__fprintf_callchain(struct hist_browser *browser,
 					   struct hist_entry *he, FILE *fp)
 {
-	u64 total = hists__total_period(he->hists);
 	struct callchain_print_arg arg  = {
 		.fp = fp,
 	};
 
-	if (symbol_conf.cumulate_callchain)
-		total = he->stat_acc->period;
-
-	hist_browser__show_callchain(browser, &he->sorted_chain, 1, 0, total,
+	hist_browser__show_callchain(browser, he, 1, 0,
 				     hist_browser__fprintf_callchain_entry, &arg,
 				     hist_browser__check_dump_full);
 	return arg.printed;
-- 
2.6.4

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

* [PATCH 08/10] perf hists browser: Pass parent_total to callchain print functions
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (6 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 07/10] perf hists browser: Fix dump to show correct callchain style Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:23   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [PATCH 09/10] perf hists browser: Fix percent display in callchains Namhyung Kim
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Pass parent node's total period to callchain print functions.  This info
is needed by later patch to determine whether it can omit percent or
not correctly.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 44 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 91804c5b0130..726c79def88d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -660,6 +660,7 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
+					     u64 parent_total __maybe_unused,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -763,6 +764,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
+					       u64 parent_total __maybe_unused,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -847,14 +849,18 @@ next:
 static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
+					u64 parent_total,
 					print_callchain_entry_fn print,
 					struct callchain_print_arg *arg,
 					check_output_full_fn is_output_full)
 {
 	struct rb_node *node;
 	int first_row = row, offset = level * LEVEL_OFFSET_STEP;
-	u64 new_total;
 	bool need_percent;
+	u64 percent_total = total;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		percent_total = parent_total;
 
 	node = rb_first(root);
 	need_percent = node && rb_next(node);
@@ -878,7 +884,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 			folded_sign = callchain_list__folded(chain);
 
 			row += hist_browser__show_callchain_list(browser, child,
-							chain, row, total,
+							chain, row, percent_total,
 							was_first && need_percent,
 							offset + extra_offset,
 							print, arg);
@@ -893,13 +899,9 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		if (folded_sign == '-') {
 			const int new_level = level + (extra_offset ? 2 : 1);
 
-			if (callchain_param.mode == CHAIN_GRAPH_REL)
-				new_total = child->children_hit;
-			else
-				new_total = total;
-
 			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
-							    new_level, row, new_total,
+							    new_level, row, total,
+							    child->children_hit,
 							    print, arg, is_output_full);
 		}
 		if (is_output_full(browser, row))
@@ -918,27 +920,29 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 					check_output_full_fn is_output_full)
 {
 	u64 total = hists__total_period(entry->hists);
+	u64 parent_total;
 	int printed;
 
-	if (callchain_param.mode == CHAIN_GRAPH_REL) {
-		if (symbol_conf.cumulate_callchain)
-			total = entry->stat_acc->period;
-		else
-			total = entry->stat.period;
-	}
+	if (symbol_conf.cumulate_callchain)
+		parent_total = entry->stat_acc->period;
+	else
+		parent_total = entry->stat.period;
 
 	if (callchain_param.mode == CHAIN_FLAT) {
 		printed = hist_browser__show_callchain_flat(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else if (callchain_param.mode == CHAIN_FOLDED) {
 		printed = hist_browser__show_callchain_folded(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else {
 		printed = hist_browser__show_callchain_graph(browser,
-						&entry->sorted_chain, level, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, level, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	}
 
 	if (arg->is_current_entry)
-- 
2.6.4

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

* [PATCH 09/10] perf hists browser: Fix percent display in callchains
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (7 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 08/10] perf hists browser: Pass parent_total to callchain print functions Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-02-03 10:24   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2016-01-27 15:40 ` [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005% Namhyung Kim
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report --percent-limit 0.01 --tui
  ...
  -    0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
           - vm_mmap_pgoff
              + 0.02% sys_mmap_pgoff
              + 0.02% vm_mmap
           + 0.02% mprotect_fixup

Current code omits the percent if 'mmap_region' becomes the only node
when percent limit is set to 0.03%, its percent is not 0.06% but users
will assume it incorrectly.

Before:

  $ perf report --percent-limit 0.03 --tui
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - mmap_region
          do_mmap_pgoff
          vm_mmap_pgoff

After:

  $ perf report --percent-limit 0.03 --stdio
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
             vm_mmap_pgoff

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 726c79def88d..4019221f6755 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -657,10 +657,24 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 	return 1;
 }
 
+static bool check_percent_display(struct rb_node *node, u64 parent_total)
+{
+	struct callchain_node *child;
+
+	if (node == NULL)
+		return false;
+
+	if (rb_next(node))
+		return true;
+
+	child = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(child) != parent_total;
+}
+
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
-					     u64 parent_total __maybe_unused,
+					     u64 parent_total,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -670,7 +684,7 @@ static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -764,7 +778,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
-					       u64 parent_total __maybe_unused,
+					       u64 parent_total,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -774,7 +788,7 @@ static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -863,7 +877,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		percent_total = parent_total;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
-- 
2.6.4

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

* [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005%
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (8 preceding siblings ...)
  2016-01-27 15:40 ` [PATCH 09/10] perf hists browser: Fix percent display in callchains Namhyung Kim
@ 2016-01-27 15:40 ` Namhyung Kim
  2016-01-27 16:06   ` Andi Kleen
  2016-01-28  8:14 ` [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Jiri Olsa
  2016-01-28  8:16 ` Jiri Olsa
  11 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 15:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan, Taeung Song

The current default limit of 0.5% is (mostly) for 'fractal' mode which
calculates the percentage relatively.  As we changed the default
callchain mode to 'graph', the existing limit is too high IMHO.
Normally there're many entries under 0.5% overhead, it'd be supprising
that they don't show callchains.

Cc: Taeung Song <treeze.taeung@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt | 2 +-
 tools/perf/builtin-report.c              | 2 +-
 tools/perf/builtin-top.c                 | 2 +-
 tools/perf/util/util.c                   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 8a301f6afb37..1591b45c79e5 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -209,7 +209,7 @@ OPTIONS
 	- none: disable call chain display.
 
 	threshold is a percentage value which specifies a minimum percent to be
-	included in the output call graph.  Default is 0.5 (%).
+	included in the output call graph.  Default is 0.005 (%).
 
 	print_limit is only applied when stdio interface is used.  It's to limit
 	number of call graph entries in a single hist entry.  Note that it needs
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 72ed0b46d5a1..ba6eba5f5d69 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -643,7 +643,7 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
-#define CALLCHAIN_DEFAULT_OPT  "graph,0.5,caller,function,percent"
+#define CALLCHAIN_DEFAULT_OPT  "graph,0.005,caller,function,percent"
 
 const char report_callchain_help[] = "Display call graph (stack chain/backtrace):\n\n"
 				     CALLCHAIN_REPORT_HELP
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index bf01cbb0ef23..1b471135ae2d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1086,7 +1086,7 @@ parse_percent_limit(const struct option *opt, const char *arg,
 }
 
 const char top_callchain_help[] = CALLCHAIN_RECORD_HELP CALLCHAIN_REPORT_HELP
-	"\n\t\t\t\tDefault: fp,graph,0.5,caller,function";
+	"\n\t\t\t\tDefault: fp,graph,0.005,caller,function";
 
 int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 {
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 7a2da7ef556e..25ae6989c89d 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -20,7 +20,7 @@
 
 struct callchain_param	callchain_param = {
 	.mode	= CHAIN_GRAPH_ABS,
-	.min_percent = 0.5,
+	.min_percent = 0.005,
 	.order  = ORDER_CALLEE,
 	.key	= CCKEY_FUNCTION,
 	.value	= CCVAL_PERCENT,
-- 
2.6.4

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

* Re: [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005%
  2016-01-27 15:40 ` [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005% Namhyung Kim
@ 2016-01-27 16:06   ` Andi Kleen
  2016-01-27 23:34     ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Andi Kleen @ 2016-01-27 16:06 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan,
	Taeung Song

On Thu, Jan 28, 2016 at 12:40:57AM +0900, Namhyung Kim wrote:
> The current default limit of 0.5% is (mostly) for 'fractal' mode which
> calculates the percentage relatively.  As we changed the default
> callchain mode to 'graph', the existing limit is too high IMHO.
> Normally there're many entries under 0.5% overhead, it'd be supprising
> that they don't show callchains.

It's too small imho. On more complex workloads with a lot of user space
code we often end up with default perf report --stdio callgraph output of several
hundred KB, and most of it is useless as it is only very small.

-Andi

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

* Re: [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005%
  2016-01-27 16:06   ` Andi Kleen
@ 2016-01-27 23:34     ` Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-01-27 23:34 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, David Ahern, Frederic Weisbecker, Wang Nan, Taeung Song

Hi Andi,

On Wed, 2016-01-27 at 17:06 +0100, Andi Kleen wrote:
> On Thu, Jan 28, 2016 at 12:40:57AM +0900, Namhyung Kim wrote:
> > The current default limit of 0.5% is (mostly) for 'fractal' mode
> > which
> > calculates the percentage relatively.  As we changed the default
> > callchain mode to 'graph', the existing limit is too high IMHO.
> > Normally there're many entries under 0.5% overhead, it'd be
> > supprising
> > that they don't show callchains.
> 
> It's too small imho. On more complex workloads with a lot of user
> space
> code we often end up with default perf report --stdio callgraph output
> of several
> hundred KB, and most of it is useless as it is only very small.

Do you think it's better to keep current default?  Or do you suggest
different value?

Thanks,
Namhyung

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (9 preceding siblings ...)
  2016-01-27 15:40 ` [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005% Namhyung Kim
@ 2016-01-28  8:14 ` Jiri Olsa
  2016-01-28  8:42   ` Namhyung Kim
  2016-01-28  8:16 ` Jiri Olsa
  11 siblings, 1 reply; 42+ messages in thread
From: Jiri Olsa @ 2016-01-28  8:14 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset tries to implement percent limit to callchains which was
> requested by Andi Kleen.  For some reason, limiting callchains by
> (overhead) percentage didn't work well.  This patch fixes it and make
> --percent-limit also works for callchains as well as hist entries.
> 
>  * Changes from v1)
>   - fix insertion path instead of changing all UI code
>   - show percent value even on single path (if needed)
>   - change default callchain percent limit
>   
> This is available on 'perf/callchain-limit-v2' branch in my tree:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any comments are welcome,
> 
> Thanks,
> Namhyung
> *** BLURB HERE ***
> 
> Namhyung Kim (10):
>   perf hists: Fix min callchain hits calculation
>   perf hists: Update hists' total period when adding entries
>   perf report: Apply --percent-limit to callchains also
>   perf report: Get rid of hist_entry__callchain_fprintf()
>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>   perf report: Fix percent display in callchains on --stdio
>   perf hists browser: Fix dump to show correct callchain style
>   perf hists browser: Pass parent_total to callchain print functions
>   perf hists browser: Fix percent display in callchains
>   perf tools: Change default calchain percent limit to 0.005%

heya,
do I get it right that:

'perf report' display all entries, but only callchains above default limit

'perf report --percent-limit X' display only entries above limit X plus
only callchains above limit X


thanks,
jirka

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
                   ` (10 preceding siblings ...)
  2016-01-28  8:14 ` [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Jiri Olsa
@ 2016-01-28  8:16 ` Jiri Olsa
  2016-01-28 10:14   ` Namhyung Kim
  11 siblings, 1 reply; 42+ messages in thread
From: Jiri Olsa @ 2016-01-28  8:16 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset tries to implement percent limit to callchains which was
> requested by Andi Kleen.  For some reason, limiting callchains by
> (overhead) percentage didn't work well.  This patch fixes it and make
> --percent-limit also works for callchains as well as hist entries.
> 
>  * Changes from v1)
>   - fix insertion path instead of changing all UI code
>   - show percent value even on single path (if needed)
>   - change default callchain percent limit
>   
> This is available on 'perf/callchain-limit-v2' branch in my tree:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any comments are welcome,
> 
> Thanks,
> Namhyung
> *** BLURB HERE ***
> 
> Namhyung Kim (10):
>   perf hists: Fix min callchain hits calculation
>   perf hists: Update hists' total period when adding entries
>   perf report: Apply --percent-limit to callchains also
>   perf report: Get rid of hist_entry__callchain_fprintf()
>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>   perf report: Fix percent display in callchains on --stdio
>   perf hists browser: Fix dump to show correct callchain style
>   perf hists browser: Pass parent_total to callchain print functions
>   perf hists browser: Fix percent display in callchains
>   perf tools: Change default calchain percent limit to 0.005%

also I see extra fo entries with callchain filtered out in stdio mode

jirka


---
     8.41%  yes      libc-2.21.so      [.] fputs_unlocked                
            |
            ---fputs_unlocked
               |          
               |--5.67%--0x757074756f206472
               |          
                --2.74%--0x3ba8e0
                          0x21e000

     2.47%  yes      yes               [.] fputs_unlocked@plt            
            |
            ---fputs_unlocked@plt
               0x3ba8e0
               0x21e000

     0.12%  yes      [kernel.vmlinux]  [k] vfs_write                     

     0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5     

     0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64              

     0.07%  yes      [kernel.vmlinux]  [k] fsnotify                      

     0.06%  yes      [kernel.vmlinux]  [k] sys_write                     

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28  8:14 ` [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Jiri Olsa
@ 2016-01-28  8:42   ` Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-01-28  8:42 UTC (permalink / raw)
  To: Jiri Olsa, Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On January 28, 2016 5:14:39 PM GMT+09:00, Jiri Olsa <jolsa@redhat.com> wrote:
>On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
>> Hello,
>> 
>> This patchset tries to implement percent limit to callchains which
>was
>> requested by Andi Kleen.  For some reason, limiting callchains by
>> (overhead) percentage didn't work well.  This patch fixes it and make
>> --percent-limit also works for callchains as well as hist entries.
>> 
>>  * Changes from v1)
>>   - fix insertion path instead of changing all UI code
>>   - show percent value even on single path (if needed)
>>   - change default callchain percent limit
>>   
>> This is available on 'perf/callchain-limit-v2' branch in my tree:
>> 
>>  
>git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>> 
>> Any comments are welcome,
>> 
>> Thanks,
>> Namhyung
>> *** BLURB HERE ***
>> 
>> Namhyung Kim (10):
>>   perf hists: Fix min callchain hits calculation
>>   perf hists: Update hists' total period when adding entries
>>   perf report: Apply --percent-limit to callchains also
>>   perf report: Get rid of hist_entry__callchain_fprintf()
>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>>   perf report: Fix percent display in callchains on --stdio
>>   perf hists browser: Fix dump to show correct callchain style
>>   perf hists browser: Pass parent_total to callchain print functions
>>   perf hists browser: Fix percent display in callchains
>>   perf tools: Change default calchain percent limit to 0.005%
>
>heya,
>do I get it right that:
>
>'perf report' display all entries, but only callchains above default
>limit
>
>'perf report --percent-limit X' display only entries above limit X plus
>only callchains above limit X
>
>
>thanks,
>jirka

Yes, actually there're two limits and they have different defaults. The --percent-limit sets both limits while -g only sets callchain limit.

Thanks
Namhyung
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28  8:16 ` Jiri Olsa
@ 2016-01-28 10:14   ` Namhyung Kim
  2016-01-28 10:16     ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-28 10:14 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
>> Hello,
>>
>> This patchset tries to implement percent limit to callchains which was
>> requested by Andi Kleen.  For some reason, limiting callchains by
>> (overhead) percentage didn't work well.  This patch fixes it and make
>> --percent-limit also works for callchains as well as hist entries.
>>
>>  * Changes from v1)
>>   - fix insertion path instead of changing all UI code
>>   - show percent value even on single path (if needed)
>>   - change default callchain percent limit
>>
>> This is available on 'perf/callchain-limit-v2' branch in my tree:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>
>> Any comments are welcome,
>>
>> Thanks,
>> Namhyung
>> *** BLURB HERE ***
>>
>> Namhyung Kim (10):
>>   perf hists: Fix min callchain hits calculation
>>   perf hists: Update hists' total period when adding entries
>>   perf report: Apply --percent-limit to callchains also
>>   perf report: Get rid of hist_entry__callchain_fprintf()
>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>>   perf report: Fix percent display in callchains on --stdio
>>   perf hists browser: Fix dump to show correct callchain style
>>   perf hists browser: Pass parent_total to callchain print functions
>>   perf hists browser: Fix percent display in callchains
>>   perf tools: Change default calchain percent limit to 0.005%
>
> also I see extra fo entries with callchain filtered out in stdio mode
>
> jirka
>
>
> ---
>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
>             |
>             ---fputs_unlocked
>                |
>                |--5.67%--0x757074756f206472
>                |
>                 --2.74%--0x3ba8e0
>                           0x21e000
>
>      2.47%  yes      yes               [.] fputs_unlocked@plt
>             |
>             ---fputs_unlocked@plt
>                0x3ba8e0
>                0x21e000
>
>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
>
>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
>
>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
>
>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
>
>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
>

I guess it's same for other UI outputs too.

The default limit of hist entries is 0 so it basically shows all
entries.  But default callchain limit is 0.5% so hist entries under
0.5% won't show callchains.

Thanks,
Namhyung

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28 10:14   ` Namhyung Kim
@ 2016-01-28 10:16     ` Namhyung Kim
  2016-01-28 12:12       ` Jiri Olsa
  0 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-01-28 10:16 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 7:14 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
>>> Hello,
>>>
>>> This patchset tries to implement percent limit to callchains which was
>>> requested by Andi Kleen.  For some reason, limiting callchains by
>>> (overhead) percentage didn't work well.  This patch fixes it and make
>>> --percent-limit also works for callchains as well as hist entries.
>>>
>>>  * Changes from v1)
>>>   - fix insertion path instead of changing all UI code
>>>   - show percent value even on single path (if needed)
>>>   - change default callchain percent limit
>>>
>>> This is available on 'perf/callchain-limit-v2' branch in my tree:
>>>
>>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>>
>>> Any comments are welcome,
>>>
>>> Thanks,
>>> Namhyung
>>> *** BLURB HERE ***
>>>
>>> Namhyung Kim (10):
>>>   perf hists: Fix min callchain hits calculation
>>>   perf hists: Update hists' total period when adding entries
>>>   perf report: Apply --percent-limit to callchains also
>>>   perf report: Get rid of hist_entry__callchain_fprintf()
>>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>>>   perf report: Fix percent display in callchains on --stdio
>>>   perf hists browser: Fix dump to show correct callchain style
>>>   perf hists browser: Pass parent_total to callchain print functions
>>>   perf hists browser: Fix percent display in callchains
>>>   perf tools: Change default calchain percent limit to 0.005%
>>
>> also I see extra fo entries with callchain filtered out in stdio mode
>>
>> jirka
>>
>>
>> ---
>>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
>>             |
>>             ---fputs_unlocked
>>                |
>>                |--5.67%--0x757074756f206472
>>                |
>>                 --2.74%--0x3ba8e0
>>                           0x21e000
>>
>>      2.47%  yes      yes               [.] fputs_unlocked@plt
>>             |
>>             ---fputs_unlocked@plt
>>                0x3ba8e0
>>                0x21e000
>>
>>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
>>
>>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
>>
>>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
>>
>>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
>>
>>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
>>
>
> I guess it's same for other UI outputs too.
>
> The default limit of hist entries is 0 so it basically shows all
> entries.  But default callchain limit is 0.5% so hist entries under
> 0.5% won't show callchains.

Btw, I changed it to 0.005% in this patchset.  Did you apply all the
patches and run 'perf report' with default value?

Thanks,
Namhyung

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28 10:16     ` Namhyung Kim
@ 2016-01-28 12:12       ` Jiri Olsa
  2016-01-28 12:24         ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Jiri Olsa @ 2016-01-28 12:12 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 07:16:43PM +0900, Namhyung Kim wrote:
> On Thu, Jan 28, 2016 at 7:14 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> > On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> >> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> >>> Hello,
> >>>
> >>> This patchset tries to implement percent limit to callchains which was
> >>> requested by Andi Kleen.  For some reason, limiting callchains by
> >>> (overhead) percentage didn't work well.  This patch fixes it and make
> >>> --percent-limit also works for callchains as well as hist entries.
> >>>
> >>>  * Changes from v1)
> >>>   - fix insertion path instead of changing all UI code
> >>>   - show percent value even on single path (if needed)
> >>>   - change default callchain percent limit
> >>>
> >>> This is available on 'perf/callchain-limit-v2' branch in my tree:
> >>>
> >>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >>>
> >>> Any comments are welcome,
> >>>
> >>> Thanks,
> >>> Namhyung
> >>> *** BLURB HERE ***
> >>>
> >>> Namhyung Kim (10):
> >>>   perf hists: Fix min callchain hits calculation
> >>>   perf hists: Update hists' total period when adding entries
> >>>   perf report: Apply --percent-limit to callchains also
> >>>   perf report: Get rid of hist_entry__callchain_fprintf()
> >>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
> >>>   perf report: Fix percent display in callchains on --stdio
> >>>   perf hists browser: Fix dump to show correct callchain style
> >>>   perf hists browser: Pass parent_total to callchain print functions
> >>>   perf hists browser: Fix percent display in callchains
> >>>   perf tools: Change default calchain percent limit to 0.005%
> >>
> >> also I see extra fo entries with callchain filtered out in stdio mode
> >>
> >> jirka
> >>
> >>
> >> ---
> >>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
> >>             |
> >>             ---fputs_unlocked
> >>                |
> >>                |--5.67%--0x757074756f206472
> >>                |
> >>                 --2.74%--0x3ba8e0
> >>                           0x21e000
> >>
> >>      2.47%  yes      yes               [.] fputs_unlocked@plt
> >>             |
> >>             ---fputs_unlocked@plt
> >>                0x3ba8e0
> >>                0x21e000
> >>
> >>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
> >>
> >>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
> >>
> >>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
> >>
> >>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
> >>
> >>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
> >>
> >
> > I guess it's same for other UI outputs too.
> >
> > The default limit of hist entries is 0 so it basically shows all
> > entries.  But default callchain limit is 0.5% so hist entries under
> > 0.5% won't show callchains.
> 
> Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> patches and run 'perf report' with default value?

yep, I had it and then reverted ;-) but I made typo
in the previous email.. what I meant was:

also I see extra LINE for entries...  ;-)

thanks,
jirka

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28 12:12       ` Jiri Olsa
@ 2016-01-28 12:24         ` Namhyung Kim
  2016-01-28 19:52           ` Jiri Olsa
  2016-02-03 10:24           ` [tip:perf/core] perf report: Don' t show blank lines if entry has no callchain tip-bot for Namhyung Kim
  0 siblings, 2 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-01-28 12:24 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 01:12:45PM +0100, Jiri Olsa wrote:
> On Thu, Jan 28, 2016 at 07:16:43PM +0900, Namhyung Kim wrote:
> > On Thu, Jan 28, 2016 at 7:14 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> > > On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > >> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> > >>> Hello,
> > >>>
> > >>> This patchset tries to implement percent limit to callchains which was
> > >>> requested by Andi Kleen.  For some reason, limiting callchains by
> > >>> (overhead) percentage didn't work well.  This patch fixes it and make
> > >>> --percent-limit also works for callchains as well as hist entries.
> > >>>
> > >>>  * Changes from v1)
> > >>>   - fix insertion path instead of changing all UI code
> > >>>   - show percent value even on single path (if needed)
> > >>>   - change default callchain percent limit
> > >>>
> > >>> This is available on 'perf/callchain-limit-v2' branch in my tree:
> > >>>
> > >>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >>>
> > >>> Any comments are welcome,
> > >>>
> > >>> Thanks,
> > >>> Namhyung
> > >>> *** BLURB HERE ***
> > >>>
> > >>> Namhyung Kim (10):
> > >>>   perf hists: Fix min callchain hits calculation
> > >>>   perf hists: Update hists' total period when adding entries
> > >>>   perf report: Apply --percent-limit to callchains also
> > >>>   perf report: Get rid of hist_entry__callchain_fprintf()
> > >>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
> > >>>   perf report: Fix percent display in callchains on --stdio
> > >>>   perf hists browser: Fix dump to show correct callchain style
> > >>>   perf hists browser: Pass parent_total to callchain print functions
> > >>>   perf hists browser: Fix percent display in callchains
> > >>>   perf tools: Change default calchain percent limit to 0.005%
> > >>
> > >> also I see extra fo entries with callchain filtered out in stdio mode
> > >>
> > >> jirka
> > >>
> > >>
> > >> ---
> > >>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
> > >>             |
> > >>             ---fputs_unlocked
> > >>                |
> > >>                |--5.67%--0x757074756f206472
> > >>                |
> > >>                 --2.74%--0x3ba8e0
> > >>                           0x21e000
> > >>
> > >>      2.47%  yes      yes               [.] fputs_unlocked@plt
> > >>             |
> > >>             ---fputs_unlocked@plt
> > >>                0x3ba8e0
> > >>                0x21e000
> > >>
> > >>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
> > >>
> > >>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
> > >>
> > >>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
> > >>
> > >>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
> > >>
> > >>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
> > >>
> > >
> > > I guess it's same for other UI outputs too.
> > >
> > > The default limit of hist entries is 0 so it basically shows all
> > > entries.  But default callchain limit is 0.5% so hist entries under
> > > 0.5% won't show callchains.
> > 
> > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> > patches and run 'perf report' with default value?
> 
> yep, I had it and then reverted ;-) but I made typo
> in the previous email.. what I meant was:
> 
> also I see extra LINE for entries...  ;-)

Ah, so you meant the blank lines..  The fix would be like following



>From 62ac44405797275aed35acb38cfe3d1afa6b709c Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 28 Jan 2016 21:18:53 +0900
Subject: [PATCH 11/10] perf report: Don't show blank lines if entry has no
 callchain

When all callchains of a hist entry is percent-limited, do not add a
blank line at the end.  It makes the entry look like it doesn't have
callchains.

Reported-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 76ff46becac8..691e52ce7510 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -233,7 +233,10 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
-	ret += fprintf(fp, "\n");
+	if (ret) {
+		/* do not add a blank line if it printed nothing */
+		ret += fprintf(fp, "\n");
+	}
 
 	return ret;
 }
-- 
2.6.4

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28 12:24         ` Namhyung Kim
@ 2016-01-28 19:52           ` Jiri Olsa
  2016-01-29 21:00             ` Arnaldo Carvalho de Melo
  2016-02-03 10:24           ` [tip:perf/core] perf report: Don' t show blank lines if entry has no callchain tip-bot for Namhyung Kim
  1 sibling, 1 reply; 42+ messages in thread
From: Jiri Olsa @ 2016-01-28 19:52 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

On Thu, Jan 28, 2016 at 09:24:54PM +0900, Namhyung Kim wrote:

SNIP

> > > > The default limit of hist entries is 0 so it basically shows all
> > > > entries.  But default callchain limit is 0.5% so hist entries under
> > > > 0.5% won't show callchains.
> > > 
> > > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> > > patches and run 'perf report' with default value?
> > 
> > yep, I had it and then reverted ;-) but I made typo
> > in the previous email.. what I meant was:
> > 
> > also I see extra LINE for entries...  ;-)
> 
> Ah, so you meant the blank lines..  The fix would be like following

yep, tested.. works ;-)

thanks,
jirka

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-28 19:52           ` Jiri Olsa
@ 2016-01-29 21:00             ` Arnaldo Carvalho de Melo
  2016-01-30 13:52               ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-01-29 21:00 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML,
	Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

Em Thu, Jan 28, 2016 at 08:52:25PM +0100, Jiri Olsa escreveu:
> On Thu, Jan 28, 2016 at 09:24:54PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > > > > The default limit of hist entries is 0 so it basically shows all
> > > > > entries.  But default callchain limit is 0.5% so hist entries under
> > > > > 0.5% won't show callchains.
> > > > 
> > > > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> > > > patches and run 'perf report' with default value?
> > > 
> > > yep, I had it and then reverted ;-) but I made typo
> > > in the previous email.. what I meant was:
> > > 
> > > also I see extra LINE for entries...  ;-)
> > 
> > Ah, so you meant the blank lines..  The fix would be like following
> 
> yep, tested.. works ;-)

Namhyung, can I try to process this patchkit just by reading these
commends and making the adjustments? Or is there something outstanding
that warrants you to push a v2?

- Arnaldo

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

* Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)
  2016-01-29 21:00             ` Arnaldo Carvalho de Melo
@ 2016-01-30 13:52               ` Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-01-30 13:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML,
	Andi Kleen, David Ahern, Frederic Weisbecker, Wang Nan

Hi Arnaldo,

On Sat, Jan 30, 2016 at 6:00 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Thu, Jan 28, 2016 at 08:52:25PM +0100, Jiri Olsa escreveu:
>> On Thu, Jan 28, 2016 at 09:24:54PM +0900, Namhyung Kim wrote:
>>
>> SNIP
>>
>> > > > > The default limit of hist entries is 0 so it basically shows all
>> > > > > entries.  But default callchain limit is 0.5% so hist entries under
>> > > > > 0.5% won't show callchains.
>> > > >
>> > > > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
>> > > > patches and run 'perf report' with default value?
>> > >
>> > > yep, I had it and then reverted ;-) but I made typo
>> > > in the previous email.. what I meant was:
>> > >
>> > > also I see extra LINE for entries...  ;-)
>> >
>> > Ah, so you meant the blank lines..  The fix would be like following
>>
>> yep, tested.. works ;-)
>
> Namhyung, can I try to process this patchkit just by reading these
> commends and making the adjustments? Or is there something outstanding
> that warrants you to push a v2?

It'd great for me if you process this (v1).  I cannot work on the v2
for a couple of days..

Thanks,
Namhyung

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-01-27 15:40 ` [PATCH 03/10] perf report: Apply --percent-limit to callchains also Namhyung Kim
@ 2016-02-01 20:19   ` Arnaldo Carvalho de Melo
  2016-02-02 13:05     ` Namhyung Kim
  2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
  1 sibling, 1 reply; 42+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-01 20:19 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> Currently --percent-limit option only works for hist entries.  However
> it'd be better to have same effect to callchains as well

Documentation needs updating? It says:

--percent-limit::
        Do not show entries which have an overhead under that percent.
        (Default: 0).

 
> Requested-by: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-report.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2bf537f190a0..72ed0b46d5a1 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  	if (!strcmp(var, "report.percent-limit")) {
> -		rep->min_percent = strtof(value, NULL);
> +		double pcnt = strtof(value, NULL);
> +
> +		rep->min_percent = pcnt;
> +		callchain_param.min_percent = pcnt;
>  		return 0;
>  	}
>  	if (!strcmp(var, "report.children")) {
> @@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
>  		    int unset __maybe_unused)
>  {
>  	struct report *rep = opt->value;
> +	double pcnt = strtof(str, NULL);
>  
> -	rep->min_percent = strtof(str, NULL);
> +	rep->min_percent = pcnt;
> +	callchain_param.min_percent = pcnt;
>  	return 0;
>  }
>  
> -- 
> 2.6.4

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-01 20:19   ` Arnaldo Carvalho de Melo
@ 2016-02-02 13:05     ` Namhyung Kim
  2016-02-02 13:55       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-02-02 13:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Hi Arnaldo,

On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > Currently --percent-limit option only works for hist entries.  However
> > it'd be better to have same effect to callchains as well
> 
> Documentation needs updating? It says:
> 
> --percent-limit::
>         Do not show entries which have an overhead under that percent.
>         (Default: 0).

Right.  Is it ok to you?

--percent-limit::
        Do not show entries and callchains which have an overhead under that
        percent.  (Default: 0).


Thanks,
Namhyung


> 
>  
> > Requested-by: Andi Kleen <andi@firstfloor.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-report.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 2bf537f190a0..72ed0b46d5a1 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
> >  		return 0;
> >  	}
> >  	if (!strcmp(var, "report.percent-limit")) {
> > -		rep->min_percent = strtof(value, NULL);
> > +		double pcnt = strtof(value, NULL);
> > +
> > +		rep->min_percent = pcnt;
> > +		callchain_param.min_percent = pcnt;
> >  		return 0;
> >  	}
> >  	if (!strcmp(var, "report.children")) {
> > @@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
> >  		    int unset __maybe_unused)
> >  {
> >  	struct report *rep = opt->value;
> > +	double pcnt = strtof(str, NULL);
> >  
> > -	rep->min_percent = strtof(str, NULL);
> > +	rep->min_percent = pcnt;
> > +	callchain_param.min_percent = pcnt;
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.6.4

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-02 13:05     ` Namhyung Kim
@ 2016-02-02 13:55       ` Arnaldo Carvalho de Melo
  2016-02-02 14:15         ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-02 13:55 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > Currently --percent-limit option only works for hist entries.  However
> > > it'd be better to have same effect to callchains as well
> > 
> > Documentation needs updating? It says:
> > 
> > --percent-limit::
> >         Do not show entries which have an overhead under that percent.
> >         (Default: 0).
> 
> Right.  Is it ok to you?
> 
> --percent-limit::
>         Do not show entries and callchains which have an overhead under that
>         percent.  (Default: 0).

Ok, but is the default zero?

That was what I was alluding to, as as soon as I applied the patch that
made callchains honour this limit, about 60% of the entries in the
particular perf.data file I was 'perf report'ing lost its '+' (callchain
expansion) signs.

- Arnaldo

> 
> 
> Thanks,
> Namhyung
> 
> 
> > 
> >  
> > > Requested-by: Andi Kleen <andi@firstfloor.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/builtin-report.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > > index 2bf537f190a0..72ed0b46d5a1 100644
> > > --- a/tools/perf/builtin-report.c
> > > +++ b/tools/perf/builtin-report.c
> > > @@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
> > >  		return 0;
> > >  	}
> > >  	if (!strcmp(var, "report.percent-limit")) {
> > > -		rep->min_percent = strtof(value, NULL);
> > > +		double pcnt = strtof(value, NULL);
> > > +
> > > +		rep->min_percent = pcnt;
> > > +		callchain_param.min_percent = pcnt;
> > >  		return 0;
> > >  	}
> > >  	if (!strcmp(var, "report.children")) {
> > > @@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
> > >  		    int unset __maybe_unused)
> > >  {
> > >  	struct report *rep = opt->value;
> > > +	double pcnt = strtof(str, NULL);
> > >  
> > > -	rep->min_percent = strtof(str, NULL);
> > > +	rep->min_percent = pcnt;
> > > +	callchain_param.min_percent = pcnt;
> > >  	return 0;
> > >  }
> > >  
> > > -- 
> > > 2.6.4

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-02 13:55       ` Arnaldo Carvalho de Melo
@ 2016-02-02 14:15         ` Namhyung Kim
  2016-02-02 14:27           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-02-02 14:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > Hi Arnaldo,
> > 
> > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > Currently --percent-limit option only works for hist entries.  However
> > > > it'd be better to have same effect to callchains as well
> > > 
> > > Documentation needs updating? It says:
> > > 
> > > --percent-limit::
> > >         Do not show entries which have an overhead under that percent.
> > >         (Default: 0).
> > 
> > Right.  Is it ok to you?
> > 
> > --percent-limit::
> >         Do not show entries and callchains which have an overhead under that
> >         percent.  (Default: 0).
> 
> Ok, but is the default zero?
> 
> That was what I was alluding to, as as soon as I applied the patch that
> made callchains honour this limit, about 60% of the entries in the
> particular perf.data file I was 'perf report'ing lost its '+' (callchain
> expansion) signs.

Yes, and this is what I want to say too. :)

The default value of percent limit is different for hist entry and
callchains.  For hist entry the default is 0, and for callchains it's
0.5%.  But using --percent-limit option, we can set both at once from
now on.

Before this patchset, percent limit of callchains didn't checked.
Once applied, users can see callchains are disppeared like your case.
 This is just because 0.5% of the default limit for callchains is too
high IMHO - I guess it was originally set for 'fractal' mode which
calculates relative percents.  So I proposed to change the default in
the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
choose other value like 0.05% ?

Thanks,
Namhyung

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-02 14:15         ` Namhyung Kim
@ 2016-02-02 14:27           ` Arnaldo Carvalho de Melo
  2016-02-02 14:35             ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-02 14:27 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > Hi Arnaldo,
> > > 
> > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > it'd be better to have same effect to callchains as well
> > > > 
> > > > Documentation needs updating? It says:
> > > > 
> > > > --percent-limit::
> > > >         Do not show entries which have an overhead under that percent.
> > > >         (Default: 0).
> > > 
> > > Right.  Is it ok to you?
> > > 
> > > --percent-limit::
> > >         Do not show entries and callchains which have an overhead under that
> > >         percent.  (Default: 0).
> > 
> > Ok, but is the default zero?
> > 
> > That was what I was alluding to, as as soon as I applied the patch that
> > made callchains honour this limit, about 60% of the entries in the
> > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > expansion) signs.
> 
> Yes, and this is what I want to say too. :)
> 
> The default value of percent limit is different for hist entry and
> callchains.  For hist entry the default is 0, and for callchains it's
> 0.5%.  But using --percent-limit option, we can set both at once from
> now on.
> 
> Before this patchset, percent limit of callchains didn't checked.
> Once applied, users can see callchains are disppeared like your case.
>  This is just because 0.5% of the default limit for callchains is too
> high IMHO - I guess it was originally set for 'fractal' mode which
> calculates relative percents.  So I proposed to change the default in
> the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> choose other value like 0.05% ?

Unsure about the limit, but please fold the nice explanation you gave
about the limits for hist entries and callchains and how the defaults
are different, and how they can be set at once using --percent-limit.

Also, would it be overengineering to allow optionally set two limits,
one for hists and one for callchains? I.e. like:

 --percent-limit 1,0.07

Then:

  --percent-limit 0.8

would be the same as:

  --percent-limit 0.8,0.8

- Arnaldo

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-02 14:27           ` Arnaldo Carvalho de Melo
@ 2016-02-02 14:35             ` Namhyung Kim
  2016-02-02 14:57               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 42+ messages in thread
From: Namhyung Kim @ 2016-02-02 14:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

On Tue, Feb 02, 2016 at 11:27:34AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> > On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > > Hi Arnaldo,
> > > > 
> > > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > > it'd be better to have same effect to callchains as well
> > > > > 
> > > > > Documentation needs updating? It says:
> > > > > 
> > > > > --percent-limit::
> > > > >         Do not show entries which have an overhead under that percent.
> > > > >         (Default: 0).
> > > > 
> > > > Right.  Is it ok to you?
> > > > 
> > > > --percent-limit::
> > > >         Do not show entries and callchains which have an overhead under that
> > > >         percent.  (Default: 0).
> > > 
> > > Ok, but is the default zero?
> > > 
> > > That was what I was alluding to, as as soon as I applied the patch that
> > > made callchains honour this limit, about 60% of the entries in the
> > > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > > expansion) signs.
> > 
> > Yes, and this is what I want to say too. :)
> > 
> > The default value of percent limit is different for hist entry and
> > callchains.  For hist entry the default is 0, and for callchains it's
> > 0.5%.  But using --percent-limit option, we can set both at once from
> > now on.
> > 
> > Before this patchset, percent limit of callchains didn't checked.
> > Once applied, users can see callchains are disppeared like your case.
> >  This is just because 0.5% of the default limit for callchains is too
> > high IMHO - I guess it was originally set for 'fractal' mode which
> > calculates relative percents.  So I proposed to change the default in
> > the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> > choose other value like 0.05% ?
> 
> Unsure about the limit, but please fold the nice explanation you gave
> about the limits for hist entries and callchains and how the defaults
> are different, and how they can be set at once using --percent-limit.

Ok, will do.

> 
> Also, would it be overengineering to allow optionally set two limits,
> one for hists and one for callchains? I.e. like:
> 
>  --percent-limit 1,0.07
> 
> Then:
> 
>   --percent-limit 0.8
> 
> would be the same as:
> 
>   --percent-limit 0.8,0.8

Originally, the percent limit (threshold) for callchains can be set
using -g/--call-graph option.  What Andi asked is to set both limits
using a single option.  Not sure it's worth adding another way to
specify the callchain limit.

Thanks,
Namhyung

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-02 14:35             ` Namhyung Kim
@ 2016-02-02 14:57               ` Arnaldo Carvalho de Melo
  2016-02-02 15:03                 ` Namhyung Kim
  0 siblings, 1 reply; 42+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-02 14:57 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

Em Tue, Feb 02, 2016 at 11:35:13PM +0900, Namhyung Kim escreveu:
> On Tue, Feb 02, 2016 at 11:27:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> > > On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > > > Hi Arnaldo,
> > > > > 
> > > > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > > > it'd be better to have same effect to callchains as well
> > > > > > 
> > > > > > Documentation needs updating? It says:
> > > > > > 
> > > > > > --percent-limit::
> > > > > >         Do not show entries which have an overhead under that percent.
> > > > > >         (Default: 0).
> > > > > 
> > > > > Right.  Is it ok to you?
> > > > > 
> > > > > --percent-limit::
> > > > >         Do not show entries and callchains which have an overhead under that
> > > > >         percent.  (Default: 0).
> > > > 
> > > > Ok, but is the default zero?
> > > > 
> > > > That was what I was alluding to, as as soon as I applied the patch that
> > > > made callchains honour this limit, about 60% of the entries in the
> > > > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > > > expansion) signs.
> > > 
> > > Yes, and this is what I want to say too. :)
> > > 
> > > The default value of percent limit is different for hist entry and
> > > callchains.  For hist entry the default is 0, and for callchains it's
> > > 0.5%.  But using --percent-limit option, we can set both at once from
> > > now on.
> > > 
> > > Before this patchset, percent limit of callchains didn't checked.
> > > Once applied, users can see callchains are disppeared like your case.
> > >  This is just because 0.5% of the default limit for callchains is too
> > > high IMHO - I guess it was originally set for 'fractal' mode which
> > > calculates relative percents.  So I proposed to change the default in
> > > the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> > > choose other value like 0.05% ?
> > 
> > Unsure about the limit, but please fold the nice explanation you gave
> > about the limits for hist entries and callchains and how the defaults
> > are different, and how they can be set at once using --percent-limit.
> 
> Ok, will do.
> 
> > 
> > Also, would it be overengineering to allow optionally set two limits,
> > one for hists and one for callchains? I.e. like:
> > 
> >  --percent-limit 1,0.07
> > 
> > Then:
> > 
> >   --percent-limit 0.8
> > 
> > would be the same as:
> > 
> >   --percent-limit 0.8,0.8
> 
> Originally, the percent limit (threshold) for callchains can be set
> using -g/--call-graph option.  What Andi asked is to set both limits
> using a single option.  Not sure it's worth adding another way to
> specify the callchain limit.

Ok, having that in the docs, i.e. when documenting --percent-limit
mention that the callchain limit can be set via the -g option as well
will help sort out confusion when trying to set those limits.

And have you considered setting these limits dynamicly? I.e. in the TUI?

- Arnaldo

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

* Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also
  2016-02-02 14:57               ` Arnaldo Carvalho de Melo
@ 2016-02-02 15:03                 ` Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2016-02-02 15:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Andi Kleen,
	David Ahern, Frederic Weisbecker, Wang Nan

On Tue, Feb 02, 2016 at 11:57:54AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 02, 2016 at 11:35:13PM +0900, Namhyung Kim escreveu:
> > On Tue, Feb 02, 2016 at 11:27:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> > > > On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > > > > Hi Arnaldo,
> > > > > > 
> > > > > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > > > > it'd be better to have same effect to callchains as well
> > > > > > > 
> > > > > > > Documentation needs updating? It says:
> > > > > > > 
> > > > > > > --percent-limit::
> > > > > > >         Do not show entries which have an overhead under that percent.
> > > > > > >         (Default: 0).
> > > > > > 
> > > > > > Right.  Is it ok to you?
> > > > > > 
> > > > > > --percent-limit::
> > > > > >         Do not show entries and callchains which have an overhead under that
> > > > > >         percent.  (Default: 0).
> > > > > 
> > > > > Ok, but is the default zero?
> > > > > 
> > > > > That was what I was alluding to, as as soon as I applied the patch that
> > > > > made callchains honour this limit, about 60% of the entries in the
> > > > > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > > > > expansion) signs.
> > > > 
> > > > Yes, and this is what I want to say too. :)
> > > > 
> > > > The default value of percent limit is different for hist entry and
> > > > callchains.  For hist entry the default is 0, and for callchains it's
> > > > 0.5%.  But using --percent-limit option, we can set both at once from
> > > > now on.
> > > > 
> > > > Before this patchset, percent limit of callchains didn't checked.
> > > > Once applied, users can see callchains are disppeared like your case.
> > > >  This is just because 0.5% of the default limit for callchains is too
> > > > high IMHO - I guess it was originally set for 'fractal' mode which
> > > > calculates relative percents.  So I proposed to change the default in
> > > > the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> > > > choose other value like 0.05% ?
> > > 
> > > Unsure about the limit, but please fold the nice explanation you gave
> > > about the limits for hist entries and callchains and how the defaults
> > > are different, and how they can be set at once using --percent-limit.
> > 
> > Ok, will do.
> > 
> > > 
> > > Also, would it be overengineering to allow optionally set two limits,
> > > one for hists and one for callchains? I.e. like:
> > > 
> > >  --percent-limit 1,0.07
> > > 
> > > Then:
> > > 
> > >   --percent-limit 0.8
> > > 
> > > would be the same as:
> > > 
> > >   --percent-limit 0.8,0.8
> > 
> > Originally, the percent limit (threshold) for callchains can be set
> > using -g/--call-graph option.  What Andi asked is to set both limits
> > using a single option.  Not sure it's worth adding another way to
> > specify the callchain limit.
> 
> Ok, having that in the docs, i.e. when documenting --percent-limit
> mention that the callchain limit can be set via the -g option as well
> will help sort out confusion when trying to set those limits.

OK.  How about this?

--percent-limit::
	Do not show entries which have an overhead under that percent.
	(Default: 0).  Note that this option also sets percent limit (threshold)
	of callchains at once.  However the default value of callchain threshold
	is different than the default value of hist entries.  Please see
	--call-graph option for details.


> 
> And have you considered setting these limits dynamicly? I.e. in the TUI?

Oh, I forgot about it.  Will try to implement.

Thanks,
Namhyung

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

* [tip:perf/core] perf hists: Fix min callchain hits calculation
  2016-01-27 15:40 ` [PATCH 01/10] perf hists: Fix min callchain hits calculation Namhyung Kim
@ 2016-02-03 10:21   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, hpa, andi, dsahern, fweisbec, jolsa, peterz, tglx,
	linux-kernel, wangnan0, namhyung, mingo

Commit-ID:  744070e0e4ac691bb43608f7bf46a9641a9cf342
Gitweb:     http://git.kernel.org/tip/744070e0e4ac691bb43608f7bf46a9641a9cf342
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 16:42:31 -0300

perf hists: Fix min callchain hits calculation

The total period should be get using hists__total_period() since it
takes filtered entries into account.  In addition, if callchain mode is
'fractal', the total period should be the entry's period.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 81ce0af..b961946 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1163,9 +1163,18 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
 
-	if (use_callchain)
+	if (use_callchain) {
+		if (callchain_param.mode == CHAIN_GRAPH_REL) {
+			u64 total = he->stat.period;
+
+			if (symbol_conf.cumulate_callchain)
+				total = he->stat_acc->period;
+
+			min_callchain_hits = total * (callchain_param.min_percent / 100);
+		}
 		callchain_param.sort(&he->sorted_chain, he->callchain,
 				      min_callchain_hits, &callchain_param);
+	}
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1195,7 +1204,7 @@ void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 	else
 		use_callchain = symbol_conf.use_callchain;
 
-	min_callchain_hits = hists->stats.total_period * (callchain_param.min_percent / 100);
+	min_callchain_hits = hists__total_period(hists) * (callchain_param.min_percent / 100);
 
 	if (sort__need_collapse)
 		root = &hists->entries_collapsed;

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

* [tip:perf/core] perf hists: Update hists' total period when adding entries
  2016-01-27 15:40 ` [PATCH 02/10] perf hists: Update hists' total period when adding entries Namhyung Kim
@ 2016-02-03 10:21   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, peterz, wangnan0, jolsa, andi, hpa, mingo, linux-kernel,
	fweisbec, dsahern, namhyung, tglx

Commit-ID:  0f58474ec835f6fc80af2cde2c7ed5495cd212ba
Gitweb:     http://git.kernel.org/tip/0f58474ec835f6fc80af2cde2c7ed5495cd212ba
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:49 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 16:45:44 -0300

perf hists: Update hists' total period when adding entries

Currently the hist entry addition path doesn't update total_period of
hists and it's calculated during 'resort' path.  But the resort path
needs to know the total period before doing its job because it's used
for calculating percent limit of callchains in hist entries.

So this patch update the total period during the addition path.  It
makes the percent limit of callchains working (again).

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 b961946..098310b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -432,8 +432,12 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 		cmp = hist_entry__cmp(he, entry);
 
 		if (!cmp) {
-			if (sample_self)
+			if (sample_self) {
 				he_stat__add_period(&he->stat, period, weight);
+				hists->stats.total_period += period;
+				if (!he->filtered)
+					hists->stats.total_non_filtered_period += period;
+			}
 			if (symbol_conf.cumulate_callchain)
 				he_stat__add_period(he->stat_acc, period, weight);
 
@@ -466,7 +470,10 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
-	hists->nr_entries++;
+	if (sample_self)
+		hists__inc_stats(hists, he);
+	else
+		hists->nr_entries++;
 
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);

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

* [tip:perf/core] perf report: Apply --percent-limit to callchains also
  2016-01-27 15:40 ` [PATCH 03/10] perf report: Apply --percent-limit to callchains also Namhyung Kim
  2016-02-01 20:19   ` Arnaldo Carvalho de Melo
@ 2016-02-03 10:22   ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: wangnan0, dsahern, hpa, fweisbec, andi, linux-kernel, jolsa,
	mingo, acme, peterz, tglx, namhyung

Commit-ID:  2665b4528d0522ef073c2bde33cf9a7bd7391164
Gitweb:     http://git.kernel.org/tip/2665b4528d0522ef073c2bde33cf9a7bd7391164
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:50 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:12:00 -0300

perf report: Apply --percent-limit to callchains also

Currently --percent-limit option only works for hist entries.  However
it'd be better to have same effect to callchains as well

Requested-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2bf537f..72ed0b4 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "report.percent-limit")) {
-		rep->min_percent = strtof(value, NULL);
+		double pcnt = strtof(value, NULL);
+
+		rep->min_percent = pcnt;
+		callchain_param.min_percent = pcnt;
 		return 0;
 	}
 	if (!strcmp(var, "report.children")) {
@@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
 		    int unset __maybe_unused)
 {
 	struct report *rep = opt->value;
+	double pcnt = strtof(str, NULL);
 
-	rep->min_percent = strtof(str, NULL);
+	rep->min_percent = pcnt;
+	callchain_param.min_percent = pcnt;
 	return 0;
 }
 

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

* [tip:perf/core] perf report: Get rid of hist_entry__callchain_fprintf()
  2016-01-27 15:40 ` [PATCH 04/10] perf report: Get rid of hist_entry__callchain_fprintf() Namhyung Kim
@ 2016-02-03 10:22   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: fweisbec, dsahern, andi, namhyung, linux-kernel, tglx, hpa, acme,
	jolsa, peterz, wangnan0, mingo

Commit-ID:  7e597d327eca3d92a759542ff707cba61af3a718
Gitweb:     http://git.kernel.org/tip/7e597d327eca3d92a759542ff707cba61af3a718
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:51 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:20:31 -0300

perf report: Get rid of hist_entry__callchain_fprintf()

It's just a wrapper function to align the start position ofcallchains to
'comm' of each thread if it's a first sort key.  But it doesn't not work
with tracepoint events and also with upcoming hierarchy view.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 387110d..8e25f7d 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -349,30 +349,6 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
-					    struct hists *hists,
-					    FILE *fp)
-{
-	int left_margin = 0;
-	u64 total_period = hists->stats.total_period;
-
-	if (field_order == NULL && (sort_order == NULL ||
-				    !prefixcmp(sort_order, "comm"))) {
-		struct perf_hpp_fmt *fmt;
-
-		perf_hpp__for_each_format(fmt) {
-			if (!perf_hpp__is_sort_entry(fmt))
-				continue;
-
-			/* must be 'comm' sort entry */
-			left_margin = fmt->width(fmt, NULL, hists_to_evsel(hists));
-			left_margin -= thread__comm_len(he->thread);
-			break;
-		}
-	}
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
-}
-
 static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -418,6 +394,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.buf		= bf,
 		.size		= size,
 	};
+	u64 total_period = hists->stats.total_period;
 
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
@@ -427,7 +404,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	ret = fprintf(fp, "%s\n", bf);
 
 	if (symbol_conf.use_callchain)
-		ret += hist_entry__callchain_fprintf(he, hists, fp);
+		ret += hist_entry_callchain__fprintf(he, total_period, 0, fp);
 
 	return ret;
 }

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

* [tip:perf/core] perf callchain: Pass parent_samples to __callchain__fprintf_graph()
  2016-01-27 15:40 ` [PATCH 05/10] perf tools: Pass parent_samples to __callchain__fprintf_graph() Namhyung Kim
@ 2016-02-03 10:22   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, hpa, wangnan0, linux-kernel, dsahern, peterz, andi, mingo,
	fweisbec, namhyung, acme, tglx

Commit-ID:  54d27b3119e2eecbb3dfbf821db90fab25f6c523
Gitweb:     http://git.kernel.org/tip/54d27b3119e2eecbb3dfbf821db90fab25f6c523
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:52 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:39:52 -0300

perf callchain: Pass parent_samples to __callchain__fprintf_graph()

Pass hist entry's period to graph callchain print function.  This info
is needed by later patch to determine whether it can omit percentage of
top-level node or not.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8e25f7d..96188ea 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -166,7 +166,8 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 }
 
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-				       u64 total_samples, int left_margin)
+				       u64 total_samples, u64 parent_samples,
+				       int left_margin)
 {
 	struct callchain_node *cnode;
 	struct callchain_list *chain;
@@ -213,6 +214,9 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 		root = &cnode->rb_root;
 	}
 
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		total_samples = parent_samples;
+
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
 	ret += fprintf(fp, "\n");
@@ -323,16 +327,19 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 					    u64 total_samples, int left_margin,
 					    FILE *fp)
 {
+	u64 parent_samples = he->stat.period;
+
+	if (symbol_conf.cumulate_callchain)
+		parent_samples = he->stat_acc->period;
+
 	switch (callchain_param.mode) {
 	case CHAIN_GRAPH_REL:
-		return callchain__fprintf_graph(fp, &he->sorted_chain,
-						symbol_conf.cumulate_callchain ?
-						he->stat_acc->period : he->stat.period,
-						left_margin);
+		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
+						parent_samples, left_margin);
 		break;
 	case CHAIN_GRAPH_ABS:
 		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
-						left_margin);
+						parent_samples, left_margin);
 		break;
 	case CHAIN_FLAT:
 		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);

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

* [tip:perf/core] perf report: Fix percent display in callchains on --stdio
  2016-01-27 15:40 ` [PATCH 06/10] perf report: Fix percent display in callchains on --stdio Namhyung Kim
@ 2016-02-03 10:22   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: fweisbec, linux-kernel, namhyung, jolsa, peterz, mingo, acme,
	andi, wangnan0, dsahern, tglx, hpa

Commit-ID:  7ed5d6e28a0a1a54f554b0ab9c38a6061e7cac9e
Gitweb:     http://git.kernel.org/tip/7ed5d6e28a0a1a54f554b0ab9c38a6061e7cac9e
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:53 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:41:32 -0300

perf report: Fix percent display in callchains on --stdio

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report -s comm --percent-limit 0.01 --stdio
  ...
     9.95%  swapper
            |
            |--7.57%--intel_idle
            |          cpuidle_enter_state
            |          cpuidle_enter
            |          call_cpuidle
            |          cpu_startup_entry
            |          |
            |          |--4.89%--start_secondary
            |          |
            |           --2.68%--rest_init
            |                     start_kernel
            |                     x86_64_start_reservations
            |                     x86_64_start_kernel
	    |
	    |--0.15%--__schedule
	    |          |
	    |          |--0.13%--schedule
	    |          |          schedule_preempt_disable
	    |          |          cpu_startup_entry
            |          |          |
            |          |          |--0.09%--start_secondary
            |          |          |
            |          |           --0.04%--rest_init
            |          |                     start_kernel
            |          |                     x86_64_start_reservations
            |          |                     x86_64_start_kernel
            |          |
            |           --0.01%--schedule_preempt_disabled
            |                     cpu_startup_entry
  ...

Current code omits the percent if 'intel_idle' becomes the only node
when percent limit is set to 0.5%, its percent is not 9.95% but users
will assume it incorrectly.

Before:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
            ---intel_idle
               cpuidle_enter_state
               cpuidle_enter
               call_cpuidle
               cpu_startup_entry
               |
               |--4.89%--start_secondary
               |
                --2.68%--rest_init
                          start_kernel
                          x86_64_start_reservations
                          x86_64_start_kernel

After:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
             --7.57%--intel_idle
                       cpuidle_enter_state
                       cpuidle_enter
                       call_cpuidle
                       cpu_startup_entry
                       |
                       |--4.89%--start_secondary
                       |
                        --2.68%--rest_init
                                  start_kernel
                                  x86_64_start_reservations
                                  x86_64_start_kernel

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-7-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 96188ea..76ff46b 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -165,6 +165,25 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	return ret;
 }
 
+/*
+ * If have one single callchain root, don't bother printing
+ * its percentage (100 % in fractal mode and the same percentage
+ * than the hist in graph mode). This also avoid one level of column.
+ *
+ * However when percent-limit applied, it's possible that single callchain
+ * node have different (non-100% in fractal mode) percentage.
+ */
+static bool need_percent_display(struct rb_node *node, u64 parent_samples)
+{
+	struct callchain_node *cnode;
+
+	if (rb_next(node))
+		return true;
+
+	cnode = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(cnode) != parent_samples;
+}
+
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 				       u64 total_samples, u64 parent_samples,
 				       int left_margin)
@@ -178,13 +197,8 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	int ret = 0;
 	char bf[1024];
 
-	/*
-	 * If have one single callchain root, don't bother printing
-	 * its percentage (100 % in fractal mode and the same percentage
-	 * than the hist in graph mode). This also avoid one level of column.
-	 */
 	node = rb_first(root);
-	if (node && !rb_next(node)) {
+	if (node && !need_percent_display(node, parent_samples)) {
 		cnode = rb_entry(node, struct callchain_node, rb_node);
 		list_for_each_entry(chain, &cnode->val, list) {
 			/*

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

* [tip:perf/core] perf hists browser: Fix dump to show correct callchain style
  2016-01-27 15:40 ` [PATCH 07/10] perf hists browser: Fix dump to show correct callchain style Namhyung Kim
@ 2016-02-03 10:23   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:23 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux-kernel, dsahern, wangnan0, acme, jolsa, tglx,
	namhyung, peterz, fweisbec, hpa, andi

Commit-ID:  0c841c6c16f320704f75970bbe6a9800c53e6cf5
Gitweb:     http://git.kernel.org/tip/0c841c6c16f320704f75970bbe6a9800c53e6cf5
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:54 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:44:30 -0300

perf hists browser: Fix dump to show correct callchain style

The commit 8c430a348699 ("perf hists browser: Support folded
callchains") missed to update hist_browser__dump() so it always shows
graph-style callchains regardless of current setting.

To fix that, factor out callchain printing code and rename the existing
function which prints graph-style callchain.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Fixes: 8c430a348699 ("perf hists browser: Support folded callchains")
Link: http://lkml.kernel.org/r/1453909257-26015-8-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 73 ++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 1da30f8..6b22baf 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -844,7 +844,7 @@ next:
 	return row - first_row;
 }
 
-static int hist_browser__show_callchain(struct hist_browser *browser,
+static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
 					print_callchain_entry_fn print,
@@ -898,7 +898,7 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 			else
 				new_total = total;
 
-			row += hist_browser__show_callchain(browser, &child->rb_root,
+			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
 							    new_level, row, new_total,
 							    print, arg, is_output_full);
 		}
@@ -910,6 +910,43 @@ out:
 	return row - first_row;
 }
 
+static int hist_browser__show_callchain(struct hist_browser *browser,
+					struct hist_entry *entry, int level,
+					unsigned short row,
+					print_callchain_entry_fn print,
+					struct callchain_print_arg *arg,
+					check_output_full_fn is_output_full)
+{
+	u64 total = hists__total_period(entry->hists);
+	int printed;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL) {
+		if (symbol_conf.cumulate_callchain)
+			total = entry->stat_acc->period;
+		else
+			total = entry->stat.period;
+	}
+
+	if (callchain_param.mode == CHAIN_FLAT) {
+		printed = hist_browser__show_callchain_flat(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else if (callchain_param.mode == CHAIN_FOLDED) {
+		printed = hist_browser__show_callchain_folded(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else {
+		printed = hist_browser__show_callchain_graph(browser,
+						&entry->sorted_chain, level, row, total,
+						print, arg, is_output_full);
+	}
+
+	if (arg->is_current_entry)
+		browser->he_selection = entry;
+
+	return printed;
+}
+
 struct hpp_arg {
 	struct ui_browser *b;
 	char folded_sign;
@@ -1084,38 +1121,14 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		--row_offset;
 
 	if (folded_sign == '-' && row != browser->b.rows) {
-		u64 total = hists__total_period(entry->hists);
 		struct callchain_print_arg arg = {
 			.row_offset = row_offset,
 			.is_current_entry = current_entry,
 		};
 
-		if (callchain_param.mode == CHAIN_GRAPH_REL) {
-			if (symbol_conf.cumulate_callchain)
-				total = entry->stat_acc->period;
-			else
-				total = entry->stat.period;
-		}
-
-		if (callchain_param.mode == CHAIN_FLAT) {
-			printed += hist_browser__show_callchain_flat(browser,
-					&entry->sorted_chain, row, total,
+		printed += hist_browser__show_callchain(browser, entry, 1, row,
 					hist_browser__show_callchain_entry, &arg,
 					hist_browser__check_output_full);
-		} else if (callchain_param.mode == CHAIN_FOLDED) {
-			printed += hist_browser__show_callchain_folded(browser,
-					&entry->sorted_chain, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		} else {
-			printed += hist_browser__show_callchain(browser,
-					&entry->sorted_chain, 1, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		}
-
-		if (arg.is_current_entry)
-			browser->he_selection = entry;
 	}
 
 	return printed;
@@ -1380,15 +1393,11 @@ do_offset:
 static int hist_browser__fprintf_callchain(struct hist_browser *browser,
 					   struct hist_entry *he, FILE *fp)
 {
-	u64 total = hists__total_period(he->hists);
 	struct callchain_print_arg arg  = {
 		.fp = fp,
 	};
 
-	if (symbol_conf.cumulate_callchain)
-		total = he->stat_acc->period;
-
-	hist_browser__show_callchain(browser, &he->sorted_chain, 1, 0, total,
+	hist_browser__show_callchain(browser, he, 1, 0,
 				     hist_browser__fprintf_callchain_entry, &arg,
 				     hist_browser__check_dump_full);
 	return arg.printed;

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

* [tip:perf/core] perf hists browser: Pass parent_total to callchain print functions
  2016-01-27 15:40 ` [PATCH 08/10] perf hists browser: Pass parent_total to callchain print functions Namhyung Kim
@ 2016-02-03 10:23   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:23 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, peterz, wangnan0, fweisbec, andi, tglx, hpa, jolsa,
	dsahern, namhyung, acme, mingo

Commit-ID:  5eca104eee7edfe7155523849750ced539b16e94
Gitweb:     http://git.kernel.org/tip/5eca104eee7edfe7155523849750ced539b16e94
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:55 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:45:42 -0300

perf hists browser: Pass parent_total to callchain print functions

Pass parent node's total period to callchain print functions.  This info
is needed by later patch to determine whether it can omit percent or not
correctly.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-9-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 44 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 6b22baf..41dbb79 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -660,6 +660,7 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
+					     u64 parent_total __maybe_unused,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -763,6 +764,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
+					       u64 parent_total __maybe_unused,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -847,14 +849,18 @@ next:
 static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
+					u64 parent_total,
 					print_callchain_entry_fn print,
 					struct callchain_print_arg *arg,
 					check_output_full_fn is_output_full)
 {
 	struct rb_node *node;
 	int first_row = row, offset = level * LEVEL_OFFSET_STEP;
-	u64 new_total;
 	bool need_percent;
+	u64 percent_total = total;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		percent_total = parent_total;
 
 	node = rb_first(root);
 	need_percent = node && rb_next(node);
@@ -878,7 +884,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 			folded_sign = callchain_list__folded(chain);
 
 			row += hist_browser__show_callchain_list(browser, child,
-							chain, row, total,
+							chain, row, percent_total,
 							was_first && need_percent,
 							offset + extra_offset,
 							print, arg);
@@ -893,13 +899,9 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		if (folded_sign == '-') {
 			const int new_level = level + (extra_offset ? 2 : 1);
 
-			if (callchain_param.mode == CHAIN_GRAPH_REL)
-				new_total = child->children_hit;
-			else
-				new_total = total;
-
 			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
-							    new_level, row, new_total,
+							    new_level, row, total,
+							    child->children_hit,
 							    print, arg, is_output_full);
 		}
 		if (is_output_full(browser, row))
@@ -918,27 +920,29 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 					check_output_full_fn is_output_full)
 {
 	u64 total = hists__total_period(entry->hists);
+	u64 parent_total;
 	int printed;
 
-	if (callchain_param.mode == CHAIN_GRAPH_REL) {
-		if (symbol_conf.cumulate_callchain)
-			total = entry->stat_acc->period;
-		else
-			total = entry->stat.period;
-	}
+	if (symbol_conf.cumulate_callchain)
+		parent_total = entry->stat_acc->period;
+	else
+		parent_total = entry->stat.period;
 
 	if (callchain_param.mode == CHAIN_FLAT) {
 		printed = hist_browser__show_callchain_flat(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else if (callchain_param.mode == CHAIN_FOLDED) {
 		printed = hist_browser__show_callchain_folded(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else {
 		printed = hist_browser__show_callchain_graph(browser,
-						&entry->sorted_chain, level, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, level, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	}
 
 	if (arg->is_current_entry)

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

* [tip:perf/core] perf hists browser: Fix percent display in callchains
  2016-01-27 15:40 ` [PATCH 09/10] perf hists browser: Fix percent display in callchains Namhyung Kim
@ 2016-02-03 10:24   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, andi, fweisbec, linux-kernel, tglx, wangnan0, namhyung,
	hpa, peterz, mingo, jolsa, dsahern

Commit-ID:  59c624e2391080fa6315a376a4ee74d0eb393d1d
Gitweb:     http://git.kernel.org/tip/59c624e2391080fa6315a376a4ee74d0eb393d1d
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:56 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:46:27 -0300

perf hists browser: Fix percent display in callchains

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report --no-children --percent-limit 0.01 --tui
  ...
  -    0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
           - vm_mmap_pgoff
              + 0.02% sys_mmap_pgoff
              + 0.02% vm_mmap
           + 0.02% mprotect_fixup

Current code omits the percent if 'mmap_region' becomes the only node
when percent limit is set to 0.03%, its percent is not 0.06% but users
will assume it incorrectly.

Before:

  $ perf report --no-children --percent-limit 0.03 --tui
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - mmap_region
          do_mmap_pgoff
          vm_mmap_pgoff

After:

  $ perf report --no-children --percent-limit 0.03 --tui
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
             vm_mmap_pgoff

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-10-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 41dbb79..61d578b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -657,10 +657,24 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 	return 1;
 }
 
+static bool check_percent_display(struct rb_node *node, u64 parent_total)
+{
+	struct callchain_node *child;
+
+	if (node == NULL)
+		return false;
+
+	if (rb_next(node))
+		return true;
+
+	child = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(child) != parent_total;
+}
+
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
-					     u64 parent_total __maybe_unused,
+					     u64 parent_total,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -670,7 +684,7 @@ static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -764,7 +778,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
-					       u64 parent_total __maybe_unused,
+					       u64 parent_total,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -774,7 +788,7 @@ static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -863,7 +877,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		percent_total = parent_total;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);

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

* [tip:perf/core] perf report: Don' t show blank lines if entry has no callchain
  2016-01-28 12:24         ` Namhyung Kim
  2016-01-28 19:52           ` Jiri Olsa
@ 2016-02-03 10:24           ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 42+ messages in thread
From: tip-bot for Namhyung Kim @ 2016-02-03 10:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, wangnan0, namhyung, dsahern, mingo, jolsa, tglx,
	linux-kernel, acme, andi, hpa, fweisbec

Commit-ID:  3848c23b19e07188bfa15e3d9a2ac27692f2ff3c
Gitweb:     http://git.kernel.org/tip/3848c23b19e07188bfa15e3d9a2ac27692f2ff3c
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 21:24:54 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:51:09 -0300

perf report: Don't show blank lines if entry has no callchain

When all callchains of a hist entry is percent-limited, do not add a
blank line at the end.  It makes the entry look like it doesn't have
callchains.

Reported-and-Tested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20160128122454.GA27446@danjae.kornet
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 76ff46b..691e52c 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -233,7 +233,10 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
-	ret += fprintf(fp, "\n");
+	if (ret) {
+		/* do not add a blank line if it printed nothing */
+		ret += fprintf(fp, "\n");
+	}
 
 	return ret;
 }

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

* [PATCH 01/10] perf hists: Fix min callchain hits calculation
  2016-02-01 21:49 [GIT PULL 00/10] perf/core callchains improvements and fixes Arnaldo Carvalho de Melo
@ 2016-02-01 21:49 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 42+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-01 21:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Peter Zijlstra, Wang Nan,
	Arnaldo Carvalho de Melo

See http://www.infradead.org/rpr.html

From: Namhyung Kim <namhyung@kernel.org>

The total period should be get using hists__total_period() since it
takes filtered entries into account.  In addition, if callchain mode is
'fractal', the total period should be the entry's period.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 81ce0aff69d1..b96194676c91 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1163,9 +1163,18 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
 
-	if (use_callchain)
+	if (use_callchain) {
+		if (callchain_param.mode == CHAIN_GRAPH_REL) {
+			u64 total = he->stat.period;
+
+			if (symbol_conf.cumulate_callchain)
+				total = he->stat_acc->period;
+
+			min_callchain_hits = total * (callchain_param.min_percent / 100);
+		}
 		callchain_param.sort(&he->sorted_chain, he->callchain,
 				      min_callchain_hits, &callchain_param);
+	}
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1195,7 +1204,7 @@ void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 	else
 		use_callchain = symbol_conf.use_callchain;
 
-	min_callchain_hits = hists->stats.total_period * (callchain_param.min_percent / 100);
+	min_callchain_hits = hists__total_period(hists) * (callchain_param.min_percent / 100);
 
 	if (sort__need_collapse)
 		root = &hists->entries_collapsed;
-- 
2.5.0

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

end of thread, other threads:[~2016-02-03 10:25 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27 15:40 [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Namhyung Kim
2016-01-27 15:40 ` [PATCH 01/10] perf hists: Fix min callchain hits calculation Namhyung Kim
2016-02-03 10:21   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 02/10] perf hists: Update hists' total period when adding entries Namhyung Kim
2016-02-03 10:21   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 03/10] perf report: Apply --percent-limit to callchains also Namhyung Kim
2016-02-01 20:19   ` Arnaldo Carvalho de Melo
2016-02-02 13:05     ` Namhyung Kim
2016-02-02 13:55       ` Arnaldo Carvalho de Melo
2016-02-02 14:15         ` Namhyung Kim
2016-02-02 14:27           ` Arnaldo Carvalho de Melo
2016-02-02 14:35             ` Namhyung Kim
2016-02-02 14:57               ` Arnaldo Carvalho de Melo
2016-02-02 15:03                 ` Namhyung Kim
2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 04/10] perf report: Get rid of hist_entry__callchain_fprintf() Namhyung Kim
2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 05/10] perf tools: Pass parent_samples to __callchain__fprintf_graph() Namhyung Kim
2016-02-03 10:22   ` [tip:perf/core] perf callchain: " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 06/10] perf report: Fix percent display in callchains on --stdio Namhyung Kim
2016-02-03 10:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 07/10] perf hists browser: Fix dump to show correct callchain style Namhyung Kim
2016-02-03 10:23   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 08/10] perf hists browser: Pass parent_total to callchain print functions Namhyung Kim
2016-02-03 10:23   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [PATCH 09/10] perf hists browser: Fix percent display in callchains Namhyung Kim
2016-02-03 10:24   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-01-27 15:40 ` [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005% Namhyung Kim
2016-01-27 16:06   ` Andi Kleen
2016-01-27 23:34     ` Namhyung Kim
2016-01-28  8:14 ` [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2) Jiri Olsa
2016-01-28  8:42   ` Namhyung Kim
2016-01-28  8:16 ` Jiri Olsa
2016-01-28 10:14   ` Namhyung Kim
2016-01-28 10:16     ` Namhyung Kim
2016-01-28 12:12       ` Jiri Olsa
2016-01-28 12:24         ` Namhyung Kim
2016-01-28 19:52           ` Jiri Olsa
2016-01-29 21:00             ` Arnaldo Carvalho de Melo
2016-01-30 13:52               ` Namhyung Kim
2016-02-03 10:24           ` [tip:perf/core] perf report: Don' t show blank lines if entry has no callchain tip-bot for Namhyung Kim
2016-02-01 21:49 [GIT PULL 00/10] perf/core callchains improvements and fixes Arnaldo Carvalho de Melo
2016-02-01 21:49 ` [PATCH 01/10] perf hists: Fix min callchain hits calculation Arnaldo Carvalho de Melo

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.